Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm #42088

eyebrowsoffire · 2023-05-17T04:17:50Z

This implements ImageFilter, ColorFilter and MaskFilter in Skwasm. This includes support on the Paint object, as well as the SceneBuilder layers that use these types.

flutter-dashboard · 2023-05-17T16:08:20Z

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.

Changes reported for pull request #42088 at sha 163ce3b

flutter-dashboard · 2023-05-17T18:44:00Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #42088 at sha 0553179

yjbanov · 2023-05-18T20:06:09Z

lib/web_ui/lib/src/engine/scene_painting.dart

+}
+
+abstract class ScenePicture implements ui.Picture {
+  ui.Rect get cullRect;


Should this be called bounds? I'm not sure how a picture could be "culling" anything 🤔

This terminology comes from skia itself, see SkPicture::cullRect. I think the idea is that if that rectangle can be culled by another operation (like a clip operation or something), it can cull the picture and skip all of its drawing commands.

lib/web_ui/lib/src/engine/scene_painting.dart

yjbanov · 2023-05-18T20:14:53Z

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart

+  final MaskFilterHandle handle;
+  bool _isDisposed = false;
+
+  void dispose() {


I believe the pattern we use is to assert(!_isDisposed) in all dispose methods because double-dispose likely points to some app-side bug.

Well, I don't actually have the finalization registry wired up yet, but the idea is for this to be safe to call more than once, since it might be called manually by the user and it might be called by the finalization registry. I think it makes sense for this to be idempotent, at least for how I envision this being used with the UniqueRef stuff.

Yeah, as long as the object reference does not leak to the framework then it should be fine. The trouble starts when the framework references the object. Then you have a chicken and egg problem, the framework drops the reference and the object is GC'd, but then FinalizationRegistry doesn't have an object to call dispose() on. In CanvasKit each Skia object is represented using a pair of Dart objects, a Ck* object (e.g. CkPaint) and a UniqueRef. Then we do FinalizationRegistry.register(ckObject, uniqueRef) and hand the ckObject reference to the framework. When ckObject is GC'd, we call uniqueRef.dispose(). UniqueRef is the only place that could potentially be disposed more than once, once manually and once via FinalizationRegistry. However, CkObject should only be disposed once, which is where we put assert(!isDisposed).

I'm guessing in Skwasm we don't need UniqueRef at all. I think Pointer can play the same role without an extra wrapper.

Yeah I see what you're saying. I know FinalizationRegistry basically needs an extra layer of indirection, but I haven't quite figured out how this is all going to work with the Skwasm objects. If you don't mind, I'm just going to kick the can on this issue and revisit it when I hook up the finalization registry stuff.

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/layers.dart

yjbanov · 2023-05-18T20:20:06Z

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paint.dart

+      -1.0, 0, 0, 1.0, 0, // row
+      0, -1.0, 0, 1.0, 0, // row
+      0, 0, -1.0, 1.0, 0, // row
+      1.0, 1.0, 1.0, 1.0, 0


Add // the boat to the last line 😛

flutter/engine#42088)

…127292) flutter/engine@aac0919...f0f3fe7 2023-05-21 skia-flutter-autoroll@skia.org Roll Skia from 7c7dff949a27 to 5b2005e47bf3 (1 revision) (flutter/engine#42199) 2023-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from gQ989rlKAuTJHQR-C... to 88pzkUAkSKsJrNG38... (flutter/engine#42198) 2023-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 868_67npyO8nD_JCx... to JU-dKW3CQIUzhbqWE... (flutter/engine#42197) 2023-05-21 skia-flutter-autoroll@skia.org Roll Skia from a60bfcb01af9 to 7c7dff949a27 (1 revision) (flutter/engine#42195) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from c_fRDyBVZX-MwW5fS... to gQ989rlKAuTJHQR-C... (flutter/engine#42194) 2023-05-20 skia-flutter-autoroll@skia.org Roll Skia from f3e9cb7d37fd to a60bfcb01af9 (1 revision) (flutter/engine#42193) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from sfLkc5VBFU6UkljF6... to 868_67npyO8nD_JCx... (flutter/engine#42191) 2023-05-20 kjlubick@users.noreply.github.com Move SkSurface::MakeNull to SkSurfaces::Null (flutter/engine#42158) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from TWjmvLCOnYAUgAzvT... to c_fRDyBVZX-MwW5fS... (flutter/engine#42189) 2023-05-20 skia-flutter-autoroll@skia.org Roll Skia from b4a4782cf89d to f3e9cb7d37fd (1 revision) (flutter/engine#42188) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from pwdDQgM88sqLmZczj... to sfLkc5VBFU6UkljF6... (flutter/engine#42187) 2023-05-20 skia-flutter-autoroll@skia.org Roll Skia from 7202b405f061 to b4a4782cf89d (19 revisions) (flutter/engine#42185) 2023-05-20 dnfield@google.com [Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan (flutter/engine#42146) 2023-05-20 jacksongardner@google.com Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm (flutter/engine#42088) 2023-05-20 30870216+gaaclarke@users.noreply.github.com [Impeller] Made other vulkan objects cleanup properly. (flutter/engine#42113) 2023-05-19 coldpalelight@gmail.com [Impeller] Fix the issue that 'coverage_coords' is incorrectly calculated in 'FillPathGeometry::GetPositionUVBuffer' (flutter/engine#42155) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from TWjmvLCOnYAU to 88pzkUAkSKsJ fuchsia/sdk/core/mac-amd64 from pwdDQgM88sqL to JU-dKW3CQIUz If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter#127292) flutter/engine@aac0919...f0f3fe7 2023-05-21 skia-flutter-autoroll@skia.org Roll Skia from 7c7dff949a27 to 5b2005e47bf3 (1 revision) (flutter/engine#42199) 2023-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from gQ989rlKAuTJHQR-C... to 88pzkUAkSKsJrNG38... (flutter/engine#42198) 2023-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 868_67npyO8nD_JCx... to JU-dKW3CQIUzhbqWE... (flutter/engine#42197) 2023-05-21 skia-flutter-autoroll@skia.org Roll Skia from a60bfcb01af9 to 7c7dff949a27 (1 revision) (flutter/engine#42195) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from c_fRDyBVZX-MwW5fS... to gQ989rlKAuTJHQR-C... (flutter/engine#42194) 2023-05-20 skia-flutter-autoroll@skia.org Roll Skia from f3e9cb7d37fd to a60bfcb01af9 (1 revision) (flutter/engine#42193) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from sfLkc5VBFU6UkljF6... to 868_67npyO8nD_JCx... (flutter/engine#42191) 2023-05-20 kjlubick@users.noreply.github.com Move SkSurface::MakeNull to SkSurfaces::Null (flutter/engine#42158) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from TWjmvLCOnYAUgAzvT... to c_fRDyBVZX-MwW5fS... (flutter/engine#42189) 2023-05-20 skia-flutter-autoroll@skia.org Roll Skia from b4a4782cf89d to f3e9cb7d37fd (1 revision) (flutter/engine#42188) 2023-05-20 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from pwdDQgM88sqLmZczj... to sfLkc5VBFU6UkljF6... (flutter/engine#42187) 2023-05-20 skia-flutter-autoroll@skia.org Roll Skia from 7202b405f061 to b4a4782cf89d (19 revisions) (flutter/engine#42185) 2023-05-20 dnfield@google.com [Impeller] avoid creating multiple concurrent message loops for Andorid Vulkan (flutter/engine#42146) 2023-05-20 jacksongardner@google.com Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm (flutter/engine#42088) 2023-05-20 30870216+gaaclarke@users.noreply.github.com [Impeller] Made other vulkan objects cleanup properly. (flutter/engine#42113) 2023-05-19 coldpalelight@gmail.com [Impeller] Fix the issue that 'coverage_coords' is incorrectly calculated in 'FillPathGeometry::GetPositionUVBuffer' (flutter/engine#42155) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from TWjmvLCOnYAU to 88pzkUAkSKsJ fuchsia/sdk/core/mac-amd64 from pwdDQgM88sqL to JU-dKW3CQIUz If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

eyebrowsoffire added 8 commits May 16, 2023 11:42

Implemented filter constructors.

0f35183

Implement setters on paint.

8872ad3

Add ColorFilter -> ImageFilter conversion.

beef090

Fixing up color filter conversion a bit.

3703765

Implemented filter-based scene layers.

17fbc22

Added tests for filters on paint.

c064963

Added scene builder tests.

781fb09

Scene builder tests.

2a1b318

flutter-dashboard bot added the platform-web Code specifically for the web engine label May 17, 2023

Fix tests on Safari and update licenses golden.

163ce3b

flutter-dashboard bot added the will affect goldens label May 17, 2023

Implement invertColors

0553179

yjbanov approved these changes May 18, 2023

View reviewed changes

eyebrowsoffire added 2 commits May 19, 2023 15:41

Merge branch 'main' into skwasm_filters

7afb542

Added doc comments and removed a debugging print statement.

d6d40f5

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2023

auto-submit bot merged commit 18aec20 into flutter:main May 20, 2023
29 checks passed

eyebrowsoffire mentioned this pull request May 20, 2023

Implement ColorFilter/ImageFilter/MaskFilter in Skwasm flutter/flutter#126342

Closed

engine-flutter-autoroll mentioned this pull request May 20, 2023

Roll Flutter Engine from aac09195688d to f939b5f062d7 (9 revisions) flutter/flutter#127268

Closed

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

18aec202b Implement ImageFilter/ColorFilter/MaskFilter in Skwasm (

53e40e8

flutter/engine#42088)

engine-flutter-autoroll mentioned this pull request May 20, 2023

Roll Flutter Engine from aac09195688d to ffd9118bc97c (10 revisions) flutter/flutter#127279

Closed

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

18aec202b Implement ImageFilter/ColorFilter/MaskFilter in Skwasm (

1e61930

flutter/engine#42088)

engine-flutter-autoroll mentioned this pull request May 21, 2023

Roll Flutter Engine from aac09195688d to 12f9a46e0035 (12 revisions) flutter/flutter#127283

Closed

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

18aec202b Implement ImageFilter/ColorFilter/MaskFilter in Skwasm (

8b3b564

flutter/engine#42088)

engine-flutter-autoroll mentioned this pull request May 21, 2023

Roll Flutter Engine from aac09195688d to 5d1b713784a7 (13 revisions) flutter/flutter#127285

Closed

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

18aec202b Implement ImageFilter/ColorFilter/MaskFilter in Skwasm (

970f308

flutter/engine#42088)

engine-flutter-autoroll mentioned this pull request May 21, 2023

Roll Flutter Engine from aac09195688d to f0f3fe729a91 (16 revisions) flutter/flutter#127292

Merged

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

18aec202b Implement ImageFilter/ColorFilter/MaskFilter in Skwasm (

cb10864

flutter/engine#42088)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm #42088

Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm #42088

eyebrowsoffire commented May 17, 2023 •

edited

flutter-dashboard bot commented May 17, 2023

flutter-dashboard bot commented May 17, 2023

yjbanov May 18, 2023

eyebrowsoffire May 18, 2023

yjbanov May 18, 2023

eyebrowsoffire May 18, 2023

yjbanov May 19, 2023

yjbanov May 19, 2023

eyebrowsoffire May 19, 2023

yjbanov May 18, 2023

Implement ImageFilter/ColorFilter/MaskFilter in Skwasm #42088

Implement ImageFilter/ColorFilter/MaskFilter in Skwasm #42088

Conversation

eyebrowsoffire commented May 17, 2023 • edited

flutter-dashboard bot commented May 17, 2023

flutter-dashboard bot commented May 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm #42088

Implement `ImageFilter`/`ColorFilter`/`MaskFilter` in Skwasm #42088

eyebrowsoffire commented May 17, 2023 •

edited