[display_list] allow applying opacity peephole to single glyph. #53160
Conversation
|
FYI @flar , not sure if we have a standard harness for testing this sort of thing, or if you have cases somewhere for textblob I could expand to textframe. |
|
dl_rendering_unittests.cc has some "compare with/without opacity" kinds of tests, but they don't run on Impeller. Would it be possible to create an Impeller golden test using DLBuilder? I thought that we recently got the DL playground tests to run as goldens. |
|
Alternately, render it with an alpha color and render it with an alpha saveLayer and see if the two produce the same image. You can also check that the saveLayer DL reports |
| // likely a glyph used as an Icon) | ||
| const bool is_single_glyph = text_frame->GetRunCount() == 1u && | ||
| text_frame->GetRuns()[0].GetGlyphCount() == 1u; | ||
| UpdateLayerOpacityCompatibility(is_single_glyph); |
There was a problem hiding this comment.
This reminds me that this code is mixing and matching attribute incompatibility with overlap incompatibility. Technically this is an overlap condition which is typically reported through the accumulator. But since this mistake is replicated in a few places I think that should be solved as a separate issue...
I filed this as flutter/flutter#149622
There was a problem hiding this comment.
Also, of note, this doesn't cause a bug, it's just a systemic logic confusion that ends up doing the right thing anyway.
jonahwilliams
added
the
autosubmit
|
reason for revert: unexpected framework goldens https://flutter-gold.skia.org/search?issue=149634&crs=github&patchsets=4&corpus=flutter |
jonahwilliams
added
the
revert
auto-submit
bot
removed
the
revert
…ph. (#53160)" (#53189) Reverts: #53160 Initiated by: jonahwilliams Reason for reverting: unexpected framework goldens https://flutter-gold.skia.org/search?issue=149634&crs=github&patchsets=4&corpus=flutter Original PR Author: jonahwilliams Reviewed By: {flar} This change reverts the following previous change: A single glyph can be opacity peepholed, this is common in the case of Icons.
…149658) flutter/engine@a6aa5d8...e211c43 2024-06-04 [email protected] Roll Fuchsia Linux SDK from lKBLxel8iBaHpT5q6... to pagJoGS4kQ8Efa_if... (flutter/engine#53192) 2024-06-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[display_list] allow applying opacity peephole to single glyph. (#53160)" (flutter/engine#53189) 2024-06-04 [email protected] Roll Dart SDK from 49b5590c8d80 to 9dce57c343ec (1 revision) (flutter/engine#53187) 2024-06-04 [email protected] [Impeller] Use varying interpolation to compute some linear gradients. (flutter/engine#53166) 2024-06-03 [email protected] [Impeller] match rrect_blur math to the gaussian math (flutter/engine#53130) 2024-06-03 [email protected] [web] Add Ethiopic font fallback. (flutter/engine#53180) 2024-06-03 [email protected] Fix rendering corruption by Flutter and GDK sharing the same OpenGL context (flutter/engine#53103) 2024-06-03 [email protected] [display_list] allow applying opacity peephole to single glyph. (flutter/engine#53160) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from lKBLxel8iBaH to pagJoGS4kQ8E 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
A single glyph can be opacity peepholed, this is common in the case of Icons.