Skip to content

Remove the Y coordinate flip workaround in the Material stretch effect shader now that it is no longer required by the Impeller GLES back end#187247

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
jason-simmons:iplr_gles_ycoord_stretch
May 29, 2026
Merged

Conversation

@jason-simmons
Copy link
Copy Markdown
Member

#186556 makes the behavior of the Y axis in the GLES back end consistent with other Impeller back ends. Shaders will render incorrectly if they continue to flip the Y axis based on "#ifdef IMPELLER_TARGET_OPENGLES".

@jason-simmons jason-simmons requested review from bdero and gaaclarke May 28, 2026 17:36
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 28, 2026
Copy link
Copy Markdown
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 removes the conditional compilation block for IMPELLER_TARGET_OPENGLES in stretch_effect.frag, making the assignment of uv.y to out_v_norm unconditional. There are no review comments, and I have no feedback to provide.

Copy link
Copy Markdown
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

thanks jason!

@github-actions github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label May 28, 2026
@justinmc
Copy link
Copy Markdown
Contributor

About the code freeze failure:

The point of the code freeze is to minimize friction when users migrate to material_ui. Ideally they can go from any version 3.44 or later to material_ui 1.0.0.

My understanding, correct me if I'm wrong: As of the landing of #186556, Material's overscroll stretch effect is unacceptably broken. The only way to fix that is to land this PR.

Implications: Since material_ui 1.0.0 is going to be published with the code from 3.44, it will not be compatible with the SDK as of the landing of #186556. We'll have to publish a ~1.0.1 that contains the fix in this PR, and users with later versions of the SDK will have to migrate directly to that.

This is a big bump in the road for our decoupling migration, but it sounds like there is no other way?

@bdero
Copy link
Copy Markdown
Member

bdero commented May 29, 2026

One way to possibly get around this would be to just get rid of (or rename) FragmentProgram's backend-specific IMPELLER_TARGET_* defines (but keep IMPELLER_GRAPHICS_BACKEND and SKIA_GRAPHICS_BACKEND for runtime_effect.glsl) so that users who wound up employing this hack for ImageFilters won't be impacted.

As an aside, doing this sort of static hack within a filtering shader only works so long as the shader can only ever sample from a render target texture (i.e. a non-collapsed layer). If a filter shader were to ever end up sampling from a readonly texture on GLES, the static IMPELLER_TARGET_OPENGLES hack would break down and wind up rendering upside-down anyway, since decoded images are not stored upside-down.
It's easy to think of hypothetical optimization scenarios where that might happen. For example: Imagine, say, the layer being filtered contains only one trivial image draw, and so we collapse the layer and just apply the filter to the internal image directly.

@jason-simmons jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue May 29, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 29, 2026
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2026
…t shader now that it is no longer required by the Impeller GLES back end

flutter#186556 makes the behavior of
the Y axis in the GLES back end consistent with other Impeller back
ends.  Shaders will render incorrectly if they continue to flip
the Y axis based on "#ifdef IMPELLER_TARGET_OPENGLES".
@jason-simmons jason-simmons force-pushed the iplr_gles_ycoord_stretch branch from cb46def to 483a76f Compare May 29, 2026 18:04
@github-actions github-actions Bot removed the CICD Run CI/CD label May 29, 2026
@jason-simmons jason-simmons added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels May 29, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2026
@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit Bot commented May 29, 2026

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

@jason-simmons jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue May 29, 2026
Merged via the queue into flutter:master with commit a191e6d May 29, 2026
99 of 100 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jun 1, 2026
flutter/flutter@b05a9d7...54e199a

2026-06-01 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from jMR_VXQi07kAk8vbR... to q27k7_um1GvVrySZS... (flutter/flutter#187338)
2026-06-01 rmacnak@google.com Remove use of deprecated API related to removal of the VM isolate. (flutter/flutter#187013)
2026-06-01 116356835+AbdeMohlbi@users.noreply.github.com Improve `dependOnInheritedWidgetOfExactType` documentation to explain why it is bad to use it in initState (flutter/flutter#186216)
2026-06-01 chris@bracken.jp Revert "Move dart-lang/ai to a top level third party dependency in en… (flutter/flutter#187370)
2026-05-30 jakemac@google.com Move dart-lang/ai to a top level third party dependency in engine (flutter/flutter#187268)
2026-05-30 evanwall@buffalo.edu add sdf golden variants for OpenGL (flutter/flutter#187246)
2026-05-30 engine-flutter-autoroll@skia.org Roll Skia from dc01525ac468 to 0aee4675e0ad (6 revisions) (flutter/flutter#187334)
2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from c480ba2eb2eb to dc01525ac468 (4 revisions) (flutter/flutter#187317)
2026-05-29 jason-simmons@users.noreply.github.com Remove the Y coordinate flip workaround in the Material stretch effect shader now that it is no longer required by the Impeller GLES back end (flutter/flutter#187247)
2026-05-29 bkonyi@google.com [flutter_tools, devicelab] Fix filesystem safety guard for symlinked temp directories (flutter/flutter#187320)
2026-05-29 30870216+gaaclarke@users.noreply.github.com Brings linux tests out of bringup. (flutter/flutter#187271)
2026-05-29 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#187321)
2026-05-29 engine-flutter-autoroll@skia.org Roll Fuchsia GN SDK from SEfYx3xgueX3aFAY3... to oOAcFhkoE2_-Sy67z... (flutter/flutter#187310)
2026-05-29 36861262+QuncCccccc@users.noreply.github.com Fix mismatch between hit-test tree and traversal tree (flutter/flutter#186826)
2026-05-29 jason-simmons@users.noreply.github.com [Impeller] Ensure that the TextureGLES destructor cleans up all objects that it holds including the sync fence (flutter/flutter#187216)
2026-05-29 engine-flutter-autoroll@skia.org Roll Packages from 10cbdc5 to e930ced (3 revisions) (flutter/flutter#187306)
2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from d9d6b440c4e7 to c480ba2eb2eb (1 revision) (flutter/flutter#187305)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD framework flutter/packages/flutter repository. See also f: labels. override code freeze Override an active code freeze.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants