Skip to content
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

'line-height: normal' definition should reflect reality of determining based on fonts #1254

Open
dbaron opened this issue Apr 20, 2017 · 10 comments
Labels

Comments

@dbaron
Copy link
Member

dbaron commented Apr 20, 2017

The definition of line-height: normal says:

Tells user agents to set the used value to a "reasonable" value based on the font of the element. The value has the same meaning as <number>. We recommend a used value for 'normal' between 1.0 to 1.2. The computed value is 'normal'.

This needs to explain what happens when font fallback occurs and multiple fonts are used. Implementations use font metrics to get the normal line-height (at least some use external leading or line gap), and these metrics can be different in different fonts. There may also be an interesting interaction with the definition of the content height, which separately considers font fallback.

More research is needed here as to what implementations do and what they should do, in particular:

  • what font metrics are used
  • how the metrics from multiple fonts are combined, and whether this has an interaction with the computation of the content box height or whether it occurs entirely separately

/cc @litherum

@dbaron
Copy link
Member Author

dbaron commented Apr 20, 2017

This is related to the discussion that the working group is having right now.

@dbaron
Copy link
Member Author

dbaron commented Apr 21, 2017

I'd also note that CSS2 10.8.1 says:

Still for each glyph, determine the leading L to add, where L = 'line-height' - AD. Half the leading is added above A and the other half below D, giving the glyph and its leading a total height above the baseline of A' = A + L/2 and a total depth of D' = D + L/2.
The height of the inline box encloses all glyphs and their half-leading on each side and is thus exactly 'line-height'. Boxes of child elements do not influence this height.

This model seems to suggest that if we add external leading from fonts for normal line height, we should probably do so separately for each glyph, i.e., using a per-glyph line-height when computing L in the algorithm above.

Gecko's implementation has a number of bugs described in bug 1358377, so I'm not sure it's particularly useful in determining what to do here. Information from other implementations may be more useful.

@kojiishi
Copy link
Contributor

kojiishi commented Apr 21, 2017

Blink does for line-height: normal:

  1. Collect used fonts for the run.
  2. For each used fonts:
    1. Compute A and D from the font metrics (note, IIUC, our A/D are slightly different from Gecko's)
    2. Compute L/2 using line spacing from the font metrics ((line spacing - (A + D)) / 2)
  3. Compute the union of A' (A + L/2) and D' (D + L/2) of all used fonts.

I believe this is mostly the same as WebKit.

@kojiishi
Copy link
Contributor

Note, the "used fonts" include system fallbacks (i.e., that are not in font-family.)

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed 'line-height: normal' definition should reflect reality of determining based on fonts, and agreed to the following resolutions:

  • RESOLVED: add leading to union of ascent and descent
  • RESOLVED: Replace definition of 'line-height: normal' with vaguer definition.
The full IRC log of that discussion <astearns> topic: 'line-height: normal' definition should reflect reality of determining based on fonts
<astearns> github topic: https://github.com//issues/1254
<surma> Florian: we were trying to do things that depend on normal and it didn’t match reality
<surma> dbaron: spec says that when you have glyphs from multiple fonts, where the fonts have different ascend and descend, the glyphs end up with different boxes
<fantasai> s/ascend/ascent/
<fantasai> s/descend/descent/
<fantasai> dbaron draws Chinese character and capital B, to represent glyphs from two fonts
<surma> dbaron: spec says that when you draw the background you use the lowest bottom and the highest top
<fantasai> dbaron draws misaligned boxes, representing the two different fonts that are triggered by fallback
<fantasai> dbaron labels height of tallest top to bottommost bottom as "content box"
<surma> dbaron: you add half of the ??? to each side (top/bottom) of the box
<fantasai> dbaron draws leading on each glyph; shorter glyph gets more half-leading on each side than taller glyph
<fantasai> dbaron labels distance from topmost edge of leading to bottommost leading edge as "line box contribution"
<surma> ScribeNick: fantasai
<fantasai> dbaron: Spec for normal says to use a value between 1.0 and 1.2 as normal line-spacing
<fantasai> dbaron: This is not the Gecko behavior
<fantasai> dbaron: This is what the spec says
<fantasai> Florian: Spec contradicts itself
<fantasai> dbaron: Spec describes this behavior, and then says "therefore you get X" where X is not what you get out of this behavior
<fantasai> [confusion about earlier resolution from yesterday]
<fantasai> Florian: The other option was to reduce this line box contribution to just the value of the leading
<fantasai> Florian: which was, iirc, what Safari does,
<fantasai> (which is what Florian and fantasai thought we had resolved on yesterday)
<fantasai> myles: We never add padding to individual boxes. We do ceiling and flooring and then add leading at the end
<fantasai> dbaron: I'm fine with resolving on this, does make this other issue simpler
<fantasai> myles: I thought we resolved already
<fantasai> Florian: dbaron and Alan thought we resolved the other way
<fantasai> dbaron: 3d paragraph on 10.8.1 says to add half leading to each glyph. We're saying to add it to the content box
<fantasai> fantasai: Content box is not well-defined
<fantasai> dbaron: It's in 10.6.1.
<fantasai> dbaron: I guess it's not actually defined.
<fantasai> dbaron: So define that you add leading to union of ascent and descent
<fantasai> fantasai: What's an em box
<fantasai> dbaron: The thing we resolved gets you 15px lines, but only if you don't have a <span> in your line
<fantasai> RESOLVED: add leading to union of ascent and descent
<fantasai> dbaron: What we're talking about is per inline box fragment
<fantasai> dbaron: Goal was to make lines 15px high
<fantasai> dbaron: This resolution will accomplish that given font fallback, as long as you have no markup whatsoever and every line is just a single box fragment
<fantasai> dbaron: The moment you have both markup and different font fallback in the different elements, it fails
<fantasai> dbaron: If you have for example
<fantasai> Get a taxi with <span>京B</span>
<fantasai> dbaron: Now you're computing the line-spacing for first piece separate from second piece
<fantasai> koji: In Blink, we use primary font to resolve content box
<fantasai> dbaron: We decided not to do that because wanted to consider all the glyphs
<fantasai> myles: If you have an element ... your entire line is that element
<fantasai> myles: what's primary font vs secondary font?
<fantasai> koji: If this is the example and no stying on <span>
<fantasai> koji: Then these two have same font family, then primary font is the same
<fantasai> koji: Even if this span starts from different font family, use the same font
<fantasai> dbaron: So you're saying you use the first available font
<fantasai> koji: If line height is not normal, yes
<fantasai> myles: Are you saying that you ignore the metrics of the non-primary font?
<fantasai> koji: yeah
<fantasai> myles: That's not what WebKit does
<fantasai> myles: At least I don't think so
<fantasai> koji: when we say we compute only once per inline box fragment, compute is based on which fonts?
<fantasai> Florian: Union of all the boxes
<fantasai> [more confusion]
<fantasai> dbaron redraws
<fantasai> dbaron: Let's say these are 16px glyphs and you want 20px line height
<fantasai> dbaron: with union thing, we take the tallst top of glyph, and lowest bottom of glyph
<fantasai> dbaron: Distance here is 19px
<fantasai> dbaron: To get to 20px, we add .5px to top and to bottom
<fantasai> dbaron: In some ways this isn't great, because it gives you baseline jitter when you have font fallback
<fantasai> dbaron: If you use only the primary font and not the others, you will not get baseline jitter
<fantasai> Florian: But you may get overlap
<fantasai> Florian: I think baseline jitter is preferable to overlap
<fantasai> dbaron: Generally the variation is not a lot, so unlikely to get overflow in most cases
<fantasai> dbaron: Unless you push your line height really close to 1.0
<fantasai> astearns: ?
<fantasai> myles: If you a really big paragraph, many lines of text, and one line in the middle has a character from a fallback font
<fantasai> dbaron: That will push the baseline of that line down, because centering union within the line height (19px) rather than just the primary font (16px)
<fantasai> myles: Webkit does that; we just have a little bit of jitter
<fantasai> dbaron: One other factor here...
<fantasai> dbaron: If we did only primary font, line-height-step would give you evenly-spaced baselines
<fantasai> dbaron: But if we don't, then even line-height-step won't give you evenly spaced baselines
<fantasai> myles: So the problem then is that if you use line-height-step, this middle line with the character, might trigger a double-spaced line
<fantasai> astearns: Sounds like maybe previous resolution, we don't want anymore
<fantasai> dbaron: Also could consider whether content-box should use primary font
<fantasai> dbaron: Let me finish
<fantasai> explanation
<fantasai> dbaron: There's a value called normal
<fantasai> dbaron: In theory this is a number
<fantasai> dbaron: But actually this is not what Gecko does
<fantasai> dbaron: Font has a metric that says what they thing the line spacing should be
<fantasai> myles: field is called line gap
<fantasai> dbaron: You could get the metric from the font and apply it to the glyph in that font, and then do the same with the other font to the other glyph
<fantasai> dbaron: And that would be easy with the previous behavior with per-glyph leading instead of union leading
<fantasai> Florian: Could do per-glyph leading and union it
<fantasai> dbaron: What gecko does is more horrible
<fantasai> dbaron: Gecko for line-height: normal
<fantasai> dbaron: Does per-glyph bounding box computation without external leading
<fantasai> dbaron: Then unions that with external leading of the primary font added to the glyph height of the primary font
<fantasai> dbaron: I don't think this was intentional
<fantasai> dbaron: so Gecko will take union of glyph boxes, and then unions that with leading box of the primary font
<fantasai> fantasai: maybe not so bad, gets you more regular spacing line to line, while still trying to avoid overlap by considering union of all glyph areas
<fantasai> Florian: I thought Koji had brought up earlier that for some languages there are very tall glyphs, and you may have that kind in the fallback
<fantasai> Florian: So might have something very tall and has external leading to get that to look nice
<fantasai> Florian: In these cases glyph can go beyond ascent and descent
<fantasai> Florian: Are these font metrics reliable
<fantasai> myles: Let's not discuss that
<fantasai> astearns: So...........
<fantasai> Florian: So spec says "line height number is like a numbe,r you just get to pick the number". This is not true
<fantasai> koji: Says use value that's appropriate to the font
<fantasai> koji: ...
<fantasai> koji: Second sentence is very questionable
<fantasai> dbaron: The sentences might ahve been written 10 years apart, as we edited CSS2
<fantasai> koji: “font of the element” is not defined
<fantasai> koji: earlier paragraph says UA may take all the fonts
<fantasai> used in the element
<fantasai> koji: this sentence doesn't make sense to me (second sentence)
<fantasai> koji: so I ignored it
<fantasai> koji: you read second sentence
<fantasai> myles: Should be fonts, not font, of the element
<fantasai> Florian: That's an improvement, but still undefined
<tantek> Indeed this has felt underspecified to me too
<fantasai> Florian: Based on it, but based how?
<fantasai> astearns: Vague is better than wrong
<fantasai> astearns: let's resolve to remove wrong parts
<fantasai> Florian quotes spec
<fantasai> discussion of prose
<fantasai> Spec is all wrong
<fantasai> Need to replace the entire definition
<fantasai> alan asks where new prose goes
<fantasai> fantasai: Errata CSS2, define in css-inline-3
<fantasai> myles: The reason we're discussing all of these computations is because if they were to differ you might get double spacing with line-height-step
<fantasai> myles: Because font metrics differ between browsers *and* platforms, will still get differences
<fantasai> Florian: Because we don't have a foundational model from which to discuss issues
<fantasai> koji: Changing defintion for non-normal case is easier.. doesn't chnage layout
<fantasai> koji: Just changes glyph position within the line
<fantasai> koji: but if we change definition of normal, might break lots of sites
<fantasai> astearns: Well, we need to change the definition in the spec in any case, because it's not matching what browsers do
<fantasai> astearns: So proposed resolution is to remove wrong definition of normal, and replace with more accurate definition
<fantasai> fantasai: Will need to be vague
<fantasai> astearns: Any objection to a vague but not wrong defintion of 'line-height: normal'?
<fantasai> RESOLVED: Replace definition of 'line-height: normal' with vaguer definition.
<fantasai> dbaron: Basically need to say that 'normal' does something based on the font that overrides algorithm for number
<fantasai> myles: Say all the fonts may be consulted, and line gap of all the fonts may be consulted
<fantasai> ACTION fantasai: make wording
<trackbot> Created ACTION-849 - Make wording [on Elika Etemad - due 2017-04-28].
<fantasai> Florian: Previous resolution, koji said it's not what they do
<fantasai> dbaron: Matches Safari, not blink or gecko
<fantasai> dbaron: Makes rhythm better wrt what spec previously said, but worse wrt blink/gecko
<fantasai> dbaron: Also only helps if you have no elements
<fantasai> discussion of what's better than what
<fantasai> dbaron: It could lead to glyph box overlap where there wasn't before
<fantasai> dbaron: You could end up now with negative half-leading
<fantasai> dbaron: Because before leading would definitely be positive
<fantasai> astearns: My understanding was that we were going to size on the content box
<fantasai> dbaron: content-box is should, not definite
<fantasai> astearns: need to discuss content box...
<fantasai> astearns: Does that affect the previous resolution?
<fantasai> dbaron: Other factor with content box
<fantasai> dbaron: tend not to have padding and border, but backgrounds is common
<fantasai> dbaron: Whether to include fallbak glyphs in content box...
<fantasai> dbaron: Do you want fallback glyphs to potentially overflow content box?
<fantasai> dbaron: Or do we want to make sure content boxes are consistent across multiple elements?
<fantasai> koji: I would really want to make heights of lines consistent. Don't care so much about positioning within the line
<fantasai> Florian: If we don't do the resolution we've just taken, the line height may be bigger than the one you asked for. With this resoution, if you say line-height: 20x, you always get 20px
<fantasai> koji doesn't believe this, so Florian is trying to explain it again
<fantasai> ...
<tantek> There is an inevitable design tension between "do exactly what I said" and maintaining a strict rhythm, and avoiding overlapping pixels / text. Not sure how that tension can be resolved in a predictable way. Especially with cross-platform/browser font differences, font substitutions etc.
<tantek> I don't have a specific suggestion, but I'm curious what methodology the group uses to try to resolve this tension.
<fantasai> tantek, in general we tend to go with "make it readable" over "make it pretty"
<fantasai> There are several diagrams on the board now
<tantek> fantasai, I agree with that. I'm more wondering about the "[ CSS is Awe ] some" type problems
<fantasai> Blue one represents "use the primary font for size and position"
<fantasai> Black one is "apply leading to each glyph and take union of all"
<fantasai> Red one is "union glyph boxes and apply leading (could be negative)"
<fantasai> dbaron: Red and blue will both give consistent line sizes, but blue will give consistency even when you have <span>s
<tantek> fantasai, cool re: diagrams. I'll check the archives later for that. Thanks and good luck!
<fantasai> In blue and Red cases, could get overflow...
<fantasai> in blue case the overflow is all wrt primary font. Red case clips a bit from tallest and lowest bits of the combination
<fantasai> Florian summarizes and asks Rossen, who suspects they don't ignore fallback so probably black or red
<fantasai> dbaron notes this is for non-normal values
<fantasai> astearns: We should have description in CSS2.1 with what browsers do, so we're actually describing reality
<fantasai> ACTION Rossen Verify what is done by each browser (by checking Edge and putting on WG to-do list for everyone else)
<trackbot> Created ACTION-850 - Verify what is done by each browser (by checking edge and putting on wg to-do list for everyone else) [on Rossen Atanassov - due 2017-04-28].
<fantasai> astearns: Anything else?
<fantasai> Meeting closed.

@astearns
Copy link
Member

astearns commented Apr 21, 2017

@dbaron
Copy link
Member Author

dbaron commented May 2, 2017

For what it's worth, by the end of the discussion above in #1254 (comment), I'm not sure we still agreed with the first of the two resolutions, although we hadn't resolved on something to replace it. (It was perhaps a little prematurely recorded as a resolution?)

@frivoal
Copy link
Collaborator

frivoal commented May 3, 2017

Right. I believe that RESOLVED: add leading to union of ascent and descent was premature, and what we now need is to check whether the various implementations are doing the blue, black, or red thing in the drawing, then assuming they're not all doing the same thing, have another round of discussions to figure out which one we can agree to align on.

@astearns @atanassov, do you want to distribute actions to the various browser vendors to go check that?

@fantasai
Copy link
Collaborator

Minutes of the 2017 Tokyo F2F discussion
Minutes of 2017 Paris F2F discussion
Minutes of 2017 Sept 27 telecon
Minutes of 2017 SF F2F discussion = resolutions on definitions of line-height calculations

Meta-bug on line-height issues

I believe most of this is captured in the current CSS Inline Layout draft in this section, which derives from CSS2.1§10.8: https://drafts.csswg.org/css-inline-3/#inline-height

What wasn't covered there was how the font's own line-gap/line-height metrics are handled. It's clear from the discussion that when they apply, the gap is split in half, and half applied above and half below the baseline. What's not entirely clear to me is if this is done:

  • only when computing the height of the line box
  • also when establishing the inline’s content edges (for background painting)
  • also when aligning by vertical-align: text-top

I suspect we only want the first one, so I've gone ahead and made that explicit.

Agenda+ to review this all with the CSSWG.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed line-height: normal' definition should reflect reality of determining based on fonts, and agreed to the following:

  • RESOLVED: publish updated working draft of CSS-Inline
The full IRC log of that discussion <Rossen_> Topic: line-height: normal' definition should reflect reality of determining based on fonts
<Rossen_> github: https://github.com//issues/1254
<dauwhe> fantasai: lots of talk about what line-height: normal does
<dauwhe> ... I went through the minutes, and incorporated into the spec
<dauwhe> ... one question: how fonts own line gap metrics are handled
<dauwhe> ... when they are there, we split the gap so half above and half below
<dauwhe> ... I think it only affects the height of the line box
<florian> q+
<dauwhe> .... so I put that in the spec
<dauwhe> ... I wanted to confirm that with the WG
<dauwhe> ... that what is now in the spec is acceptable
<dauwhe> myles: is this a question of researching of what browsers do? Make fonts with varying line gaps and see what happens?
<dauwhe> florian: at least start with finding what it is
<dauwhe> dbaron: my memory is that gecko's choice of whether to use the line gap metric is complex
<dauwhe> florian: I agree that line gap does not affect height of content box
<dauwhe> ... I didn't test vertical align
<florian> q-
<dauwhe> myles: rather than asking us to remember what our browsers do, better to test the browsers?
<dauwhe> ... I'm happy to make a bunch of fonts to help
<dbaron> s/complex/complex, and that what it affects is what number line-height: normal means/
<dauwhe> florian: we can split it up. we'll make fonts and test
<dauwhe> Rossen_: how shall we resolve? try to match reality of current implementations?
<dauwhe> fantasai: we can leave the issue open until we have definitive test results
<dauwhe> ... I'll make sure that what dbaron said is allowed
<dauwhe> ... the rest is already in the spec
<dauwhe> ... if people want to review, that would be great
<dauwhe> ... and I'll ask if I should publish a new WD?
<myles> q+ to talk about 90 minute meetings
<dauwhe> Rossen_: a new WD would include edits with today's resolutions?
<dbaron> I don't think I mentioned using the line gap metric of a different font...
<dauwhe> fantasai: yes, I'll make those edits before I publish
<dauwhe> Rossen_: any thoughts?
<dauwhe> q?
<dauwhe> Rossen_: hearing no pushback
<dauwhe> RESOLVED: publish updated working draft of CSS-Inline
<dauwhe> myles: 90 minutes is too long for m
<dauwhe> ...e
<dauwhe> ... we made good progress
<dauwhe> ... don't we usually just let the agenda overflow
<dauwhe> ... I'd rather have lots of topics for the future
<dauwhe> fantasai: I had things lined up this week, which was why
<fantasai> s/things lined up/deadlines/
<dauwhe> Rossen_: most people had similar sentiments, Myles
<dauwhe> ... maybe we don't repeat this, unless at F2F
<dauwhe> ... there's a thread on Private ML about upcoming F2F planning
<dauwhe> ... thanks everyone
<fantasai> I also figured that the line sizing topic would take awhile and wanted to make sure we had the time to dig into it rather than skimming over it over the course a few calls
<dauwhe> I think this helped

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants