Repository navigation
[web] Unify ui.Paint across CanvasKit and Skwasm - #196071
auto-submit[bot] merged 9 commits into
Conversation
Replace the duplicated CkPaint and SkwasmPaint implementations of ui.Paint with a single shared EnginePaint that owns all paint state, defaults and toString. ui.Paint() now constructs an EnginePaint directly. Native paints are created per draw call through EnginePaint.createBackendPaint(), which asks the renderer for a BackendPaint. The caller owns the result and disposes it after the draw; backend paints are never cached or finalized. CkPaint and SkwasmPaint are now thin BackendPaint wrappers around an SkPaint and a native paint handle. Behavior that previously differed between renderers is now shared: - the imageFilter getter returns exactly the filter that was set, even when it is a ColorFilter; - mask filters with a zero, negative or infinite sigma are dropped before reaching either backend; - setting an equal shader, color filter or image filter is a no-op; - the EngineImageFilter derived from imageFilter is cached on the paint. Also attach the finalizer for a ColorFilter's image-filter backend object to the ColorFilter that caches it, rather than to the throwaway EngineColorFilterImageFilter wrapper, so the cached backend filter is not freed while the ColorFilter is still alive.
Add a test showing the CanvasKit SkPaint counter observes a live backend paint, assert exact SkPaint creation counts for the paint consumers, make every expectation reason explain why it holds, and reword the test about changing a paint between draws.
Override the getters with EngineColorFilter? and EngineShader? return types (setters still accept the public ui types) and drop the separate engineColorFilter and engineShader getters.
There was a problem hiding this comment.
Code Review
This pull request refactors the paint implementation in the Flutter Web engine by separating the Dart-side state from the native backend paint objects. It introduces EnginePaint (implementing ui.Paint) to hold the Dart-side state, and BackendPaint (with concrete implementations CkPaint for CanvasKit and SkwasmPaint for Skwasm) to represent the native paint objects used for individual draw calls. Native paint objects are now created per draw call and eagerly disposed of immediately after use, preventing memory leaks. Corresponding unit tests have been added to verify the lifecycle and disposal of these backend paint objects. I have no feedback to provide as there are no review comments.
…ross-canvaskit-and-skwasm
|
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 Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
flutter/flutter@d463821...fffd8c2 2026-11-10 [email protected] Roll Fuchsia Test Scripts from 8QcsP_SmbsT09VgUs... to PXc5D7S_q6sIYpfFJ... (flutter/flutter#196273) 2026-11-10 [email protected] [web] Accept the capped letter spacing line shift in kitchensink (flutter/flutter#196148) 2026-11-10 [email protected] Fix Vulkan MSAA diagonal seam in FramebufferBlend (flutter/flutter#196197) 2026-11-10 [email protected] Read back images on the raster thread on OpenGLES (flutter/flutter#195927) 2026-11-10 [email protected] [web] Unify ui.Paint across CanvasKit and Skwasm (flutter/flutter#196071) 2026-11-10 [email protected] Roll Fuchsia Test Scripts from p3N3B-Kchh1bthB6n... to 8QcsP_SmbsT09VgUs... (flutter/flutter#196249) 2026-11-09 [email protected] [Impeller] Reland clipped BackdropFilter square corners fix (flutter/flutter#195716) 2026-11-09 [email protected] [web] Recreate canvas and context on CkOffscreenSurface resize to fix WebGL clipping (flutter/flutter#192769) 2026-11-09 [email protected] Roll Dart SDK from ee0f66fa0ad2 to a2baae273bb2 (1 revision) (flutter/flutter#196245) 2026-11-09 [email protected] Add Mac web_tool_tests_1_2 and _2_2 in bringup (flutter/flutter#196241) 2026-11-09 [email protected] Roll pub packages (flutter/flutter#196246) 2026-11-09 [email protected] Increase number of 'Windows tool_integration_tests' subshards (flutter/flutter#196243) 2026-11-09 [email protected] Roll Fuchsia Test Scripts from Jo-ZgQSNDuPsjlZ4s... to p3N3B-Kchh1bthB6n... (flutter/flutter#196238) 2026-11-09 [email protected] Skip UIScene pre-migration test cases on Xcode 27 (flutter/flutter#194755) 2026-11-09 [email protected] [flutter_tools] Share Analytics.isBot and make isRunningOnBot sync (flutter/flutter#194744) 2026-11-09 [email protected] [flutter_tools] Retry chromedriver startup when its port is taken (flutter/flutter#196150) 2026-11-09 [email protected] Remove unused vulkan_provider.cc (flutter/flutter#195513) 2026-11-09 [email protected] Roll Dart SDK from 3b174b7217fe to ee0f66fa0ad2 (13 revisions) (flutter/flutter#196217) 2026-11-09 [email protected] Goma is dead, long live RBE! (Take 2) (flutter/flutter#196212) 2026-11-09 [email protected] Split DevTools customer tests into its own customer_testing_devtools shard (flutter/flutter#195871) 2026-11-09 [email protected] Split mac_host_{debug,profile,release}_arm64_tests into one step per type (flutter/flutter#196031) If this roll has caused a breakage, revert this CL and set the roller to dry run mode 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
Flutter Web's two renderers each had their own copy of
ui.Paint:CkPaintfor CanvasKit andSkwasmPaintfor Skwasm (which also drives Web Impeller). The two classes were about 85% identical (fields, defaults, setters, and a ~70-linetoString), and they had started to drift apart. This PR replaces both with one shared implementation, following the same pattern already used for the unifiedShader,ColorFilter,MaskFilter,ImageFilterandVerticesprimitives:EnginePaint(new,primitives/paint.dart) is the singleui.Paintimplementation for every renderer. It owns all Dart-side state, the defaults,toString, and the logic that used to differ between backends (theimageFiltergetter, the blur-sigma guard, wrapping aColorFilterused as an image filter).ui.Paint()now constructs anEnginePaintdirectly.BackendPaint(new,backend/paint.dart) is a minimal boundary with onlydispose().EnginePaint.createBackendPaint()gets one fromRenderer.createPaint(EnginePaint, ...).CkPaint/SkwasmPaintare now thin per-drawBackendPaintwrappers. Each constructor reads anEnginePaintand builds the native paint the same waytoSkPaint()/toRawPaint()did. They exposeskiaObject/handle.saveLayervariants,drawAtlas, paragraph foreground/background paints, and the CanvasKit surface) move fromtoSkPaint()…delete()/toRawPaint()…paintDispose()tocreateBackendPaint()…dispose().The native paint keeps its current per-draw lifecycle: it is created for one draw call and freed right after, and is never cached or registered with a finalizer. A cached native paint would go stale, because fragment shaders and image shaders replace their native shader without going through the
Paint.The PR also fixes a latent lifetime bug in
EngineColorFilterImageFilter.getBackendFilter(). The native filter is cached in anExpandokeyed on theColorFilter, but its finalizer was attached to the short-lived wrapper. When a wrapper was garbage collected, the finalizer could free a native filter that the cache was still handing out. The finalizer is now attached to theColorFilter, so it lives exactly as long as the cache entry.This unblocks the
Canvasunification, since every draw call takes aPaint.Part of #175630
Behavioral Changes
Paint.imageFiltergetter now returns the filter that was set.(Paint()..imageFilter = someColorFilter).imageFilterreturned an internalEngineColorFilterImageFilterwrapper.someColorFilter, matching Skwasm and nativedart:ui.ColorFilterno longer risk having their native filter freed while it is still cached and in use (see above).ui.Paint()is now anEnginePaint. Assigning a value==to the current one toshader,colorFilterorimageFilteris a no-op, so the getter keeps returning the earlier, equal instance.Where to focus
lib/src/engine/primitives/paint.dart: the new shared state. Check theimageFiltersetter, which caches the effectiveEngineImageFilterso repeated draws don't allocate a newColorFilterwrapper each time, andeffectiveMaskFilter, the shared sigma guard.lib/src/engine/canvaskit/painting.dartandlib/src/engine/skwasm/skwasm_impl/paint.dart: these constructors are meant to be line-for-line ports of the oldtoSkPaint()/toRawPaint(), including CanvasKit composinginvertColorswith the color filter and deleting the composed filter before the constructor returns, gradient dithering, and the null native-filter checks.createBackendPaint()must be followed bydispose()in the same scope. The call sites incanvaskit/canvas.dartandskwasm_impl/canvas.dartare mechanical. New live-count tests check this for every paint-consumingCanvasmethod: Skwasm's nativepaintCountintest/skwasm/paint_lifecycle_test.dart, and anSkPaintconstructor spy intest/canvaskit/painting_test.dart.lib/src/engine/primitives/image_filter.dart: a one-line finalizer target change, independent of the Paint refactor but surfaced by it.EnginePaint. They pick upEnginePaint's defaults, including anti-aliasing on, where a rawSkPainthad it off. Glyph anti-aliasing is controlled by the font, and no goldens changed.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.