From f585dddc15e4f8d25884e80a5fa0316139daa6d4 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Sun, 20 Aug 2023 21:06:03 +0000 Subject: [PATCH] Merge pull request #12606 from AvaloniaUI/fixes/correct-render-bounds When calculating geometry bounds take into account parameters that affect geometry bounds --- .../Drawing/Nodes/RenderDataGeometryNode.cs | 2 +- .../SceneGraph/GeometryBoundsHelper.cs | 31 ---- src/Skia/Avalonia.Skia/DrawingContextImpl.cs | 27 +--- src/Skia/Avalonia.Skia/GeometryImpl.cs | 151 +++++++----------- src/Skia/Avalonia.Skia/SkiaSharpExtensions.cs | 20 +++ .../Avalonia.Direct2D1/Media/GeometryImpl.cs | 19 ++- .../RenderBoundsTests.cs | 43 +++++ 7 files changed, 141 insertions(+), 152 deletions(-) delete mode 100644 src/Avalonia.Base/Rendering/SceneGraph/GeometryBoundsHelper.cs create mode 100644 tests/Avalonia.Skia.UnitTests/RenderBoundsTests.cs diff --git a/src/Avalonia.Base/Rendering/Composition/Drawing/Nodes/RenderDataGeometryNode.cs b/src/Avalonia.Base/Rendering/Composition/Drawing/Nodes/RenderDataGeometryNode.cs index 67231c9012..5a40a6b279 100644 --- a/src/Avalonia.Base/Rendering/Composition/Drawing/Nodes/RenderDataGeometryNode.cs +++ b/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; } \ No newline at end of file diff --git a/src/Avalonia.Base/Rendering/SceneGraph/GeometryBoundsHelper.cs b/src/Avalonia.Base/Rendering/SceneGraph/GeometryBoundsHelper.cs deleted file mode 100644 index b1129e81c4..0000000000 --- a/src/Avalonia.Base/Rendering/SceneGraph/GeometryBoundsHelper.cs +++ /dev/null @@ -1,31 +0,0 @@ -using System; -using Avalonia.Media; -using Avalonia.Utilities; - -namespace Avalonia.Rendering.SceneGraph; - -internal static class GeometryBoundsHelper -{ - /// - /// Calculates the bounds of a given geometry with respect to the pens - /// - /// The calculated bounds without s - /// The pen with information about the s - /// - 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(); - } - } -} diff --git a/src/Skia/Avalonia.Skia/DrawingContextImpl.cs b/src/Skia/Avalonia.Skia/DrawingContextImpl.cs index b1b1de0ffe..76d236e18a 100644 --- a/src/Skia/Avalonia.Skia/DrawingContextImpl.cs +++ b/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; diff --git a/src/Skia/Avalonia.Skia/GeometryImpl.cs b/src/Skia/Avalonia.Skia/GeometryImpl.cs index aee84d1346..c1ce4a661f 100644 --- a/src/Skia/Avalonia.Skia/GeometryImpl.cs +++ b/src/Skia/Avalonia.Skia/GeometryImpl.cs @@ -43,46 +43,11 @@ namespace Avalonia.Skia /// 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); - } - - /// - /// Update path cache for given stroke width. - /// - /// Stroke width. - 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); } - + /// /// Check Skia path if it contains a point. /// @@ -106,14 +71,8 @@ namespace Avalonia.Skia /// 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; } /// @@ -180,66 +139,70 @@ namespace Avalonia.Skia /// protected void InvalidateCaches() { - _pathCache.Invalidate(); + _pathCache.Dispose(); + _pathCache = default; } private struct PathCache { - private float _cachedStrokeWidth; - - /// - /// Tolerance for two stroke widths to be deemed equal - /// - public const float Tolerance = float.Epsilon; - - /// - /// Cached contour path. - /// - public SKPath? CachedStrokePath { get; private set; } - - /// - /// Cached geometry render bounds. - /// - public Rect CachedGeometryRenderBounds { get; private set; } - - /// - /// Is cached valid for given stroke width. - /// - /// Stroke width to check. - /// True, if CachedStrokePath can be used for given stroke width. - 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; - } - - /// - /// Cache path for given stroke width. Takes ownership of a passed path. - /// - /// Path to cache. - /// Stroke width to cache. - /// Render bounds to use. - 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); } - /// - /// Invalidate cache state. - /// - public void Invalidate() + public void Dispose() { - CachedStrokePath?.Dispose(); - CachedStrokePath = null; - CachedGeometryRenderBounds = default; - _cachedStrokeWidth = default; + _path?.Dispose(); + _path = null; } + } } } diff --git a/src/Skia/Avalonia.Skia/SkiaSharpExtensions.cs b/src/Skia/Avalonia.Skia/SkiaSharpExtensions.cs index 455e415ce1..44fe7aed89 100644 --- a/src/Skia/Avalonia.Skia/SkiaSharpExtensions.cs +++ b/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) diff --git a/src/Windows/Avalonia.Direct2D1/Media/GeometryImpl.cs b/src/Windows/Avalonia.Direct2D1/Media/GeometryImpl.cs index 9a93d1afd3..e1c08e0814 100644 --- a/src/Windows/Avalonia.Direct2D1/Media/GeometryImpl.cs +++ b/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 /// 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(); + } } /// diff --git a/tests/Avalonia.Skia.UnitTests/RenderBoundsTests.cs b/tests/Avalonia.Skia.UnitTests/RenderBoundsTests.cs new file mode 100644 index 0000000000..a659c2b8b2 --- /dev/null +++ b/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); + } + } + } +}