From 629ff8e73e237ad6b35c2d2c7663c71942dd3506 Mon Sep 17 00:00:00 2001 From: alanwutang11 <50225138+alanwutang11@users.noreply.github.com> Date: Thu, 12 Jan 2023 10:16:14 -0800 Subject: [PATCH] Revert "fix canvas drawLine bugs (#38753)" This reverts commit 24eb954dac512c74299314c4d376ef2e17052b2c. --- .../lib/src/engine/html/bitmap_canvas.dart | 21 ++++- lib/web_ui/lib/src/engine/html/clip.dart | 4 +- .../lib/src/engine/html/dom_canvas.dart | 12 +-- .../drawing/canvas_lines_golden_test.dart | 80 ------------------- 4 files changed, 27 insertions(+), 90 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart index 3ea737c4d77d3..5468b0bb066ea 100644 --- a/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart @@ -538,6 +538,20 @@ 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) { @@ -549,15 +563,16 @@ class BitmapCanvas extends EngineCanvas { drawRRect(pathAsRRect, paint); return; } - final DomElement svgElm = pathToSvgElement(surfacePath, paint); + final ui.Rect pathBounds = surfacePath.getBounds(); + final DomElement svgElm = pathToSvgElement( + surfacePath, paint, '${pathBounds.right}', '${pathBounds.bottom}'); if (!_canvasPool.isClipped) { final DomCSSStyleDeclaration style = svgElm.style; style.position = 'absolute'; if (!transform.isIdentity()) { style ..transform = matrix4ToCssTransform(transform) - ..transformOrigin = '0 0 0' - ..overflow = 'visible'; + ..transformOrigin = '0 0 0'; } } _applyFilter(svgElm, paint); diff --git a/lib/web_ui/lib/src/engine/html/clip.dart b/lib/web_ui/lib/src/engine/html/clip.dart index dc826ff88a5b0..892bc88403825 100644 --- a/lib/web_ui/lib/src/engine/html/clip.dart +++ b/lib/web_ui/lib/src/engine/html/clip.dart @@ -392,7 +392,9 @@ class PersistedPhysicalShape extends PersistedContainerSurface path, SurfacePaintData() ..style = ui.PaintingStyle.fill - ..color = color.value); + ..color = color.value, + '${pathBounds2.right}', + '${pathBounds2.bottom}'); /// Render element behind the clipped content. rootElement!.insertBefore(_svgElement!, childContainer); diff --git a/lib/web_ui/lib/src/engine/html/dom_canvas.dart b/lib/web_ui/lib/src/engine/html/dom_canvas.dart index 467053a014a28..3e63a0b99ff03 100644 --- a/lib/web_ui/lib/src/engine/html/dom_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/dom_canvas.dart @@ -14,7 +14,6 @@ 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'; @@ -334,10 +333,14 @@ String _borderStrokeToCssUnit(double value) { return '${value.toStringAsFixed(3)}px'; } -SVGSVGElement pathToSvgElement(SurfacePath path, SurfacePaintData paint) { +SVGSVGElement pathToSvgElement( + SurfacePath path, SurfacePaintData paint, String width, String height) { // In Firefox some SVG typed attributes are returned as null without a // setter. So we use strings here. - final SVGSVGElement root = createSVGSVGElement(); + final SVGSVGElement root = createSVGSVGElement() + ..setAttribute('width', '${width}px') + ..setAttribute('height', '${height}px') + ..setAttribute('viewBox', '0 0 $width $height'); final SVGPathElement svgPath = createSVGPathElement(); root.append(svgPath); @@ -347,9 +350,6 @@ SVGSVGElement pathToSvgElement(SurfacePath path, SurfacePaintData paint) { 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)!); diff --git a/lib/web_ui/test/html/drawing/canvas_lines_golden_test.dart b/lib/web_ui/test/html/drawing/canvas_lines_golden_test.dart index db0d0fcb148fc..166df80c58277 100644 --- a/lib/web_ui/test/html/drawing/canvas_lines_golden_test.dart +++ b/lib/web_ui/test/html/drawing/canvas_lines_golden_test.dart @@ -17,17 +17,13 @@ Future 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 { @@ -37,75 +33,6 @@ Future 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 points = [ - 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 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 points = [ - 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 shiftedPoints = points.map((Offset point) => point.translate(50, 50)).toList(); - final List 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) { @@ -143,10 +70,3 @@ 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 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); - } -}