[Impeller] Vulkan: Top line of text is cropped sometimes in wonderous app

[gaaclarke]

description

I've seen this with the opengles backend and the vulkan backend. It's shown up in a couple places in the app, not just here.

I checked against the skia backend and this was not a problem.

doctor

[✓] Flutter (Channel main, 3.12.0-3.0.pre.44, on macOS 13.4 22F66 darwin-arm64, locale en)
    • Flutter version 3.12.0-3.0.pre.44 on channel main at /Users/aaclarke/dev/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision e84fa2ce47 (4 hours ago), 2023-06-09 23:06:21 +0500
    • Engine revision bc6e047570
    • Dart version 3.1.0 (build 3.1.0-180.0.dev)
    • DevTools version 2.24.0

engine version

64d0cc34e7949bd6e5b3891670e124c5cb233fa0

os

Android

screenshot

cc @bdero

[bdero]

Is this happening on the Metal backend too? If that's the case, then this is definitely a regression introduced after the 3.10 branch cut.

[bdero]

I checked wondrous at engine head and am not reproducing it on iOS.

[jonahwilliams]

Please check if this is fixed after flutter/engine#42746 lands

[jonahwilliams]

(I cannot repro it before or after, but it probably depends on screen size)

[chinmaygarde]

Closing as fixed in flutter/engine#42746. We can re-open if this happens again.

[jonahwilliams]

I can repro this, will need a slightly different fix

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

[jonahwilliams]

Also does not repro on iPhone 13.

[jonahwilliams]

Not only that, but the text itself is jittery as I scroll which does not happen on Pixels either. I wonder if there is some sort of "helpful" correction being applied somewhere? Though I'm not sure where that would be.

[jonahwilliams]

From running locally, I can see that the glyphs in the atlas are rendered right against the edge of the glyph atlas, obviously cutting off parts of the glyph and/or antialiased pixels on some of them. Given that we should be rendering everything with 2px padding it seems like something is off...

[jonahwilliams]

Something I am noticing is that on Android Skia starts from the bottom of the atlas...

[chinmaygarde]

Perhaps this is caused by incorrect UV mapping needing after we added the 2px padding? Also, atlases on Android generated by Skia won't use the CoreText. There could be differences in glyph AA in atlases. So maybe we need more padding perhaps?

[jonahwilliams]

I tried adding more padding but it made no difference :( . I think the glyph bounds themselves are wrong or need some other adjustment

[jason-simmons]

Reproduced this on a Pixel 3.

The glyphs render correctly if I remove the sk_font.setHinting(SkFontHinting::kSlight) call in text_render_context_skia.cc.

[jonahwilliams]

FYI @bdero

IRC the goal of adding that flag was better fidelity on iOS at the time, but obvs its not helping us on Android so we could disable it with an ifdef for now.

That said, we should probably figure out how to handle these flags correctly.

[jonahwilliams]

If I comment out that line, then I can still reproduce this issue on an S10 on Flutter gallery. ☹️

[chinmaygarde]

Just adding a note that glyph offsets on Android are also reproducible with stroked text (perhaps even more so?). This is perhaps the same underlying issue.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

[jason-simmons]

Looked into the behavior of the flutter/engine@4ad970f patch.

Skia is rounding the font metrics to integers in the SkScalerContext. Specifically, when SkScalerContext constructs an SkGlyph it calls SkScalerContext::SaturateGlyphBounds, which does an SkRect::roundOut on the glyph's bounds rectangle.

flutter/engine@4ad970f is changing the font's size to 100000 to compute metrics and then scaling the metrics down to the original font size. With larger metric values, there will be less loss of precision due to the SkScalerContext rounding.

When given a huge font size like that Skia actually builds the glyphs at a smaller size and then applies scaling to produce the large size. SkStrikeSpec::MakeCanonicalized calls ShouldDrawAsPath, which checks whether the font exceeds various limits that require path rendering. If so, MakeCanonicalized creates a font using a standard size for path rendering. That size is SkFontPriv::kCanonicalTextSizeForPaths, which is defined as 64.

Impeller should get the same results by obtaining the metrics from a font of size 64 instead of 100000. Or Impeller may be able to further improve the precision by using a size larger than 64 but less than the threshold that triggers ShouldDrawAsPath.