-
Notifications
You must be signed in to change notification settings - Fork 642
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
[css-fonts-5] advance-override details #5983
Comments
In Chrome's implementation, there's no font size changes or stretches. It shares quite a lot of code with IMO this seems better than adding spacing around each glyph. Otherwise, the first character may look misaligned with the start of the line. Anyway, all these choices have no difference for the purpose of reducing layout shift...
This is one of the topics that apply to
Maybe we should recommend NO for complex scripts and fonts. Reasons:
|
+1 to skipping scripts with cursive representations. I think this is in accord with the F2F discussion also, and is what (*) Spec text: "if the UA cannot expand text from a cursive script without breaking its cursive connections, it must not apply spacing between any pair of that script’s typographic letter units at all" https://drafts.csswg.org/css-text-3/#cursive-tracking |
It is indeed a common case for complex scripts to use different letter shapes, font styles, and ligating or conjoining approaches from font to font. It is also common that the text is formed from clusters of glyphs, with complex relative offsets and rule-base variants, rather than a simple succession of isolated letter glyphs. These things are so common, in fact, that they are what lead to them being called 'complex scripts'. But even where there is little to no interaction between glyphs, other than placement of diacritics, such as for Thai, differences between individual letter shapes can vary. Going from one font to another, some letters are thinner, others wider. Actually i'd have thought that would also be problematic for Latin text unless you choose quite similar fonts. Would it be fair to say that advance-override is really most suitable for monospaced, non-complex fonts such as CJK? |
Yeah, for this reason We never intend to do that accurately, though. Our goal is just to reduce layout shift. Given that web developers are already using |
It sounds like the description for this should point out the limitations, and thus avoid over-promising |
Here are some topics and my proposals for the next meeting. Please add more if needed.
Proposal: no. It has no effect for the purpose of reducing layout shift, but adds a lot of complexity to spec and implementation. If using
Proposal: spacing is added after each character, for the reasons I gave in #5983 (comment) for reasons.
Proposal: Use two descriptors Other ideas:
|
Just a nit: It's similar to letter-spacing for cursive scripts (although there's still a remote possibility that someone might come up with a use case that makes sense, esp. for non-Arabic scripts). I don't think however that it's similar to letter-spacing wrt complex scripts in general, because letter-spacing is expected to handle those (albeit that some additional functionality needs to be specified/implemented*). hth
|
Agreed, no. (I think there should be a separate
Disagree; I think spacing should be added equally on both sides of each glyph. Adding it asymmetrically (as some current letter-spacing implementations do) is often visually inferior IMO, particularly at run boundaries. In general this feature should not be used to insert (or remove) more than a small percentage of the glyph width -- if you need more than that, you're using a bad choice of fallback font, or should be using
I don't have a strong preference among these options. I think I'd lean towards either the first (separate width/height descriptors) or the third (second param defaults to
+1 to mentioning that it may not work well with cursive/complex scripts. However, I think we should not have spec text (like for By all means include a warning for authors about applying this to glyphs that are supposed to visually join, but if an author nevertheless chooses to do so, they can. (I have seen old printed examples from lead-type days of If an author wants to (for example) use Arial as a fallback, with |
I thought the reason we decided it would not just add a constant to each advance was because that looked best for cursive scripts? Maybe I'm misremembering? I also don't understand why the behavior here is different than Corollary: I think my counter-proposal to the above discussion would be: "intentionally don't innovate here, and make it behave exactly like Possibly related: #5965 |
That would even come into play if you have two immediately adjacent
I think Now, if you want to support changing the definition of |
Right, I'm not proposing just adjusting the metrics. I'm proposing an implementation which is intentionally more involved than simply overwriting some values from the file. |
A couple points:
Agreed, and I'd go further: I think shipping
Agreed. I'd probably go with the third option, seems the friendliest syntactically.
Agree with jfkthame's rationale. Yes, adding it just to the right side works best for left-aligned paragraphs. But not all affected content is left-aligned.
Note that letter-spacing adds a constant per typographic character unit. Advance-override is defined to scale the advance of each glyph proportionally. Also it's being defined as a font metric override, not a text layout feature... |
After discussing with the rest of the team, I retract my proposal. Sorry for the noise. |
One thing that is important to note here - these overridden advances will have to be applied after shaping, not before. If the input to shaping doesn't match what's in the font file, bad things happen (see https://bugs.webkit.org/show_bug.cgi?id=214769). |
I don't have a strong opinion on where to add spacing: before, after or around each glyph. All work the same for the purpose of reducing layout shift, so I'm fine to accept any resolution that gives the best typography.
We actually have prototyped
I'm happy to see if |
@xiaochengh No, no units should be affected by font-size override. It's a scaling factor applied to the glyphs and metrics, but not to CSS. as for how it interacts with other overrides, probably it should apply first. |
The CSS Working Group just discussed The full IRC log of that discussion<fantasai> Topic: advance-override details<Rossen_> github: https://github.com//issues/5983 <myles> fantasai: We decided we wanted to add an advance-override descriptor to the @font-face rule, but we didnt' define what it actually does other htan to say it takes a percentage that's resovled agains the advance itself <chris> q+ <myles> fantasai: A couple of points that would need to be resolved: 1. How are we increasing or decreasing the advance width? The size of the glyph in bohth axes together? Just x axis? Space around each glyph? left side? right side, start end, end end? <myles> fantasai: There's been some discussion in the issue about what it should apply to and how it should apply, but no conclusions. <myles> fantasai: I think the first thing we'd have to decide is how we increase the size of the glyphs <myles> fantasai: to do text layout <fantasai> myles: Shouldn't distort the outlines <Rossen_> ack chris <fantasai> chris: We're changing the advance, not the bounding box of the glyph. Not changing the outline <fantasai> chris: just changing the amount you need to move forward <myles> chris: We're changing the advance, not the bounding box of the glyphs, n or teh geometry. We're changing the amount you need to move forward before painting the next glyphs. Changing spacing in the text advance direction <fantasai> chris: very clearly not changing any glyph geometry <myles> chris: There's also a size override, but there's separate <fantasai> chris: There is a proposal to have a scale factor, but that's separate <fantasai> myles: I think that the exact way that space is added, I think that's not a super important decision, so doing whatever makes sense is easiest. For us that would be applying to the right side always <fantasai> Rossen_: Is that affected by directionality? <fantasai> myles: No, always on the right side <fantasai> chris: Why? <fantasai> myles: it's just easier to implement. Base-level API renders text LTR <fantasai> chris: So you're saying you do bidi reordering in the backing store and then apply space after that? <jfkthame> q+ <fantasai> Rossen_: So proposal is to keep it uniform on one side <fantasai> jfkthame: I would not like to specify that space is only applied on one side <fantasai> jfkthame: if that's easiest to apply, maybe allow it <fantasai> jfkthame: but I don't think that's the best behavior <fantasai> jfkthame: browser should be able to apply equally across both sides of glyph <fantasai> jfkthame: which is superior <Rossen_> ack jfkthame <Rossen_> ack fantasai <chris> I'm a bit concerned about margins not lining up, with space at the margins <Rossen_> q <fantasai> chris: could add space not at margins <fantasai> myles: bleeds into my point. naive implementation is to just multiply the number in the font file <fantasai> myles: but that applies before shaping <fantasai> myles: when you apply shaping, have to match what's in the font file. <fantasai> myles: Correct implementation of this needs to apply this after shaping <fantasai> myles: which is a more complicated implementation <fantasai> myles: doing something like what chris said is not hard, because you're already at the level of knowing that information <fantasai> Rossen_: So sounds like we're aligning on adding space on both sides? <fantasai> Rossen_: any other opinions? or resolve? <fantasai> myles: If you do it on both sides, then alignment won't look good on either side <fantasai> jfkthame: We're putting the space equally makes for a less-bad result than you will get than if it's all on one side <fantasai> jfkthame: in practice, I don't think this is a feature that should be used for a large adjustment <fantasai> jfkthame: it's for fine-tuning the metrics of a fallback font to match another font <fantasai> jfkthame: so the adjustment is going to be a small fraction of a glyph width. Not too bad. <fantasai> myles: I suggest leaving it undefined <chris> we should put that wording from jfkthame into the spec - fine adjustments, not major ones. fine tuning <fantasai> myles: Major point is that it makes text more/less compact <fantasai> myles: Might be worth getting impl experience <fantasai> jfkthame: Wrt trimming space at line boundaries, I'd be opposed. <fantasai> jfkthame: This is effectively about modifying the metrics of the font <fantasai> jfkthame: if you modify metrics of a glyph, should be everywhere that glyph occurs, not different based on position in the line <Rossen_> ack fantasai <TabAtkins> q+ to remind that we're trying to better solve a case where people are *already* doing letter-spacing <astearns> AFAIK this feature is NOT about making things look good, it’s merely about reducing layout shift once a font loads <fantasai> fantasai: Slight tangent, but <fantasai> fantasai: This feature has a lot of badness to it <fantasai> fantasai: First, as we've been discussing, it distorts alignment <fantasai> fantasai: Second, it breaks complex and cursive writing systems <fantasai> fantasai: Third, because it introduces uneven amounts of spacing between pairs of characters, it distorts the rhythm of the text and hence impacts readability in a negative way <fantasai> fantasai: None of these problems apply if the font is scaled in both axes instead of adding space between characters <fantasai> fantasai: And I think it would be harmful if we ship this feature without also shipping a scaling-factor descriptor that could be used in any cases where that would be usable in place of advance-override <fantasai> jfkthame: Strongly agree with that. <Rossen_> q? <Rossen_> ack TabAtkins <Zakim> TabAtkins, you wanted to remind that we're trying to better solve a case where people are *already* doing letter-spacing <xiaochengh> q+ <fantasai> TabAtkins: Keep in mind, the goal is not to define a new typographic feature for text. It's to replace existing hacks that people use to reduce layout shift when fonts load <fantasai> TabAtkins: if we can reduce layout shift, even if it looks bad, still successful <fantasai> TabAtkins: Want it to look good and make it work across writing systems <fantasai> TabAtkins: but if not ideal, that's fine, because that's not the point <fantasai> TabAtkins: should be for fonts that not there for very long, will be replaced by the real font later <fantasai> TabAtkins: Want a solution that works across scripts and works OK, better than current hacks <Rossen_> ack xiaochengh <fantasai> xiaochengh: Regarding scaling / font-size override <fantasai> xiaochengh: we have prototyped in Chrome, and found it tricky to nicely specify <fantasai> xiaochengh: This might affect ascent-override etc. <fantasai> xiaochengh: they have a %, which font-size do these resolve against? resolve against the font-size property value of the element or used font size? <fantasai> xiaochengh: Either way going to surprise some users <fantasai> xiaochengh: since our main focus is to reduce layout shift, wouldn't want to introduce a blocking issue on top of advance override <TabAtkins> fantasai: To reply to TAb, I udnerstand this is for reducing layout shift <Rossen_> ack fantasai <TabAtkins> fantasai: But there's different way to do that, and I think in many cases scaling outlines without affecting resolution of length units would also have that affect <TabAtkins> fantasai: wrt using it as a fallback, what happens if the other font doesn't load <TabAtkins> fantasai: This isn't "onlya pplies in a fallback font" <TabAtkins> fantasai: People on slow connections, etc are stuck with a font that's not just non-ideal but also has weird spacing <TabAtkins> fantasai: And this doesn't work for complex or cursive script as specified today. <TabAtkins> fantasai: You can handle those scripts if you scale, not if you intro space. <fantasai> TabAtkins: if your fallback font plus scaling advance looks terrible, you chose a bad fallback font <fantasai> TabAtkins: we don't need to protect author from that. They should tweak it to be closer. <fantasai> TabAtkins: If it is, it's a bad page. <jfkthame> instead of `font-size-override`, suggest something like `glyph-scale-factor`, then it's clearer that it doesn't affect anything else <fantasai> myles: What is the exist for this discussion? We've talked about several issues here. <myles> s/exist/exit/ <fantasai> TabAtkins: Trying to figure out how to fix this for awhile <myles> s/exit/exit criteria/ <fantasai> TabAtkins: keep getting objections that it's not ideal <fantasai> TabAtkins: but nobody is helping making a better solution <fantasai> TabAtkins: we did exploration with scaling, it doesn't lookg good <fantasai> TabAtkins: maybe we need to scale on some scripts <fantasai> TabAtkins: but essentially just letter-spacing works well <fantasai> fantasai: letter-spacing applies evenly <fantasai> fantasai: and if it's supposed to apply for temporary font, should shut off if you're not loading another font and sticking with this one <fantasai> TabAtkins: but then you have a layout shift <fantasai> fantasai: If downloading is turned off, then no layout shift <fantasai> myles: rogue character with accent in the middle of a paragraph causes ... <fantasai> Rossen_: Proposed path forward... not hearing a clear consensus around a) where the adjustment should take place or b) should we even have that <fantasai> Rossen_: I think we resolve in backwards order <TabAtkins> summary of myles: also useful if you have a fallback used for certain characters, which has an oddly different character size - this lets you adjust the fallback characters to feel better <fantasai> Rossen_: First question, should we still pursue this? <chris> OK so close this discussin with "needs proposal"? <fantasai> fantasai: I can live with having this plus a scaling factor, but not having just this one, because if only have a broken option that's not good enough <fantasai> myles: I think we should have both, changing scaling factor makes a lot of sense <fantasai> TabAtkins: Not hearing any objections to what we're doing here, other than fantasai's point. <fantasai> Rossen_: so no objection to continuing on this <fantasai> Rossen_: So specific proposal here, fantasai mentioned adding scaling factor and then we have to decide where space is added <fantasai> Rossen_: My proposal is to take back to the issue and iron it out there <Rossen_> ack fantasai <fantasai> fantasai: suggest to take a resolution on a) adding a scale-factor descriptor that only affects the glyph outline b) add space on both sides, maybe *allow* (MAY) UAs to do one side only if it's an implementation problem c) apply it to all glyphs in all writing systems, d) warn about how that can break things in certain writing systems / if too much space is added |
@xiaochengh Actually, thinking about it more, it shouldn't affect 'em' but probably should affect things that measure off the glyph like ex and ic. (There's a lot of use cases for per-font scaling adjustments that aren't about fallback.) |
For this issue based on the discussion above I propose that we
I'm not really happy about that last one, though. @xiaochengh @jfkthame Filed #6075 on glyph scaling, with answers to most of the questions I think you have. Lmk what you think. |
@fantasai Could you clarify how this works? Especially when we have a grapheme cluster (e.g., Edit: Maybe Latin is not a good example since many fonts just use single glyphs for letters with diacritics. How about CJK characters represented as ideographic description sequences? |
In the case of If the font didn't actually attach the glyph, but just used a zero-width glyph with negative left sidebearing, so that it "overstrikes" the preceding letter, then adjusting the advance of the base letter could have the effect of shifting the diacritic relative to its base. But I don't think that's a common situation. (Contrary to what I understood @litherum to be saying on the recent call, my current thinking is that it should be possible to apply |
Now I see how per glyph adjustment is supposed to work. It should be straightforward if spacing is only added behind each glyph. But, if we add spacing to both both sides, then the horizontal offset of the glyphs inside a grapheme cluster should also be adjusted. It should still work, though. @fantasai's last proposal looks acceptable to me, and I don't have further opinions. |
DO NOT COMMIT. Need to find an example of cursive script using grapheme cluster to connect glyphs. This patch implements the per-glyph version of advance-override following proposal [1], plus maintaining the relative position of each glyph within each grapheme cluster. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: I1802307e7e0ba1d08a28247843f6797d5b454d8c
DO NOT COMMIT. Need to find an example of cursive script using grapheme cluster to connect glyphs. This patch implements the per-glyph version of advance-override following proposal [1], plus maintaining the relative position of each glyph within each grapheme cluster. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: I1802307e7e0ba1d08a28247843f6797d5b454d8c
Can we consider emoji type fonts? |
I'm prototyping @fantasai's proposal, which is very implementable. And I found one detail that I'd like to discuss regarding whether we break grapheme clusters. Consider Bengali text This is because a grapheme cluster (e.g., (Btw the situation seems common in many Indic and related scripts) One thing I like about the proposal is that, it affects the text width (and hence CLS) in a very predictable way: the result text width is scaled by exactly the given percentage. So I'd like to do some small tweaks to make the text layout look better, while preserving this property. The result on the previous Bengali text looks like: (Note that the total text width is exactly the same as before) The tweak is basically adjusting the total advance of each grapheme cluster as a whole, instead of glyphs:
Similar as before, the adjustment is unconditionally applied to each grapheme cluster after shaping. This also handles simpler cases like diacritics well, since diacritic glyphs have zero advance: Unfortunately, cursive joining of e.g. Arabic may still be broken because each letter is a single grapheme cluster (at least when using Noto Naskh Arabic). This seems unfixable, because after shaping, there's no way to know whether adding spacing between two grapheme clusters breaks cursive joining or not. The situation is the same as when applying |
@yisibl I think all proposals here naturally handle emoji fonts well, because emoji fonts are monospaced |
fyi @r12a please see #5983 (comment) from @xiaochengh above. |
I agree with @xiaochengh's comment about applying to grapheme clusters rather than individual characters; this will generally be less broken. However, I also agree with @fantasai's point that I believe that the use cases that prompted the invention of @xiaochengh has already experimented with scaling the font size instead of overriding glyph advances, and reported that it does seem to work better. I have also prototyped such a feature in Gecko and it seems to work as expected there. In the light of this, I propose that we should reverse the resolution in #5533 to add the |
The CSS Working Group just discussed
The full IRC log of that discussion<dael> Topic: [css-fonts-5] advance-override details<dael> github: https://github.com//issues/5983 <fantasai> https://github.com//issues/5983#issuecomment-800123577 <dael> fantasai: Where we're at is jfkthame last comment ^ <Rossen_> q? <dael> fantasai: Suggestion is reveerse a resolution, drop advance-override and focus on scaling descriptor in 6075 <dael> chris: Good prop. Had a lot of problems with advance-override. This seems better approach <dael> TabAtkins: Last I heard from chrishtr yesterday, this is reasonable to us <dael> fantasai: And [missed] agrees <fantasai> s/[missed]/xiaocheng/ <dael> Rossen_: Objections? <dael> RESOLVED: Reverse the resolution and drop advance-override |
Since CSSWG has reversed the resolution on advance-override [1], there shouldn't be tests in WPT. Hence, this patch moves the tests from WPT to Blink's wpt_internal folder, as a preparation for a final removal when the spec edit has settled. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: If6e8b2f7f739e9555f0aa92a72cc83327b550566 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2774143 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#864490}
Since CSSWG has reversed the resolution on advance-override [1], there shouldn't be tests in WPT. Hence, this patch moves the tests from WPT to Blink's wpt_internal folder, as a preparation for a final removal when the spec edit has settled. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: If6e8b2f7f739e9555f0aa92a72cc83327b550566 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2774143 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#864490}
Since CSSWG has reversed the resolution on advance-override [1], there shouldn't be tests in WPT. Hence, this patch moves the tests from WPT to Blink's wpt_internal folder, as a preparation for a final removal when the spec edit has settled. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: If6e8b2f7f739e9555f0aa92a72cc83327b550566 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2774143 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#864490}
…wpt_internal, a=testonly Automatic update from web-platform-tests Move advance-override tests from WPT to wpt_internal Since CSSWG has reversed the resolution on advance-override [1], there shouldn't be tests in WPT. Hence, this patch moves the tests from WPT to Blink's wpt_internal folder, as a preparation for a final removal when the spec edit has settled. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: If6e8b2f7f739e9555f0aa92a72cc83327b550566 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2774143 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#864490} -- wpt-commits: c5794e1d395729bd3d7a333aa8cf6bde10a4bb5e wpt-pr: 28145
Since CSSWG has reversed the resolution on advance-override [1], there shouldn't be tests in WPT. Hence, this patch moves the tests from WPT to Blink's wpt_internal folder, as a preparation for a final removal when the spec edit has settled. [1] w3c/csswg-drafts#5983 (comment) Bug: 1137633 Change-Id: If6e8b2f7f739e9555f0aa92a72cc83327b550566 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2774143 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#864490} GitOrigin-RevId: 6f2e7b414588fc0cb14e2ad057208d614bc8933e
We resolved to add an advance-override descriptor in #5533 but didn't define quite what it means. Here are some fun questions:
The text was updated successfully, but these errors were encountered: