[Impeller] let drawImage nine use porter duff optimization. #169611
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
May 28, 2025 12:33
This comment was marked as off-topic.
This comment was marked as off-topic.
gaaclarke
reviewed
May 29, 2025
Member
gaaclarke
left a comment
There was a problem hiding this comment.
Looking good, just needs cleaned up documentation.
| float16_t output_alpha; | ||
| float tmx; | ||
| float tmy; | ||
| vec4 ai_ao_tmx_tmy; |
Comment on lines
31
to
32
| float tmx = frag_info.ai_ao_tmx_tmy.z; | ||
| float tmy = frag_info.ai_ao_tmx_tmy.w; |
Member
There was a problem hiding this comment.
No abbreviations: https://google.github.io/styleguide/cppguide.html#General_Naming_Rules
Contributor
Author
There was a problem hiding this comment.
This isn't C++ :)
Member
There was a problem hiding this comment.
Oh I forgot, the glsl style guide says abbreviate everything. Nevermind, change everything else then.
Contributor
Author
There was a problem hiding this comment.
(do you get the joke, I used an abbreviation to respond to you)
Contributor
Author
There was a problem hiding this comment.
(also yes, I will change this :) )
Member
There was a problem hiding this comment.
Sorry, I was actually talking about the tmx, tmy variables. The packed one was unfortunate but I figured the docstring would help.
- float tmx = frag_info.ai_ao_tmx_tmy.z;
- float tmy = frag_info.ai_ao_tmx_tmy.w;
+ vec2 tileMode = vec2(frag_info.ai_ao_tmx_tmy.z, frag_info.ai_ao_tmx_tmy.w);
engine/src/flutter/impeller/entity/shaders/blending/porter_duff_blend.frag
Show resolved
Hide resolved
|
|
||
| virtual bool ShouldInvertBlendMode() const { return true; } | ||
|
|
||
| virtual std::optional<Rect> GetStrictSrcRect() const { return std::nullopt; } |
Member
There was a problem hiding this comment.
Docstring please, just add a "see also" that points to the definition of strict source rects.
gaaclarke
reviewed
May 29, 2025
Comment on lines
31
to
32
| float tmx = frag_info.ai_ao_tmx_tmy.z; | ||
| float tmy = frag_info.ai_ao_tmx_tmy.w; |
Member
There was a problem hiding this comment.
Sorry, I was actually talking about the tmx, tmy variables. The packed one was unfortunate but I figured the docstring would help.
- float tmx = frag_info.ai_ao_tmx_tmy.z;
- float tmy = frag_info.ai_ao_tmx_tmy.w;
+ vec2 tileMode = vec2(frag_info.ai_ao_tmx_tmy.z, frag_info.ai_ao_tmx_tmy.w);|
|
||
| virtual bool ShouldInvertBlendMode() const { return true; } | ||
|
|
||
| /// @brief the source rect of the draw if a strict source rect should |
Member
There was a problem hiding this comment.
Suggested change
| /// @brief the source rect of the draw if a strict source rect should | |
| /// @brief The source rect of the draw if a strict source rect should |
gaaclarke
approved these changes
May 29, 2025
Member
gaaclarke
left a comment
There was a problem hiding this comment.
LGTM modulo the tilemode abbreviation.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 30, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 30, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 30, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 3, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #169492
Currently we apply a color filter nine times for the drawImageNine calls, which is very expensive. We can do the same porterduff optimization we did for flame for drawImage nine by allowing the porter duff shader to specify a strict src rect.
Before
After