Started adjusting uvs to match pixel snapping.#162049
Started adjusting uvs to match pixel snapping.#162049auto-submit[bot] merged 11 commits intoflutter:masterfrom
Conversation
|
Note to self: one way you can verify this is by printing out the aspect ratio of the UVs before and after. The hypothesis is that the "before" has more noisy aspect ratios for the uvs. |
|
I've thought about this more and I think this isn't quite the right approach, but its close. I think =the scaling problem can be caused by scaling up/down and not the rounding. With the rounding, we're only rounding/pixel snapping the origin point. Then computing the remaining coordinates off of that origin point. As long as the size is correct, that should not change the uv width. |
|
Here's the test I thought up for this, it fails with or without this patch though with the exact same results: TEST_P(TextContentsTest, MaintainsAspectRatio) {
#ifndef FML_OS_MACOSX
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif
GlyphAtlasPipeline::VertexShader::PerVertexData data[6];
std::shared_ptr<TextFrame> text_frame =
MakeTextFrame("1", "ahem.ttf", /*font_size=*/50);
std::shared_ptr<TypographerContext> context = TypographerContextSkia::Make();
std::shared_ptr<GlyphAtlasContext> atlas_context =
context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap);
std::shared_ptr<HostBuffer> host_buffer = HostBuffer::Create(
GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter());
ASSERT_TRUE(context && context->IsValid());
for (int i = 0; i < 10; ++i) {
Scalar font_scale = 0.440f + i * 0.001f;
std::shared_ptr<GlyphAtlas> atlas = CreateGlyphAtlas(
*GetContext(), context.get(), *host_buffer,
GlyphAtlas::Type::kAlphaBitmap, font_scale, atlas_context, text_frame);
ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(
data, text_frame, font_scale,
/*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}),
/*offset=*/Vector2(0, 0),
/*glyph_properties=*/std::nullopt, atlas);
Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
EXPECT_NEAR(GetAspectRatio(position_rect), GetAspectRatio(uv_rect), 0.001)
<< "scale: " << font_scale;
}
} |
|
To be clear the results are 4/10 times it passes, but 6/10 times the difference is aspect ratios is 0.15%. |
|
Here is another example of before and after, I think you can more clearly see the benefit of this PR with this. This was generated by drawing 300pt font at scale 0.444x and 0.445x. beforeafter
|
ba10e73 to
90e6548
Compare
|
Okay @jonahwilliams , I just merged in the RMSE test and switched it to just look for a jump in the maximum y value for a tiny scale. The diff should be 1 based on observation, but we are seeing 2 as noted in #162049 (comment) I wish we had a more holistic test for this but this is what I could come up with. |
|
autosubmit label was removed for flutter/flutter/162049, because - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
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 Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
The golden's in that particular case don't look ideal since there is a 1px space between the baseline and some of the glyphs. I think the old test was a fluke though since we know the glyphs would bounce around with certain scalars. I'll audit that particular test on monday. |
|
I made the test animated and looked through different scales, looks good. I'll approve the goldens and then push my PR with the test tweak. |
This reverts commit dc580af.
This comment was marked as outdated.
This comment was marked as outdated.
|
Reason for revert: Negatively affected Android rendering ( #162361) |
This reverts commit dc580af.
…162392) <!-- start_original_pr_link --> Reverts: #162049 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: gaaclarke <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: Negatively affected Android rendering ( #162361) <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: gaaclarke <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {jonahwilliams} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: issue: #149652 doc: (currently google only) https://docs.google.com/document/d/1Rulw_noQi0G8Glb47vk17uBbb6sySDxlQe-l-0kHn14/edit?tab=t.0 This increases the RMSE value in the test in #161445 by a slight amount. I do believe this reduces the time where we get non uniform scalars and protects the integrity of relative spacing, thus being more what we expect. There is still a bug that has to do with pixel alignment that does give the illusion of stretching and shrinking though because of hard/soft lines. ## Before https://github.com/user-attachments/assets/e9b80b70-0961-4e02-9053-84d4457348e5 ## After https://github.com/user-attachments/assets/2494a2b1-497d-4a2b-afd7-23064acba293 ## 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]. <!-- Links --> [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 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…ter#162049)" (flutter#162392)" This reverts some of commit d048c77, just the testing parts.
Roll Flutter from c1ffaa9 to b007899 (43 revisions) flutter/flutter@c1ffaa9...b007899 2025-01-29 [email protected] Fix `Tab` linear and elastic animation blink (flutter/flutter#162315) 2025-01-29 [email protected] Pass-through `textInputAction` in `DropdownMenu` (flutter/flutter#162309) 2025-01-29 [email protected] Fix scrollUntilVisible in WidgetTester (flutter/flutter#159582) 2025-01-29 [email protected] Pass-through `maxLines` in `DropdownMenu` (flutter/flutter#161903) 2025-01-29 [email protected] fix: appbar leading width is not square for custom toolbar height (flutter/flutter#161880) 2025-01-29 [email protected] [DisplayList] Don't call Skia Ganesh methods when its not available. (flutter/flutter#162345) 2025-01-29 [email protected] Update README.md to include googler post verification steps (flutter/flutter#162272) 2025-01-29 [email protected] [engine, web] return switch expressions in many places (flutter/flutter#162336) 2025-01-29 [email protected] Update README.md to not have engine link for android (flutter/flutter#162330) 2025-01-29 [email protected] Reland "[ Widget Previews ] Add support for detecting previews and generating code (#161911)"" (flutter/flutter#162337) 2025-01-29 [email protected] Add instructions to download the Gradle wrapper to FGP readme, and add to gitignore (flutter/flutter#162332) 2025-01-29 [email protected] Fix tests to prepare for `--explicit-package-dependencies` and a bug. (flutter/flutter#162289) 2025-01-29 [email protected] Add a currently unused `runs_in_merge_queue` property to `Linux analyze`. (flutter/flutter#162335) 2025-01-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ Widget Previews ] Add support for detecting previews and generating code (#161911)" (flutter/flutter#162327) 2025-01-28 [email protected] Support hot restart for DDC library bundle format (flutter/flutter#162123) 2025-01-28 [email protected] Started adjusting uvs to match pixel snapping. (flutter/flutter#162049) 2025-01-28 [email protected] Refactor code inside flutter.groovy (flutter/flutter#160250) 2025-01-28 [email protected] Table implements redepth (flutter/flutter#162282) 2025-01-28 [email protected] [ Widget Previews ] Add support for detecting previews and generating code (flutter/flutter#161911) 2025-01-28 [email protected] remove dependency on `Usage` from `Pub` class (flutter/flutter#162279) 2025-01-28 [email protected] Roll Packages from 258f6dc to 02c6fef (6 revisions) (flutter/flutter#162313) 2025-01-28 [email protected] Remove `scenario_app/android` and rename to `ios_scenario_app`. (flutter/flutter#160992) 2025-01-28 [email protected] Apparently it is illegal to use `stderr` in this project. (flutter/flutter#162294) 2025-01-28 [email protected] Fix update_engine_version_test in presence of FLUTTER_PREBUILT_ENGINE_VERSION env vars. (flutter/flutter#162270) 2025-01-28 [email protected] Add missing `properties: ...` and move to presubmit. (flutter/flutter#162170) 2025-01-27 [email protected] [Impeller] make swapchain related external fence/semaphore extensions optional. (flutter/flutter#162205) 2025-01-27 49699333+dependabot[bot]@users.noreply.github.com Bump the all-github-actions group with 2 updates (flutter/flutter#162277) 2025-01-27 [email protected] Don't depend on Dart from FML. (flutter/flutter#162271) 2025-01-27 [email protected] [DisplayList] Move nested canvas enums into their own TU. (flutter/flutter#162037) 2025-01-27 [email protected] Avoid iOS text selection crash by returning nil range (flutter/flutter#161996) 2025-01-27 [email protected] fix `felt` link to point to flutter repo instead of the engine repo (flutter/flutter#161423) 2025-01-27 [email protected] Enable the Android Engine OpenGLES/Vulkan suites. (flutter/flutter#162258) 2025-01-27 [email protected] [canvaskit] Fix debug build for CanvasKit (flutter/flutter#162198) 2025-01-27 [email protected] Roll Packages from 3d3ab7b to 258f6dc (19 revisions) (flutter/flutter#162254) 2025-01-25 [email protected] Pin `customer_testing` to the SHA specified in `tests.version` (flutter/flutter#162048) 2025-01-25 [email protected] Formalize `update_engine_version.{sh|ps1}`. (flutter/flutter#162118) 2025-01-25 [email protected] Rename 'SelectionChangedCause.scribble' to 'SelectionChangedCause.stylusHandwriting' (flutter/flutter#161518) 2025-01-25 [email protected] Don't install xcode when doing `local_engine` web builds on mac. (flutter/flutter#162164) 2025-01-25 [email protected] Force Impeller backend for `android_engine_test`, and test both OpenGLES and Vulkan (flutter/flutter#162089) 2025-01-24 [email protected] [Impeller] when a command pool has many unused buffers, reset with release resources flag. (flutter/flutter#162171) 2025-01-24 [email protected] [web] Remove HTML renderer from framework tests (flutter/flutter#162038) 2025-01-24 [email protected] [Impeller] Skip clip entity replay that cannot impact current clip. (flutter/flutter#162113) 2025-01-24 [email protected] Update Android integration test package for newer AGP (flutter/flutter#161856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: ...
Roll Flutter from c1ffaa9 to b007899 (43 revisions) flutter/flutter@c1ffaa9...b007899 2025-01-29 [email protected] Fix `Tab` linear and elastic animation blink (flutter/flutter#162315) 2025-01-29 [email protected] Pass-through `textInputAction` in `DropdownMenu` (flutter/flutter#162309) 2025-01-29 [email protected] Fix scrollUntilVisible in WidgetTester (flutter/flutter#159582) 2025-01-29 [email protected] Pass-through `maxLines` in `DropdownMenu` (flutter/flutter#161903) 2025-01-29 [email protected] fix: appbar leading width is not square for custom toolbar height (flutter/flutter#161880) 2025-01-29 [email protected] [DisplayList] Don't call Skia Ganesh methods when its not available. (flutter/flutter#162345) 2025-01-29 [email protected] Update README.md to include googler post verification steps (flutter/flutter#162272) 2025-01-29 [email protected] [engine, web] return switch expressions in many places (flutter/flutter#162336) 2025-01-29 [email protected] Update README.md to not have engine link for android (flutter/flutter#162330) 2025-01-29 [email protected] Reland "[ Widget Previews ] Add support for detecting previews and generating code (#161911)"" (flutter/flutter#162337) 2025-01-29 [email protected] Add instructions to download the Gradle wrapper to FGP readme, and add to gitignore (flutter/flutter#162332) 2025-01-29 [email protected] Fix tests to prepare for `--explicit-package-dependencies` and a bug. (flutter/flutter#162289) 2025-01-29 [email protected] Add a currently unused `runs_in_merge_queue` property to `Linux analyze`. (flutter/flutter#162335) 2025-01-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ Widget Previews ] Add support for detecting previews and generating code (#161911)" (flutter/flutter#162327) 2025-01-28 [email protected] Support hot restart for DDC library bundle format (flutter/flutter#162123) 2025-01-28 [email protected] Started adjusting uvs to match pixel snapping. (flutter/flutter#162049) 2025-01-28 [email protected] Refactor code inside flutter.groovy (flutter/flutter#160250) 2025-01-28 [email protected] Table implements redepth (flutter/flutter#162282) 2025-01-28 [email protected] [ Widget Previews ] Add support for detecting previews and generating code (flutter/flutter#161911) 2025-01-28 [email protected] remove dependency on `Usage` from `Pub` class (flutter/flutter#162279) 2025-01-28 [email protected] Roll Packages from 258f6dc to 02c6fef (6 revisions) (flutter/flutter#162313) 2025-01-28 [email protected] Remove `scenario_app/android` and rename to `ios_scenario_app`. (flutter/flutter#160992) 2025-01-28 [email protected] Apparently it is illegal to use `stderr` in this project. (flutter/flutter#162294) 2025-01-28 [email protected] Fix update_engine_version_test in presence of FLUTTER_PREBUILT_ENGINE_VERSION env vars. (flutter/flutter#162270) 2025-01-28 [email protected] Add missing `properties: ...` and move to presubmit. (flutter/flutter#162170) 2025-01-27 [email protected] [Impeller] make swapchain related external fence/semaphore extensions optional. (flutter/flutter#162205) 2025-01-27 49699333+dependabot[bot]@users.noreply.github.com Bump the all-github-actions group with 2 updates (flutter/flutter#162277) 2025-01-27 [email protected] Don't depend on Dart from FML. (flutter/flutter#162271) 2025-01-27 [email protected] [DisplayList] Move nested canvas enums into their own TU. (flutter/flutter#162037) 2025-01-27 [email protected] Avoid iOS text selection crash by returning nil range (flutter/flutter#161996) 2025-01-27 [email protected] fix `felt` link to point to flutter repo instead of the engine repo (flutter/flutter#161423) 2025-01-27 [email protected] Enable the Android Engine OpenGLES/Vulkan suites. (flutter/flutter#162258) 2025-01-27 [email protected] [canvaskit] Fix debug build for CanvasKit (flutter/flutter#162198) 2025-01-27 [email protected] Roll Packages from 3d3ab7b to 258f6dc (19 revisions) (flutter/flutter#162254) 2025-01-25 [email protected] Pin `customer_testing` to the SHA specified in `tests.version` (flutter/flutter#162048) 2025-01-25 [email protected] Formalize `update_engine_version.{sh|ps1}`. (flutter/flutter#162118) 2025-01-25 [email protected] Rename 'SelectionChangedCause.scribble' to 'SelectionChangedCause.stylusHandwriting' (flutter/flutter#161518) 2025-01-25 [email protected] Don't install xcode when doing `local_engine` web builds on mac. (flutter/flutter#162164) 2025-01-25 [email protected] Force Impeller backend for `android_engine_test`, and test both OpenGLES and Vulkan (flutter/flutter#162089) 2025-01-24 [email protected] [Impeller] when a command pool has many unused buffers, reset with release resources flag. (flutter/flutter#162171) 2025-01-24 [email protected] [web] Remove HTML renderer from framework tests (flutter/flutter#162038) 2025-01-24 [email protected] [Impeller] Skip clip entity replay that cannot impact current clip. (flutter/flutter#162113) 2025-01-24 [email protected] Update Android integration test package for newer AGP (flutter/flutter#161856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: ...
Roll Flutter from c1ffaa9 to b007899 (43 revisions) flutter/flutter@c1ffaa9...b007899 2025-01-29 [email protected] Fix `Tab` linear and elastic animation blink (flutter/flutter#162315) 2025-01-29 [email protected] Pass-through `textInputAction` in `DropdownMenu` (flutter/flutter#162309) 2025-01-29 [email protected] Fix scrollUntilVisible in WidgetTester (flutter/flutter#159582) 2025-01-29 [email protected] Pass-through `maxLines` in `DropdownMenu` (flutter/flutter#161903) 2025-01-29 [email protected] fix: appbar leading width is not square for custom toolbar height (flutter/flutter#161880) 2025-01-29 [email protected] [DisplayList] Don't call Skia Ganesh methods when its not available. (flutter/flutter#162345) 2025-01-29 [email protected] Update README.md to include googler post verification steps (flutter/flutter#162272) 2025-01-29 [email protected] [engine, web] return switch expressions in many places (flutter/flutter#162336) 2025-01-29 [email protected] Update README.md to not have engine link for android (flutter/flutter#162330) 2025-01-29 [email protected] Reland "[ Widget Previews ] Add support for detecting previews and generating code (#161911)"" (flutter/flutter#162337) 2025-01-29 [email protected] Add instructions to download the Gradle wrapper to FGP readme, and add to gitignore (flutter/flutter#162332) 2025-01-29 [email protected] Fix tests to prepare for `--explicit-package-dependencies` and a bug. (flutter/flutter#162289) 2025-01-29 [email protected] Add a currently unused `runs_in_merge_queue` property to `Linux analyze`. (flutter/flutter#162335) 2025-01-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ Widget Previews ] Add support for detecting previews and generating code (#161911)" (flutter/flutter#162327) 2025-01-28 [email protected] Support hot restart for DDC library bundle format (flutter/flutter#162123) 2025-01-28 [email protected] Started adjusting uvs to match pixel snapping. (flutter/flutter#162049) 2025-01-28 [email protected] Refactor code inside flutter.groovy (flutter/flutter#160250) 2025-01-28 [email protected] Table implements redepth (flutter/flutter#162282) 2025-01-28 [email protected] [ Widget Previews ] Add support for detecting previews and generating code (flutter/flutter#161911) 2025-01-28 [email protected] remove dependency on `Usage` from `Pub` class (flutter/flutter#162279) 2025-01-28 [email protected] Roll Packages from 258f6dc to 02c6fef (6 revisions) (flutter/flutter#162313) 2025-01-28 [email protected] Remove `scenario_app/android` and rename to `ios_scenario_app`. (flutter/flutter#160992) 2025-01-28 [email protected] Apparently it is illegal to use `stderr` in this project. (flutter/flutter#162294) 2025-01-28 [email protected] Fix update_engine_version_test in presence of FLUTTER_PREBUILT_ENGINE_VERSION env vars. (flutter/flutter#162270) 2025-01-28 [email protected] Add missing `properties: ...` and move to presubmit. (flutter/flutter#162170) 2025-01-27 [email protected] [Impeller] make swapchain related external fence/semaphore extensions optional. (flutter/flutter#162205) 2025-01-27 49699333+dependabot[bot]@users.noreply.github.com Bump the all-github-actions group with 2 updates (flutter/flutter#162277) 2025-01-27 [email protected] Don't depend on Dart from FML. (flutter/flutter#162271) 2025-01-27 [email protected] [DisplayList] Move nested canvas enums into their own TU. (flutter/flutter#162037) 2025-01-27 [email protected] Avoid iOS text selection crash by returning nil range (flutter/flutter#161996) 2025-01-27 [email protected] fix `felt` link to point to flutter repo instead of the engine repo (flutter/flutter#161423) 2025-01-27 [email protected] Enable the Android Engine OpenGLES/Vulkan suites. (flutter/flutter#162258) 2025-01-27 [email protected] [canvaskit] Fix debug build for CanvasKit (flutter/flutter#162198) 2025-01-27 [email protected] Roll Packages from 3d3ab7b to 258f6dc (19 revisions) (flutter/flutter#162254) 2025-01-25 [email protected] Pin `customer_testing` to the SHA specified in `tests.version` (flutter/flutter#162048) 2025-01-25 [email protected] Formalize `update_engine_version.{sh|ps1}`. (flutter/flutter#162118) 2025-01-25 [email protected] Rename 'SelectionChangedCause.scribble' to 'SelectionChangedCause.stylusHandwriting' (flutter/flutter#161518) 2025-01-25 [email protected] Don't install xcode when doing `local_engine` web builds on mac. (flutter/flutter#162164) 2025-01-25 [email protected] Force Impeller backend for `android_engine_test`, and test both OpenGLES and Vulkan (flutter/flutter#162089) 2025-01-24 [email protected] [Impeller] when a command pool has many unused buffers, reset with release resources flag. (flutter/flutter#162171) 2025-01-24 [email protected] [web] Remove HTML renderer from framework tests (flutter/flutter#162038) 2025-01-24 [email protected] [Impeller] Skip clip entity replay that cannot impact current clip. (flutter/flutter#162113) 2025-01-24 [email protected] Update Android integration test package for newer AGP (flutter/flutter#161856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: ...


issue: #149652
doc: (currently google only) https://docs.google.com/document/d/1Rulw_noQi0G8Glb47vk17uBbb6sySDxlQe-l-0kHn14/edit?tab=t.0
This increases the RMSE value in the test in #161445 by a slight amount. I do believe this reduces the time where we get non uniform scalars and protects the integrity of relative spacing, thus being more what we expect. There is still a bug that has to do with pixel alignment that does give the illusion of stretching and shrinking though because of hard/soft lines.
Before
ondevice.mov
After
adjustuvs.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.