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

[canvaskit] Clip before applying ColorFilter so it doesn't filter beyond child bounds #52704

Merged
merged 3 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1352,7 +1352,7 @@ extension SkPaintExtension on SkPaint {

@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);
Expand Down
36 changes: 31 additions & 5 deletions lib/web_ui/lib/src/engine/canvaskit/layer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -409,12 +411,23 @@ class ImageFilterEngineLayer extends ContainerLayer
childMatrix.translate(_offset.dx, _offset.dy);
prerollContext.mutatorsStack
.pushTransform(Matrix4.translationValues(_offset.dx, _offset.dy, 0.0));
final CkManagedSkImageFilterConvertible convertible;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a field that's populated in the constructor and reused by both preroll and paint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only used in preroll. In paint we use the filter directly

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();
}
Expand All @@ -424,6 +437,8 @@ class ImageFilterEngineLayer extends ContainerLayer
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);
Expand Down Expand Up @@ -518,10 +533,21 @@ class ColorFilterEngineLayer extends ContainerLayer
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();
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/canvaskit/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import 'shader.dart';
class CkPaint implements ui.Paint {
CkPaint() : skiaObject = SkPaint() {
skiaObject.setAntiAlias(_isAntiAlias);
skiaObject.setColorInt(_defaultPaintColor.toDouble());
skiaObject.setColorInt(_defaultPaintColor);
_ref = UniqueRef<SkPaint>(this, skiaObject, 'Paint');
}

Expand Down Expand Up @@ -127,7 +127,7 @@ class CkPaint implements ui.Paint {
return;
}
_color = value.value;
skiaObject.setColorInt(value.value.toDouble());
skiaObject.setColorInt(value.value);
}

int _color = _defaultPaintColor;
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/canvaskit/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder {
SkPaint? foreground = skStyle.foreground?.skiaObject;
if (foreground == null) {
_defaultTextForeground.setColorInt(
(skStyle.color?.value ?? 0xFF000000).toDouble(),
skStyle.color?.value ?? 0xFF000000,
);
foreground = _defaultTextForeground;
}
Expand Down
84 changes: 84 additions & 0 deletions lib/web_ui/test/canvaskit/color_filter_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,90 @@ void testMain() {
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);
}