Skip to content

[css-fonts-5] How do font-size-adjust property and size-adjust descriptor work together? #6128

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

Closed
xiaochengh opened this issue Mar 23, 2021 · 6 comments · Fixed by #6184
Closed

Comments

@xiaochengh
Copy link
Contributor

font-size-adjust is defined as:

This property allows authors to specify an aspect value for an element that will effectively preserve the x-height of the first choice font, whether it is substituted or not.
...
The value of ‘font-size-adjust’ affects the used value of ‘font-size’ but does not affect the computed value.

While size-adjust is recently resolved and specified as

... when the descriptor value is a percentage, all metrics values of this font, including glyph advance sizes, are resolved as the given percentage multiplied by the used font size.

So they affect the exact same value. What should we do when both of them are set?

In particular, while size-adjust scales the x-height, while font-size-adjust depends on the unadjusted x-height and tries to preserve its value by scaling the used font size. So there seems to be a dependency issue here to clarify, or which one to apply first.

Also @jfkthame since Gecko has already shipped font-size-adjust and prototyped size-adjust. It would be great to know Gecko's behavior and considerations.

@jfkthame
Copy link
Contributor

Given that size-adjust is a descriptor on the font face, while font-size-adjust is a property on an element, I think the logical behavior is that size-adjust applies first, scaling all metrics retrieved from the font (including its x-height). This happens independently in each face that's being used.

These adjusted metrics then provide the basis for the computation and adjustment that the font-size-adjust property will do.

I think this is the behavior that will naturally arise from the patches currently in progress for Gecko, though I admit I have not actually tested the interaction yet.

(This feels somewhat parallel to the interaction of the font-feature-settings and font-variation-settings descriptors and corresponding properties, where the descriptor can set up defaults that go along with the specific font face, but then the property, if present, can adjust them further.)

@xiaochengh
Copy link
Contributor Author

Makes sense to me. Thank you!

@chrishtr
Copy link
Contributor

@jfkthame's reasoning make sense to me. Agenda+ to agree on it next week and add to the spec.

@drott
Copy link
Collaborator

drott commented Mar 31, 2021

Here's a quick Codepen I put together https://codepen.io/drott_chrome/pen/bGggEeB to test the interaction in Firefox Nightly. Thanks @jfkthame for prototyping this.

I agree with Jonathan's comment and Xiaocheng. This sequence of computation effective font size seems reasonable to me as well, but let's look at how this behaves.

Looking at the codepen that combines the two in FF nightly, I wonder what exactly is happening. It seems that once font-size-adjust is in the style, the size-adjust no longer has an effect (rendered font size does not change when changing font-size-adjust.

The way the formula is set up for font-size adjust it only looks at aspects (x-height to font-size / ascent + descent) of the fonts, so a scaling done through the size-adjust descriptor does not effectively change that ratio. In the end, it looks like font-size-adjust overrules size-adjust because the aspect stays unchanged and the font-size denoted with s in the c = ( a / a' ) s formula for font-size-adjust is the font-size from CSS, which also seems correct.

This looks to me like in fact it is applied after size-adjust, but the effect of size-adjust becomes void because font-size-adjust operates on style font-size and font aspects only. This seems fine to me.

@jfkthame
Copy link
Contributor

Yes, I agree this is the result. (Sorry I didn't spell this out better before, but it became clear in the course of working on this that it has to be this way.)

The aim of font-size-adjust is essentially to harmonize a specific metric (x-height) that otherwise tends to vary between different fonts (for a given font-size). size-adjust provides a way for authors to customize that metric within each face, by rescaling the glyphs relative to the em size, but whatever the author does at that level, font-size-adjust (if applied) will still end up harmonizing the end result.

@astearns astearns added this to the EUR VF2F-2021-04-06 milestone Apr 2, 2021
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-fonts-5] How do font-size-adjust property and size-adjust descriptor work together?, and agreed to the following:

  • RESOLVED: clarify that font-size-adjust computations will effect things in such a way that the descriptor overrides have no effect
The full IRC log of that discussion <astearns> topic: [css-fonts-5] How do font-size-adjust property and size-adjust descriptor work together?
<astearns> github: https://github.com//issues/6128
<fremy> jfkthame: the question came up about the interaction between these two properties
<fremy> jfkthame: the proposed solution in the issue
<fremy> jfkthame: is that the font-size-adjust takes precedence in a given element
<fremy> jfkthame: that does erase the changes in the descriptor
<fremy> jfkthame: I think this makes sense
<fremy> myles: one way of looking at it is that they both occur, but font-size-adjust changes things to the same solution regardless
<fremy> myles: when writing the spec text, I would go with this
<fremy> jfkthame: I think that is true
<dbaron> (Myles's description makes sense to me.)
<fremy> jfkthame: basically the calculations of font-size-adjust cancel out any previous adjustment
<fremy> astearns: sounds like we have a general agreement
<fremy> astearns: any objection to this clarification?
<fantasai> +1 to jfkthame's proposal
<fremy> RESOLVED: clarify that font-size-adjust computations will effect things in such a way that the descriptor overrides have no effect

webkit-commit-queue pushed a commit to vitorroriz/WebKit that referenced this issue May 2, 2023
https://bugs.webkit.org/show_bug.cgi?id=253062
rdar://106349717

Reviewed by Myles C. Maxfield.

Implements @font-face size-adjust descriptor according to
https://www.w3.org/TR/css-fonts-5/#size-adjust-desc

* LayoutTests/TestExpectations:
* Source/WebCore/css/FontFace.cpp:
(WebCore::FontFace::create):
(WebCore::FontFace::setSizeAdjust):
(WebCore::FontFace::sizeAdjust const):
* Source/WebCore/css/FontFace.h:
* Source/WebCore/css/FontFace.idl:
- Integrating size-adjust descriptor to FontFace() api.

* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp:
(WebCore::CSSPropertyParserWorkerSafe::parseFontFaceSizeAdjust):
* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.h:
- size-adjust parser wrapper for being used by FontFace().

* Source/WebCore/platform/graphics/FontPlatformData.cpp:
(WebCore::FontPlatformData::usedFontSize):
    It is not worth modifying the used size with @font-face size-adjust if
    we are to re-adjust it later with font-size-adjust.
This is because font-size-adjust will overrule this change, since
size-adjust also modifies the font's metric values and thus, keeps the
aspect-value unchanged (see discussion at w3c/csswg-drafts#6128).

* LayoutTests/TestExpectations:
* LayoutTests/platform/ios/size-adjust-interacts-with-text-size-adjust-auto-expected-mismatch.html: Added.
* LayoutTests/platform/ios/size-adjust-interacts-with-text-size-adjust-auto-notref.html: Added.
* LayoutTests/platform/ios/size-adjust-interacts-with-text-size-adjust-auto.html: Added.
* LayoutTests/platform/ios/size-adjust-interacts-with-text-size-adjust-percentage-expected.html: Added.
* LayoutTests/platform/ios/size-adjust-interacts-with-text-size-adjust-percentage-ref.html: Added.
* LayoutTests/platform/ios/size-adjust-interacts-with-text-size-adjust-percentage.html: Added.
- Theses tests won't be exported to WPT because -webkit-text-size-adjust
is a webkit-only property.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml

Canonical link: https://commits.webkit.org/263604@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants