Merge pull request #12606 from AvaloniaUI/fixes/correct-render-bounds

When calculating geometry bounds take into account parameters that affect geometry bounds

src/Avalonia.Base/Rendering/Composition/Drawing/Nodes/RenderDataGeometryNode.cs

@@ -25,5 +25,5 @@ class RenderDataGeometryNode : RenderDataBrushAndPenNode
         context.Context.DrawGeometry(ServerBrush, ServerPen, Geometry!);
     }
 
-    public override Rect? Bounds => Geometry?.GetRenderBounds(ServerPen).CalculateBoundsWithLineCaps(ServerPen) ?? default;
+    public override Rect? Bounds => Geometry?.GetRenderBounds(ServerPen) ?? default;
 }

src/Avalonia.Base/Rendering/SceneGraph/GeometryBoundsHelper.cs

@@ -1,31 +0,0 @@
-using System;
-using Avalonia.Media;
-using Avalonia.Utilities;
-
-namespace Avalonia.Rendering.SceneGraph;
-
-internal static class GeometryBoundsHelper
-{
-    /// <summary>
-    /// Calculates the bounds of a given geometry with respect to the pens <see cref="IPen.LineCap"/>
-    /// </summary>
-    /// <param name="originalBounds">The calculated bounds without <see cref="IPen.LineCap"/>s</param>
-    /// <param name="pen">The pen with information about the <see cref="IPen.LineCap"/>s</param>
-    /// <returns></returns>
-    public static Rect CalculateBoundsWithLineCaps(this Rect originalBounds, IPen? pen)
-    {
-        if (pen is null || MathUtilities.IsZero(pen.Thickness)) return originalBounds;
-
-        switch (pen.LineCap)
-        {
-            case PenLineCap.Flat:
-                return originalBounds;
-            case PenLineCap.Round:
-                return originalBounds.Inflate(pen.Thickness / 2);
-            case PenLineCap.Square:
-                return originalBounds.Inflate(pen.Thickness);
-            default:
-                throw new ArgumentOutOfRangeException();
-        }
-    }
-}

src/Skia/Avalonia.Skia/DrawingContextImpl.cs

@@ -1247,31 +1247,8 @@ namespace Avalonia.Skia
             // https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/graphics/skiasharp/paths/dots
             // TODO: Still something is off, dashes are now present, but don't look the same as D2D ones.
 
-            switch (pen.LineCap)
-            {
-                case PenLineCap.Round:
-                    paint.StrokeCap = SKStrokeCap.Round;
-                    break;
-                case PenLineCap.Square:
-                    paint.StrokeCap = SKStrokeCap.Square;
-                    break;
-                default:
-                    paint.StrokeCap = SKStrokeCap.Butt;
-                    break;
-            }
-
-            switch (pen.LineJoin)
-            {
-                case PenLineJoin.Miter:
-                    paint.StrokeJoin = SKStrokeJoin.Miter;
-                    break;
-                case PenLineJoin.Round:
-                    paint.StrokeJoin = SKStrokeJoin.Round;
-                    break;
-                default:
-                    paint.StrokeJoin = SKStrokeJoin.Bevel;
-                    break;
-            }
+            paint.StrokeCap = pen.LineCap.ToSKStrokeCap();
+            paint.StrokeJoin = pen.LineJoin.ToSKStrokeJoin();
 
             paint.StrokeMiter = (float) pen.MiterLimit;
 

src/Skia/Avalonia.Skia/GeometryImpl.cs

@@ -43,46 +43,11 @@ namespace Avalonia.Skia
         /// <inheritdoc />
         public bool StrokeContains(IPen? pen, Point point)
         {
-            // Skia requires to compute stroke path to check for point containment.
-            // Due to that we are caching using stroke width.
-            // Usually this function is being called with same stroke width per path, so this saves a lot of Skia traffic.
+            _pathCache.UpdateIfNeeded(StrokePath, pen);
 
-            var strokeWidth = (float)(pen?.Thickness ?? 0);
-
-            if (!_pathCache.HasCacheFor(strokeWidth))
-            {
-                UpdatePathCache(strokeWidth);
-            }
-
-            return PathContainsCore(_pathCache.CachedStrokePath, point);
-        }
-
-        /// <summary>
-        /// Update path cache for given stroke width.
-        /// </summary>
-        /// <param name="strokeWidth">Stroke width.</param>
-        private void UpdatePathCache(float strokeWidth)
-        {
-            var strokePath = new SKPath();
-
-            // For stroke widths close to 0 simply use empty path. Render bounds are cached from fill path.
-            if (Math.Abs(strokeWidth) < float.Epsilon)
-            {
-                _pathCache.Cache(strokePath, strokeWidth, Bounds);
-            }
-            else
-            {
-                var paint = SKPaintCache.Shared.Get();
-                paint.IsStroke = true;
-                paint.StrokeWidth = strokeWidth;
-                paint.GetFillPath(StrokePath, strokePath);
-
-                SKPaintCache.Shared.ReturnReset(paint);
-
-                _pathCache.Cache(strokePath, strokeWidth, strokePath.TightBounds.ToAvaloniaRect());
-            }
+            return PathContainsCore(_pathCache.ExpandedPath, point);
         }
-
+        
         /// <summary>
         /// Check Skia path if it contains a point.
         /// </summary>
@@ -106,14 +71,8 @@ namespace Avalonia.Skia
         /// <inheritdoc />
         public Rect GetRenderBounds(IPen? pen)
         {
-            var strokeWidth = (float)(pen?.Thickness ?? 0);
-
-            if (!_pathCache.HasCacheFor(strokeWidth))
-            {
-                UpdatePathCache(strokeWidth);
-            }
-
-            return _pathCache.CachedGeometryRenderBounds;
+            _pathCache.UpdateIfNeeded(StrokePath, pen);
+            return _pathCache.RenderBounds;
         }
 
         /// <inheritdoc />
@@ -180,66 +139,70 @@ namespace Avalonia.Skia
         /// </summary>
         protected void InvalidateCaches()
         {
-            _pathCache.Invalidate();
+            _pathCache.Dispose();
+            _pathCache = default;
         }
 
         private struct PathCache
         {
-            private float _cachedStrokeWidth;
-
-            /// <summary>
-            /// Tolerance for two stroke widths to be deemed equal
-            /// </summary>
-            public const float Tolerance = float.Epsilon;
-
-            /// <summary>
-            /// Cached contour path.
-            /// </summary>
-            public SKPath? CachedStrokePath { get; private set; }
-
-            /// <summary>
-            /// Cached geometry render bounds.
-            /// </summary>
-            public Rect CachedGeometryRenderBounds { get; private set; }
-
-            /// <summary>
-            /// Is cached valid for given stroke width.
-            /// </summary>
-            /// <param name="strokeWidth">Stroke width to check.</param>
-            /// <returns>True, if CachedStrokePath can be used for given stroke width.</returns>
-            public bool HasCacheFor(float strokeWidth)
+            private double _width, _miterLimit;
+            private PenLineCap _cap;
+            private PenLineJoin _join;
+            private SKPath? _path, _cachedFor;
+            private Rect? _renderBounds;
+            private static readonly SKPath s_emptyPath = new();
+            
+
+            public Rect RenderBounds => _renderBounds ??= (_path ?? _cachedFor ?? s_emptyPath).Bounds.ToAvaloniaRect();
+            public SKPath ExpandedPath => _path ?? s_emptyPath;
+
+            public void UpdateIfNeeded(SKPath? strokePath, IPen? pen)
             {
-                return CachedStrokePath != null && Math.Abs(_cachedStrokeWidth - strokeWidth) < Tolerance;
-            }
-
-            /// <summary>
-            /// Cache path for given stroke width. Takes ownership of a passed path.
-            /// </summary>
-            /// <param name="path">Path to cache.</param>
-            /// <param name="strokeWidth">Stroke width to cache.</param>
-            /// <param name="geometryRenderBounds">Render bounds to use.</param>
-            public void Cache(SKPath path, float strokeWidth, Rect geometryRenderBounds)
-            {
-                if (CachedStrokePath != path)
+                var strokeWidth = pen?.Thickness ?? 0;
+                var miterLimit = pen?.MiterLimit ?? 0;
+                var cap = pen?.LineCap ?? default;
+                var join = pen?.LineJoin ?? default;
+                
+                if (_cachedFor == strokePath
+                    && _path != null
+                    && cap == _cap
+                    && join == _join
+                    && Math.Abs(_width - strokeWidth) < float.Epsilon
+                    && (join != PenLineJoin.Miter || Math.Abs(_miterLimit - miterLimit) > float.Epsilon))
+                    // We are up to date
+                    return;
+
+                _renderBounds = null;
+                _cachedFor = strokePath;
+                _width = strokeWidth;
+                _cap = cap;
+                _join = join;
+                _miterLimit = miterLimit;
+                
+                if (strokePath == null || Math.Abs(strokeWidth) < float.Epsilon)
                 {
-                    CachedStrokePath?.Dispose();
+                    _path = null;
+                    return;
                 }
 
-                CachedStrokePath = path;
-                CachedGeometryRenderBounds = geometryRenderBounds;
-                _cachedStrokeWidth = strokeWidth;
+                var paint = SKPaintCache.Shared.Get();
+                paint.IsStroke = true;
+                paint.StrokeWidth = (float)_width;
+                paint.StrokeCap = cap.ToSKStrokeCap();
+                paint.StrokeJoin = join.ToSKStrokeJoin();
+                paint.StrokeMiter = (float)miterLimit;
+                _path = new SKPath();
+                paint.GetFillPath(strokePath, _path);
+
+                SKPaintCache.Shared.ReturnReset(paint);
             }
 
-            /// <summary>
-            /// Invalidate cache state.
-            /// </summary>
-            public void Invalidate()
+            public void Dispose()
             {
-                CachedStrokePath?.Dispose();
-                CachedStrokePath = null;
-                CachedGeometryRenderBounds = default;
-                _cachedStrokeWidth = default;
+                _path?.Dispose();
+                _path = null;
             }
+
         }
     }
 }

src/Skia/Avalonia.Skia/SkiaSharpExtensions.cs

@@ -196,6 +196,26 @@ namespace Avalonia.Skia
             }
         }
 
+        public static SKStrokeCap ToSKStrokeCap(this PenLineCap cap)
+        {
+            return cap switch
+            {
+                PenLineCap.Round => SKStrokeCap.Round,
+                PenLineCap.Square => SKStrokeCap.Square,
+                _ => SKStrokeCap.Butt
+            };
+        }
+
+        public static SKStrokeJoin ToSKStrokeJoin(this PenLineJoin join)
+        {
+            return join switch
+            {
+                PenLineJoin.Bevel => SKStrokeJoin.Bevel,
+                PenLineJoin.Round => SKStrokeJoin.Round,
+                _ => SKStrokeJoin.Miter
+            };
+        }
+
         public static TextAlignment ToAvalonia(this SKTextAlign a)
         {
             switch (a)

src/Windows/Avalonia.Direct2D1/Media/GeometryImpl.cs

@@ -1,6 +1,10 @@
+using System;
 using Avalonia.Logging;
+using Avalonia.Media;
 using Avalonia.Platform;
 using SharpDX.Direct2D1;
+using Geometry = SharpDX.Direct2D1.Geometry;
+using PathGeometry = SharpDX.Direct2D1.PathGeometry;
 
 namespace Avalonia.Direct2D1.Media
 {
@@ -27,7 +31,20 @@ namespace Avalonia.Direct2D1.Media
         /// <inheritdoc/>
         public Rect GetRenderBounds(Avalonia.Media.IPen pen)
         {
-            return Geometry.GetWidenedBounds((float)(pen?.Thickness ?? 0)).ToAvalonia();
+            if (pen == null || Math.Abs(pen.Thickness) < float.Epsilon)
+                return Geometry.GetBounds().ToAvalonia();
+            var originalBounds = Geometry.GetWidenedBounds((float)pen.Thickness).ToAvalonia();
+            switch (pen.LineCap)
+            {
+                case PenLineCap.Flat:
+                    return originalBounds;
+                case PenLineCap.Round:
+                    return originalBounds.Inflate(pen.Thickness / 2);
+                case PenLineCap.Square:
+                    return originalBounds.Inflate(pen.Thickness);
+                default:
+                    throw new ArgumentOutOfRangeException();
+            }
         }
 
         /// <inheritdoc/>

tests/Avalonia.Skia.UnitTests/RenderBoundsTests.cs

@@ -0,0 +1,43 @@
+using System;
+using Avalonia.Controls.Shapes;
+using Avalonia.Layout;
+using Avalonia.Media;
+using Avalonia.Platform;
+using Avalonia.Rendering;
+using Avalonia.UnitTests;
+using Xunit;
+
+namespace Avalonia.Skia.UnitTests
+{
+    public class RenderBoundsTests
+    {
+        [Theory,
+            InlineData("M10 20 L 20 10 L 30 20", PenLineCap.Round, PenLineJoin.Miter, 2, 10, 
+                8.585786819458008, 8.585786819458008, 22.828428268432617, 12.828428268432617),
+            InlineData("M10 10 L 20 10", PenLineCap.Round, PenLineJoin.Miter,2, 10,
+                9,9,12,2),
+            InlineData("M10 10 L 20 15 L 10 20", PenLineCap.Flat, PenLineJoin.Miter, 2, 20,
+                9.552786827087402, 9.105572700500488, 12.683281898498535, 11.788853645324707)
+        
+        ]
+        public void RenderBoundsAreCorrectlyCalculated(string path, PenLineCap cap, PenLineJoin join, double thickness, double miterLimit, double x, double y, double width, double height)
+        {
+            using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface
+                       .With(renderInterface: new PlatformRenderInterface())))
+            {
+                var geo = PathGeometry.Parse(path);
+                var pen = new Pen(Brushes.Black, thickness, null, cap, join, miterLimit);
+                var bounds = geo.GetRenderBounds(pen);
+                var tolerance = 0.001;
+                if (
+                    Math.Abs(bounds.X - x) > tolerance
+                    || Math.Abs(bounds.Y - y) > tolerance
+                    || Math.Abs(bounds.Width - width) > tolerance
+                    || Math.Abs(bounds.Height - height) > tolerance)
+                    Assert.Fail($"Expected {x}:{y}:{width}:{height}, got {bounds}");
+                
+                Assert.Equal(new Rect(x, y, width, height), bounds);
+            }
+        }
+    }
+}