Skip to content

Commit

Permalink
chore: dont invalidate BorderVisual paths when the brush changes
Browse files Browse the repository at this point in the history
  • Loading branch information
ramezgerges committed Oct 17, 2024
1 parent c8ae3e3 commit 449a174
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 19 deletions.
49 changes: 31 additions & 18 deletions src/Uno.UI.Composition/Composition/BorderVisual.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ internal class BorderVisual(Compositor compositor) : ShapeVisual(compositor)
private RectangleClip? _childClipCausedByCornerRadius;

// We do this instead of a direct SetProperty call so that SetProperty automatically gets an accurate propertyName
// we need the SetProperty calls to get notified on brush updates.
// (<Border|Background>Brush internals change -> <Border|Background>Shape is notified through FillBrush -> render invalidation)
private CompositionSpriteShape? BackgroundShape { set => SetProperty(ref _backgroundShape, value); }
private CompositionSpriteShape? BorderShape { set => SetProperty(ref _borderShape, value); }

Expand All @@ -55,13 +57,29 @@ public bool UseInnerBorderBoundsAsAreaForBackground
public CompositionBrush? BackgroundBrush
{
private get => _backgroundBrush;
set => SetProperty(ref _backgroundBrush, value);
set
{
if (_backgroundBrush is null)
{
// we skip the background path calculations while the background is null;
_backgroundPathValid = false;
}
SetProperty(ref _backgroundBrush, value);
}
}

public CompositionBrush? BorderBrush
{
private get => _borderBrush;
set => SetProperty(ref _borderBrush, value);
set
{
if (_borderBrush is null)
{
// we skip the border path calculations while the border is null;
_borderPathValid = false;
}
SetProperty(ref _borderBrush, value);
}
}

private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange)
Expand All @@ -78,11 +96,8 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool
// BackgroundShape and BorderShape are NOT added to this.Shapes, which both makes it easier
// to reason about (no external tampering) and is also closer to what WinUI does.
case nameof(BorderBrush):
_borderPathValid = false; // to update _borderPath if previously skipped
if (BorderBrush is not null && _borderShape is null)
{
// we need this to get notified on brush updates
// (BorderBrush internals change -> BorderShape is notified through FillBrush -> render invalidation)
var borderShape = Compositor.CreateSpriteShape();
borderShape.Geometry = Compositor.CreatePathGeometry();
#if DEBUG
Expand All @@ -97,11 +112,8 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool
}
break;
case nameof(BackgroundBrush):
_backgroundPathValid = false; // to update _backgroundPath if previously skipped
if (BackgroundBrush is not null && _backgroundShape is null)
{
// we need this to get notified on brush updates.
// (BackgroundBrush internals change -> BackgroundShape is notified through FillBrush -> render invalidation)
var backgroundShape = Compositor.CreateSpriteShape();

backgroundShape.Geometry = Compositor.CreatePathGeometry();
Expand Down Expand Up @@ -170,7 +182,7 @@ private void UpdatePathsAndCornerClip()
fullCornerRadius.Outer.GetRadii(outerRadii);
fullCornerRadius.Inner.GetRadii(innerRadii);

if (_backgroundBrush is { } && !_backgroundPathValid)
if (_backgroundBrush is { } && !_backgroundPathValid) // no point calculating background state if the brush is null
{
_backgroundPathValid = true;
// We don't pass down <inner|outer>Area directly, since it contains the thickness offsets.
Expand Down Expand Up @@ -214,13 +226,16 @@ private void UpdatePathsAndCornerClip()
// | |
// | |
// |-----------------300px---------------------|
UpdateBackgroundPath(_useInnerBorderBoundsAsAreaForBackground, innerArea.Size, outerArea.Size, outerRadii, innerRadii);

var backgroundPath = CreateBackgroundPath(_useInnerBorderBoundsAsAreaForBackground, innerArea.Size, outerArea.Size, outerRadii, innerRadii);
((CompositionPathGeometry)_backgroundShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(backgroundPath));
_backgroundShape!.Offset = _useInnerBorderBoundsAsAreaForBackground ? new Vector2((float)_borderThickness.Left, (float)_borderThickness.Top) : Vector2.Zero;
}
if (_borderBrush is { } && !_borderPathValid)
if (_borderBrush is { } && !_borderPathValid) // no point calculating border state if the brush is null
{
_borderPathValid = true;
UpdateBorderPath(innerArea, outerArea, outerRadii, innerRadii);
var borderPath = CreateBorderPath(innerArea, outerArea, outerRadii, innerRadii);
((CompositionPathGeometry)_borderShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(borderPath));
}
}

Expand Down Expand Up @@ -251,7 +266,7 @@ private void UpdatePathsAndCornerClip()
}
}

private unsafe void UpdateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackground, SKSize innerArea, SKSize outerArea, SKPoint* outerRadii, SKPoint* innerRadii)
private static unsafe SKPath CreateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackground, SKSize innerArea, SKSize outerArea, SKPoint* outerRadii, SKPoint* innerRadii)
{
var backgroundPath = new SKPath();
var roundRect = new SKRoundRect();
Expand All @@ -265,11 +280,10 @@ private unsafe void UpdateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackg
backgroundPath.AddRoundRect(roundRect);
backgroundPath.Close();

// Unfortunately, this will cause an unnecessary render invalidation
((CompositionPathGeometry)_backgroundShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(backgroundPath));
return backgroundPath;
}

private unsafe void UpdateBorderPath(SKRect innerArea, SKRect outerArea, SKPoint* outerRadii, SKPoint* innerRadii)
private static unsafe SKPath CreateBorderPath(SKRect innerArea, SKRect outerArea, SKPoint* outerRadii, SKPoint* innerRadii)
{
var borderPath = new SKPath();

Expand All @@ -289,8 +303,7 @@ private unsafe void UpdateBorderPath(SKRect innerArea, SKRect outerArea, SKPoint
borderPath.Close();
}

// Unfortunately, this will cause an unnecessary render invalidation
((CompositionPathGeometry)_borderShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(borderPath));
return borderPath;
}

internal override bool HitTest(Point point)
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI.Composition/Composition/Uno/UnoSkiaApi.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal static class UnoSkiaApi
unsafe internal static extern void sk_textblob_builder_alloc_run_pos(IntPtr builder, IntPtr font, int count, SKRect* bounds, UnoSKRunBufferInternal* runbuffer);

/// <summary>
/// We use this instead of the equivalent SKRoundRect.SetRectRadii because it take the array with a
/// We use this instead of the equivalent SKRoundRect.SetRectRadii because it takes an array with a
/// length of _exactly_ 4. If we rent the SKPoint array to reduce allocations, we're not guaranteed to
/// get the exact length we need.
/// </summary>
Expand Down

0 comments on commit 449a174

Please sign in to comment.