[Impeller] Correctly compute UVs in texture fill #43028

bdero · 2023-06-21T00:45:09Z

Depends on #43026.

Maps the texel coordinate space correctly relative to the geometry pixel coordinates.
Use decal sampling for subpass textures.
Simplify the computation by using new rect utilities such as Rect::Project.

Texture in subpass (the issue was that when copying to the parent pass, this image gets screwed up):

Before Impeller:

After Impeller:

bdero · 2023-06-21T00:48:45Z

The subpass image is getting copied to the parent pass perfectly, and the remaining visible imperfections are due to having only 4 subpixel weights with MSAA.

jonahwilliams

LGTM!

flutter-dashboard · 2023-06-21T23:44:48Z

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.

Changes reported for pull request #43028 at sha a10f8cb

flutter-dashboard · 2023-06-22T01:20:48Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #43028 at sha 40f9af7

flutter-dashboard · 2023-06-22T06:08:48Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #43028 at sha 24c239c

Use decal sampling for subpass textures

bdero · 2023-06-22T06:42:59Z

I fixed an issue where I was not mapping the source rect in terms of the texture size, and ended up writing a solution that's much nicer/easier to understand.

bdero · 2023-06-22T06:44:05Z

impeller/entity/contents/texture_contents.cc

+  // pixel grid if both the source rect and destination rect are given in screen
+  // space.
+  auto texture_coords =
+      Rect::MakeSize(texture_->GetSize()).Project(source_rect_.Expand(0.5));


This line is all that's needed to correctly compute the texture coordinates. :)

…ngine#43028)

…129339) flutter/engine@703c9a1...8cc6d6d 2023-06-22 [email protected] [Impeller] Correctly compute UVs in texture fill (flutter/engine#43028) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This reverts commit 8cc6d6d.

Reverts #43028 An engine roll containing only this commit turned the framework tree red with a crasher on Android. I'm a bit concerned because it looked like one of the benchmarks that crashed should not have been using Impeller see flutter/flutter#129353 and https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_android%20animated_blur_backdrop_filter_perf__timeline_summary/1120/overview

…ngine#43028)

…129378) flutter/engine@703c9a1...ed477de 2023-06-22 [email protected] Roll ANGLE from bbcf54bcb738 to 4ed2d403a329 (7 revisions) (flutter/engine#43105) 2023-06-22 [email protected] Workaround a release blocker after libc++ change (flutter/engine#43091) 2023-06-22 [email protected] Roll Skia from 8168c802c391 to 09b36b8ce0db (1 revision) (flutter/engine#43102) 2023-06-22 [email protected] [Impeller] Reland: Correctly compute UVs in texture fill (flutter/engine#43093) 2023-06-22 [email protected] [Impeller] Add validation forbidding SamplerAddressMode::kDecal on the OpenGLES backend (flutter/engine#43094) 2023-06-22 [email protected] Use minor version, ignore patches for CodeQL (flutter/engine#43088) 2023-06-22 [email protected] Print a warning when a message channel is used on the wrong thread. (flutter/engine#42928) 2023-06-22 [email protected] Roll Skia from 3f3e1da4b7eb to 8168c802c391 (4 revisions) (flutter/engine#43096) 2023-06-22 [email protected] [Impeller] default sample count back to 1 (but configure to 4 in defaults). (flutter/engine#43089) 2023-06-22 [email protected] [web] Don't get break type from v8BreakIterator (flutter/engine#43053) 2023-06-22 [email protected] Roll dart to 3.1.0-239.0.dev (flutter/engine#43083) 2023-06-22 [email protected] Revert "[Impeller] dont use concurrent runner to decode images on Android." (flutter/engine#43061) 2023-06-22 [email protected] [Impeller] Add fence waiter trace event. (flutter/engine#43092) 2023-06-22 [email protected] [Impeller] remove Vulkan pipeline cache mutex. (flutter/engine#43085) 2023-06-22 [email protected] Revert "[Impeller] Correctly compute UVs in texture fill" (flutter/engine#43087) 2023-06-22 [email protected] Roll Fuchsia Linux SDK from 7EZeNE4aGd29VfDly... to tcVndpnH_jzGm5LsJ... (flutter/engine#43081) 2023-06-22 [email protected] Roll Skia from 117f57a53215 to 3f3e1da4b7eb (4 revisions) (flutter/engine#43080) 2023-06-22 [email protected] Roll ANGLE from 7658525166a4 to bbcf54bcb738 (1 revision) (flutter/engine#43079) 2023-06-22 [email protected] Roll Skia from 5013b651f8ec to 117f57a53215 (1 revision) (flutter/engine#43078) 2023-06-22 [email protected] Roll Fuchsia Mac SDK from QtQznuUmHMTyORqxJ... to Ylc35wOk0_j0NLzDv... (flutter/engine#43076) 2023-06-22 [email protected] Roll ANGLE from a2b3f9b64670 to 7658525166a4 (1 revision) (flutter/engine#43075) 2023-06-22 [email protected] Roll ANGLE from ac263582dda4 to a2b3f9b64670 (1 revision) (flutter/engine#43074) 2023-06-22 [email protected] Roll Skia from 71047dca9f77 to 5013b651f8ec (4 revisions) (flutter/engine#43073) 2023-06-22 [email protected] [Impeller] Correctly compute UVs in texture fill (flutter/engine#43028) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 7EZeNE4aGd29 to tcVndpnH_jzG fuchsia/sdk/core/mac-amd64 from QtQznuUmHMTy to Ylc35wOk0_j0 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

bdero added the e: impeller label Jun 21, 2023

bdero requested review from chinmaygarde, dnfield and jonahwilliams June 21, 2023 00:45

bdero self-assigned this Jun 21, 2023

jonahwilliams approved these changes Jun 21, 2023

View reviewed changes

bdero force-pushed the bdero/texture-fill-uvs branch from 84ba9b4 to a10f8cb Compare June 21, 2023 17:43

bdero mentioned this pull request Jun 21, 2023

Long CI scheduling delays flutter/flutter#129297

Closed

flutter-dashboard bot added the will affect goldens label Jun 21, 2023

bdero force-pushed the bdero/texture-fill-uvs branch from a10f8cb to 40f9af7 Compare June 22, 2023 00:22

bdero added 3 commits June 21, 2023 23:21

[Impeller] Correctly compute UVs in texture fill

5f4a812

Use decal sampling for subpass textures

Fix tex coords

ca8da50

Offset half texel correctly

bb9c781

bdero force-pushed the bdero/texture-fill-uvs branch from a005071 to bb9c781 Compare June 22, 2023 06:21

bdero commented Jun 22, 2023

View reviewed changes

Use a simpler and more consistent solution for computing the UVs

c4f3031

bdero force-pushed the bdero/texture-fill-uvs branch from aa60990 to c4f3031 Compare June 22, 2023 06:46

bdero merged commit 8cc6d6d into flutter:main Jun 22, 2023

engine-flutter-autoroll mentioned this pull request Jun 22, 2023

Roll Flutter Engine from 703c9a14ac7f to 8cc6d6d5efdb (1 revision) flutter/flutter#129339

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2023

8cc6d6d5e [Impeller] Correctly compute UVs in texture fill (flutter/e…

054c2ea

…ngine#43028)

zanderso added a commit that referenced this pull request Jun 22, 2023

Revert "[Impeller] Correctly compute UVs in texture fill (#43028)"

402ac9a

This reverts commit 8cc6d6d.

zanderso mentioned this pull request Jun 22, 2023

Revert "[Impeller] Correctly compute UVs in texture fill" #43087

Merged

engine-flutter-autoroll mentioned this pull request Jun 22, 2023

Roll Flutter Engine from 703c9a14ac7f to 1225ac5dd6e9 (11 revisions) flutter/flutter#129362

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2023

8cc6d6d5e [Impeller] Correctly compute UVs in texture fill (flutter/e…

7e1c144

…ngine#43028)

engine-flutter-autoroll mentioned this pull request Jun 22, 2023

Roll Flutter Engine from 703c9a14ac7f to a3c5a319fccb (23 revisions) flutter/flutter#129374

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2023

8cc6d6d5e [Impeller] Correctly compute UVs in texture fill (flutter/e…

3f7d0ae

…ngine#43028)

engine-flutter-autoroll mentioned this pull request Jun 22, 2023

Roll Flutter Engine from 703c9a14ac7f to ed477deba7c5 (24 revisions) flutter/flutter#129378

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2023

8cc6d6d5e [Impeller] Correctly compute UVs in texture fill (flutter/e…

b319992

…ngine#43028)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] Correctly compute UVs in texture fill #43028

[Impeller] Correctly compute UVs in texture fill #43028

Uh oh!

bdero commented Jun 21, 2023

Uh oh!

bdero commented Jun 21, 2023

Uh oh!

jonahwilliams left a comment

Uh oh!

flutter-dashboard bot commented Jun 21, 2023

Uh oh!

flutter-dashboard bot commented Jun 22, 2023

Uh oh!

flutter-dashboard bot commented Jun 22, 2023

Uh oh!

bdero commented Jun 22, 2023

Uh oh!

bdero Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[Impeller] Correctly compute UVs in texture fill #43028

[Impeller] Correctly compute UVs in texture fill #43028

Uh oh!

Conversation

bdero commented Jun 21, 2023

Uh oh!

bdero commented Jun 21, 2023

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jun 21, 2023

Uh oh!

flutter-dashboard bot commented Jun 22, 2023

Uh oh!

flutter-dashboard bot commented Jun 22, 2023

Uh oh!

bdero commented Jun 22, 2023

Uh oh!

bdero Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants