Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit ba8e0d3

Browse files
[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: ![canvaskit_colorfilter_bounds_before](https://github.com/flutter/engine/assets/1961493/25394b40-c40d-44fb-9c78-9638a40d3329) After: ![canvaskit_colorfilter_bounds_after](https://github.com/flutter/engine/assets/1961493/b25e4084-ccae-4e41-b6e6-37e8cbbd9d54) Fixes flutter/flutter#88866 Fixes flutter/flutter#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
1 parent f553caa commit ba8e0d3

File tree

5 files changed

+119
-9
lines changed

5 files changed

+119
-9
lines changed

lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1352,7 +1352,7 @@ extension SkPaintExtension on SkPaint {
13521352

13531353
@JS('setColorInt')
13541354
external JSVoid _setColorInt(JSNumber color);
1355-
void setColorInt(double color) => _setColorInt(color.toJS);
1355+
void setColorInt(int color) => _setColorInt(color.toJS);
13561356

13571357
external JSVoid setShader(SkShader? shader);
13581358
external JSVoid setMaskFilter(SkMaskFilter? maskFilter);

lib/web_ui/lib/src/engine/canvaskit/layer.dart

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44

55
import 'package:ui/ui.dart' as ui;
66

7+
import '../color_filter.dart';
78
import '../vector_math.dart';
89
import 'canvas.dart';
910
import 'canvaskit_api.dart';
11+
import 'color_filter.dart';
1012
import 'embedded_views.dart';
1113
import 'image_filter.dart';
1214
import 'n_way_canvas.dart';
@@ -409,12 +411,23 @@ class ImageFilterEngineLayer extends ContainerLayer
409411
childMatrix.translate(_offset.dx, _offset.dy);
410412
prerollContext.mutatorsStack
411413
.pushTransform(Matrix4.translationValues(_offset.dx, _offset.dy, 0.0));
414+
final CkManagedSkImageFilterConvertible convertible;
415+
if (_filter is ui.ColorFilter) {
416+
convertible = createCkColorFilter(_filter as EngineColorFilter)!;
417+
} else {
418+
convertible = _filter as CkManagedSkImageFilterConvertible;
419+
}
412420
final ui.Rect childPaintBounds =
413421
prerollChildren(prerollContext, childMatrix);
414-
(_filter as CkManagedSkImageFilterConvertible)
415-
.imageFilter((SkImageFilter filter) {
416-
paintBounds =
417-
rectFromSkIRect(filter.getOutputBounds(toSkRect(childPaintBounds)));
422+
convertible.imageFilter((SkImageFilter filter) {
423+
// If the filter is a ColorFilter, the extended paint bounds will be the
424+
// entire screen, which is not what we want.
425+
if (_filter is ui.ColorFilter) {
426+
paintBounds = childPaintBounds;
427+
} else {
428+
paintBounds =
429+
rectFromSkIRect(filter.getOutputBounds(toSkRect(childPaintBounds)));
430+
}
418431
});
419432
prerollContext.mutatorsStack.pop();
420433
}
@@ -424,6 +437,8 @@ class ImageFilterEngineLayer extends ContainerLayer
424437
assert(needsPainting);
425438
paintContext.internalNodesCanvas.save();
426439
paintContext.internalNodesCanvas.translate(_offset.dx, _offset.dy);
440+
paintContext.internalNodesCanvas
441+
.clipRect(paintBounds, ui.ClipOp.intersect, false);
427442
final CkPaint paint = CkPaint();
428443
paint.imageFilter = _filter;
429444
paintContext.internalNodesCanvas.saveLayer(paintBounds, paint);
@@ -518,10 +533,21 @@ class ColorFilterEngineLayer extends ContainerLayer
518533
final CkPaint paint = CkPaint();
519534
paint.colorFilter = filter;
520535

536+
// We need to clip because if the ColorFilter affects transparent black,
537+
// then it will fill the entire `cullRect` of the picture, ignoring the
538+
// `paintBounds` passed to `saveLayer`. See:
539+
// https://github.com/flutter/flutter/issues/88866
540+
paintContext.internalNodesCanvas.save();
541+
542+
// TODO(hterkelsen): Only clip if the ColorFilter affects transparent black.
543+
paintContext.internalNodesCanvas
544+
.clipRect(paintBounds, ui.ClipOp.intersect, false);
545+
521546
paintContext.internalNodesCanvas.saveLayer(paintBounds, paint);
522-
paint.dispose();
523547
paintChildren(paintContext);
524548
paintContext.internalNodesCanvas.restore();
549+
paintContext.internalNodesCanvas.restore();
550+
paint.dispose();
525551
}
526552
}
527553

lib/web_ui/lib/src/engine/canvaskit/painting.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import 'shader.dart';
2424
class CkPaint implements ui.Paint {
2525
CkPaint() : skiaObject = SkPaint() {
2626
skiaObject.setAntiAlias(_isAntiAlias);
27-
skiaObject.setColorInt(_defaultPaintColor.toDouble());
27+
skiaObject.setColorInt(_defaultPaintColor);
2828
_ref = UniqueRef<SkPaint>(this, skiaObject, 'Paint');
2929
}
3030

@@ -127,7 +127,7 @@ class CkPaint implements ui.Paint {
127127
return;
128128
}
129129
_color = value.value;
130-
skiaObject.setColorInt(value.value.toDouble());
130+
skiaObject.setColorInt(value.value);
131131
}
132132

133133
int _color = _defaultPaintColor;

lib/web_ui/lib/src/engine/canvaskit/text.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1187,7 +1187,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder {
11871187
SkPaint? foreground = skStyle.foreground?.skiaObject;
11881188
if (foreground == null) {
11891189
_defaultTextForeground.setColorInt(
1190-
(skStyle.color?.value ?? 0xFF000000).toDouble(),
1190+
skStyle.color?.value ?? 0xFF000000,
11911191
);
11921192
foreground = _defaultTextForeground;
11931193
}

lib/web_ui/test/canvaskit/color_filter_golden_test.dart

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,90 @@ void testMain() {
195195
await matchSceneGolden('canvaskit_dst_colorfilter.png', builder.build(),
196196
region: region);
197197
});
198+
199+
test('ColorFilter only applies to child bounds', () async {
200+
final LayerSceneBuilder builder = LayerSceneBuilder();
201+
202+
builder.pushOffset(0, 0);
203+
204+
// Draw a red circle and add it to the scene.
205+
final CkPictureRecorder recorder = CkPictureRecorder();
206+
final CkCanvas canvas = recorder.beginRecording(region);
207+
208+
canvas.drawCircle(
209+
const ui.Offset(75, 125),
210+
50,
211+
CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
212+
);
213+
final CkPicture redCircle = recorder.endRecording();
214+
215+
builder.addPicture(ui.Offset.zero, redCircle);
216+
217+
// Apply a green color filter.
218+
builder.pushColorFilter(
219+
const ui.ColorFilter.mode(ui.Color(0xff00ff00), ui.BlendMode.color));
220+
// Draw another red circle and apply it to the scene.
221+
// This one should be green since we have the color filter.
222+
final CkPictureRecorder recorder2 = CkPictureRecorder();
223+
final CkCanvas canvas2 = recorder2.beginRecording(region);
224+
225+
canvas2.drawCircle(
226+
const ui.Offset(425, 125),
227+
50,
228+
CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
229+
);
230+
final CkPicture greenCircle = recorder2.endRecording();
231+
232+
builder.addPicture(ui.Offset.zero, greenCircle);
233+
234+
await matchSceneGolden(
235+
'canvaskit_colorfilter_bounds.png',
236+
builder.build(),
237+
region: region,
238+
);
239+
});
240+
241+
test('ColorFilter works as an ImageFilter', () async {
242+
final LayerSceneBuilder builder = LayerSceneBuilder();
243+
244+
builder.pushOffset(0, 0);
245+
246+
// Draw a red circle and add it to the scene.
247+
final CkPictureRecorder recorder = CkPictureRecorder();
248+
final CkCanvas canvas = recorder.beginRecording(region);
249+
250+
canvas.drawCircle(
251+
const ui.Offset(75, 125),
252+
50,
253+
CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
254+
);
255+
final CkPicture redCircle = recorder.endRecording();
256+
257+
builder.addPicture(ui.Offset.zero, redCircle);
258+
259+
// Apply a green color filter as an ImageFilter.
260+
builder.pushImageFilter(
261+
const ui.ColorFilter.mode(ui.Color(0xff00ff00), ui.BlendMode.color));
262+
// Draw another red circle and apply it to the scene.
263+
// This one should be green since we have the color filter.
264+
final CkPictureRecorder recorder2 = CkPictureRecorder();
265+
final CkCanvas canvas2 = recorder2.beginRecording(region);
266+
267+
canvas2.drawCircle(
268+
const ui.Offset(425, 125),
269+
50,
270+
CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0),
271+
);
272+
final CkPicture greenCircle = recorder2.endRecording();
273+
274+
builder.addPicture(ui.Offset.zero, greenCircle);
275+
276+
await matchSceneGolden(
277+
'canvaskit_colorfilter_as_imagefilter.png',
278+
builder.build(),
279+
region: region,
280+
);
281+
});
198282
// TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520
199283
}, skip: isSafari || isFirefox);
200284
}

0 commit comments

Comments
 (0)