[Impeller] Remove the y_coord_scale Y-flip plumbing#187224
Conversation
Follow-up to flutter#186556, which absorbed GLES's Y-axis difference into the vertex stage and pinned Texture::GetYCoordScale() to 1.0. With every call site now passing 1.0 and every IPRemapCoords call reducing to identity, this PR deletes the cruft: the y_coord_scale parameter from the IPSample helpers in texture.glsl, the *_y_coord_scale uniform field from every affected vertex and fragment shader, the IPRemapCoords helper (and the now-empty conversions.glsl), the C++ RemapQuadCoords helper in gaussian_blur_filter_contents.cc, the frame_info.*_y_coord_scale = ...->GetYCoordScale() assignments in 21 callers, the dead DCHECK branch in texture_contents.cc, and Texture::GetYCoordScale() (plus its TextureGLES override).
|
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
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the Y-coordinate scaling logic (y_coord_scale and GetYCoordScale) across Impeller's shaders, textures, and rendering contents, simplifying texture sampling coordinates. The review feedback highlights two instances in atlas_contents.cc where removing the texture_sampler_y_coord_scale assignment leaves a declared frame_info variable completely uninitialized, which could lead to rendering issues if it is bound to the vertex shader.
|
Merry Christmas @gaaclarke 
It ain't July, but close enough. |
|
test-exempt: code refactor with no semantic change |
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
ahh, that code review was warm and fuzzy.
LGTM, CI isn't happy about the formatting though.
Apply clang-format to the gradient and helper shaders (whitespace/layout only, no semantic change) and refresh impeller/tools/malioc.json so the perf baseline reflects the removal of the *_y_coord_scale uniforms (uniform_registers_used drops by ~2 across the affected shaders).
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/187224, because - The status or check suite Windows tool_integration_tests_10_10 has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter-dashboard
Bot
removed
the
autosubmit
flutter/flutter@e70534d...b05a9d7 2026-05-29 [email protected] Roll Skia from 47155534833e to d9d6b440c4e7 (1 revision) (flutter/flutter#187301) 2026-05-29 [email protected] Roll Skia from f93ed13d77fb to 47155534833e (4 revisions) (flutter/flutter#187291) 2026-05-29 [email protected] [web_ui] Optimize skwasm text layout and path decoding to eliminate dynamic boxing churn under Wasm (flutter/flutter#186978) 2026-05-29 [email protected] Roll Fuchsia Linux SDK from SBpmmPxqx3lAvGojE... to jMR_VXQi07kAk8vbR... (flutter/flutter#187279) 2026-05-29 [email protected] Remove Material import from sliver tree rendering test (flutter/flutter#187000) 2026-05-29 [email protected] [Impeller] Remove the y_coord_scale Y-flip plumbing (flutter/flutter#187224) 2026-05-29 [email protected] Add/remove overlay child RenderObject from the tree in `attach`/`detach` (flutter/flutter#186564) 2026-05-29 [email protected] Roll Skia from fcfe5975c945 to f93ed13d77fb (4 revisions) (flutter/flutter#187273) 2026-05-28 [email protected] Handle complex RSE rendering in the uber SDF pipeline (flutter/flutter#186434) 2026-05-28 [email protected] Roll Dart SDK from 082191101fcc to 683322426411 (2 revisions) (flutter/flutter#187270) 2026-05-28 [email protected] Clarify route transition animations (flutter/flutter#186552) 2026-05-28 [email protected] document that the default Key is null and explain proper usage in list diffing (flutter/flutter#185197) 2026-05-28 [email protected] [flutter_tools] Use super parameters in missed spots (flutter/flutter#186197) 2026-05-28 [email protected] docs: Document MediaQueryData.alwaysUse24HourFormat on macOS, Windows, Linux, web (#160664) (flutter/flutter#186642) 2026-05-28 [email protected] Roll Skia from 5493e4c144cd to fcfe5975c945 (3 revisions) (flutter/flutter#187256) 2026-05-28 [email protected] Shares opengles golden context (flutter/flutter#187243) 2026-05-28 [email protected] Update the Curl CIPD package in .ci.yaml to version 8.20.0 (flutter/flutter#187133) 2026-05-28 [email protected] Improve SizedBox's docs (flutter/flutter#187208) 2026-05-28 [email protected] [Impeller] Support instanced rendering across all backends (flutter/flutter#186653) 2026-05-28 [email protected] [Android] Reset system UI visibility flags when setting edge-to-edge mode (flutter/flutter#187207) 2026-05-28 [email protected] Roll Skia from a38708fb7926 to 5493e4c144cd (7 revisions) (flutter/flutter#187241) 2026-05-28 [email protected] Turned on impeller by default on macos (flutter/flutter#186546) 2026-05-28 [email protected] Clarify lazy scroll extent docs (flutter/flutter#186864) 2026-05-28 [email protected] [web] Fix WebParagraph locales test (flutter/flutter#186813) 2026-05-28 [email protected] Roll Packages from 4b424d7 to 10cbdc5 (3 revisions) (flutter/flutter#187238) 2026-05-28 [email protected] Roll Dart SDK from f3db7b7d9801 to 082191101fcc (8 revisions) (flutter/flutter#187235) 2026-05-28 [email protected] Roll Skia from 32acea791248 to a38708fb7926 (1 revision) (flutter/flutter#187221) 2026-05-28 [email protected] [Flutter GPU] Add r32Float and remove Apple-only XR pixel formats (flutter/flutter#187069) 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 [email protected],[email protected] 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
3 participants
GOODNIGHT, SWEET PRINCE.
No semantic change, baby.
Follow-up to #186556 (and its tracking issue #186554), where I deprecated one of the worst landmines in Impeller by absorbing OpenGL ES's Y-axis difference into the vertex stage and pinned
Texture::GetYCoordScale()to 1.0. Every call site is now passing 1.0 and everyIPRemapCoordscall reduces to identity, so this PR removes the cruft. :)Pre-launch Checklist
///).