Font rendering (line-height) in 2024.3.X

[MercinaM]

Describe the bug
Line height calculations seem to be behaving very differently in the 2024.3.X versions than they did in 2023.12.6 and before, in a way that seems like it might be a bug.

When using a line height of 1, multiple lines will now overlap, which is something I would only expect with a line height of less than 1 (but I am not an expert).

In 2023.12.6 multiple lines of text did not overlap with the same line height.

To Reproduce

public void Compose(IDocumentContainer container)
{
    container.Page(page =>
    {
        page.Margin(10f, Unit.Millimetre);

        page.DefaultTextStyle(TextStyle
            .Default
            .FontFamily(Fonts.SegoeUI)
            .LineHeight(1f)
            .FontSize(FontSizeConverter.CrystalVerdanaToQuestSegoeUI(9))
            .NormalWeight());

        page.Content().Column(column =>
        {
            column.Item().Text("Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed non dictum est.").BackgroundColor("#FF0000");
            column.Item().Height(5f, Unit.Millimetre);
            column.Item().Text("(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)(g)(j)(q)");
            column.Item().Text("OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO");
        });
    });
}

Expected behavior
I don't know what the expected behaviour is. I realize the 2024.3.X branch has an entirely new text renderer, and the way line heights are calculated might just be the way things work now. If so, we can obviously work around the issue by re-adjusting the line heights in our documents. But we are holding off on updating for now until this is clarified.

Screenshots

Before:

After:

Environment
Library version: 2024.3.1
OS: Windows 11

Additional context
Add any other context about the problem here.

[ebarnard]

I've noticed this as well when looking to upgrade. It seems like font height measuring has changed from top to bottom, to ascent to descent (as shown in the image below).

Source: https://stackoverflow.com/questions/27631736/meaning-of-top-ascent-baseline-descent-bottom-and-leading-in-androids-font

Another example of this is a header with filled background component that is less tall in 2024.3.X compared to previous versions (no changes have been made aside from QuestPDF version).

[MarcinZiabek]

Thank you for noticing those changes 😄

Indeed, the text engine has been rebuilt from the ground up in the 2024.3.X release. Now, it is based on the SkParagraph module from the Skia project (used in Flutter) and should be more compliant with various Unicode standards (including better handling of RTL languages, text bidirectionality, splitting text to a new line, and applying text styles).

While I am confident that Skia SkParagraph is a superior solution, I am also aware that I could have made certain mistakes during the integration. Your feedback on this would be greatly appreciated.

@MercinaM While experimenting with CSS in the web browser, I found that the Verdana font behaves this way (putting certain glyph parts below the bottom line). It looks ugly, but does it seem to be the intended way?

@ebarnard Do you think it is correct to extend the first line to use top; and the last line to use bottom? I am just wondering what solution is proper from a typography perspective. I am for consistency with other technologies (Flutter, HTML, CSS, Word).

I am happy to collaborate on this topic.

[MercinaM]

@MarcinZiabek I may be a little out of my depth here 😄.

That said, I did test how Browsers (tested in Firefox, but Chrome seems to behave the same way) handle the very same example (Segoe UI, line-height: 1), and I found that the way 2024.3.1 handles is mostly consistent (it does seem to be just slightly more squished together), and the way 2023.12.6 handled it was actually very inconsistent, so the new renderer may very well be the better choice in that regard:

2023.12.6

2024.3.1

Firefox (HTML)

Like I said, I don't actually know what the correct way of handling this is, I was mostly just looking for confirmation that this is intended behavior before I start modifying our documents.

[ebarnard]

@MarcinZiabek I think @MercinaM is correct that 2023.12.6 did not lay out text in a "standard" way, but I'm not sure 2024.3.1 is correct either.

There seems to be a mismatch between how QuestPDF line-height is and how SkParagraph uses it. The example in the post above isn't too bad, but the SegoeUI example in the first post definitely seems incorrect.

It looks like font files include a default "leading" value which describes the spacing between adjacent lines. Typically this is the specified as the distance between the descent of one line and the ascent of the following line.

QuestPDF unconditionally sets SkParagraph's font height override/multiplier to be the line height, and uses 1 if the user does not specify a line height.

https://github.com/QuestPDF/QuestPDF.Native/blob/c9f384f08bf8941b7aa88ba33832420e668ebd1a/native/src/text/textStyle.cpp#L70C1-L71C40

This overrides the default leading specified in the font file (below and in other places in SkParagraph)

https://github.com/google/skia/blob/aeab79038011b6e3869834ebd2577da37859527f/modules/skparagraph/src/Run.cpp#L65C1-L67C6

The fix would seem to be to not override the font's default leading if the user does not specify a line height, and allow passing a line height of null or adding a DefaultLineHeight() method to reset any previously set line height.

[ebarnard]

I forgot to add why I think we should use the font's default leading/line height by default - in CSS the default value of line-height is normal which AFAIK uses the font's default built-in leading/line height.

[ebarnard]

The other thing is that we should ensure fUseHalfLeading is always true here and elsewhere in SkParagraph

https://github.com/google/skia/blob/aeab79038011b6e3869834ebd2577da37859527f/modules/skparagraph/src/Run.cpp#L65C1-L67C6

This also matches the behaviour of CSS where text remains centered in its parent element as you increase line-height (see jsfiddle below). This might already be the case.

https://jsfiddle.net/stao857n/

[MarcinZiabek]

I forgot to add why I think we should use the font's default leading/line height by default - in CSS the default value of line-height is normal which AFAIK uses the font's default built-in leading/line height.

This is an important observation. Indeed, in CSS, the line-height property set to 1 or 100% produces slightly different results than the default normal value.

In Chrome, line-height set to normal:

And to 100%:

If I am not mistaken, in QuestPDF, the default line-height value (calculated with rules up to 2023.12.X revision) roughly corresponds to normal.

In new releases (starting from 2024.3.X), the library can follow default font metrics when the line-height is set to null, and also use null as a default.

[MarcinZiabek]

The 2024.3.3 release fixes the font line-height rendering problem. Would you please give it a try?

Also, let me express my gratitude for helping me understand and resolve this problem. You are great! ❤️

[ebarnard]

Default line height seems to be working sensibly in 2024.3.3.

Spacing around text is still not as expected. We should be using half leadings. Skia's default behaviour is to add the extra leading between lines unevenly so text is no longer centered in its parent container. This is the issue mentioned in #863.

We should be using half leadings as it's the behaviour used by HTML/CSS and is a lot more intuitive. If you draw some text with a line hight set to 2, and then draw a box around it, the text should be centered in the box.

This is done by setting setHalfLeading(true) on SkParahraph's TextStyle.

[blogcraft]

2024.3.4 fixed a lot of text differences for me!

[MarcinZiabek]

In 2024.3.4, I followed @ebarnard suggestion. Applying half-leading indeed produces much better results.

@ebarnard Thank you for investigating this issue and helping me localize the appropriate enhancement. I highly appreciate your help!

[ebarnard]

Thanks @MarcinZiabek, 2024.3.4 looks so much better.

I had a look at the change to QuestPDF.Native and I think we should always be using half leading, not just when line height is specified manually. When line height is automatic, there is still a line height which is set by the font, and in that case I think we still want text with automatic/default line height to be centered within its parent container.