Skip to content

Commit

Permalink
fix canvas drawLine bugs (flutter#38753)
Browse files Browse the repository at this point in the history
  • Loading branch information
alanwutang11 authored Jan 12, 2023
1 parent 2d2c5e7 commit 24eb954
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 27 deletions.
21 changes: 3 additions & 18 deletions lib/web_ui/lib/src/engine/html/bitmap_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -538,20 +538,6 @@ class BitmapCanvas extends EngineCanvas {
if (_useDomForRenderingFill(paint)) {
final Matrix4 transform = _canvasPool.currentTransform;
final SurfacePath surfacePath = path as SurfacePath;
final ui.Rect? pathAsLine = surfacePath.toStraightLine();
if (pathAsLine != null) {
ui.Rect rect = (pathAsLine.top == pathAsLine.bottom)
? ui.Rect.fromLTWH(
pathAsLine.left, pathAsLine.top, pathAsLine.width, 1)
: ui.Rect.fromLTWH(
pathAsLine.left, pathAsLine.top, 1, pathAsLine.height);

rect = adjustRectForDom(rect, paint);
final DomHTMLElement element = buildDrawRectElement(
rect, paint, 'draw-rect', _canvasPool.currentTransform);
_drawElement(element, rect.topLeft, paint);
return;
}

final ui.Rect? pathAsRect = surfacePath.toRect();
if (pathAsRect != null) {
Expand All @@ -563,16 +549,15 @@ class BitmapCanvas extends EngineCanvas {
drawRRect(pathAsRRect, paint);
return;
}
final ui.Rect pathBounds = surfacePath.getBounds();
final DomElement svgElm = pathToSvgElement(
surfacePath, paint, '${pathBounds.right}', '${pathBounds.bottom}');
final DomElement svgElm = pathToSvgElement(surfacePath, paint);
if (!_canvasPool.isClipped) {
final DomCSSStyleDeclaration style = svgElm.style;
style.position = 'absolute';
if (!transform.isIdentity()) {
style
..transform = matrix4ToCssTransform(transform)
..transformOrigin = '0 0 0';
..transformOrigin = '0 0 0'
..overflow = 'visible';
}
}
_applyFilter(svgElm, paint);
Expand Down
4 changes: 1 addition & 3 deletions lib/web_ui/lib/src/engine/html/clip.dart
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface
path,
SurfacePaintData()
..style = ui.PaintingStyle.fill
..color = color.value,
'${pathBounds2.right}',
'${pathBounds2.bottom}');
..color = color.value);

/// Render element behind the clipped content.
rootElement!.insertBefore(_svgElement!, childContainer);
Expand Down
12 changes: 6 additions & 6 deletions lib/web_ui/lib/src/engine/html/dom_canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import '../svg.dart';
import '../text/canvas_paragraph.dart';
import '../util.dart';
import '../vector_math.dart';
import 'bitmap_canvas.dart';
import 'painting.dart';
import 'path/path.dart';
import 'path/path_to_svg.dart';
Expand Down Expand Up @@ -333,14 +334,10 @@ String _borderStrokeToCssUnit(double value) {
return '${value.toStringAsFixed(3)}px';
}

SVGSVGElement pathToSvgElement(
SurfacePath path, SurfacePaintData paint, String width, String height) {
SVGSVGElement pathToSvgElement(SurfacePath path, SurfacePaintData paint) {
// In Firefox some SVG typed attributes are returned as null without a
// setter. So we use strings here.
final SVGSVGElement root = createSVGSVGElement()
..setAttribute('width', '${width}px')
..setAttribute('height', '${height}px')
..setAttribute('viewBox', '0 0 $width $height');
final SVGSVGElement root = createSVGSVGElement();

final SVGPathElement svgPath = createSVGPathElement();
root.append(svgPath);
Expand All @@ -350,6 +347,9 @@ SVGSVGElement pathToSvgElement(
paint.strokeWidth != null)) {
svgPath.setAttribute('stroke', colorValueToCssString(paint.color)!);
svgPath.setAttribute('stroke-width', '${paint.strokeWidth ?? 1.0}');
if (paint.strokeCap != null) {
svgPath.setAttribute('stroke-linecap', '${stringForStrokeCap(paint.strokeCap)}');
}
svgPath.setAttribute('fill', 'none');
} else if (paint.color != null) {
svgPath.setAttribute('fill', colorValueToCssString(paint.color)!);
Expand Down
80 changes: 80 additions & 0 deletions lib/web_ui/test/html/drawing/canvas_lines_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@ Future<void> testMain() async {
const Rect region = Rect.fromLTWH(0, 0, 300, 300);

late BitmapCanvas canvas;
late BitmapCanvas domCanvas;

setUp(() {
canvas = BitmapCanvas(region, RenderStrategy());
// setting isInsideSvgFilterTree true forces use of DOM canvas
domCanvas = BitmapCanvas(region, RenderStrategy()..isInsideSvgFilterTree = true);
});

tearDown(() {
canvas.rootElement.remove();
domCanvas.rootElement.remove();
});

test('draws lines with varying strokeWidth', () async {
Expand All @@ -33,6 +37,75 @@ Future<void> testMain() async {
domDocument.body!.append(canvas.rootElement);
await matchGoldenFile('canvas_lines_thickness.png', region: region);
});
test('draws lines with varying strokeWidth with dom canvas', () async {

paintLines(domCanvas);

domDocument.body!.append(domCanvas.rootElement);
await matchGoldenFile('canvas_lines_thickness_dom_canvas.png', region: region);
});
test('draws lines with negative Offset values with dom canvas', () async {
// test rendering lines correctly with negative offset when using DOM
final SurfacePaintData paintWithStyle = SurfacePaintData()
..color = 0xFFE91E63 // Colors.pink
..style = PaintingStyle.stroke
..strokeWidth = 16
..strokeCap = StrokeCap.round;

// canvas.drawLine ignores paint.style (defaults to fill) according to api docs.
// expect lines are rendered the same regardless of the set paint.style
final SurfacePaintData paintWithoutStyle = SurfacePaintData()
..color = 0xFF4CAF50 // Colors.green
..strokeWidth = 16
..strokeCap = StrokeCap.round;

// test vertical, horizontal, and diagonal lines
final List<Offset> points = <Offset>[
const Offset(-25, 50), const Offset(45, 50),
const Offset(100, -25), const Offset(100, 200),
const Offset(-150, -145), const Offset(100, 200),
];
final List<Offset> shiftedPoints = points.map((Offset point) => point.translate(20, 20)).toList();

paintLinesFromPoints(domCanvas, paintWithStyle, points);
paintLinesFromPoints(domCanvas, paintWithoutStyle, shiftedPoints);

domDocument.body!.append(domCanvas.rootElement);
await matchGoldenFile('canvas_lines_with_negative_offset.png', region: region);
});

test('drawLines method respects strokeCap with dom canvas', () async {
final SurfacePaintData paintStrokeCapRound = SurfacePaintData()
..color = 0xFFE91E63 // Colors.pink
..strokeWidth = 16
..strokeCap = StrokeCap.round;

final SurfacePaintData paintStrokeCapSquare = SurfacePaintData()
..color = 0xFF4CAF50 // Colors.green
..strokeWidth = 16
..strokeCap = StrokeCap.square;

final SurfacePaintData paintStrokeCapButt = SurfacePaintData()
..color = 0xFFFF9800 // Colors.orange
..strokeWidth = 16
..strokeCap = StrokeCap.butt;

// test vertical, horizontal, and diagonal lines
final List<Offset> points = <Offset>[
const Offset(5, 50), const Offset(45, 50),
const Offset(100, 5), const Offset(100, 200),
const Offset(5, 10), const Offset(100, 200),
];
final List<Offset> shiftedPoints = points.map((Offset point) => point.translate(50, 50)).toList();
final List<Offset> twiceShiftedPoints = shiftedPoints.map((Offset point) => point.translate(50, 50)).toList();

paintLinesFromPoints(domCanvas, paintStrokeCapRound, points);
paintLinesFromPoints(domCanvas, paintStrokeCapSquare, shiftedPoints);
paintLinesFromPoints(domCanvas, paintStrokeCapButt, twiceShiftedPoints);

domDocument.body!.append(domCanvas.rootElement);
await matchGoldenFile('canvas_lines_with_strokeCap.png', region: region);
});
}

void paintLines(BitmapCanvas canvas) {
Expand Down Expand Up @@ -70,3 +143,10 @@ void paintLines(BitmapCanvas canvas) {
paint3.color = 0xFFFF9800; // Colors.orange;
canvas.drawLine(const Offset(50, 70), const Offset(150, 70), paint3);
}

void paintLinesFromPoints(BitmapCanvas canvas, SurfacePaintData paint, List<Offset> points) {
// points list contains pairs of Offset points, so for loop step is 2
for (int i = 0; i < points.length - 1; i += 2) {
canvas.drawLine(points[i], points[i + 1], paint);
}
}

0 comments on commit 24eb954

Please sign in to comment.