Skip to content

Fix incorrect rendering from drawImageNine when running on impeller with opengles#182179

Merged
auto-submit[bot] merged 3 commits intoflutter:masterfrom
b-luk:texurecontentssourcerectyscale
Feb 11, 2026
Merged

Fix incorrect rendering from drawImageNine when running on impeller with opengles#182179
auto-submit[bot] merged 3 commits intoflutter:masterfrom
b-luk:texurecontentssourcerectyscale

Conversation

@b-luk
Copy link
Contributor

@b-luk b-luk commented Feb 10, 2026

The frag_info.source_rect passed into https://github.com/flutter/flutter/blob/98977409b2ac8209be9e918abb0bbb9c37a23d0f/engine/src/flutter/impeller/entity/shaders/texture_fill_strict_src.frag from texture_contents.cc did not account for an inverted y scale (used with an opengles impeller backend).

This ends up causing the coordinates used to sample the texture in the fragment shader to be incorrectly clamped. A lot of the y coordinates end up incorrectly being considered outside of the source_rect, so we end up with lots of incorrect y values and vertical smearing caused by the clamping.

The texture_fill_strict_src.frag shader is used primarily by calls to drawImageNIne. NinePatchConverter calls DrawImageRect with SourceRectConstraint::kStrict, which uses texture_fill_strict_src.frag.

#182101 shows a repro case for this. The golden test issue discussed in #181933 (comment) is also caused by this.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

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. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Feb 10, 2026
@b-luk b-luk changed the title Account for inverted y coordinates when setting frag_info.source_rect… Fix incorrect rendering from drawImageNine when running on impeller with opengles Feb 10, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request corrects an issue with texture coordinate clamping when using an inverted Y-axis, which is common with the OpenGL ES backend. The change adjusts the source_rect for the fragment shader to account for the inverted Y-coordinates, preventing vertical smearing artifacts. My review includes a minor suggestion to use 1.0f for float literals to align with the project's style guide and ensure type consistency.

@b-luk
Copy link
Contributor Author

b-luk commented Feb 10, 2026

I expect golden changes that update the opengles canvas_test_blurred_rendering_ tests discussed in #181933 (comment)

@b-luk
Copy link
Contributor Author

b-luk commented Feb 11, 2026

https://flutter-gold.skia.org/search?crs=github&issue=182179&patchsets=2 does show the expected fixed golden

@b-luk b-luk requested a review from gaaclarke February 11, 2026 00:36
FSStrict::FragInfo frag_info;
frag_info.source_rect = Vector4(strict_texture_coords.GetLTRB());
if (texture_->GetYCoordScale() < 0.0) {
frag_info.source_rect = Vector4(strict_texture_coords.GetLeft(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's FML_DCHECK that it is -1 since you aren't actually using the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@gaaclarke
Copy link
Member

Nice find!

@b-luk b-luk added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 11, 2026
@auto-submit
Copy link
Contributor

auto-submit bot commented Feb 11, 2026

autosubmit label was removed for flutter/flutter/182179, because - The status or check suite Linux linux_host_engine_test has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 11, 2026
@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 11, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Feb 11, 2026
Merged via the queue into flutter:master with commit 9cde519 Feb 11, 2026
184 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 11, 2026
@b-luk b-luk deleted the texurecontentssourcerectyscale branch February 11, 2026 18:52
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2026
rickhohler pushed a commit to rickhohler/flutter that referenced this pull request Feb 19, 2026
…ith opengles (flutter#182179)

The `frag_info.source_rect` passed into
https://github.com/flutter/flutter/blob/98977409b2ac8209be9e918abb0bbb9c37a23d0f/engine/src/flutter/impeller/entity/shaders/texture_fill_strict_src.frag
from
[texture_contents.cc](https://github.com/flutter/flutter/blob/98977409b2ac8209be9e918abb0bbb9c37a23d0f/engine/src/flutter/impeller/entity/contents/texture_contents.cc#L187)
did not account for an inverted y scale (used with an opengles impeller
backend).

This ends up causing the coordinates used to sample the texture in the
fragment shader to be incorrectly clamped. A lot of the y coordinates
end up incorrectly being considered outside of the source_rect, so we
end up with lots of incorrect y values and vertical smearing caused by
the clamping.

The texture_fill_strict_src.frag shader is used primarily by calls to
drawImageNIne. NinePatchConverter calls DrawImageRect with
SourceRectConstraint::kStrict, which uses texture_fill_strict_src.frag.

flutter#182101 shows a repro case for
this. The golden test issue discussed in
flutter#181933 (comment)
is also caused by this.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants