Fixed translated text's subpixel alignment. #162824

gaaclarke · 2025-02-06T18:53:38Z

Previously we weren't calculating the subpixel offset correctly. We weren't using the transform of the object being drawn to get global coordinates, now we are.

Video of #149652 after PR

gotit.mov

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gaaclarke · 2025-02-06T22:33:03Z

There is some cleanup that needs to come to the font rendering code. I'm not happy with how this is all laid out but this is the minimal change to fix the issue. We can consider refactors later.

I want to remove scale and offset from TextFrame and just rely on the transform
I don't like the text frame "per frame data", that should just come in the arguments instead of getting stored on the text frame

gaaclarke · 2025-02-06T22:33:48Z

engine/src/flutter/impeller/display_list/dl_golden_unittests.cc

+
+// Checks that the left most edge of the glyph is fading out as we push
+// it to the right by fractional pixels.
+TEST_P(DlGoldenTest, SubpixelScaledTranslated) {


This is the test that would actually fail before this change, the other passed.

gaaclarke · 2025-02-06T22:43:12Z

engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc

@@ -452,7 +452,8 @@ TypographerContextSkia::CollectNewGlyphs(
      for (const auto& glyph_position : run.GetGlyphPositions()) {
        Point subpixel = TextFrame::ComputeSubpixelPosition(
            glyph_position, scaled_font.font.GetAxisAlignment(),
-            frame->GetOffset(), frame->GetScale());
+            frame->GetTransform() *
+                Matrix::MakeTranslation(frame->GetOffset()));


This is annoying, the transform here doesn't include the offset, but when you go to render it in TextContents it has the offset included.

jonahwilliams

LGTM!

jonahwilliams · 2025-02-07T00:22:36Z

engine/src/flutter/impeller/display_list/dl_golden_unittests.cc

+  const uint32_t* ptr = reinterpret_cast<const uint32_t*>(img->GetBytes());
+  for (size_t i = 0; i < img->GetHeight(); ++i) {
+    for (int32_t j = 0; j < static_cast<int32_t>(img->GetWidth()); ++j) {
+      if (((*ptr & 0x00ffffff) != 0)) {


yes, of course

indeed, sir

gaaclarke · 2025-02-07T01:54:28Z

Weird, something is going on with the vulkan renderer where we are getting zero values in the blue channel. We should look into that. For now I switched to the green channel which works as expected locally.

auto-submit · 2025-02-07T01:55:25Z

autosubmit label was removed for flutter/flutter/162824, because - The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-02-07T04:56:24Z

autosubmit label was removed for flutter/flutter/162824, because - The status or check suite Linux linux_web_engine_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

flutter-dashboard · 2025-02-07T06:00:18Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #162824 at sha 7e0ef41

…162824)

fixes flutter#162776 fixes flutter#149652 Previously we weren't calculating the subpixel offset correctly. We weren't using the transform of the object being drawn to get global coordinates, now we are. ## Video of flutter#149652 after PR https://github.com/user-attachments/assets/3e9063d5-f70c-46d0-a7a4-892819b247b8 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

…162824)

This is a cherry-pick for all the changes that went into fixing #149652 It looks like a lot but most of it is testing and refactoring. ## PRs included - #161625 - #162351 - #162415 - #162555 - #162824 ## Impacted Users All users of Impeller. ## Impact Description Animating text with translations and scales can cause: - jitter between glyphs - jitter between glyphs and the baseline - artifacts when rendering glyphs at non integer scales ## Workaround Use skia. ## Risk Since this edits how text is rendered, the risk is pretty high. The actual changes are small and there are unit tests for them. Golden test coverage for cherry-picks is not complete and text rendering golden coverage for android is problematic. ## Test Coverage Yes. ## Validation Steps The reproduction code in #149652 is good.

This is a cherry-pick for all the changes that went into fixing flutter#149652 It looks like a lot but most of it is testing and refactoring. - flutter#161625 - flutter#162351 - flutter#162415 - flutter#162555 - flutter#162824 All users of Impeller. Animating text with translations and scales can cause: - jitter between glyphs - jitter between glyphs and the baseline - artifacts when rendering glyphs at non integer scales Use skia. Since this edits how text is rendered, the risk is pretty high. The actual changes are small and there are unit tests for them. Golden test coverage for cherry-picks is not complete and text rendering golden coverage for android is problematic. Yes. The reproduction code in flutter#149652 is good.

This is a cherry-pick for all the changes that went into fixing flutter/flutter#149652 It looks like a lot but most of it is testing and refactoring. ## PRs included - flutter/flutter#161625 - flutter/flutter#162351 - flutter/flutter#162415 - flutter/flutter#162555 - flutter/flutter#162824 ## Impacted Users All users of Impeller. ## Impact Description Animating text with translations and scales can cause: - jitter between glyphs - jitter between glyphs and the baseline - artifacts when rendering glyphs at non integer scales ## Workaround Use skia. ## Risk Since this edits how text is rendered, the risk is pretty high. The actual changes are small and there are unit tests for them. Golden test coverage for cherry-picks is not complete and text rendering golden coverage for android is problematic. ## Test Coverage Yes. ## Validation Steps The reproduction code in flutter/flutter#149652 is good.

…162824)

github-actions bot added a: text input Entering text in a text field or keyboard related problems engine flutter/engine repository. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Feb 6, 2025

Fixed translated text's subpixel alignment.

037a0af

gaaclarke force-pushed the subpixel-translation-fix branch from 3cd5c84 to 037a0af Compare February 6, 2025 18:58

gaaclarke added 7 commits February 6, 2025 13:11

removed traces

3d64402

cleanup

93b1c07

added subpixel golden test

4b7e56f

added scaled test

6649eca

moved over scaled and translated test

9f253f0

removed golden tests

e2311c4

added docstring

7492bca

gaaclarke marked this pull request as ready for review February 6, 2025 22:29

gaaclarke requested review from jonahwilliams and bdero February 6, 2025 22:29

gaaclarke commented Feb 6, 2025

View reviewed changes

made sure we aren't jumping more than once

857aeb6

jonahwilliams approved these changes Feb 7, 2025

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2025

I switched to the green channel

7e0ef41

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2025

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2025

engine-flutter-autoroll mentioned this pull request Feb 8, 2025

Roll Flutter from 8e2a6fc3fd2c to 829fe68e9dff (100 revisions) flutter/packages#8591

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

a4f31f7

…162824)

engine-flutter-autoroll mentioned this pull request Feb 9, 2025

Roll Flutter from 8e2a6fc3fd2c to 5156de731b45 (102 revisions) flutter/packages#8593

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

a70270d

…162824)

engine-flutter-autoroll mentioned this pull request Feb 9, 2025

Roll Flutter from 8e2a6fc3fd2c to 5156de731b45 (102 revisions) flutter/packages#8594

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

21304cb

…162824)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to e7cb7a30bb1c (104 revisions) flutter/packages#8597

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

58b237e

…162824)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to c89d563944e3 (105 revisions) flutter/packages#8599

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

c191882

…162824)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to 33a4c95de079 (106 revisions) flutter/packages#8600

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

62a1fcd

…162824)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to 1d766ed8d93e (107 revisions) flutter/packages#8601

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

ffe9a6c

…162824)

engine-flutter-autoroll mentioned this pull request Feb 10, 2025

Roll Flutter from 8e2a6fc3fd2c to 78e5d7995cdb (108 revisions) flutter/packages#8602

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

31e4f64

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

fe7f6a7

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

87ec097

…162824)

gaaclarke mentioned this pull request Feb 11, 2025

[CP] all jittery glyph fixes #163058

Merged

9 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

da553b1

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

e6cd6db

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

45f6a72

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

090b734

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

1cf448b

…162824)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

68e785e Fixed translated text's subpixel alignment. (flutter/flutter#…

7e5d237

…162824)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed translated text's subpixel alignment. #162824

Fixed translated text's subpixel alignment. #162824

Uh oh!

gaaclarke commented Feb 6, 2025 •

edited

Loading

Uh oh!

gaaclarke commented Feb 6, 2025

Uh oh!

gaaclarke Feb 6, 2025

Uh oh!

gaaclarke Feb 6, 2025

Uh oh!

jonahwilliams left a comment

Uh oh!

jonahwilliams Feb 7, 2025

Uh oh!

gaaclarke Feb 7, 2025

Uh oh!

gaaclarke commented Feb 7, 2025

Uh oh!

auto-submit bot commented Feb 7, 2025

Uh oh!

auto-submit bot commented Feb 7, 2025

Uh oh!

flutter-dashboard bot commented Feb 7, 2025

Uh oh!

Uh oh!

Fixed translated text's subpixel alignment. #162824

Fixed translated text's subpixel alignment. #162824

Uh oh!

Conversation

gaaclarke commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Video of #149652 after PR

Pre-launch Checklist

Uh oh!

gaaclarke commented Feb 6, 2025

Uh oh!

gaaclarke Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Feb 7, 2025

Uh oh!

auto-submit bot commented Feb 7, 2025

Uh oh!

auto-submit bot commented Feb 7, 2025

Uh oh!

flutter-dashboard bot commented Feb 7, 2025

Uh oh!

Uh oh!

gaaclarke commented Feb 6, 2025 •

edited

Loading