Skip to content

Commit

Permalink
Merge pull request #19023 from unoplatform/dev/xygu/20241205/shape-perf
Browse files Browse the repository at this point in the history
refactor: shape perf
  • Loading branch information
jeromelaban authored Dec 11, 2024
2 parents 1f34a6b + 338b32d commit 826aac9
Show file tree
Hide file tree
Showing 9 changed files with 300 additions and 13 deletions.
28 changes: 28 additions & 0 deletions src/Uno.UI/FeatureConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -877,5 +877,33 @@ internal static class DebugOptions

public static bool WaitIndefinitelyInEventTester { get; set; }
}

public static class Shape
{
/// <summary>
/// [WebAssembly Only] Gets or sets whether native svg attributes assignments can be postponed until the first arrange pass.
/// </summary>
/// <remarks>This avoid double assignments(with js interop call) from both OnPropertyChanged and UpdateRender.</remarks>
public static bool WasmDelayUpdateUntilFirstArrange { get; set; } = true;

/// <summary>
/// [WebAssembly Only] Gets or sets whether native getBBox() result will be cached.
/// </summary>
public static bool WasmCacheBBoxCalculationResult { get; set; } = true;

internal const int WasmDefaultBBoxCacheSize = 64;
/// <summary>
/// [WebAssembly Only] Gets or sets the size of getBBox cache. The default size is 64.
/// </summary>
#if __WASM__
public static int WasmBBoxCacheSize
{
get => Microsoft.UI.Xaml.Shapes.Shape.BBoxCacheSize;
set => Microsoft.UI.Xaml.Shapes.Shape.BBoxCacheSize = value;
}
#else
public static int WasmBBoxCacheSize { get; set; } = WasmDefaultBBoxCacheSize;
#endif
}
}
}
10 changes: 9 additions & 1 deletion src/Uno.UI/UI/Xaml/Shapes/Ellipse.wasm.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Uno.Extensions;
using System;
using Uno.Extensions;
using Windows.Foundation;
using Microsoft.UI.Xaml.Wasm;

Expand Down Expand Up @@ -28,5 +29,12 @@ protected override Size ArrangeOverride(Size finalSize)

return finalSize;
}

private protected override string GetBBoxCacheKeyImpl() =>
#if DEBUG
throw new InvalidOperationException("Elipse doesnt use GetBBox. Should the impl change in the future, add key-gen and invalidation mechanism.");
#else
null;
#endif
}
}
22 changes: 22 additions & 0 deletions src/Uno.UI/UI/Xaml/Shapes/Line.wasm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,27 @@ protected override Size ArrangeOverride(Size finalSize)
UpdateRender();
return ArrangeAbsoluteShape(finalSize, this);
}

internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs args)
{
base.OnPropertyChanged2(args);

// invalidate cache key if dp of interests changed
if (_bboxCacheKey != null && (
args.Property == X1Property ||
args.Property == Y1Property ||
args.Property == X2Property ||
args.Property == Y2Property
))
{
_bboxCacheKey = null;
}
}

private protected override string GetBBoxCacheKeyImpl() => string.Join(',',
"line",
X1.ToStringInvariant(), Y1.ToStringInvariant(),
X2.ToStringInvariant(), Y2.ToStringInvariant()
);
}
}
10 changes: 9 additions & 1 deletion src/Uno.UI/UI/Xaml/Shapes/Path.wasm.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#nullable enable
using Windows.Foundation;
using Microsoft.UI.Xaml.Wasm;
using Uno.UI.Xaml;
using Microsoft.UI.Xaml.Wasm;
using Microsoft.UI.Xaml.Media;

namespace Microsoft.UI.Xaml.Shapes
{
Expand Down Expand Up @@ -37,9 +38,16 @@ internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs arg
{
_mainSvgElement.AddChild(data.GetSvgElement());
}

_bboxCacheKey = null;
}
}

private protected override string? GetBBoxCacheKeyImpl() =>
Data is GeometryData g
? ("path," + g.Data)
: null;

internal override bool HitTest(Point relativePosition)
{
// ContainsPoint acts on SVGGeometryElement, and "g" HTML element is not SVGGeometryElement.
Expand Down
17 changes: 17 additions & 0 deletions src/Uno.UI/UI/Xaml/Shapes/Polygon.wasm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,22 @@ protected override Size ArrangeOverride(Size finalSize)
UpdateRender();
return ArrangeAbsoluteShape(finalSize, this);
}

internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs args)
{
base.OnPropertyChanged2(args);

if (_bboxCacheKey != null && (
args.Property == PointsProperty
))
{
_bboxCacheKey = null;
}
}

private protected override string GetBBoxCacheKeyImpl() =>
Points is { } points
? ("polygone:" + points.ToCssString())
: null;
}
}
17 changes: 17 additions & 0 deletions src/Uno.UI/UI/Xaml/Shapes/Polyline.wasm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,22 @@ protected override Size ArrangeOverride(Size finalSize)
UpdateRender();
return ArrangeAbsoluteShape(finalSize, this);
}

internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs args)
{
base.OnPropertyChanged2(args);

if (_bboxCacheKey != null && (
args.Property == PointsProperty
))
{
_bboxCacheKey = null;
}
}

private protected override string GetBBoxCacheKeyImpl() =>
Points is { } points
? ("polygone:" + points.ToCssString())
: null;
}
}
8 changes: 8 additions & 0 deletions src/Uno.UI/UI/Xaml/Shapes/Rectangle.wasm.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Uno.Extensions;
using System;
using Uno.UI;
using Windows.Foundation;
using Microsoft.UI.Xaml.Media;
Expand Down Expand Up @@ -27,5 +28,12 @@ protected override Size ArrangeOverride(Size finalSize)

return finalSize;
}

private protected override string GetBBoxCacheKeyImpl() =>
#if DEBUG
throw new InvalidOperationException("Rectangle doesnt use GetBBox. Should the impl change in the future, add key-gen and invalidation mechanism");
#else
null;
#endif
}
}
82 changes: 71 additions & 11 deletions src/Uno.UI/UI/Xaml/Shapes/Shape.wasm.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,44 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Uno;
using Uno.Disposables;
using Uno.Extensions;
using System.Numerics;
using Windows.Foundation;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Media;
using Microsoft.UI.Xaml.Wasm;
using Uno;
using Uno.Collections;
using Uno.Disposables;
using Uno.Extensions;
using Uno.UI;
using Uno.UI.Xaml;

using RadialGradientBrush = Microsoft/* UWP don't rename */.UI.Xaml.Media.RadialGradientBrush;
using System.Numerics;
using System.Diagnostics;
using Uno.UI.Xaml;

namespace Microsoft.UI.Xaml.Shapes
{
partial class Shape
{
private static readonly LruCache<string, Rect> _bboxCache = new(FeatureConfiguration.Shape.WasmDefaultBBoxCacheSize);

internal static int BBoxCacheSize
{
get => _bboxCache.Capacity;
set => _bboxCache.Capacity = value;
}
}

partial class Shape
{
private protected string _bboxCacheKey;

private readonly SerialDisposable _fillBrushSubscription = new SerialDisposable();
private readonly SerialDisposable _strokeBrushSubscription = new SerialDisposable();

private DefsSvgElement _defs;
private protected readonly SvgElement _mainSvgElement;
private protected bool _shouldUpdateNative = !FeatureConfiguration.Shape.WasmDelayUpdateUntilFirstArrange;

protected Shape() : base("svg", isSvg: true)
{
Expand All @@ -37,14 +54,30 @@ private protected Shape(string mainSvgElementTag) : base("svg", isSvg: true)

private protected void UpdateRender()
{
OnFillBrushChanged();
OnStrokeBrushChanged();
UpdateStrokeThickness();
UpdateStrokeDashArray();
// We can delay the setting of these property until first arrange pass,
// so to prevent double-updates from both OnPropertyChanged & ArrangeOverride.
// These updates are a little costly as we need to cross the cs-js bridge.
_shouldUpdateNative = true;

// Regarding to caching of GetPathBoundingBox(js: getBBox) result:
// On shapes that depends on it, so: Line, Path, Polygon, Polyline
// all the properties below (at the time of written) has no effect on getBBox:
// > Note that the values of the opacity, visibility, fill, fill-opacity, fill-rule, stroke-dasharray and stroke-dashoffset properties on an element have no effect on the bounding box of an element.
// > -- https://svgwg.org/svg2-draft/coords.html#BoundingBoxes
// while not mentioned, stroke-width doesnt affect getBBox neither (for the 4 classes of shape mentioned above).

// StrokeThickness can alter getBBox on Ellipse and Rectangle, but we dont use getBBox in these two.

OnFillBrushChanged(); // fill
OnStrokeBrushChanged(); // stroke
UpdateStrokeThickness(); // stroke-width
UpdateStrokeDashArray(); // stroke-dasharray
}

private void OnFillBrushChanged()
{
if (!_shouldUpdateNative) return;

// We don't request an update of the HitTest (UpdateHitTest()) since this element is never expected to be hit testable.
// Note: We also enforce that the default hit test == false is not altered in the OnHitTestVisibilityChanged.

Expand Down Expand Up @@ -111,6 +144,8 @@ private void OnFillBrushChanged()

private void OnStrokeBrushChanged()
{
if (!_shouldUpdateNative) return;

var svgElement = _mainSvgElement;
var stroke = Stroke;

Expand Down Expand Up @@ -159,6 +194,8 @@ private void OnStrokeBrushChanged()

private void UpdateStrokeThickness()
{
if (!_shouldUpdateNative) return;

var svgElement = _mainSvgElement;
var strokeThickness = ActualStrokeThickness;

Expand All @@ -174,6 +211,8 @@ private void UpdateStrokeThickness()

private void UpdateStrokeDashArray()
{
if (!_shouldUpdateNative) return;

var svgElement = _mainSvgElement;

if (StrokeDashArray is not { } strokeDashArray)
Expand Down Expand Up @@ -203,7 +242,22 @@ private UIElementCollection GetDefs()

private static Rect GetPathBoundingBox(Shape shape)
{
return shape._mainSvgElement.GetBBox();
if (FeatureConfiguration.Shape.WasmCacheBBoxCalculationResult)
{
var key = shape.GetBBoxCacheKey();
if (!string.IsNullOrEmpty(key))
{
if (!_bboxCache.TryGetValue(key, out var rect))
{
_bboxCache[key] = rect = shape._mainSvgElement.GetBBox();
}

return rect;
}
}

var result = shape._mainSvgElement.GetBBox();
return result;
}

private protected void Render(Shape shape, Size? size = null, double scaleX = 1d, double scaleY = 1d, double renderOriginX = 0d, double renderOriginY = 0d)
Expand All @@ -225,5 +279,11 @@ internal override bool HitTest(Point relativePosition)
return (considerFill || considerStroke) &&
WindowManagerInterop.ContainsPoint(_mainSvgElement.HtmlId, relativePosition.X, relativePosition.Y, considerFill, considerStroke);
}

// lazy impl, and _cacheKey can be invalidated by setting to null
private string GetBBoxCacheKey() => _bboxCacheKey ?? (_bboxCacheKey = GetBBoxCacheKeyImpl());

// note: perf is of concern here. avoid $"string interpolation" and current-culture .ToString, and use string.concat and ToStringInvariant
private protected abstract string GetBBoxCacheKeyImpl();
}
}
Loading

0 comments on commit 826aac9

Please sign in to comment.