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 <[email protected]>

src/Avalonia.Base/Media/PolyLineSegment.cs

@@ -53,7 +53,7 @@ internal override void ApplyTo(StreamGeometryContext ctx)
  {
  for (int i = 0; i < points.Count; i++)
  {
- ctx.LineTo(points[i]);
+ ctx.LineTo(points[i], IsStroked);
  }
  }
  }

src/Skia/Avalonia.Skia/CombinedGeometryImpl.cs

@@ -13,7 +13,13 @@ public CombinedGeometryImpl(SKPath? stroke, SKPath? fill)
  {
  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)

src/Skia/Avalonia.Skia/GeometryImpl.cs

@@ -80,7 +80,10 @@ public Rect GetRenderBounds(IPen? pen)
  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;
  }
  }
 

src/Skia/Avalonia.Skia/StreamGeometryImpl.cs

@@ -88,6 +88,22 @@ private class StreamContext : IStreamGeometryContextImpl, IGeometryContext2
  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();
+ }
+
+ }
+
  /// <summary>
  /// Initializes a new instance of the <see cref="StreamContext"/> class.
  /// <param name="geometryImpl">Geometry to operate on.</param>
@@ -134,11 +150,8 @@ public void ArcTo(Point point, Size size, double rotationAngle, bool isLargeArc,
  /// <inheritdoc />
  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 @@ public void EndFigure(bool isClosed)
  {
  if (_isFigureBroken)
  {
- LineTo(_startPoint);
+ Stroke.LineTo(_startPoint.ToSKPoint());
  _isFigureBroken = false;
  }
  else
@@ -204,11 +217,7 @@ public void LineTo(Point point, bool isStroked)
  }
  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 @@ public void ArcTo(Point point, Size size, double rotationAngle, bool isLargeArc,
  }
  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 @@ public void CubicBezierTo(Point point1, Point point2, Point point3, bool isStrok
  }
  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 @@ public void QuadraticBezierTo(Point point1, Point point2, bool isStroked)
  }
  else
  {
- if (Stroke == Fill)
- _geometryImpl._fillPath = Stroke.Clone();
-
- _isFigureBroken = true;
-
+ BreakFigure();
  Stroke.MoveTo((float)point2.X, (float)point2.Y);
  }
  if (Duplicate)

tests/Avalonia.RenderTests.WpfCompare/CrossUI.Wpf.cs

@@ -190,6 +190,7 @@ s switch
  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)))
  };

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 void Should_Render_Geometry_With_Strokeless_Lines()
  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,

tests/Avalonia.RenderTests/CrossUI/CrossUI.Avalonia.cs

@@ -201,6 +201,11 @@ s switch
  Point2 = q.Point2,
  IsStroked = q.IsStroked
  },
+ CrossPathSegment.PolyLine p => new PolyLineSegment()
+ {
+ Points = p.Points.ToList(),
+ IsStroked = p.IsStroked
+ },
  _ => throw new InvalidOperationException()
  }))
  }))

tests/Avalonia.RenderTests/CrossUI/CrossUI.cs

@@ -158,6 +158,7 @@ public record Line(Point To, bool IsStroked) : CrossPathSegment(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<Point> Points, bool IsStroked) : CrossPathSegment(IsStroked);
 }
 
 public class CrossDrawingBrush : CrossTileBrush

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);
+ }
+}