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 3 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
16 changes: 12 additions & 4 deletions lib/web_ui/lib/src/engine/recording_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,20 @@ class RecordingCanvas {
}

void drawDRRect(ui.RRect outer, ui.RRect inner, ui.Paint paint) {
// If inner rect is not contained inside outer, flutter engine skips
// painting rectangle.
if (!(outer.contains(ui.Offset(inner.left, inner.top)) &&
outer.contains(ui.Offset(inner.right, inner.bottom)))) {
// Ensure inner is fully contained within outer, by comparing its
// defining points (including its border radius)
final ui.RRect scaled = inner.scaleRadii(); // Measure the same rrect that is going to be painted
if (!(outer.contains(ui.Offset(scaled.left, scaled.top + scaled.tlRadiusY))
&& outer.contains(ui.Offset(scaled.left + scaled.tlRadiusX, scaled.top)) // Left-Top
&& outer.contains(ui.Offset(scaled.right - scaled.trRadiusX, scaled.top))
&& outer.contains(ui.Offset(scaled.right, scaled.top + scaled.trRadiusY)) // Right-Top
&& outer.contains(ui.Offset(scaled.right, scaled.bottom - scaled.brRadiusY))
&& outer.contains(ui.Offset(scaled.right - scaled.brRadiusX, scaled.bottom)) // Right-Bottom
&& outer.contains(ui.Offset(scaled.left + scaled.blRadiusX, scaled.bottom))
&& outer.contains(ui.Offset(scaled.left, scaled.bottom - scaled.blRadiusY)))) { // Left-Bottom
Copy link
Contributor

@yjbanov yjbanov Sep 16, 2019

Choose a reason for hiding this comment

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

In the happy case when the drrect is properly specified, this check will allocate 8 ui.Offset objects. Is it possible to optimize that away?

Copy link
Contributor

Choose a reason for hiding this comment

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

/cc @rakudrama Am I being too paranoid here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can try with a different check, but I'm not sure it'll be geometrically precise, let me give that a quick shot, I can always come back to this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mean rewrite the check. I mean do exactly the same math without allocation. Or is there something that prevents you from doing that?

Copy link
Member Author

Choose a reason for hiding this comment

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

The API of contains is what it is, and once the radius information is used to construct the RRect, everything is stored as doubles, so I don't see any way of computing the offsets of each of the points of the corner without allocating an Offset. Also Offsets seem to be immutable, so even if I try to move it around, it'll return a new instance all the time, right?

Anyway, I wrote a slightly different revision that does the same with a little bit less allocation (but in a slightly less compact code).

return;
}

_hasArbitraryPaint = true;
_didDraw = true;
final double strokeWidth =
Expand Down
59 changes: 59 additions & 0 deletions lib/web_ui/test/engine/recording_canvas_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// 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.

import 'package:ui/ui.dart';
import 'package:ui/src/engine.dart';
import 'package:test/test.dart';

import '../mock_engine_canvas.dart';

void main() {

RecordingCanvas underTest;
MockEngineCanvas mockCanvas;

setUp(() {
underTest = RecordingCanvas(Rect.largest);
mockCanvas = MockEngineCanvas();
});

group('drawDRRect', () {
final RRect rrect = RRect.fromLTRBR(10, 10, 50, 50, Radius.circular(3));
final Paint somePaint = Paint()..color = const Color(0xFFFF0000);
test('Happy case', () {
underTest.drawDRRect(rrect, rrect.deflate(1), somePaint);
underTest.apply(mockCanvas);
// Expect drawDRRect to be called
expect(mockCanvas.methodCallLog.length, equals(1));
MockCanvasCall mockCall = mockCanvas.methodCallLog[0];
expect(mockCall.methodName, equals('drawDRRect'));
expect(mockCall.arguments, equals({
'outer': rrect,
'inner': rrect.deflate(1),
'paint': somePaint.webOnlyPaintData,
}));
});

test('Inner RRect > Outer RRect', () {
underTest.drawDRRect(rrect, rrect.inflate(1), somePaint);
underTest.apply(mockCanvas);
// Expect nothing to be called
expect(mockCanvas.methodCallLog.length, equals(0));
});

test('Inner RRect not completely inside Outer RRect', () {
underTest.drawDRRect(rrect, rrect.deflate(1).shift(const Offset(0.0, 10)), somePaint);
underTest.apply(mockCanvas);
// Expect nothing to be called
expect(mockCanvas.methodCallLog.length, equals(0));
});

test('Inner RRect same as Outer RRect', () {
underTest.drawDRRect(rrect, rrect, somePaint);
underTest.apply(mockCanvas);
// Expect nothing to be called
expect(mockCanvas.methodCallLog.length, equals(0));
});
});
}