Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: shape perf #19023

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading