[web] Refactor webparagraph painters to separate CK properly by mdebbar · Pull Request #186684 · flutter/flutter

mdebbar · 2026-05-18T15:34:52Z

Unify under one Painter class.
Move CanvasKitPainter to the canvaskit/ folder.
Create the entry point for a future SkwasmPainter.
Move decoration logic to a different file (it'll probably be deleted later).
Always draw background because it's not cached in the paragraph image.
Change the bytesPerRow parameter from double to int.

flutter-dashboard · 2026-05-18T15:34:56Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

This pull request refactors the web paragraph painting architecture by introducing the WebParagraphPainter abstraction and moving decoration logic to a specialized painter. It updates the CanvasKit API to use integer values for row bytes and implements a caching mechanism for paragraph images. Review feedback highlights a coordinate system mismatch in decoration painting, a naming convention violation for constants, and the need for safer handling of null styles, empty dimensions, and buffer conversions.

gemini-code-assist · 2026-05-18T15:36:49Z

+              ui.Offset(line.advance.left + line.formattingShift, line.advance.top),
+            );
+            // TODO(jlavrova): Implement decorations entirely on ui.Canvas
+            DomCanvasDecorationPainter.fillDecorations(_paintContext, block, sourceRect);


There is a coordinate system mismatch here. sourceRect is calculated in physical pixels (scaled by DPR), but _paintContext has already been scaled by DPR in _resizePaintCanvas. Passing sourceRect to fillDecorations will result in double-scaling of the decoration dimensions because fillDecorations uses the rect width/height directly for drawing. You should pass a rectangle in logical pixels instead.

Suggested change

DomCanvasDecorationPainter.fillDecorations(_paintContext, block, sourceRect);

DomCanvasDecorationPainter.fillDecorations(_paintContext, block, ui.Rect.fromLTWH(0, 0, targetRect.width, targetRect.height));

gemini-code-assist · 2026-05-18T15:36:49Z

+
+    final double thickness = _calculateThickness(block.style);
+
+    const DoubleDecorationSpacing = 3.0;


The constant DoubleDecorationSpacing should follow the Dart naming convention for local constants, which is lowerCamelCase. Using UpperCamelCase makes it look like a class or type name.

References

Effective Dart: Style recommends using lowerCamelCase for constant names. ^(link)

gemini-code-assist · 2026-05-18T15:36:49Z

+      paintContext.lineWidth = thickness;
+      paintContext.strokeStyle = block.style.decorationColor!.toCssString();
+
+      switch (block.style.decorationStyle!) {


The force unwrap block.style.decorationStyle! is potentially unsafe if decorationStyle is null. It's better to provide a default value, such as ui.TextDecorationStyle.solid.

Suggested change

switch (block.style.decorationStyle!) {

switch (block.style.decorationStyle ?? ui.TextDecorationStyle.solid) {

gemini-code-assist · 2026-05-18T15:36:49Z

+      layout,
+      offset,
+      ui.window.devicePixelRatio,
+    );


If the paragraph is empty or has zero width/height, sourceRect will be empty. Calling getImageData with zero dimensions on a 2D context will throw an IndexSizeError. It's safer to check if the rectangle is empty before proceeding with painting.

Suggested change

);

);

if (sourceRect.isEmpty) {

return;

}

gemini-code-assist · 2026-05-18T15:36:49Z

+          sourceRect.width.ceil(),
+          sourceRect.height.ceil(),
+        );
+        return imageData.data.buffer.asUint8List();


When converting Uint8ClampedList to Uint8List via the buffer, it is safer to specify the offset and length to ensure you are viewing the correct portion of the buffer, even if getImageData currently returns a fresh buffer.

Suggested change

return imageData.data.buffer.asUint8List();

return imageData.data.buffer.asUint8List(imageData.data.offsetInBytes, imageData.data.lengthInBytes);

flutter-dashboard · 2026-05-19T21:36:16Z

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 #186684 at sha 988b794

flutter/flutter@259aeae...e03b91f 2026-05-20 engine-flutter-autoroll@skia.org Roll Packages from ade10ca to 1dfbada (6 revisions) (flutter/flutter#186811) 2026-05-20 brunocorona.alcantar@gmail.com Fix AnimatedList.separated assert when removing last item mid-removal… (flutter/flutter#186389) 2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from d45969a5752e to 5f4f454b9662 (2 revisions) (flutter/flutter#186809) 2026-05-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from -F9Ci3Opxt06MixDl... to iKCvaD58jKStYGla0... (flutter/flutter#186796) 2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 19ad9707e5c6 to d45969a5752e (2 revisions) (flutter/flutter#186792) 2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 3471ebf5af0c to 19ad9707e5c6 (9 revisions) (flutter/flutter#186772) 2026-05-20 mdebbar@google.com [web] Refactor webparagraph painters to separate CK properly (flutter/flutter#186684) 2026-05-19 31859944+LongCatIsLooong@users.noreply.github.com Enable Swift testing in the iOS embedder (flutter/flutter#185712) 2026-05-19 mdebbar@google.com [web] Rename WebParagraph goldens (flutter/flutter#186680) 2026-05-19 engine-flutter-autoroll@skia.org Roll Skia from f1b406860c5e to 3471ebf5af0c (5 revisions) (flutter/flutter#186745) 2026-05-19 154381524+flutteractionsbot@users.noreply.github.com Revert "Ship gen_snapshot for linux-arm64 hosts targeting Android" (flutter/flutter#186693) 2026-05-19 bkonyi@google.com [ Tool ] Remove legacy analytics code (flutter/flutter#184994) 2026-05-19 chingjun@google.com Update Vulkan enum values (flutter/flutter#186694) 2026-05-19 1961493+harryterkelsen@users.noreply.github.com fix(web): Fixes CSS override detection when the browser has a default font size (flutter/flutter#186474) 2026-05-19 30870216+gaaclarke@users.noreply.github.com adds linux impeller hello world integration test (flutter/flutter#186715) 2026-05-19 jason-simmons@users.noreply.github.com Update the list of binaries in the code signing verification test to include new Dart snapshots (flutter/flutter#186754) 2026-05-19 brunocorona.alcantar@gmail.com Make EdgeDraggingAutoScroller respect ScrollPhysics (flutter/flutter#186541) 2026-05-19 bkonyi@google.com [ Widget Preview ] Fix inspector split resize focus loss over WebViews (flutter/flutter#186432) 2026-05-19 engine-flutter-autoroll@skia.org Roll Packages from b9bdd37 to ade10ca (1 revision) (flutter/flutter#186746) 2026-05-19 jason-simmons@users.noreply.github.com Manual Dart roll from 8e30b88e4d5a to 66873d2da857 (flutter/flutter#186690) 2026-05-19 bkonyi@google.com [ Widget Preview ] Improve zoom behavior and add zoom slider (flutter/flutter#186422) 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 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

…#186684) - Unify under one `Painter` class. - Move `CanvasKitPainter` to the `canvaskit/` folder. - Create the entry point for a future `SkwasmPainter`. - Move decoration logic to a different file (it'll probably be deleted later). - Always draw background because it's not cached in the paragraph image. - Change the `bytesPerRow` parameter from `double` to `int`. Part of flutter#172561

…r#11747) flutter/flutter@259aeae...e03b91f 2026-05-20 engine-flutter-autoroll@skia.org Roll Packages from ade10ca to 1dfbada (6 revisions) (flutter/flutter#186811) 2026-05-20 brunocorona.alcantar@gmail.com Fix AnimatedList.separated assert when removing last item mid-removal… (flutter/flutter#186389) 2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from d45969a5752e to 5f4f454b9662 (2 revisions) (flutter/flutter#186809) 2026-05-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from -F9Ci3Opxt06MixDl... to iKCvaD58jKStYGla0... (flutter/flutter#186796) 2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 19ad9707e5c6 to d45969a5752e (2 revisions) (flutter/flutter#186792) 2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 3471ebf5af0c to 19ad9707e5c6 (9 revisions) (flutter/flutter#186772) 2026-05-20 mdebbar@google.com [web] Refactor webparagraph painters to separate CK properly (flutter/flutter#186684) 2026-05-19 31859944+LongCatIsLooong@users.noreply.github.com Enable Swift testing in the iOS embedder (flutter/flutter#185712) 2026-05-19 mdebbar@google.com [web] Rename WebParagraph goldens (flutter/flutter#186680) 2026-05-19 engine-flutter-autoroll@skia.org Roll Skia from f1b406860c5e to 3471ebf5af0c (5 revisions) (flutter/flutter#186745) 2026-05-19 154381524+flutteractionsbot@users.noreply.github.com Revert "Ship gen_snapshot for linux-arm64 hosts targeting Android" (flutter/flutter#186693) 2026-05-19 bkonyi@google.com [ Tool ] Remove legacy analytics code (flutter/flutter#184994) 2026-05-19 chingjun@google.com Update Vulkan enum values (flutter/flutter#186694) 2026-05-19 1961493+harryterkelsen@users.noreply.github.com fix(web): Fixes CSS override detection when the browser has a default font size (flutter/flutter#186474) 2026-05-19 30870216+gaaclarke@users.noreply.github.com adds linux impeller hello world integration test (flutter/flutter#186715) 2026-05-19 jason-simmons@users.noreply.github.com Update the list of binaries in the code signing verification test to include new Dart snapshots (flutter/flutter#186754) 2026-05-19 brunocorona.alcantar@gmail.com Make EdgeDraggingAutoScroller respect ScrollPhysics (flutter/flutter#186541) 2026-05-19 bkonyi@google.com [ Widget Preview ] Fix inspector split resize focus loss over WebViews (flutter/flutter#186432) 2026-05-19 engine-flutter-autoroll@skia.org Roll Packages from b9bdd37 to ade10ca (1 revision) (flutter/flutter#186746) 2026-05-19 jason-simmons@users.noreply.github.com Manual Dart roll from 8e30b88e4d5a to 66873d2da857 (flutter/flutter#186690) 2026-05-19 bkonyi@google.com [ Widget Preview ] Improve zoom behavior and add zoom slider (flutter/flutter#186422) 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 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

…#186684) - Unify under one `Painter` class. - Move `CanvasKitPainter` to the `canvaskit/` folder. - Create the entry point for a future `SkwasmPainter`. - Move decoration logic to a different file (it'll probably be deleted later). - Always draw background because it's not cached in the paragraph image. - Change the `bytesPerRow` parameter from `double` to `int`. Part of flutter#172561

[web] Refactor webparagraph painters to separate CK properly

d8b23e5

mdebbar requested a review from Rusino May 18, 2026 15:34

flutter-dashboard Bot added the CICD Run CI/CD label May 18, 2026

github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels May 18, 2026

gemini-code-assist Bot reviewed May 18, 2026

View reviewed changes

Rusino previously approved these changes May 18, 2026

View reviewed changes

lints

0df7af3

mdebbar dismissed Rusino’s stale review via 0df7af3 May 19, 2026 16:12

github-actions Bot removed the CICD Run CI/CD label May 19, 2026

mdebbar added the CICD Run CI/CD label May 19, 2026

Merge branch 'master' into webpargraph_painter

988b794

github-actions Bot removed the CICD Run CI/CD label May 19, 2026

mdebbar added the CICD Run CI/CD label May 19, 2026

flutter-dashboard Bot added the will affect goldens Changes to golden files label May 19, 2026

mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2026

mdebbar requested a review from Rusino May 20, 2026 01:36

Rusino approved these changes May 20, 2026

View reviewed changes

auto-submit Bot added this pull request to the merge queue May 20, 2026

Merged via the queue into flutter:master with commit 06a22c5 May 20, 2026
204 of 205 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2026

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

Roll Flutter from 259aeae0e0ba to 645a4e5fe508 (18 revisions) flutter/packages#11744

Closed

mdebbar deleted the webpargraph_painter branch May 20, 2026 16:37

This was referenced May 20, 2026

Roll Flutter from 259aeae0e0ba to 31f6edd8f6de (20 revisions) flutter/packages#11746

Closed

Roll Flutter from 259aeae0e0ba to e03b91f1fe34 (21 revisions) flutter/packages#11747

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[web] Refactor webparagraph painters to separate CK properly#186684

[web] Refactor webparagraph painters to separate CK properly#186684
auto-submit[bot] merged 3 commits into
flutter:masterfrom
mdebbar:webpargraph_painter

mdebbar commented May 18, 2026 •

edited

Loading

Uh oh!

flutter-dashboard Bot commented May 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 18, 2026

Uh oh!

gemini-code-assist Bot May 18, 2026

Uh oh!

gemini-code-assist Bot May 18, 2026

Uh oh!

gemini-code-assist Bot May 18, 2026

Uh oh!

gemini-code-assist Bot May 18, 2026

Uh oh!

flutter-dashboard Bot commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	DomCanvasDecorationPainter.fillDecorations(_paintContext, block, sourceRect);
	DomCanvasDecorationPainter.fillDecorations(_paintContext, block, ui.Rect.fromLTWH(0, 0, targetRect.width, targetRect.height));


		final double thickness = _calculateThickness(block.style);

		const DoubleDecorationSpacing = 3.0;

	switch (block.style.decorationStyle!) {
	switch (block.style.decorationStyle ?? ui.TextDecorationStyle.solid) {

-    );
+    );
+    if (sourceRect.isEmpty) {
+      return;
+    }

	return imageData.data.buffer.asUint8List();
	return imageData.data.buffer.asUint8List(imageData.data.offsetInBytes, imageData.data.lengthInBytes);

Uh oh!

Conversation

mdebbar commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard Bot commented May 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard Bot commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mdebbar commented May 18, 2026 •

edited

Loading