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

perf: reduce SKPaint and SKPath allocations #18476

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public partial class CompositionMaskBrush : CompositionBrush, IOnlineBrush
{
private SKPaint? _sourcePaint;
private SKPaint? _maskPaint;
private SKPaint? _resultPaint;

bool IOnlineBrush.IsOnline
{
Expand All @@ -36,10 +35,11 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds)

void IOnlineBrush.Paint(in Visual.PaintingSession session, SKRect bounds)
{
_resultPaint ??= new SKPaint() { IsAntialias = true };
var resultPaint = SkiaHelper.GetTempSKPaint();
resultPaint.IsAntialias = true;

UpdatePaint(_resultPaint, bounds);
session.Canvas?.DrawRect(bounds, _resultPaint);
UpdatePaint(resultPaint, bounds);
session.Canvas?.DrawRect(bounds, resultPaint);
}

private protected override void DisposeInternal()
Expand All @@ -48,7 +48,6 @@ private protected override void DisposeInternal()

_sourcePaint?.Dispose();
_maskPaint?.Dispose();
_resultPaint?.Dispose();
}
}
}
74 changes: 29 additions & 45 deletions src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ namespace Microsoft.UI.Composition
{
public partial class CompositionSpriteShape : CompositionShape
{
private SKPaint? _strokePaint;
private SKPaint? _fillPaint;
private SKPath? _geometryWithTransformations;

internal override void Paint(in Visual.PaintingSession session)
Expand All @@ -19,7 +17,7 @@ internal override void Paint(in Visual.PaintingSession session)
{
if (FillBrush is { } fill)
{
var fillPaint = TryCreateAndClearFillPaint(session.Filters.OpacityColorFilter);
var fillPaint = GetTempFillPaint(session.Filters.OpacityColorFilter);

if (Compositor.TryGetEffectiveBackgroundColor(this, out var colorFromTransition))
{
Expand Down Expand Up @@ -52,8 +50,8 @@ internal override void Paint(in Visual.PaintingSession session)

if (StrokeBrush is { } stroke && StrokeThickness > 0)
{
var fillPaint = TryCreateAndClearFillPaint(session.Filters.OpacityColorFilter);
var strokePaint = TryCreateAndClearStrokePaint(session.Filters.OpacityColorFilter);
var fillPaint = GetTempFillPaint(session.Filters.OpacityColorFilter);
var strokePaint = GetTempStrokePaint(session.Filters.OpacityColorFilter);

// Set stroke thickness
strokePaint.StrokeWidth = StrokeThickness;
Expand All @@ -78,57 +76,41 @@ internal override void Paint(in Visual.PaintingSession session)
// On Windows, the stroke is simply 1px, it doesn't scale with the height.
// So, to get a correct stroke geometry, we must apply the transformations first.

var strokeFillPath = SkiaHelper.GetTempSKPath();
// Get the stroke geometry, after scaling has been applied.
var strokeGeometry = strokePaint.GetFillPath(geometryWithTransformations);
strokePaint.GetFillPath(geometryWithTransformations, strokeFillPath);

stroke.UpdatePaint(fillPaint, strokeGeometry.Bounds);
stroke.UpdatePaint(fillPaint, strokeFillPath.Bounds);

session.Canvas.DrawPath(strokeGeometry, fillPaint);
session.Canvas.DrawPath(strokeFillPath, fillPaint);
}
}
}

private SKPaint TryCreateAndClearStrokePaint(SKColorFilter? colorFilter)
=> TryCreateAndClearPaint(ref _strokePaint, true, colorFilter, CompositionConfiguration.UseBrushAntialiasing);
private SKPaint GetTempStrokePaint(SKColorFilter? colorFilter)
=> GetTempPaint(true, colorFilter, CompositionConfiguration.UseBrushAntialiasing);

private SKPaint TryCreateAndClearFillPaint(SKColorFilter? colorFilter)
=> TryCreateAndClearPaint(ref _fillPaint, false, colorFilter, CompositionConfiguration.UseBrushAntialiasing);
private SKPaint GetTempFillPaint(SKColorFilter? colorFilter)
=> GetTempPaint(false, colorFilter, CompositionConfiguration.UseBrushAntialiasing);

private static SKPaint TryCreateAndClearPaint(ref SKPaint? paint, bool isStroke, SKColorFilter? colorFilter, bool isHighQuality = false)
private static SKPaint GetTempPaint(bool isStroke, SKColorFilter? colorFilter, bool isHighQuality = false)
{
if (paint == null)
{
// Initialization
paint = new SKPaint();
paint.IsStroke = isStroke;
paint.IsAntialias = true;
paint.IsAutohinted = true;
var paint = isStroke ? SkiaHelper.GetTempSKPaint() : SkiaHelper.GetAnotherTempSKPaint();
paint.IsAntialias = true;
paint.ColorFilter = colorFilter;

if (isHighQuality)
{
paint.FilterQuality = SKFilterQuality.High;
}
}
else
{
// Cleanup
// - Brushes can change, we cant leave color and shader garbage
// from last rendering around for the next pass.
paint.Color = SKColors.White; // Transparent color wouldn't draw anything
if (paint.Shader is { } shader)
{
shader.Dispose();
paint.Shader = null;
}
paint.IsStroke = isStroke;

if (paint.PathEffect is { } pathEffect)
{
pathEffect.Dispose();
paint.PathEffect = null;
}
}
// uno-specific defaults
paint.Color = SKColors.White; // Transparent color wouldn't draw anything
paint.IsAutohinted = true;
// paint.IsAntialias = true; // IMPORTANT: don't set this to true by default. It breaks canvas clipping on Linux for some reason.
ramezgerges marked this conversation as resolved.
Show resolved Hide resolved

paint.ColorFilter = colorFilter;
if (isHighQuality)
{
paint.FilterQuality = SKFilterQuality.High;
}

return paint;
}
Expand Down Expand Up @@ -177,10 +159,12 @@ internal override bool HitTest(Point point)

if (StrokeBrush is { } stroke && StrokeThickness > 0)
{
var strokePaint = TryCreateAndClearStrokePaint(null);
var strokePaint = GetTempStrokePaint(null);
strokePaint.StrokeWidth = StrokeThickness;
var strokeGeometry = strokePaint.GetFillPath(geometryWithTransformations);
if (strokeGeometry.Contains((float)point.X, (float)point.Y))

var hitTestStrokeFillPath = SkiaHelper.GetTempSKPath();
strokePaint.GetFillPath(geometryWithTransformations, hitTestStrokeFillPath);
if (hitTestStrokeFillPath.Contains((float)point.X, (float)point.Y))
{
return true;
}
Expand Down
23 changes: 14 additions & 9 deletions src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#nullable enable

using System;
using System.Numerics;
using Windows.Foundation;
using SkiaSharp;
Expand All @@ -9,12 +10,16 @@ namespace Microsoft.UI.Composition;

public partial class ShapeVisual
{
[ThreadStatic] // safety
private static SKPath? _prePaintingClipPath;

private protected override void ApplyPrePaintingClipping(in SKCanvas canvas)
{
base.ApplyPrePaintingClipping(in canvas);
if (GetViewBoxPathInElementCoordinateSpace() is { } path)
_prePaintingClipPath ??= new SKPath();
if (GetViewBoxPathInElementCoordinateSpace(_prePaintingClipPath))
{
canvas.ClipPath(path, antialias: true);
canvas.ClipPath(_prePaintingClipPath, antialias: true);
}
}

Expand All @@ -32,28 +37,28 @@ internal override void Paint(in PaintingSession session)
base.Paint(in session);
}

internal SKPath? GetViewBoxPathInElementCoordinateSpace()
/// <returns>The same <see cref="dst"/> object.</returns>
internal bool GetViewBoxPathInElementCoordinateSpace(SKPath dst)
{
if (ViewBox is not { } viewBox)
{
return null;
return false;
}

var shape = new SKPath();
dst.Rewind();
var clipRect = new SKRect(viewBox.Offset.X, viewBox.Offset.Y, viewBox.Offset.X + viewBox.Size.X, viewBox.Offset.Y + viewBox.Size.Y);
shape.AddRect(clipRect);
dst.AddRect(clipRect);
if (viewBox.IsAncestorClip)
{
Matrix4x4.Invert(TotalMatrix, out var totalMatrixInverted);
var childToParentTransform = Parent!.TotalMatrix * totalMatrixInverted;
if (!childToParentTransform.IsIdentity)
{

shape.Transform(childToParentTransform.ToSKMatrix());
dst.Transform(childToParentTransform.ToSKMatrix());
}
}

return shape;
return true;
}

/// <remarks>This does NOT take the clipping into account.</remarks>
Expand Down
43 changes: 43 additions & 0 deletions src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#nullable enable

using SkiaSharp;
using System;
using System.Collections.Generic;
using System.Numerics;
using System.Text;
using Windows.Foundation;
using Windows.UI;

namespace Microsoft.UI.Composition
{
public static class SkiaHelper
{
[ThreadStatic] private static readonly SKPath _tempPath = new SKPath();

Check warning on line 15 in src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs#L15

Remove this initialization of '_tempPath' or make it lazy.
[ThreadStatic] private static readonly SKPaint _tempPaint = new SKPaint();

Check warning on line 16 in src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs#L16

Remove this initialization of '_tempPaint' or make it lazy.
[ThreadStatic] private static readonly SKPaint _tempPaint2 = new SKPaint();

Check warning on line 17 in src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.Composition/Composition/SkiaHelper.skia.cs#L17

Remove this initialization of '_tempPaint2' or make it lazy.

public static SKPath GetTempSKPath()
{
// Note the difference between Rewind and Reset
// https://api.skia.org/classSkPath.html#a8dc858ee4c95a59b3dd4bdd3f7b85fdc : "Use rewind() instead of reset() if SkPath storage will be reused and performance is critical."
_tempPath.Rewind();
return _tempPath;
}

public static SKPaint GetTempSKPaint()
{
// https://api.skia.org/classSkPaint.html#a6c7118c97a0e8819d75aa757afbc4c49
// "This is equivalent to replacing SkPaint with the result of SkPaint()."
_tempPaint.Reset();
return _tempPaint;
}

public static SKPaint GetAnotherTempSKPaint()
ramezgerges marked this conversation as resolved.
Show resolved Hide resolved
{
// https://api.skia.org/classSkPaint.html#a6c7118c97a0e8819d75aa757afbc4c49
// "This is equivalent to replacing SkPaint with the result of SkPaint()."
_tempPaint2.Reset();
return _tempPaint2;
}
}
}
34 changes: 31 additions & 3 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/Given_Path.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
using System;
using Windows.Foundation;
using System.Threading.Tasks;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Markup;
using Microsoft.UI.Xaml.Media;
using Microsoft.UI.Xaml.Shapes;
using MUXControlsTestApp.Utilities;
using SamplesApp.UITests;
using Uno.UI.RuntimeTests.Helpers;
using Size = Windows.Foundation.Size;

namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Shapes
{
[TestClass]
[RunsOnUIThread]
public class Given_Path
{
[TestMethod]
[RunsOnUIThread]
[UnoWorkItem("https://github.com/unoplatform/uno/issues/6846")]
public void Should_not_throw_if_Path_Data_is_set_to_null()
{
Expand All @@ -23,7 +28,6 @@ public void Should_not_throw_if_Path_Data_is_set_to_null()
}

[TestMethod]
[RunsOnUIThread]
public void Should_Not_Include_Control_Points_Bounds()
{
#if WINAPPSDK
Expand All @@ -41,5 +45,29 @@ public void Should_Not_Include_Control_Points_Bounds()
Assert.IsTrue(Math.Abs(50 - SUT.DesiredSize.Height) <= 1, $"Actual size: {SUT.DesiredSize}");
#endif
}

// This tests a workaround for a Linux-specific skia bug.
// This doesn't actually repro for some reason, so it's here for documentation purposes.
ramezgerges marked this conversation as resolved.
Show resolved Hide resolved
[TestMethod]
[UnoWorkItem("https://github.com/unoplatform/uno/issues/18473")]
public async Task When_Path_Clipped_Inside_ScrollViewer()
{
var root = (FrameworkElement)XamlReader.Load(
"""
<StackPanel xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
<Border Height="100" HorizontalAlignment="Stretch" Background="Blue" />
<ScrollViewer Height="30">
<Path Fill="Red" Data="m 0,0 c -0.639,0 -1.276,0.243 -1.765,0.729 -0.977,0.974 -0.98,2.557 -0.006,3.535 L 16.925,23.032 -1.768,41.725 c -0.976,0.976 -0.976,2.559 0,3.535 0.977,0.976 2.559,0.976 3.536,0 L 22.225,24.803 c 0.975,-0.975 0.976,-2.555 0.004,-3.532 L 1.771,0.736 C 1.283,0.245 0.642,0 0,0" />
</ScrollViewer>
</StackPanel>
""");
await UITestHelper.Load(root);

root.FindVisualChildByType<ScrollViewer>().ScrollToVerticalOffset(9999);
await UITestHelper.WaitForIdle();

var screenshot = await UITestHelper.ScreenShot(root);
ImageAssert.DoesNotHaveColorInRectangle(screenshot, new System.Drawing.Rectangle(0, 0, screenshot.Width, screenshot.Height * 60 / 100), Microsoft.UI.Colors.Red);
}
}
}
11 changes: 6 additions & 5 deletions src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Uno.UI.Xaml.Core;
using Uno.UI.Xaml.Media;
using Uno.UI.Xaml.Core.Scaling;
using SkiaExtensions = Microsoft.UI.Composition.SkiaExtensions;

#nullable enable

Expand Down Expand Up @@ -188,11 +189,11 @@ partial void SetupInlines()
{
var canvas = t.canvas;
var rect = t.rect;
canvas.DrawRect(new SKRect((float)rect.Left, (float)rect.Top, (float)rect.Right, (float)rect.Bottom), new SKPaint
{
Color = SelectionHighlightColor.Color.ToSKColor(),
Style = SKPaintStyle.Fill
});

var paint = SkiaHelper.GetTempSKPaint();
paint.Color = SelectionHighlightColor.Color.ToSKColor();
paint.Style = SKPaintStyle.Fill;
canvas.DrawRect(new SKRect((float)rect.Left, (float)rect.Top, (float)rect.Right, (float)rect.Bottom), paint);
};

_inlines.RenderSelection = IsTextSelectionEnabled;
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ private void UpdateTextBoxView()
var caretRect = args.rect;
var compositor = _visual.Compositor;
var brush = DefaultBrushes.TextForegroundBrush.GetOrCreateCompositionBrush(compositor);
var caretPaint = new SKPaint(); // we create a new caret everytime to dodge the complications that occur when trying to "reset" an SKPaint.
var caretPaint = SkiaHelper.GetTempSKPaint();
brush.UpdatePaint(caretPaint, caretRect.ToSKRect());
args.canvas.DrawRect(
new SKRect((float)caretRect.Left, (float)caretRect.Top, (float)caretRect.Right,
Expand Down
16 changes: 0 additions & 16 deletions src/Uno.UI/UI/Xaml/Documents/Inline.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,6 @@ namespace Microsoft.UI.Xaml.Documents
partial class Inline
{
private FontDetails? _fontInfo;
private SKPaint? _paint;

internal SKPaint Paint
{
get
{
var paint = _paint ??= new SKPaint()
{
TextEncoding = SKTextEncoding.Utf16,
IsStroke = false,
IsAntialias = true,
};

return paint;
}
}

internal FontDetails FontInfo
{
Expand Down
Loading
Loading