From eb4f575aa820e755724d926f6755bbbd5516401f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 13 Jun 2024 15:06:40 +0200 Subject: [PATCH] Fix some issues with strokeless geometry segments (#16019) * Added failing cross test. For polyline segment with `IsStroked = false`. * Fixed some issues with non-stroked segments * Fix CombinedGeometryImpl with empty paths. If an empty (but non-null) stroke was passed to `CombinedGeometryImpl` then its empty bounds would be used and the fill bounds ignored. Added a test and fixed that. --------- Co-authored-by: Nikita Tsukanov --- src/Avalonia.Base/Media/PolyLineSegment.cs | 2 +- .../Avalonia.Skia/CombinedGeometryImpl.cs | 8 ++- src/Skia/Avalonia.Skia/GeometryImpl.cs | 5 +- src/Skia/Avalonia.Skia/StreamGeometryImpl.cs | 49 ++++++++---------- .../CrossUI.Wpf.cs | 1 + .../CrossTests/CrossGeometryTests.cs | 33 +++++++++++- .../CrossUI/CrossUI.Avalonia.cs | 5 ++ tests/Avalonia.RenderTests/CrossUI/CrossUI.cs | 1 + .../CombinedGeometryImplTests.cs | 23 ++++++++ ...yLineSegment_With_Strokeless_Lines.wpf.png | Bin 0 -> 362 bytes 10 files changed, 97 insertions(+), 30 deletions(-) create mode 100644 tests/Avalonia.Skia.UnitTests/CombinedGeometryImplTests.cs create mode 100644 tests/TestFiles/CrossTests/Media/Geometry/Should_Render_PolyLineSegment_With_Strokeless_Lines.wpf.png diff --git a/src/Avalonia.Base/Media/PolyLineSegment.cs b/src/Avalonia.Base/Media/PolyLineSegment.cs index da87a2155b..c97c016ab3 100644 --- a/src/Avalonia.Base/Media/PolyLineSegment.cs +++ b/src/Avalonia.Base/Media/PolyLineSegment.cs @@ -53,7 +53,7 @@ namespace Avalonia.Media { for (int i = 0; i < points.Count; i++) { - ctx.LineTo(points[i]); + ctx.LineTo(points[i], IsStroked); } } } diff --git a/src/Skia/Avalonia.Skia/CombinedGeometryImpl.cs b/src/Skia/Avalonia.Skia/CombinedGeometryImpl.cs index 1e9240ec70..3ba4738ce6 100644 --- a/src/Skia/Avalonia.Skia/CombinedGeometryImpl.cs +++ b/src/Skia/Avalonia.Skia/CombinedGeometryImpl.cs @@ -13,7 +13,13 @@ namespace Avalonia.Skia { StrokePath = stroke; FillPath = fill; - Bounds = (stroke ?? fill)?.TightBounds.ToAvaloniaRect() ?? default; + + var bounds = stroke?.TightBounds ?? default; + + if (fill != null) + bounds.Union(fill.TightBounds); + + Bounds = bounds.ToAvaloniaRect(); } public static CombinedGeometryImpl ForceCreate(GeometryCombineMode combineMode, IGeometryImpl g1, IGeometryImpl g2) diff --git a/src/Skia/Avalonia.Skia/GeometryImpl.cs b/src/Skia/Avalonia.Skia/GeometryImpl.cs index 1685dc9d20..47508970d5 100644 --- a/src/Skia/Avalonia.Skia/GeometryImpl.cs +++ b/src/Skia/Avalonia.Skia/GeometryImpl.cs @@ -80,7 +80,10 @@ namespace Avalonia.Skia lock (_lock) { _pathCache.UpdateIfNeeded(StrokePath, pen); - return _pathCache.RenderBounds; + var bounds = _pathCache.RenderBounds; + if (StrokePath != FillPath && FillPath != null) + bounds = bounds.Union(FillPath.Bounds.ToAvaloniaRect()); + return bounds; } } diff --git a/src/Skia/Avalonia.Skia/StreamGeometryImpl.cs b/src/Skia/Avalonia.Skia/StreamGeometryImpl.cs index e66c77c9a3..940a1c7247 100644 --- a/src/Skia/Avalonia.Skia/StreamGeometryImpl.cs +++ b/src/Skia/Avalonia.Skia/StreamGeometryImpl.cs @@ -88,6 +88,22 @@ namespace Avalonia.Skia private bool _isFigureBroken; private bool Duplicate => _isFilled && !ReferenceEquals(_geometryImpl._fillPath, Stroke); + private void EnsureSeparateFillPath() + { + if (Stroke == Fill) + _geometryImpl._fillPath = Stroke.Clone(); + } + + private void BreakFigure() + { + if (!_isFigureBroken) + { + _isFigureBroken = true; + EnsureSeparateFillPath(); + } + + } + /// /// Initializes a new instance of the class. /// Geometry to operate on. @@ -134,11 +150,8 @@ namespace Avalonia.Skia /// public void BeginFigure(Point startPoint, bool isFilled) { - if (!isFilled) - { - if (Stroke == Fill) - _geometryImpl._fillPath = Stroke.Clone(); - } + if (!isFilled) + EnsureSeparateFillPath(); _isFilled = isFilled; _startPoint = startPoint; @@ -179,7 +192,7 @@ namespace Avalonia.Skia { if (_isFigureBroken) { - LineTo(_startPoint); + Stroke.LineTo(_startPoint.ToSKPoint()); _isFigureBroken = false; } else @@ -204,11 +217,7 @@ namespace Avalonia.Skia } else { - if (Stroke == Fill) - _geometryImpl._fillPath = Stroke.Clone(); - - _isFigureBroken = true; - + BreakFigure(); Stroke.MoveTo((float)point.X, (float)point.Y); } if (Duplicate) @@ -236,11 +245,7 @@ namespace Avalonia.Skia } else { - if (Stroke == Fill) - _geometryImpl._fillPath = Stroke.Clone(); - - _isFigureBroken = true; - + BreakFigure(); Stroke.MoveTo((float)point.X, (float)point.Y); } if (Duplicate) @@ -263,11 +268,7 @@ namespace Avalonia.Skia } else { - if (Stroke == Fill) - _geometryImpl._fillPath = Stroke.Clone(); - - _isFigureBroken = true; - + BreakFigure(); Stroke.MoveTo((float)point3.X, (float)point3.Y); } if (Duplicate) @@ -283,11 +284,7 @@ namespace Avalonia.Skia } else { - if (Stroke == Fill) - _geometryImpl._fillPath = Stroke.Clone(); - - _isFigureBroken = true; - + BreakFigure(); Stroke.MoveTo((float)point2.X, (float)point2.Y); } if (Duplicate) diff --git a/tests/Avalonia.RenderTests.WpfCompare/CrossUI.Wpf.cs b/tests/Avalonia.RenderTests.WpfCompare/CrossUI.Wpf.cs index 3ed994ca8e..32e29a5193 100644 --- a/tests/Avalonia.RenderTests.WpfCompare/CrossUI.Wpf.cs +++ b/tests/Avalonia.RenderTests.WpfCompare/CrossUI.Wpf.cs @@ -190,6 +190,7 @@ namespace Avalonia.RenderTests.WpfCompare CrossPathSegment.Arc arc => new ArcSegment(arc.Point.ToWpf(), arc.Size.ToWpf(), arc.RotationAngle, arc.IsLargeArc, (SweepDirection)arc.SweepDirection, s.IsStroked), CrossPathSegment.CubicBezier cubicBezier => new BezierSegment(cubicBezier.Point1.ToWpf(), cubicBezier.Point2.ToWpf(), cubicBezier.Point3.ToWpf(), cubicBezier.IsStroked), CrossPathSegment.QuadraticBezier quadraticBezier => new QuadraticBezierSegment(quadraticBezier.Point1.ToWpf(), quadraticBezier.Point2.ToWpf(), quadraticBezier.IsStroked), + CrossPathSegment.PolyLine polyLine => new PolyLineSegment(polyLine.Points.Select(p => p.ToWpf()).ToList(), polyLine.IsStroked), _ => throw new NotImplementedException(), }), f.Closed))) }; diff --git a/tests/Avalonia.RenderTests/CrossTests/CrossGeometryTests.cs b/tests/Avalonia.RenderTests/CrossTests/CrossGeometryTests.cs index 0fdd2fc0d9..0761e09a13 100644 --- a/tests/Avalonia.RenderTests/CrossTests/CrossGeometryTests.cs +++ b/tests/Avalonia.RenderTests/CrossTests/CrossGeometryTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Drawing.Drawing2D; using System.Linq; using Avalonia.Media; using CrossUI; @@ -109,7 +110,37 @@ public class CrossGeometryTests : CrossTestBase Background = brush }); } - + + + [CrossFact] + public void Should_Render_PolyLineSegment_With_Strokeless_Lines() + { + var brush = new CrossSolidColorBrush(Colors.Blue); + var pen = new CrossPen() + { + Brush = new CrossSolidColorBrush(Colors.Red), + Thickness = 8 + }; + var figure = new CrossPathFigure() + { + Closed = true, + Segments = + { + new CrossPathSegment.PolyLine([new(0, 0), new(100, 0), new(100, 100), new(0, 100), new(0, 0)], false) + } + }; + var geometry = new CrossPathGeometry { Figures = { figure } }; + + var control = new CrossFuncControl(ctx => ctx.DrawGeometry(brush, pen, geometry)) + { + Width = 100, + Height = 100, + }; + + RenderAndCompare(control, + $"{nameof(Should_Render_PolyLineSegment_With_Strokeless_Lines)}"); + } + // Skip the test for now #if !AVALONIA_SKIA [CrossTheory, diff --git a/tests/Avalonia.RenderTests/CrossUI/CrossUI.Avalonia.cs b/tests/Avalonia.RenderTests/CrossUI/CrossUI.Avalonia.cs index 168aeeb096..96574db757 100644 --- a/tests/Avalonia.RenderTests/CrossUI/CrossUI.Avalonia.cs +++ b/tests/Avalonia.RenderTests/CrossUI/CrossUI.Avalonia.cs @@ -201,6 +201,11 @@ namespace Avalonia.Direct2D1.RenderTests.CrossUI Point2 = q.Point2, IsStroked = q.IsStroked }, + CrossPathSegment.PolyLine p => new PolyLineSegment() + { + Points = p.Points.ToList(), + IsStroked = p.IsStroked + }, _ => throw new InvalidOperationException() })) })) diff --git a/tests/Avalonia.RenderTests/CrossUI/CrossUI.cs b/tests/Avalonia.RenderTests/CrossUI/CrossUI.cs index 13a3a3029c..587e8d0f83 100644 --- a/tests/Avalonia.RenderTests/CrossUI/CrossUI.cs +++ b/tests/Avalonia.RenderTests/CrossUI/CrossUI.cs @@ -158,6 +158,7 @@ public abstract record class CrossPathSegment(bool IsStroked) public record Arc(Point Point, Size Size, double RotationAngle, bool IsLargeArc, SweepDirection SweepDirection, bool IsStroked) : CrossPathSegment(IsStroked); public record CubicBezier(Point Point1, Point Point2, Point Point3, bool IsStroked) : CrossPathSegment(IsStroked); public record QuadraticBezier(Point Point1, Point Point2, bool IsStroked) : CrossPathSegment(IsStroked); + public record PolyLine(IEnumerable Points, bool IsStroked) : CrossPathSegment(IsStroked); } public class CrossDrawingBrush : CrossTileBrush diff --git a/tests/Avalonia.Skia.UnitTests/CombinedGeometryImplTests.cs b/tests/Avalonia.Skia.UnitTests/CombinedGeometryImplTests.cs new file mode 100644 index 0000000000..018a56205d --- /dev/null +++ b/tests/Avalonia.Skia.UnitTests/CombinedGeometryImplTests.cs @@ -0,0 +1,23 @@ +using SkiaSharp; +using Xunit; + +namespace Avalonia.Skia.UnitTests; + +public class CombinedGeometryImplTests +{ + [Fact] + public void Combining_Fill_With_Empty_Stroke_Returns_Fill_Bounds() + { + var fill = new SKPath(); + fill.LineTo(100, 0); + fill.LineTo(100, 100); + fill.LineTo(0, 100); + fill.Close(); + + var stroke = new SKPath(); + + var result = new CombinedGeometryImpl(stroke, fill); + + Assert.Equal(new Rect(0, 0, 100, 100), result.Bounds); + } +} diff --git a/tests/TestFiles/CrossTests/Media/Geometry/Should_Render_PolyLineSegment_With_Strokeless_Lines.wpf.png b/tests/TestFiles/CrossTests/Media/Geometry/Should_Render_PolyLineSegment_With_Strokeless_Lines.wpf.png new file mode 100644 index 0000000000000000000000000000000000000000..3efd30c5bd133a76a86a9f49cf44f90ebf7887ce GIT binary patch literal 362 zcmeAS@N?(olHy`uVBq!ia0vp^DImgTe~DWM4f5@B>p literal 0 HcmV?d00001