[canvaskit] Clip before applying ColorFilter so it doesn't filter beyond child bounds (#52704)
When a ColorFilter affects transparent black, it will expand its bounds
to the entire screen, even if the `saveLayer` call is bounded. This
applies a clip before applying the ColorFilter so the filter is bounded
to just the child drawings.
Also fixes bug with ColorFilter being used as an ImageFilter.
Before:
After:
Fixes https://github.com/flutter/flutter/issues/88866
Fixes https://github.com/flutter/flutter/issues/144015
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
index 39608d2..8c0eba1 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
@@ -1352,7 +1352,7 @@
@JS('setColorInt')
external JSVoid _setColorInt(JSNumber color);
- void setColorInt(double color) => _setColorInt(color.toJS);
+ void setColorInt(int color) => _setColorInt(color.toJS);
external JSVoid setShader(SkShader? shader);
external JSVoid setMaskFilter(SkMaskFilter? maskFilter);
diff --git a/lib/web_ui/lib/src/engine/canvaskit/layer.dart b/lib/web_ui/lib/src/engine/canvaskit/layer.dart
index 143ff86..325322d 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/layer.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/layer.dart
@@ -4,9 +4,11 @@
import 'package:ui/ui.dart' as ui;
+import '../color_filter.dart';
import '../vector_math.dart';
import 'canvas.dart';
import 'canvaskit_api.dart';
+import 'color_filter.dart';
import 'embedded_views.dart';
import 'image_filter.dart';
import 'n_way_canvas.dart';
@@ -409,12 +411,23 @@
childMatrix.translate(_offset.dx, _offset.dy);
prerollContext.mutatorsStack
.pushTransform(Matrix4.translationValues(_offset.dx, _offset.dy, 0.0));
+ final CkManagedSkImageFilterConvertible convertible;
+ if (_filter is ui.ColorFilter) {
+ convertible = createCkColorFilter(_filter as EngineColorFilter)!;
+ } else {
+ convertible = _filter as CkManagedSkImageFilterConvertible;
+ }
final ui.Rect childPaintBounds =
prerollChildren(prerollContext, childMatrix);
- (_filter as CkManagedSkImageFilterConvertible)
- .imageFilter((SkImageFilter filter) {
- paintBounds =
- rectFromSkIRect(filter.getOutputBounds(toSkRect(childPaintBounds)));
+ convertible.imageFilter((SkImageFilter filter) {
+ // If the filter is a ColorFilter, the extended paint bounds will be the
+ // entire screen, which is not what we want.
+ if (_filter is ui.ColorFilter) {
+ paintBounds = childPaintBounds;
+ } else {
+ paintBounds =
+ rectFromSkIRect(filter.getOutputBounds(toSkRect(childPaintBounds)));
+ }
});
prerollContext.mutatorsStack.pop();
}
@@ -424,6 +437,8 @@
assert(needsPainting);
paintContext.internalNodesCanvas.save();
paintContext.internalNodesCanvas.translate(_offset.dx, _offset.dy);
+ paintContext.internalNodesCanvas
+ .clipRect(paintBounds, ui.ClipOp.intersect, false);
final CkPaint paint = CkPaint();
paint.imageFilter = _filter;
paintContext.internalNodesCanvas.saveLayer(paintBounds, paint);
@@ -518,10 +533,21 @@
final CkPaint paint = CkPaint();
paint.colorFilter = filter;
+ // We need to clip because if the ColorFilter affects transparent black,
+ // then it will fill the entire `cullRect` of the picture, ignoring the
+ // `paintBounds` passed to `saveLayer`. See:
+ // https://github.com/flutter/flutter/issues/88866
+ paintContext.internalNodesCanvas.save();
+
+ // TODO(hterkelsen): Only clip if the ColorFilter affects transparent black.
+ paintContext.internalNodesCanvas
+ .clipRect(paintBounds, ui.ClipOp.intersect, false);
+
paintContext.internalNodesCanvas.saveLayer(paintBounds, paint);
- paint.dispose();
paintChildren(paintContext);
paintContext.internalNodesCanvas.restore();
+ paintContext.internalNodesCanvas.restore();
+ paint.dispose();
}
}
diff --git a/lib/web_ui/lib/src/engine/canvaskit/painting.dart b/lib/web_ui/lib/src/engine/canvaskit/painting.dart
index 00bead7..3bd9f79 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/painting.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/painting.dart
@@ -24,7 +24,7 @@
class CkPaint implements ui.Paint {
CkPaint() : skiaObject = SkPaint() {
skiaObject.setAntiAlias(_isAntiAlias);
- skiaObject.setColorInt(_defaultPaintColor.toDouble());
+ skiaObject.setColorInt(_defaultPaintColor);
_ref = UniqueRef<SkPaint>(this, skiaObject, 'Paint');
}
@@ -127,7 +127,7 @@
return;
}
_color = value.value;
- skiaObject.setColorInt(value.value.toDouble());
+ skiaObject.setColorInt(value.value);
}
int _color = _defaultPaintColor;
diff --git a/lib/web_ui/lib/src/engine/canvaskit/text.dart b/lib/web_ui/lib/src/engine/canvaskit/text.dart
index 4a928cb..bdcd666 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/text.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/text.dart
@@ -1187,7 +1187,7 @@
SkPaint? foreground = skStyle.foreground?.skiaObject;
if (foreground == null) {
_defaultTextForeground.setColorInt(
- (skStyle.color?.value ?? 0xFF000000).toDouble(),
+ skStyle.color?.value ?? 0xFF000000,
);
foreground = _defaultTextForeground;
}
diff --git a/lib/web_ui/test/canvaskit/color_filter_golden_test.dart b/lib/web_ui/test/canvaskit/color_filter_golden_test.dart
index cf5ba12..49b8c89 100644
--- a/lib/web_ui/test/canvaskit/color_filter_golden_test.dart
+++ b/lib/web_ui/test/canvaskit/color_filter_golden_test.dart
@@ -195,6 +195,90 @@
await matchSceneGolden('canvaskit_dst_colorfilter.png', builder.build(),
region: region);
});
+
+ test('ColorFilter only applies to child bounds', () async {
+ final LayerSceneBuilder builder = LayerSceneBuilder();
+
+ builder.pushOffset(0, 0);
+
+ // Draw a red circle and add it to the scene.
+ final CkPictureRecorder recorder = CkPictureRecorder();
+ final CkCanvas canvas = recorder.beginRecording(region);
+
+ canvas.drawCircle(
+ const ui.Offset(75, 125),
+ 50,
+ CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
+ );
+ final CkPicture redCircle = recorder.endRecording();
+
+ builder.addPicture(ui.Offset.zero, redCircle);
+
+ // Apply a green color filter.
+ builder.pushColorFilter(
+ const ui.ColorFilter.mode(ui.Color(0xff00ff00), ui.BlendMode.color));
+ // Draw another red circle and apply it to the scene.
+ // This one should be green since we have the color filter.
+ final CkPictureRecorder recorder2 = CkPictureRecorder();
+ final CkCanvas canvas2 = recorder2.beginRecording(region);
+
+ canvas2.drawCircle(
+ const ui.Offset(425, 125),
+ 50,
+ CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
+ );
+ final CkPicture greenCircle = recorder2.endRecording();
+
+ builder.addPicture(ui.Offset.zero, greenCircle);
+
+ await matchSceneGolden(
+ 'canvaskit_colorfilter_bounds.png',
+ builder.build(),
+ region: region,
+ );
+ });
+
+ test('ColorFilter works as an ImageFilter', () async {
+ final LayerSceneBuilder builder = LayerSceneBuilder();
+
+ builder.pushOffset(0, 0);
+
+ // Draw a red circle and add it to the scene.
+ final CkPictureRecorder recorder = CkPictureRecorder();
+ final CkCanvas canvas = recorder.beginRecording(region);
+
+ canvas.drawCircle(
+ const ui.Offset(75, 125),
+ 50,
+ CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
+ );
+ final CkPicture redCircle = recorder.endRecording();
+
+ builder.addPicture(ui.Offset.zero, redCircle);
+
+ // Apply a green color filter as an ImageFilter.
+ builder.pushImageFilter(
+ const ui.ColorFilter.mode(ui.Color(0xff00ff00), ui.BlendMode.color));
+ // Draw another red circle and apply it to the scene.
+ // This one should be green since we have the color filter.
+ final CkPictureRecorder recorder2 = CkPictureRecorder();
+ final CkCanvas canvas2 = recorder2.beginRecording(region);
+
+ canvas2.drawCircle(
+ const ui.Offset(425, 125),
+ 50,
+ CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
+ );
+ final CkPicture greenCircle = recorder2.endRecording();
+
+ builder.addPicture(ui.Offset.zero, greenCircle);
+
+ await matchSceneGolden(
+ 'canvaskit_colorfilter_as_imagefilter.png',
+ builder.build(),
+ region: region,
+ );
+ });
// TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520
}, skip: isSafari || isFirefox);
}
