[Impeller] organize texture shaders / delete blend.frag + external_texture_fill #52137
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
I think we don't need the strict src rect fragment shader, but when I run the golden NinePatchImagePrecision before/after or with the feature disabled altogether I don't notice a difference. |
|
This PR changes the output for the example app from flutter/flutter#140393. With |
|
Does this require a specific screen size or device? Visually this looks correct for me on a Pixel 7 Pro, and the playground test you added also looks correct for me on an m1 mac. But I can reproduce the pixel bleeding if I turn of the kStrict flag entirely. |
|
The issue is that with this PR the colors of the corners are lighter than expected.
The above was taken from a Pixel 6a, but this should reproduce on any device. The bleeding outside the corners does not happen with this PR. But the corners themselves look like they are not sampling exclusively from the color of the corners in the nine-patch image. |
|
Okay I see it now! I think I'll leave this one as is. Since we only use it in the drawRect case its not going to cause problems with the tiled texture refacor. |
|
@chinmaygarde speaking of reducing shader count |
jonahwilliams
changed the title
jonahwilliams
added
the
autosubmit
…xternal_texture_fill (flutter/engine#52137)
…147091) flutter/engine@a9a6f59...55670b7 2024-04-19 [email protected] [Impeller] organize texture shaders / delete blend.frag + external_texture_fill (flutter/engine#52137) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
jonahwilliams
added
the
revert
|
Time to revert pull request flutter/engine/52137 has elapsed. |
auto-submit
bot
removed
the
revert
|
reason for revert: investigating breakages in #52299 |
jonahwilliams
added
the
revert
|
Time to revert pull request flutter/engine/52137 has elapsed. |
auto-submit
bot
removed
the
revert
…lutter#147091) flutter/engine@a9a6f59...55670b7 2024-04-19 [email protected] [Impeller] organize texture shaders / delete blend.frag + external_texture_fill (flutter/engine#52137) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
* Deletes strict src rect fragment shader. We should be able to compute the UVs on the CPU, which is trivial since this is only used for rectangular draws.