Skip to content

Fix variable font wght synthesis for shaping - #723

Open
taj-p wants to merge 1 commit into
linebender:mainfrom
taj-p:tajp/fixVariableFontWgtSelection
Open

Fix variable font wght synthesis for shaping#723
taj-p wants to merge 1 commit into
linebender:mainfrom
taj-p:tajp/fixVariableFontWgtSelection

Conversation

@taj-p

@taj-p taj-p commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

When a caller declares a variable font with weight X, it could be shaped with weight Y because shaping uses the font's default weight, rather than the specified X value.

This PR ensures that Fontique synthesizes the correct weight when the default and declared weight conflict.

See the provided unit test.

Before Fix After Fix
style_font_weight_uses_variable_axis_default-0 style_font_weight_uses_variable_axis_default-0

@taj-p taj-p changed the title Fix variable font wgt synthesis for shaping Fix variable font wght synthesis for shaping Jul 31, 2026
Comment thread fontique/src/font.rs
} else if weight.value() > self.weight.value() {
synth.embolden = true;
}
} else if weight.value() > self.weight.value() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition needs tweaking IMO. Should only embolden if requested weight is at least 200/300 CSS units more, not just 1, or 100 for that matter.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, doesn't the embolden() get a strength by any chance?

@nicoburns nicoburns Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a strength parameter on the actual embolden impl in kurbo (exposed on vello). Don't think we have it wired through the text parts of the stack which I think might still be using more of an RBIZ model (yes/no) for embolden.

@taj-p taj-p Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Chromium uses a "at least 200 units more" approach to trigger embolden:

https://chromium.googlesource.com/chromium/src/%2B/39f1c9a7161786d7c54f394dfbfca8941f78fd8b/third_party/blink/renderer/platform/fonts/skia/font_cache_skia.cc#311

I'm happy to add that change in as a separate PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants