-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Render small-caps font variant #1611
base: master
Are you sure you want to change the base?
Conversation
- pango has a PANGO_VARIANT_SMALL_CAPS variant that can be added to the font description, but setting it doesn't cause text to be rendered in small-caps, even if the font supports it - adding the appropriate opentype features to the layout's attributes list will use the font's small-caps glyphs if it provides them
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.
Thanks for this. Looks good in general. pango_attr_font_features_new
is not available in the (old) version of Pango for Windows that we link to in the wiki and use in CI, so we might have to #ifdef PANGO_VERSION(...
the new code.
We should also probably change CI testing to use MSYS like the prebuilds do.
- the font-variant state will get out of sync with the fontDescription across context-saves & restores unless we keep a copy of the PangoAttrList around
I realized that my initial version didn't account for state and would get out of sync with the displayed font after a save() and a restore(). So I've added a field to the state struct called |
- the pango NEWS file suggests 1.37.1 was where the `pango_attr_font_features_new` call was added...
- it can be enabled if/when the pango version is updated
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.
Looking good, I have one comment about state management.
@@ -258,6 +262,8 @@ Context2d::save() { | |||
states[++stateno] = (canvas_state_t *) malloc(sizeof(canvas_state_t)); | |||
memcpy(states[stateno], state, sizeof(canvas_state_t)); | |||
states[stateno]->fontDescription = pango_font_description_copy(states[stateno-1]->fontDescription); | |||
states[stateno]->textAttributes = pango_attr_list_copy(states[stateno-1]->textAttributes); | |||
pango_layout_set_attributes(_layout, states[stateno]->textAttributes); |
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.
This doesn't quite rhyme with how we handle the font description on the state (continued below)
} else { | ||
features = pango_attr_font_features_new(""); | ||
} | ||
pango_attr_list_change(context->state->textAttributes, features); |
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.
It's a little more code, but you could instead free the attr list on the stack first, then set the a fresh attr list on the layout and stack here. That way it's a little more like the code above and we're not mutating objects on the state.
It looks like no progress on this in awhile. Is it a complicated fix to add small-caps support in? |
64ed3d8
to
ff0f2ab
Compare
Thanks for contributing!