[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. |
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
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).
|
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/flutter@e70534d...b05a9d7 2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from 47155534833e to d9d6b440c4e7 (1 revision) (flutter/flutter#187301) 2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from f93ed13d77fb to 47155534833e (4 revisions) (flutter/flutter#187291) 2026-05-29 kevmoo@users.noreply.github.com [web_ui] Optimize skwasm text layout and path decoding to eliminate dynamic boxing churn under Wasm (flutter/flutter#186978) 2026-05-29 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from SBpmmPxqx3lAvGojE... to jMR_VXQi07kAk8vbR... (flutter/flutter#187279) 2026-05-29 burak.karahan@mail.ru Remove Material import from sliver tree rendering test (flutter/flutter#187000) 2026-05-29 bdero@google.com [Impeller] Remove the y_coord_scale Y-flip plumbing (flutter/flutter#187224) 2026-05-29 31859944+LongCatIsLooong@users.noreply.github.com Add/remove overlay child RenderObject from the tree in `attach`/`detach` (flutter/flutter#186564) 2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from fcfe5975c945 to f93ed13d77fb (4 revisions) (flutter/flutter#187273) 2026-05-28 evanwall@buffalo.edu Handle complex RSE rendering in the uber SDF pipeline (flutter/flutter#186434) 2026-05-28 engine-flutter-autoroll@skia.org Roll Dart SDK from 082191101fcc to 683322426411 (2 revisions) (flutter/flutter#187270) 2026-05-28 mvincentong@gmail.com Clarify route transition animations (flutter/flutter#186552) 2026-05-28 116356835+AbdeMohlbi@users.noreply.github.com document that the default Key is null and explain proper usage in list diffing (flutter/flutter#185197) 2026-05-28 srawlins@google.com [flutter_tools] Use super parameters in missed spots (flutter/flutter#186197) 2026-05-28 mr_nadeem_iqbal@yahoo.com docs: Document MediaQueryData.alwaysUse24HourFormat on macOS, Windows, Linux, web (#160664) (flutter/flutter#186642) 2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from 5493e4c144cd to fcfe5975c945 (3 revisions) (flutter/flutter#187256) 2026-05-28 30870216+gaaclarke@users.noreply.github.com Shares opengles golden context (flutter/flutter#187243) 2026-05-28 jason-simmons@users.noreply.github.com Update the Curl CIPD package in .ci.yaml to version 8.20.0 (flutter/flutter#187133) 2026-05-28 737941+loic-sharma@users.noreply.github.com Improve SizedBox's docs (flutter/flutter#187208) 2026-05-28 bdero@google.com [Impeller] Support instanced rendering across all backends (flutter/flutter#186653) 2026-05-28 43054281+camsim99@users.noreply.github.com [Android] Reset system UI visibility flags when setting edge-to-edge mode (flutter/flutter#187207) 2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from a38708fb7926 to 5493e4c144cd (7 revisions) (flutter/flutter#187241) 2026-05-28 30870216+gaaclarke@users.noreply.github.com Turned on impeller by default on macos (flutter/flutter#186546) 2026-05-28 mvincentong@gmail.com Clarify lazy scroll extent docs (flutter/flutter#186864) 2026-05-28 mdebbar@google.com [web] Fix WebParagraph locales test (flutter/flutter#186813) 2026-05-28 engine-flutter-autoroll@skia.org Roll Packages from 4b424d7 to 10cbdc5 (3 revisions) (flutter/flutter#187238) 2026-05-28 engine-flutter-autoroll@skia.org Roll Dart SDK from f3db7b7d9801 to 082191101fcc (8 revisions) (flutter/flutter#187235) 2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from 32acea791248 to a38708fb7926 (1 revision) (flutter/flutter#187221) 2026-05-28 bdero@google.com [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 boetger@google.com,stuartmorgan@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
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
///).