Skip to content

Conversation

@jason-simmons
Copy link
Member

Includes updates for the removal of SkApplyPerspectiveClip

Includes updates for the removal of SkApplyPerspectiveClip
@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Oct 14, 2025
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 updates the code to accommodate the removal of SkApplyPerspectiveClip from Skia. The changes correctly update calls to mapRect. However, this makes the implementations of TransformRectFast and TransformAndClipRect identical for SkMatrixAdapter and SkM44Adapter. As a result, the BM_TransformAndClipRect benchmark no longer measures clipping performance. I've added comments with suggestions to restore the clipping logic for these benchmarks to ensure their correctness.

Comment on lines 226 to 230
void TransformAndClipRect(const TestTransform& transform,
const TestRect& in,
TestRect& out) const override {
out.sk_rect =
transform.sk_matrix.mapRect(in.sk_rect, SkApplyPerspectiveClip::kYes);
out.sk_rect = transform.sk_matrix.mapRect(in.sk_rect);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

high

With the removal of the SkApplyPerspectiveClip parameter, this implementation of TransformAndClipRect is now identical to TransformRectFast. This means the BM_TransformAndClipRect benchmark for SkMatrix will no longer measure clipping performance, which is likely not the intent.

To restore the clipping behavior, you could implement it manually, similar to how impeller::Rect::TransformAndClipBounds is implemented. This would involve:

  1. Transforming the rectangle's corners to homogeneous coordinates using transform.sk_matrix.mapHomogeneousPoints.
  2. Implementing helper functions similar to ClipAndInsert and InterpolateAndInsert from impeller::geometry::Rect to perform clipping against the near plane.
  3. Calculating the bounds of the resulting clipped points.

This would ensure the benchmark accurately measures clipping performance for SkMatrix.

@jason-simmons jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 14, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Oct 15, 2025
Merged via the queue into flutter:master with commit 879d0fb Oct 15, 2025
186 of 187 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 16, 2025
Manual roll requested by tarrinneal@google.com

flutter/flutter@7cd821c...a873a27

2025-10-16 30870216+gaaclarke@users.noreply.github.com [tool] makes listing a shader also as an asset a build failure (flutter/flutter#176866)
2025-10-16 engine-flutter-autoroll@skia.org Roll Packages from d062181 to 835dccb (7 revisions) (flutter/flutter#177100)
2025-10-16 jason-simmons@users.noreply.github.com Handle the new location of Perfetto in create_updated_flutter_deps.py (flutter/flutter#177099)
2025-10-16 matt.kosarek@canonical.com Implement dialog windows for the win32 platform (flutter/flutter#176309)
2025-10-16 rmolivares@renzo-olivares.dev `SelectableRegion` should not show flutter rendered context menu when web context menu is enabled (flutter/flutter#176855)
2025-10-16 jason-simmons@users.noreply.github.com Manual roll Skia to 2d9df7c70b6f (flutter/flutter#177074)
2025-10-16 31685655+SalehTZ@users.noreply.github.com feat: add `OptionsViewOpenDirection.mostSpace` to `RawAutocomplete` (flutter/flutter#172997)
2025-10-16 43054281+camsim99@users.noreply.github.com [Android] Refactor `ImageReaderSurfaceProducer` restoration after app resumes (flutter/flutter#175937)
2025-10-15 34465683+rkishan516@users.noreply.github.com Refactor: migrate fade upwards page transition builder to widgets (flutter/flutter#175560)
2025-10-15 fluttergithubbot@gmail.com Marks Windows windowing_test to be unflaky (flutter/flutter#176701)
2025-10-15 72062416+Yash-Dhrangdhariya@users.noreply.github.com fix: 🐛 Add equality and hashCode implementations to ScrollAwareImageProvider (flutter/flutter#175038)
2025-10-15 mohellebiabdessalem@gmail.com Updates `sliver_tree.1.dart‎` to use `MediaQuery.widthOf(context)`  (flutter/flutter#176888)
2025-10-15 mdebbar@google.com [web] Fix focus issues in newer versions of Chrome (flutter/flutter#176938)
2025-10-15 jessiewong401@gmail.com [Android 16] Update `android_engine_vulkan_tests` to Test Against SDK 36 Emulator (flutter/flutter#176985)
2025-10-15 sokolovskyi.konstantin@gmail.com Fix key events interception by RadioGroup when no Radio is focused. (flutter/flutter#176335)
2025-10-15 21270878+elliette@users.noreply.github.com Update cherry-pick instructions to include instructions for pre-release CPs (flutter/flutter#177020)
2025-10-15 jason-simmons@users.noreply.github.com Manual roll Skia to c501c727a007 (flutter/flutter#177015)
2025-10-15 robert.ancell@canonical.com Update examples to latest Linux runner style (flutter/flutter#177033)
2025-10-15 59215665+davidhicks980@users.noreply.github.com [material/menu_anchor.dart] Create internal menu controller if external controller is changed to null. (flutter/flutter#176375)
2025-10-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix - TalkBack does not announce list information (#174374)" (flutter/flutter#177062)
2025-10-14 robert.ancell@canonical.com Implement Regular Windows for Linux (flutter/flutter#176187)
2025-10-14 31510811+jwlilly@users.noreply.github.com Fix - TalkBack does not announce list information (flutter/flutter#174374)

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
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
Includes updates for the removal of SkApplyPerspectiveClip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants