From f949d8eaedf3084224ecff94a36be2eef543abd0 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Fri, 14 Feb 2025 09:21:30 -0800 Subject: [PATCH] Tweaked TextContents math to avoid floating point errors (#162480) This gets rid of artifacts in characters. ## before ![before_445](https://github.com/user-attachments/assets/c66d3d22-baaf-4d66-bff3-3ad5b4d5747c) ## after ![after_445](https://github.com/user-attachments/assets/0e23a593-b68a-4334-a82e-4f7dc1ecdc17) ## 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 `///`). - [ ] 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 --- .../display_list/aiks_dl_text_unittests.cc | 22 +++++++++++++++++++ .../impeller/entity/contents/text_contents.cc | 4 +++- .../testing/impeller_golden_tests_output.txt | 3 +++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc b/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc index 4f11e10f29..c26530af02 100644 --- a/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc +++ b/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc @@ -155,6 +155,28 @@ TEST_P(AiksTest, CanRenderTextFrameWithHalfScaling) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +// This is a test that looks for glyph artifacts we've see. +TEST_P(AiksTest, ScaledK) { + DisplayListBuilder builder; + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0.1, 0.1, 0.1)); + builder.DrawPaint(paint); + for (int i = 0; i < 6; ++i) { + builder.Save(); + builder.Translate(300 * i, 0); + Scalar scale = 0.445 - (i / 1000.f); + builder.Scale(scale, scale); + RenderTextInCanvasSkia( + GetContext(), builder, "k", "Roboto-Regular.ttf", + TextRenderOptions{.font_size = 600, .position = DlPoint(10, 500)}); + RenderTextInCanvasSkia( + GetContext(), builder, "k", "Roboto-Regular.ttf", + TextRenderOptions{.font_size = 300, .position = DlPoint(10, 800)}); + builder.Restore(); + } + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) { Scalar fine_scale = 0.f; bool is_subpixel = false; diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index 7c8be8c684..d3d6f6e36e 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents.cc @@ -187,7 +187,9 @@ void TextContents::ComputeVertexData( Point position; if (is_translation_scale) { position = (screen_glyph_position + - (basis_transform * point * scaled_bounds.GetSize())) + ((basis_transform.m[0] < 0 ? Matrix::MakeScale({-1, 1, 1}) + : Matrix()) * + point * glyph_bounds.GetSize())) .Round(); } else { position = entity_transform * diff --git a/engine/src/flutter/testing/impeller_golden_tests_output.txt b/engine/src/flutter/testing/impeller_golden_tests_output.txt index 5cb5a2cc76..c7318d7805 100644 --- a/engine/src/flutter/testing/impeller_golden_tests_output.txt +++ b/engine/src/flutter/testing/impeller_golden_tests_output.txt @@ -880,6 +880,9 @@ impeller_Play_AiksTest_SaveLayerDrawsBehindSubsequentEntities_Vulkan.png impeller_Play_AiksTest_SaveLayerFiltersScaleWithTransform_Metal.png impeller_Play_AiksTest_SaveLayerFiltersScaleWithTransform_OpenGLES.png impeller_Play_AiksTest_SaveLayerFiltersScaleWithTransform_Vulkan.png +impeller_Play_AiksTest_ScaledK_Metal.png +impeller_Play_AiksTest_ScaledK_OpenGLES.png +impeller_Play_AiksTest_ScaledK_Vulkan.png impeller_Play_AiksTest_SetContentsWithRegion_Metal.png impeller_Play_AiksTest_SetContentsWithRegion_OpenGLES.png impeller_Play_AiksTest_SetContentsWithRegion_Vulkan.png