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
…10244)

Manual roll requested by [email protected]

flutter/flutter@7cd821c...a873a27

2025-10-16 [email protected] [tool] makes listing a shader also as an asset a build failure (flutter/flutter#176866)
2025-10-16 [email protected] Roll Packages from d062181 to 835dccb (7 revisions) (flutter/flutter#177100)
2025-10-16 [email protected] Handle the new location of Perfetto in create_updated_flutter_deps.py (flutter/flutter#177099)
2025-10-16 [email protected] Implement dialog windows for the win32 platform (flutter/flutter#176309)
2025-10-16 [email protected] `SelectableRegion` should not show flutter rendered context menu when web context menu is enabled (flutter/flutter#176855)
2025-10-16 [email protected] Manual roll Skia to 2d9df7c70b6f (flutter/flutter#177074)
2025-10-16 [email protected] feat: add `OptionsViewOpenDirection.mostSpace` to `RawAutocomplete` (flutter/flutter#172997)
2025-10-16 [email protected] [Android] Refactor `ImageReaderSurfaceProducer` restoration after app resumes (flutter/flutter#175937)
2025-10-15 [email protected] Refactor: migrate fade upwards page transition builder to widgets (flutter/flutter#175560)
2025-10-15 [email protected] Marks Windows windowing_test to be unflaky (flutter/flutter#176701)
2025-10-15 [email protected] fix: 🐛 Add equality and hashCode implementations to ScrollAwareImageProvider (flutter/flutter#175038)
2025-10-15 [email protected] Updates `sliver_tree.1.dart‎` to use `MediaQuery.widthOf(context)`  (flutter/flutter#176888)
2025-10-15 [email protected] [web] Fix focus issues in newer versions of Chrome (flutter/flutter#176938)
2025-10-15 [email protected] [Android 16] Update `android_engine_vulkan_tests` to Test Against SDK 36 Emulator (flutter/flutter#176985)
2025-10-15 [email protected] Fix key events interception by RadioGroup when no Radio is focused. (flutter/flutter#176335)
2025-10-15 [email protected] Update cherry-pick instructions to include instructions for pre-release CPs (flutter/flutter#177020)
2025-10-15 [email protected] Manual roll Skia to c501c727a007 (flutter/flutter#177015)
2025-10-15 [email protected] Update examples to latest Linux runner style (flutter/flutter#177033)
2025-10-15 [email protected] [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 [email protected] Implement Regular Windows for Linux (flutter/flutter#176187)
2025-10-14 [email protected] 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 [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
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