Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
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/dev/goldens_lock.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
repository: https://github.com/flutter/goldens.git
revision: 672510dc52daa5b059081f6990582bccdb4ea48f
revision: 1556280d6f1d70fac9ddff9b38639757e105b4b0
16 changes: 13 additions & 3 deletions lib/web_ui/lib/src/engine/bitmap_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,18 @@ class BitmapCanvas extends EngineCanvas {
}

@override
void clipRect(ui.Rect rect) {
_canvasPool.clipRect(rect);
void clipRect(ui.Rect rect, ui.ClipOp op) {
if (op == ui.ClipOp.difference) {
// Create 2 rectangles inside each other that represents
// clip area difference using even-odd fill rule.
final SurfacePath path = new SurfacePath();
path.fillType = ui.PathFillType.evenOdd;
path.addRect(ui.Rect.fromLTWH(0, 0, _bounds.width, _bounds.height));
path.addRect(rect);
_canvasPool.clipPath(path);
} else {
_canvasPool.clipRect(rect);
}
}

@override
Expand Down Expand Up @@ -466,7 +476,7 @@ class BitmapCanvas extends EngineCanvas {
} else {
if (requiresClipping) {
save();
clipRect(dst);
clipRect(dst, ui.ClipOp.intersect);
}
double targetLeft = dst.left;
double targetTop = dst.top;
Expand Down
15 changes: 12 additions & 3 deletions lib/web_ui/lib/src/engine/canvas_pool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,13 @@ class _CanvasPool extends _SaveStackTracking {
} else if (clipEntry.rrect != null) {
_clipRRect(ctx, clipEntry.rrect!);
} else if (clipEntry.path != null) {
_runPath(ctx, clipEntry.path as SurfacePath);
ctx.clip();
final SurfacePath path = clipEntry.path as SurfacePath;
_runPath(ctx, path);
if (path.fillType == ui.PathFillType.nonZero) {
ctx.clip();
} else {
ctx.clip('evenodd');
}
}
}
}
Expand Down Expand Up @@ -443,7 +448,11 @@ class _CanvasPool extends _SaveStackTracking {
if (_canvas != null) {
html.CanvasRenderingContext2D ctx = context;
_runPath(ctx, path as SurfacePath);
ctx.clip();
if (path.fillType == ui.PathFillType.nonZero) {
ctx.clip();
} else {
ctx.clip('evenodd');
}
Comment on lines +451 to +455
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you did the opposite check here compared to the above. Should we keep them consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/dom_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DomCanvas extends EngineCanvas with SaveElementStackTracking {
}

@override
void clipRect(ui.Rect rect) {
void clipRect(ui.Rect rect, ui.ClipOp op) {
throw UnimplementedError();
}

Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/engine_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ abstract class EngineCanvas {

void transform(Float32List matrix4);

void clipRect(ui.Rect rect);
void clipRect(ui.Rect rect, ui.ClipOp clipOp);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can add comment here since it's a public method, or we can add the comments to the implementing classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At some point all comments were removed so we didn't have to keep them in sync.


void clipRRect(ui.RRect rrect);

Expand Down Expand Up @@ -222,7 +222,7 @@ mixin SaveStackTracking on EngineCanvas {
///
/// Classes that override this method must call `super.clipRect()`.
@override
void clipRect(ui.Rect rect) {
void clipRect(ui.Rect rect, ui.ClipOp op) {
_clipStack ??= <_SaveClipEntry>[];
_clipStack!.add(_SaveClipEntry.rect(rect, _currentTransform.clone()));
}
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/html/recording_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ class RecordingCanvas {

void clipRect(ui.Rect rect, ui.ClipOp clipOp) {
assert(!_recordingEnded);
final PaintClipRect command = PaintClipRect(rect, clipOp);
final DrawCommand command = PaintClipRect(rect, clipOp);
switch (clipOp) {
case ui.ClipOp.intersect:
_paintBounds.clipRect(rect, command);
Expand Down Expand Up @@ -810,7 +810,7 @@ class PaintClipRect extends DrawCommand {

@override
void apply(EngineCanvas canvas) {
canvas.clipRect(rect);
canvas.clipRect(rect, clipOp);
}

@override
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/test/golden_tests/engine/canvas_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ void testMain() async {

canvas = BitmapCanvas(canvasSize);
canvas.debugChildOverdraw = true;
canvas.clipRect(outerClip);
canvas.clipRect(outerClip, ClipOp.intersect);
canvas.drawParagraph(paragraph, const Offset(8.5, 8.5));
canvas.clipRect(innerClip);
canvas.clipRect(innerClip, ClipOp.intersect);
canvas.drawParagraph(paragraph, Offset(8.5, 8.5 + innerClip.top));

expect(
Expand Down
42 changes: 42 additions & 0 deletions lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.6
import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
import 'package:ui/ui.dart';
import 'package:ui/src/engine.dart';
import 'screenshot.dart';

void main() {
internalBootstrapBrowserTest(() => testMain);
}

void testMain() async {
setUp(() async {
debugEmulateFlutterTesterEnvironment = true;
});

/// Regression test for https://github.com/flutter/flutter/issues/64734.
test('Clips using difference', () async {
final Rect region = const Rect.fromLTRB(0, 0, 400, 300);
final RecordingCanvas canvas = RecordingCanvas(region);
final Rect titleRect = Rect.fromLTWH(20, 0, 50, 20);
final Paint paint = Paint()
..style = PaintingStyle.stroke
..color = const Color(0xff000000)
..strokeWidth = 1;
canvas.save();
try {
final Rect borderRect = Rect.fromLTRB(0, 10, region.width, region.height);
canvas.clipRect(titleRect, ClipOp.difference);
canvas.drawRect(borderRect, paint);
} finally {
canvas.restore();
}
canvas..drawRect(titleRect, paint);
await canvasScreenshot(canvas, 'clip_op_difference',
region: const Rect.fromLTRB(0, 0, 420, 360));
});
}
43 changes: 43 additions & 0 deletions lib/web_ui/test/golden_tests/engine/screenshot.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have test/golden_tests/engine/scuba.dart and EngineScubaTester that contains all this stuff. Is there a reason for the duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its a start to dedup a lot of test code. There are only 4 tests using scuba and DomCanvas soon going away. BitmapCanvas will write out DOM nodes for rects etc soon as well and desire to run canvaskit+html tests in a uniform way. Plan to move scuba bits into screenshot and clean things up across all tests.

// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.6
import 'dart:html' as html;
import 'package:ui/ui.dart' as ui;
import 'package:ui/src/engine.dart';
import 'package:web_engine_tester/golden_tester.dart';
import 'package:test/test.dart';

/// Commit a recording canvas to a bitmap, and compare with the expected.
Future<void> canvasScreenshot(RecordingCanvas rc, String fileName,
{ui.Rect region = const ui.Rect.fromLTWH(0, 0, 600, 800),
double maxDiffRatePercent = 0.0, bool write: false}) async {
final EngineCanvas engineCanvas = BitmapCanvas(region);

rc.endRecording();
rc.apply(engineCanvas, region);

// Wrap in <flt-scene> so that our CSS selectors kick in.
final html.Element sceneElement = html.Element.tag('flt-scene');
try {
sceneElement.append(engineCanvas.rootElement);
html.document.body.append(sceneElement);
await matchGoldenFile('$fileName.png',
region: region, maxDiffRatePercent: maxDiffRatePercent, write: write);
} finally {
// The page is reused across tests, so remove the element after taking the
// Scuba screenshot.
sceneElement.remove();
}
}

/// Configures the test to use bundled Roboto and Ahem fonts to avoid golden
/// screenshot differences due to differences in the preinstalled system fonts.
void setUpStableTestFonts() {
setUp(() async {
await ui.webOnlyInitializePlatform();
ui.webOnlyFontCollection.debugRegisterTestFonts();
await ui.webOnlyFontCollection.ensureFontsLoaded();
});
}
2 changes: 1 addition & 1 deletion lib/web_ui/test/mock_engine_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class MockEngineCanvas implements EngineCanvas {
}

@override
void clipRect(Rect rect) {
void clipRect(Rect rect, ClipOp op) {
_called('clipRect', arguments: rect);
}

Expand Down