-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Provide fragment uniform data through varyings for solid color, glyph atlas, texture shaders. #43838
Conversation
|
|
||
| uniform FragInfo { | ||
| float16_t x_tile_mode; | ||
| float16_t y_tile_mode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't used the tiled texture shader on vulkan, but we still need to move the alpha so that this can share the same vertex shader as texture.frag.
| #ifdef IMPELLER_TARGET_OPENGLES | ||
| #define IMPELLER_FLAT | ||
| #else | ||
| #define IMPELLER_FLAT flat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should call thin IMPELLER_MAYBE_FLAT. Right now, we only use flat as an optimization to reduce load on the varying unit. But someone might actually start using it in the future to disregard non-provoking vertices and will be confused as to why it doesn't work on OpenGL ES.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…or solid color, glyph atlas, texture shaders. (flutter/engine#43838)
…131015) flutter/engine@9b2ebf2...ab7d424 2023-07-20 [email protected] [Impeller] Provide fragment uniform data through varyings for solid color, glyph atlas, texture shaders. (flutter/engine#43838) 2023-07-20 [email protected] Roll ANGLE from a4c283be741f to f2e0f8a0b236 (2 revisions) (flutter/engine#43864) 2023-07-20 [email protected] Roll Skia from 18e834916f47 to 100d0f858f02 (7 revisions) (flutter/engine#43863) 2023-07-20 [email protected] Roll Fuchsia Mac SDK from SmAtKPfGzPllC9gfO... to -SaPL-46jpiYbnCAu... (flutter/engine#43862) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from SmAtKPfGzPll to -SaPL-46jpiY 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
…olor, glyph atlas, texture shaders. (flutter#43838) From experimentation, providing these values through varyings runs substantially faster on Pixel devices. Alternative solutions to use f16 or push constants did not improve things substantially. These subset of shaders are used very heavily and so they benefit from this change more substantially. I don't think this is applicable to all shaders as there is going to be an additional varying interpolation cost. See also: [go/impeller-slow-fragment-shader](http://goto.google.com/impeller-slow-fragment-shader)
… solid color, glyph atlas, texture shaders. (flutter#43838)" This reverts commit ab7d424.
…lutter#131015) flutter/engine@9b2ebf2...ab7d424 2023-07-20 [email protected] [Impeller] Provide fragment uniform data through varyings for solid color, glyph atlas, texture shaders. (flutter/engine#43838) 2023-07-20 [email protected] Roll ANGLE from a4c283be741f to f2e0f8a0b236 (2 revisions) (flutter/engine#43864) 2023-07-20 [email protected] Roll Skia from 18e834916f47 to 100d0f858f02 (7 revisions) (flutter/engine#43863) 2023-07-20 [email protected] Roll Fuchsia Mac SDK from SmAtKPfGzPllC9gfO... to -SaPL-46jpiYbnCAu... (flutter/engine#43862) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from SmAtKPfGzPll to -SaPL-46jpiY 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
From experimentation, providing these values through varyings runs substantially faster on Pixel devices. Alternative solutions to use f16 or push constants did not improve things substantially.
These subset of shaders are used very heavily and so they benefit from this change more substantially. I don't think this is applicable to all shaders as there is going to be an additional varying interpolation cost.
See also: go/impeller-slow-fragment-shader