Implement `clipPath` Mutator for hcpp #164525

gmackall · 2025-03-04T01:03:49Z

Implements clipPath mutator for hcpp.

Fixes #164219

working_clippath.mp4

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gmackall · 2025-03-04T21:49:12Z

Screenshots should look like (where red is the platform view):

hybrid_composition_pp_platform_view complex_clippath

and
hybrid_composition_pp_platform_view complex_clippath_no_triangle

jonahwilliams

change LGTM, though slightly worried about the FTL crash. Maybe unrelated, I hope?

dev/integration_tests/android_engine_test/lib/hcpp/platform_view_clippath_main.dart

...r/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorsStack.java

gmackall · 2025-03-04T22:33:59Z

change LGTM, though slightly worried about the FTL crash. Maybe unrelated, I hope?

Huh I think that also crashed on a previous commit, I'll take a look

gmackall · 2025-03-04T23:30:13Z

change LGTM, though slightly worried about the FTL crash. Maybe unrelated, I hope?

Huh I think that also crashed on a previous commit, I'll take a look

Okay it looks like the failure is related:

[ERROR:flutter/shell/platform/android/platform_view_android_jni_impl.cc(1256)] Could not locate android.graphics.Path.conicTo method

And I assume I need to only register these methods if using API 34+ (conicTo was added then, but also our hcpp only supports 34+ regardless)

gmackall · 2025-03-05T00:00:04Z

@jonahwilliams does this approach of guarding the method registration seem reasonable? I notice other places in the code we simply just ignore if registration fails, e.g.:

flutter/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc

Line 1144 in b831b26

// Continue on as this method may not exist at API <= 29.

  if (g_image_get_hardware_buffer_method == nullptr) {
    // Continue on as this method may not exist at API <= 29.
    fml::jni::ClearException(env, true);
  }

I could also change to that here if you prefer. Or I could change those instances to behave like this, i.e. guard on the version code.

jonahwilliams · 2025-03-05T00:05:26Z

If that is the existing pattern that seems reasonable to me. Yeah none of this will get used pre 34, unless we accidentally refactor some of the old HC mode support later. So maybe also add a DCHECK in the path converter that the method is defined?

flutter-dashboard · 2025-03-05T01:35:27Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #164525 at sha 9887180

jonahwilliams

Still LGTM

flutter/flutter@2578d97...321fbc0 2025-03-06 [email protected] Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712) 2025-03-06 [email protected] [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491) 2025-03-06 [email protected] [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296) 2025-03-06 [email protected] Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701) 2025-03-06 [email protected] Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677) 2025-03-06 [email protected] Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669) 2025-03-06 [email protected] [Impeller] use device private on non-iOS devices. (flutter/flutter#164601) 2025-03-05 [email protected] Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661) 2025-03-05 [email protected] Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657) 2025-03-05 [email protected] Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649) 2025-03-05 [email protected] Adds animateToItem to the CarouselController (flutter/flutter#162694) 2025-03-05 [email protected] Cleanup content context (flutter/flutter#164229) 2025-03-05 [email protected] Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700) 2025-03-05 [email protected] [Impeller] fix macOS managed memory. (flutter/flutter#164635) 2025-03-05 [email protected] [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588) 2025-03-05 [email protected] Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392) 2025-03-05 [email protected] Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582) 2025-03-05 [email protected] Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630) 2025-03-05 [email protected] Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622) 2025-03-05 [email protected] Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308) 2025-03-05 [email protected] Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599) 2025-03-05 [email protected] Implement `clipPath` Mutator for hcpp (flutter/flutter#164525) 2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600) 2025-03-05 [email protected] [macos] prefer integrated GPU. (flutter/flutter#164569) 2025-03-05 [email protected] Enforce minSdk constraint for Android Flutter (flutter/flutter#164251) 2025-03-05 [email protected] Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111) 2025-03-05 [email protected] Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590) 2025-03-05 [email protected] when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456) 2025-03-05 [email protected] Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583) 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

impleement clipPath for hcpp

6e85bc8

github-actions bot added platform-android Android applications specifically engine flutter/engine related. See also e: labels. labels Mar 4, 2025

Gray Mackall and others added 4 commits March 3, 2025 17:13

conform to logging not erroring standard

a29d5f8

Merge branch 'master' into clean_branch

6dc07d3

some tests - complex clip, and complex clip with some toggling

4eab335

Merge branch 'master' into clean_branch

24d9a49

gmackall changed the title ~~[wip] Implement clipPath Mutator for hcpp~~ Implement clipPath Mutator for hcpp Mar 4, 2025

gmackall marked this pull request as ready for review March 4, 2025 21:46

gmackall requested a review from matanlurey as a code owner March 4, 2025 21:46

gmackall requested review from a team and jonahwilliams March 4, 2025 21:46

undo local testing

5943989

jonahwilliams approved these changes Mar 4, 2025

View reviewed changes

dev/integration_tests/android_engine_test/lib/hcpp/platform_view_clippath_main.dart Outdated Show resolved Hide resolved

...r/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorsStack.java Show resolved Hide resolved

Gray Mackall added 2 commits March 4, 2025 14:54

format

909d9aa

javadoc

1611830

Gray Mackall added 2 commits March 4, 2025 15:57

make registration of path methods conditional on api 34+

7b01269

format

e67a45d

remove line that was only to see the path

8612c7a

undo guarding, conform to previous pattern of just not erroring

9887180

gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025

flutter-dashboard bot added the will affect goldens Changes to golden files label Mar 5, 2025

jonahwilliams approved these changes Mar 5, 2025

View reviewed changes

matanlurey approved these changes Mar 5, 2025

View reviewed changes

gmackall added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Mar 5, 2025

auto-submit bot added this pull request to the merge queue Mar 5, 2025

Merged via the queue into flutter:master with commit 1301475 Mar 5, 2025
179 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 296c54f7a001 (42 revisions) flutter/packages#8797

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

9cc3ddc

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 5ca20d8d0e78 (43 revisions) flutter/packages#8798

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

3dc4d60

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 86e4c12a068a (44 revisions) flutter/packages#8799

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

d062437

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Manual roll Flutter from 04d39343a691 to 57cddce92438 (45 revisions) flutter/packages#8800

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

54f058f

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 57cddce92438 (45 revisions) flutter/packages#8801

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

4c5273f

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 1e9676fc9527 (46 revisions) flutter/packages#8802

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

e5f2410

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

e1360e9

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to 099e6d39fe38 (27 revisions) flutter/packages#8806

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

38cee53

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to d778ed25a485 (28 revisions) flutter/packages#8807

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

68fee21

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to 321fbc0e7e81 (29 revisions) flutter/packages#8808

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

ba1f54d

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

83ea5ee

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

1301475 Implement clipPath Mutator for hcpp (flutter/flutter#164525)

914ffd5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement `clipPath` Mutator for hcpp #164525

Implement `clipPath` Mutator for hcpp #164525

Uh oh!

gmackall commented Mar 4, 2025 •

edited

Loading

Uh oh!

gmackall commented Mar 4, 2025 •

edited

Loading

Uh oh!

jonahwilliams left a comment

Uh oh!

Uh oh!

Uh oh!

gmackall commented Mar 4, 2025

Uh oh!

gmackall commented Mar 4, 2025

Uh oh!

gmackall commented Mar 5, 2025

Uh oh!

jonahwilliams commented Mar 5, 2025

Uh oh!

flutter-dashboard bot commented Mar 5, 2025

Uh oh!

jonahwilliams left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implement clipPath Mutator for hcpp #164525

Implement clipPath Mutator for hcpp #164525

Uh oh!

Conversation

gmackall commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gmackall commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gmackall commented Mar 4, 2025

Uh oh!

gmackall commented Mar 4, 2025

Uh oh!

gmackall commented Mar 5, 2025

Uh oh!

jonahwilliams commented Mar 5, 2025

Uh oh!

flutter-dashboard bot commented Mar 5, 2025

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implement `clipPath` Mutator for hcpp #164525

Implement `clipPath` Mutator for hcpp #164525

gmackall commented Mar 4, 2025 •

edited

Loading

gmackall commented Mar 4, 2025 •

edited

Loading