-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OfTrueTypeFont constructor+misc #7740
base: master
Are you sure you want to change the base?
OfTrueTypeFont constructor+misc #7740
Conversation
…/artificiel/openFrameworks into ofTrueTypeFont--constructor+misc
One suggestion for future PRs is splitting different subjects in different PRs, so they can be reviewed, discussed and merged faster. |
I don't mind splitting but at the same time got the opposite suggestion from @ofTheo vs my jumbo OSC PR #7658. what I'm aiming for is limiting individual commits to single code changes with a name that indicates clearly the change, and especially keep formatting is separate commits so the individual functional changes can be audited separately without being obfuscated in whitespace changes. so for instance in the above we see 4 code changes which are self-contained and well labeled. would it be preferable to have 4 distinct PR's? (and also consider the trickiness that they modify the same file, sometimes with sequential dependency, hence enhancing the risk of merge conflicts?) a strategy seen elsewhere is "a PR must close a single issue, and the issue must exist prior to the PR" (which implies the mandate has been clarified). this is in the spirit of discussing a future change before spending time on implementation. for some things it makes sense, but other minor changes (like this one) seems like an overhead for everyone. before GitHub integrates very well the discussion process within the PR, discussions can occur in there too, with a head start "proposal". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebase and figure out conflict
const ofUnicode::range ofUnicode::MiscTechnical {0x2300, 0x23FF}; | ||
const ofUnicode::range ofUnicode::BoxDrawing {0x2500, 0x257F}; | ||
const ofUnicode::range ofUnicode::BlockElement {0x2580, 0x259F}; | ||
const ofUnicode::range ofUnicode::GeometricShapes {0x25A0, 0x25FF}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot of formatting changes here hence the stale conflict. if you wanna figure it out
the constructor allows .h use like this:
other tweaks allow positioning from glm::vec2, and additional (0) default values for getStringBoundingBox.
[edit] also aligned the ofTrueTypeFontSettings constructor to ofTrueTypeFont's. while the idea of Settings class is evidently to provide explicitly named parameters (and fine-grained) configs, a given Setting should be as configurable as the class' most complex available setup(). It is arguable that the ofTrueTypeFont's setup is too complicated and should be reduced to enforce the Settings, but until that happens they should at least be coherent.