From 6d4ee55b29b95e01db5cae997dbec109733a2ccc Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 3 May 2024 10:33:47 +0300 Subject: [PATCH 01/38] chore: undo changes from #16502 --- .../Composition/ShapeVisual.skia.cs | 74 +++++-------------- src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 3 +- 2 files changed, 20 insertions(+), 57 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index dff31d523e85..7ee6a804f7a1 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -1,10 +1,8 @@ #nullable enable -using System.Numerics; using SkiaSharp; using Uno.Extensions; using Uno.UI.Composition; -using Windows.Foundation; namespace Microsoft.UI.Composition; @@ -20,14 +18,10 @@ internal override void Render(in DrawingSession parentSession) // First we render the shapes (a.k.a. the "local content") // For UIElement, those are background and border or shape's content // WARNING: As we are overriding the "Render" method, at this point we are still in the parent's coordinate system - - // This shouldn't be inside the if condition. - // When inside the if, the session.Dispose will be called before base.Render, - // which can cause clipping to be removed before rendering children. - using var session = BeginShapesDrawing(in parentSession); - if (_shapes is { Count: not 0 } shapes) { + using var session = BeginShapesDrawing(in parentSession); + for (var i = 0; i < shapes.Count; i++) { // TODO: If the shape will end up being not in Window bounds, @@ -47,19 +41,29 @@ internal override void Render(in DrawingSession parentSession) base.Render(in parentSession); } + /// + internal override void Draw(in DrawingSession session) + { + if (ViewBox is { } viewBox) + { + session.Canvas.ClipRect(viewBox.GetSKRect(), antialias: true); + } + + base.Draw(in session); + } + private DrawingSession BeginShapesDrawing(in DrawingSession parentSession) { parentSession.Canvas.Save(); + var totalOffset = this.GetTotalOffset(); + // Set the position of the visual on the canvas (i.e. change coordinates system to the "XAML element" one) + parentSession.Canvas.Translate(totalOffset.X + AnchorPoint.X, totalOffset.Y + AnchorPoint.Y); + var transform = this.GetTransform().ToSKMatrix(); if (ViewBox is { } viewBox) { - if (!viewBox.IsAncestorClip) - { - ApplyTranslation(parentSession.Canvas); - } - // We apply the transformed viewbox clipping if (transform.IsIdentity) { @@ -68,25 +72,10 @@ private DrawingSession BeginShapesDrawing(in DrawingSession parentSession) else { var shape = new SKPath(); - 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); - if (!viewBox.IsAncestorClip) - { - shape.Transform(transform); - } - + shape.AddRect(new SKRect(viewBox.Offset.X, viewBox.Offset.Y, viewBox.Offset.X + viewBox.Size.X, viewBox.Offset.Y + viewBox.Size.Y)); + shape.Transform(transform); parentSession.Canvas.ClipPath(shape, antialias: true); } - - if (viewBox.IsAncestorClip) - { - ApplyTranslation(parentSession.Canvas); - } - } - else - { - ApplyTranslation(parentSession.Canvas); } if (!transform.IsIdentity) @@ -104,29 +93,4 @@ private DrawingSession BeginShapesDrawing(in DrawingSession parentSession) return session; } - - private void ApplyTranslation(SKCanvas canvas) - { - var totalOffset = this.GetTotalOffset(); - // Set the position of the visual on the canvas (i.e. change coordinates system to the "XAML element" one) - canvas.Translate(totalOffset.X + AnchorPoint.X, totalOffset.Y + AnchorPoint.Y); - } - - internal Rect? GetViewBoxRectInElementCoordinateSpace() - { - if (ViewBox is null) - { - return null; - } - - var rect = ViewBox.GetRect(); - if (ViewBox.IsAncestorClip) - { - var totalOffset = this.GetTotalOffset(); - rect.X -= totalOffset.X; - rect.Y -= totalOffset.Y; - } - - return rect; - } } diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index 58112d2bfd46..e69798a556b7 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -492,10 +492,9 @@ private static (UIElement? element, Branch? stale) SearchDownForTopMostElementAt // The maximum region where the current element and its children might draw themselves // This is expressed in the window (absolute) coordinate space. - var clippingBounds = element.Visual.GetViewBoxRectInElementCoordinateSpace() is { } rect + var clippingBounds = element.Visual.ViewBox?.GetRect() is { } rect ? transformToElement.Transform(rect) : Rect.Infinite; - if (element.Visual.Clip?.GetBounds(element.Visual) is { } clip) { clippingBounds = clippingBounds.IntersectWith(transformToElement.Transform(clip)) ?? default; From 001ba85b2fa1cbc1c1238c4694175c14c1bcd2f6 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 15 Apr 2024 19:24:59 +0200 Subject: [PATCH 02/38] chore: simplify ShapeVisual and make Visual.Render not virtual --- .../Composition/ShapeVisual.skia.cs | 80 ++----------------- .../Composition/Visual.skia.cs | 5 +- 2 files changed, 9 insertions(+), 76 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 7ee6a804f7a1..9d05d65bbcb9 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -1,96 +1,32 @@ #nullable enable -using SkiaSharp; -using Uno.Extensions; using Uno.UI.Composition; namespace Microsoft.UI.Composition; public partial class ShapeVisual { - internal override void Render(in DrawingSession parentSession) - { - if (this is { Opacity: 0 } or { IsVisible: false }) - { - return; - } - - // First we render the shapes (a.k.a. the "local content") - // For UIElement, those are background and border or shape's content - // WARNING: As we are overriding the "Render" method, at this point we are still in the parent's coordinate system - if (_shapes is { Count: not 0 } shapes) - { - using var session = BeginShapesDrawing(in parentSession); - - for (var i = 0; i < shapes.Count; i++) - { - // TODO: If the shape will end up being not in Window bounds, - // we should skip rendering it for performance reasons. - // Maybe get canvas total matrix and multiply that by shape transform - // Then, use that to transform Rect(0, 0, Size.X, Size.Y) - // and check if that intersects with Rect(0, 0, RootVisualSize.X, RootVisualSize.Y) - // This should also done in other parts, e.g, text rendering. - // NOTE: We probably shouldn't skip a whole Visual as it could have a child that - // is translated and gets back into the Window bounds. - // Maybe we could only skip the visual if we know it's clipped, but that won't always the case. - shapes[i].Render(in session); - } - } - - // Second we render the children - base.Render(in parentSession); - } - /// internal override void Draw(in DrawingSession session) { + // Note that Visual.Clip is already applied in Visual.Render -> Visual.BeginDrawing + if (ViewBox is { } viewBox) { session.Canvas.ClipRect(viewBox.GetSKRect(), antialias: true); } - base.Draw(in session); - } - - private DrawingSession BeginShapesDrawing(in DrawingSession parentSession) - { - parentSession.Canvas.Save(); - - var totalOffset = this.GetTotalOffset(); - // Set the position of the visual on the canvas (i.e. change coordinates system to the "XAML element" one) - parentSession.Canvas.Translate(totalOffset.X + AnchorPoint.X, totalOffset.Y + AnchorPoint.Y); - - var transform = this.GetTransform().ToSKMatrix(); - - if (ViewBox is { } viewBox) + if (_shapes is { Count: not 0 } shapes) { - // We apply the transformed viewbox clipping - if (transform.IsIdentity) + foreach (var t in shapes) { - parentSession.Canvas.ClipRect(viewBox.GetSKRect(), antialias: true); + t.Render(in session); } - else - { - var shape = new SKPath(); - shape.AddRect(new SKRect(viewBox.Offset.X, viewBox.Offset.Y, viewBox.Offset.X + viewBox.Size.X, viewBox.Offset.Y + viewBox.Size.Y)); - shape.Transform(transform); - parentSession.Canvas.ClipPath(shape, antialias: true); - } - } - - if (!transform.IsIdentity) - { - // Applied rending transformation matrix (i.e. change coordinates system to the "rendering" one) - parentSession.Canvas.Concat(ref transform); } - // Note: Here only the `Clip` is relevant for the shapes, `CornerRadiusClip` applies only for children (i.e. UIElement's content) - Clip?.Apply(parentSession.Canvas, this); + // The CornerRadiusClip doesn't affect the shapes of the ShapeVisual, only its children + CornerRadiusClip?.Apply(session.Canvas, this); - var session = parentSession; // Creates a new session (clone the struct) - - DrawingSession.PushOpacity(ref session, Opacity); - - return session; + base.Draw(in session); } } diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 6f5eb36bed2e..3ad54f102e4e 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -163,7 +163,7 @@ internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) /// Position a sub visual on the canvas and draw its content. /// /// The drawing session of the visual. - internal virtual void Render(in DrawingSession parentSession) + internal void Render(in DrawingSession parentSession) { #if TRACE_COMPOSITION var indent = int.TryParse(Comment?.Split(new char[] { '-' }, 2, StringSplitOptions.TrimEntries).FirstOrDefault(), out var depth) @@ -217,9 +217,6 @@ private DrawingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in Drawi // Note: The Clip is applied after the transformation matrix, so it's also transformed. Clip?.Apply(canvas, this); - // CornerRadiusClip applies to the children only. - CornerRadiusClip?.Apply(canvas, this); - var session = new DrawingSession(surface, canvas, in filters, in initialTransform); DrawingSession.PushOpacity(ref session, Opacity); From 0f6877643c831c525559b8b0ac4c3b68eea18bba Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 15 Apr 2024 19:36:24 +0200 Subject: [PATCH 03/38] chore: combine CompositionObject.Context and Compositionobject into one class --- .../Composition/CompositionObject.Context.cs | 148 +++++++++--------- .../Composition/CompositionObject.cs | 13 +- 2 files changed, 71 insertions(+), 90 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs b/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs index 4dea09744da8..ff7d85b76d55 100644 --- a/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs +++ b/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs @@ -7,118 +7,110 @@ namespace Microsoft.UI.Composition { public partial class CompositionObject { - private class ContextStore - { - private readonly object _lock = new object(); - private List? _contextEntries; + private readonly object _lock = new object(); + private List? _contextEntries; - public ContextStore() + public void AddContext(CompositionObject context, string? propertyName) + { + lock (_lock) { - + AddContextImpl(context, propertyName); } + } - public void AddContext(CompositionObject context, string? propertyName) + public void RemoveContext(CompositionObject context, string? propertyName) + { + lock (_lock) { - lock (_lock) - { - AddContextImpl(context, propertyName); - } + RemoveContextImpl(context, propertyName); } + } - public void RemoveContext(CompositionObject context, string? propertyName) + public void PropagateChanged() + { + lock (_lock) { - lock (_lock) - { - RemoveContextImpl(context, propertyName); - } + PropagateChangedImpl(); } + } - public void RaiseChanged() + private void AddContextImpl(CompositionObject newContext, string? propertyName) + { + var contextEntries = _contextEntries; + if (contextEntries == null) { - lock (_lock) - { - RaiseChangedImpl(); - } + contextEntries = new List(); + _contextEntries = contextEntries; } - private void AddContextImpl(CompositionObject newContext, string? propertyName) - { - var contextEntries = _contextEntries; - if (contextEntries == null) - { - contextEntries = new List(); - _contextEntries = contextEntries; - } + contextEntries.Add(new ContextEntry(newContext, propertyName)); + } - contextEntries.Add(new ContextEntry(newContext, propertyName)); + private void RemoveContextImpl(CompositionObject oldContext, string? propertyName) + { + var contextEntries = _contextEntries; + if (contextEntries == null) + { + return; } - private void RemoveContextImpl(CompositionObject oldContext, string? propertyName) + for (int i = contextEntries.Count - 1; i >= 0; i--) { - var contextEntries = _contextEntries; - if (contextEntries == null) - { - return; - } + var contextEntry = contextEntries[i]; - for (int i = contextEntries.Count - 1; i >= 0; i--) + if (!contextEntry.Context.TryGetTarget(out CompositionObject? context)) { - var contextEntry = contextEntries[i]; - - if (!contextEntry.Context.TryGetTarget(out CompositionObject? context)) - { - // Clean up dead references. - contextEntries.RemoveAt(i); - continue; - } - - if (context == oldContext && contextEntry.PropertyName == propertyName) - { - contextEntries.RemoveAt(i); - break; - } + // Clean up dead references. + contextEntries.RemoveAt(i); + continue; } - if (contextEntries.Count == 0) + if (context == oldContext && contextEntry.PropertyName == propertyName) { - _contextEntries = null; + contextEntries.RemoveAt(i); + break; } } - private void RaiseChangedImpl() + if (contextEntries.Count == 0) { - var contextEntries = _contextEntries; - if (contextEntries == null) - { - return; - } - - for (int i = contextEntries.Count - 1; i >= 0; i--) - { - var contextEntry = contextEntries[i]; - - if (!contextEntry.Context.TryGetTarget(out CompositionObject? context)) - { - // Clean up dead references. - contextEntries.RemoveAt(i); - continue; - } + _contextEntries = null; + } + } - context!.OnPropertyChanged(contextEntry.PropertyName, true); - } + private void PropagateChangedImpl() + { + var contextEntries = _contextEntries; + if (contextEntries == null) + { + return; } - private class ContextEntry + for (int i = contextEntries.Count - 1; i >= 0; i--) { - public ContextEntry(CompositionObject context, string? propertyName) + var contextEntry = contextEntries[i]; + + if (!contextEntry.Context.TryGetTarget(out var context)) { - Context = new WeakReference(context); - PropertyName = propertyName; + // Clean up dead references. + contextEntries.RemoveAt(i); + continue; } - public WeakReference Context { get; } - public string? PropertyName { get; } + context.OnPropertyChanged(contextEntry.PropertyName, true); + } + } + + private class ContextEntry + { + public ContextEntry(CompositionObject context, string? propertyName) + { + Context = new WeakReference(context); + PropertyName = propertyName; } + + public WeakReference Context { get; } + public string? PropertyName { get; } } } } diff --git a/src/Uno.UI.Composition/Composition/CompositionObject.cs b/src/Uno.UI.Composition/Composition/CompositionObject.cs index 2b9370f82ac1..b0cd6283f7bf 100644 --- a/src/Uno.UI.Composition/Composition/CompositionObject.cs +++ b/src/Uno.UI.Composition/Composition/CompositionObject.cs @@ -16,7 +16,6 @@ namespace Microsoft.UI.Composition { public partial class CompositionObject : IDisposable { - private readonly ContextStore _contextStore = new ContextStore(); private CompositionPropertySet? _properties; private Dictionary? _animations; @@ -157,16 +156,6 @@ internal virtual bool StartAnimationCore(string propertyName, CompositionAnimati => false; #endif - internal void AddContext(CompositionObject context, string? propertyName) - { - _contextStore.AddContext(context, propertyName); - } - - internal void RemoveContext(CompositionObject context, string? propertyName) - { - _contextStore.RemoveContext(context, propertyName); - } - private protected void SetProperty(ref bool field, bool value, [CallerMemberName] string? propertyName = null) { if (field == value) @@ -344,7 +333,7 @@ private protected void OnCompositionPropertyChanged(CompositionObject? oldValue, private protected void OnPropertyChanged(string? propertyName, bool isSubPropertyChange) { OnPropertyChangedCore(propertyName, isSubPropertyChange); - _contextStore.RaiseChanged(); + PropagateChanged(); } private protected virtual void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) From 703f3cba0903f0e591dc08e1ed2578dc4c31e9cd Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 15 Apr 2024 19:58:00 +0200 Subject: [PATCH 04/38] chore: remove ContainerVisual.Draw and move the child rendering logic up to Visual --- .../Composition/ContainerVisual.skia.cs | 14 +------------- src/Uno.UI.Composition/Composition/Visual.skia.cs | 13 +++++++++++++ .../Given_ContainerVisual.cs | 6 +++--- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs index 22e8d6d5f471..fe9de5e724af 100644 --- a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs @@ -18,7 +18,7 @@ partial void InitializePartial() Children.CollectionChanged += (s, e) => IsChildrenRenderOrderDirty = true; } - internal List GetChildrenInRenderOrder() + private protected override List? GetChildrenInRenderOrder() { if (IsChildrenRenderOrderDirty) { @@ -45,18 +45,6 @@ internal void ResetRenderOrder() IsChildrenRenderOrderDirty = false; } - internal override void Draw(in DrawingSession session) - { - base.Draw(in session); - - var children = GetChildrenInRenderOrder(); - var childrenCount = children.Count; - for (var i = 0; i < childrenCount; i++) - { - children[i].Render(in session); - } - } - internal override bool SetMatrixDirty() { if (base.SetMatrixDirty()) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 3ad54f102e4e..35ac89d3b4d6 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -1,6 +1,7 @@ #nullable enable //#define TRACE_COMPOSITION +using System.Collections.Generic; using System.Diagnostics; using System.Numerics; using SkiaSharp; @@ -159,6 +160,10 @@ internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) } } + private protected virtual List? GetChildrenInRenderOrder() => null; + + internal List? GetChildrenInRenderOrderTestingOnly() => GetChildrenInRenderOrder(); + /// /// Position a sub visual on the canvas and draw its content. /// @@ -179,6 +184,14 @@ internal void Render(in DrawingSession parentSession) using var session = BeginDrawing(in parentSession); Draw(in session); + + if (GetChildrenInRenderOrder() is { } children) + { + foreach (var child in children) + { + child.Render(in session); + } + } } /// diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs index 694657a2d843..71b51247295e 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs @@ -24,19 +24,19 @@ public void When_Children_Change() var shape = compositor.CreateShapeVisual(); containerVisual.Children.InsertAtTop(shape); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - var children = containerVisual.GetChildrenInRenderOrder(); + var children = containerVisual.GetChildrenInRenderOrderTestingOnly(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); Assert.AreEqual(1, children.Count); containerVisual.Children.InsertAtTop(compositor.CreateShapeVisual()); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - children = containerVisual.GetChildrenInRenderOrder(); + children = containerVisual.GetChildrenInRenderOrderTestingOnly(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); Assert.AreEqual(2, children.Count); containerVisual.Children.Remove(shape); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - children = containerVisual.GetChildrenInRenderOrder(); + children = containerVisual.GetChildrenInRenderOrderTestingOnly(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); Assert.AreEqual(1, children.Count); } From ec00c675743d95c9c16b505b2b280017cf3f0257 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 22 Apr 2024 14:15:53 +0200 Subject: [PATCH 05/38] feat: only "repaint" Visuals that actually changed --- .../Composition/ContainerVisual.skia.cs | 12 +++++- src/Uno.UI.Composition/Composition/Visual.cs | 1 + .../Composition/Visual.skia.cs | 40 +++++++++++++------ .../Given_ContainerVisual.cs | 6 +-- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs index fe9de5e724af..04eceb67e794 100644 --- a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs @@ -18,7 +18,7 @@ partial void InitializePartial() Children.CollectionChanged += (s, e) => IsChildrenRenderOrderDirty = true; } - private protected override List? GetChildrenInRenderOrder() + internal List GetChildrenInRenderOrder() { if (IsChildrenRenderOrderDirty) { @@ -61,4 +61,14 @@ internal override bool SetMatrixDirty() return false; } + + internal override void Render(in DrawingSession parentSession) + { + base.Render(in parentSession); + + foreach (var child in GetChildrenInRenderOrder()) + { + child.Render(in parentSession); + } + } } diff --git a/src/Uno.UI.Composition/Composition/Visual.cs b/src/Uno.UI.Composition/Composition/Visual.cs index 48a1bcc188ce..40f084f5ec0f 100644 --- a/src/Uno.UI.Composition/Composition/Visual.cs +++ b/src/Uno.UI.Composition/Composition/Visual.cs @@ -133,6 +133,7 @@ internal ICompositionTarget? CompositionTarget private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) { Compositor.InvalidateRender(this); + InvalidatePaint(); // TODO: only repaint when "dependent" properties are changed } internal override object GetAnimatableProperty(string propertyName, string subPropertyName) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 35ac89d3b4d6..c4e6d79e2fb9 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -1,6 +1,7 @@ #nullable enable //#define TRACE_COMPOSITION +using System; using System.Collections.Generic; using System.Diagnostics; using System.Numerics; @@ -19,9 +20,24 @@ public partial class Visual : global::Microsoft.UI.Composition.CompositionObject private int _zIndex; private bool _matrixDirty = true; private Matrix4x4 _totalMatrix = Matrix4x4.Identity; + private bool _requiresRepaint = true; + private SKPictureRecorder? _recorder; + private SKPicture? _picture; /// true if wasn't dirty - internal virtual bool SetMatrixDirty() => _matrixDirty = true; + internal virtual bool SetMatrixDirty() + { + var matrixDirty = _matrixDirty; + _matrixDirty = true; + return !matrixDirty; + } + + internal void InvalidatePaint() + { + _picture?.Dispose(); + _picture = null; + _requiresRepaint = true; + } /// /// This is the final transformation matrix from the origin to this Visual. @@ -160,15 +176,11 @@ internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) } } - private protected virtual List? GetChildrenInRenderOrder() => null; - - internal List? GetChildrenInRenderOrderTestingOnly() => GetChildrenInRenderOrder(); - /// /// Position a sub visual on the canvas and draw its content. /// /// The drawing session of the visual. - internal void Render(in DrawingSession parentSession) + internal virtual void Render(in DrawingSession parentSession) { #if TRACE_COMPOSITION var indent = int.TryParse(Comment?.Split(new char[] { '-' }, 2, StringSplitOptions.TrimEntries).FirstOrDefault(), out var depth) @@ -183,15 +195,17 @@ internal void Render(in DrawingSession parentSession) } using var session = BeginDrawing(in parentSession); - Draw(in session); - - if (GetChildrenInRenderOrder() is { } children) + if (_requiresRepaint) { - foreach (var child in children) - { - child.Render(in session); - } + _requiresRepaint = false; + _recorder ??= new SKPictureRecorder(); + var recorderSession = session with { Canvas = _recorder.BeginRecording(new SKRect(float.NegativeInfinity, float.NegativeInfinity, float.PositiveInfinity, float.PositiveInfinity)) }; + // To debug what exactly gets repainted, replace the following line with `Draw(in session);` + Draw(in recorderSession); + _picture = _recorder.EndRecording(); } + + session.Canvas.DrawPicture(_picture); } /// diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs index 71b51247295e..694657a2d843 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs @@ -24,19 +24,19 @@ public void When_Children_Change() var shape = compositor.CreateShapeVisual(); containerVisual.Children.InsertAtTop(shape); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - var children = containerVisual.GetChildrenInRenderOrderTestingOnly(); + var children = containerVisual.GetChildrenInRenderOrder(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); Assert.AreEqual(1, children.Count); containerVisual.Children.InsertAtTop(compositor.CreateShapeVisual()); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - children = containerVisual.GetChildrenInRenderOrderTestingOnly(); + children = containerVisual.GetChildrenInRenderOrder(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); Assert.AreEqual(2, children.Count); containerVisual.Children.Remove(shape); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - children = containerVisual.GetChildrenInRenderOrderTestingOnly(); + children = containerVisual.GetChildrenInRenderOrder(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); Assert.AreEqual(1, children.Count); } From 8ebbff39a9f397e1d79d5d5ccedc5b7038129996 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 22 Apr 2024 14:16:13 +0200 Subject: [PATCH 06/38] chore: repaint TextVisuals whenever InlineCollection changes --- .../Xaml/Controls/TextBlock/TextBlock.skia.cs | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs index 02fffbe38cc0..dd49450c51ed 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs @@ -158,39 +158,22 @@ internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs arg private int GetCharacterIndexAtPoint(Point point, bool extended = false) => Inlines.GetIndexAt(point, false, extended); - partial void OnInlinesChangedPartial() - { - Inlines.InvalidateMeasure(); - } - - // Invalidate Inlines measure when any IBlock properties used during measuring change: - - partial void OnMaxLinesChangedPartial() - { - Inlines.InvalidateMeasure(); - } + // Invalidate Inlines measure and repaint text when any IBlock properties used during measuring change: - partial void OnTextWrappingChangedPartial() + private void InvalidateInlineAndRequireRepaint() { Inlines.InvalidateMeasure(); + _textVisual.InvalidatePaint(); } - partial void OnLineHeightChangedPartial() - { - Inlines.InvalidateMeasure(); - } - - partial void OnLineStackingStrategyChangedPartial() - { - Inlines.InvalidateMeasure(); - } - - partial void OnSelectionHighlightColorChangedPartial(SolidColorBrush brush) - { - Inlines.InvalidateMeasure(); - } + partial void OnInlinesChangedPartial() => InvalidateInlineAndRequireRepaint(); + partial void OnMaxLinesChangedPartial() => InvalidateInlineAndRequireRepaint(); + partial void OnTextWrappingChangedPartial() => InvalidateInlineAndRequireRepaint(); + partial void OnLineHeightChangedPartial() => InvalidateInlineAndRequireRepaint(); + partial void OnLineStackingStrategyChangedPartial() => InvalidateInlineAndRequireRepaint(); + partial void OnSelectionHighlightColorChangedPartial(SolidColorBrush brush) => InvalidateInlineAndRequireRepaint(); - void IBlock.Invalidate(bool updateText) => InvalidateInlines(updateText); + void IBlock.Invalidate(bool updateText) => InvalidateInlineAndRequireRepaint(); string IBlock.GetText() => Text; partial void OnSelectionChanged() From 4d87ac5e8bc70319f589d900499d3cc90e8beb4b Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 22 Apr 2024 23:02:40 +0200 Subject: [PATCH 07/38] chore: use a single SKPictureRecorder for all the visuals --- src/Uno.UI.Composition/Composition/Visual.skia.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index c4e6d79e2fb9..866358b59e4b 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -14,6 +14,10 @@ namespace Microsoft.UI.Composition; public partial class Visual : global::Microsoft.UI.Composition.CompositionObject { + // Since painting (and recording) is done on the UI thread, we need a single SKPictureRecorder per UI thread. + // If we move to a UI-thread-per-window model, then we need multiple recorders. + [ThreadStatic] private static SKPictureRecorder? _recorder; + private CompositionClip? _clip; private RectangleClip? _cornerRadiusClip; private Vector2 _anchorPoint = Vector2.Zero; // Backing for scroll offsets @@ -21,7 +25,6 @@ public partial class Visual : global::Microsoft.UI.Composition.CompositionObject private bool _matrixDirty = true; private Matrix4x4 _totalMatrix = Matrix4x4.Identity; private bool _requiresRepaint = true; - private SKPictureRecorder? _recorder; private SKPicture? _picture; /// true if wasn't dirty From 9b0d6df6e9471b96ed95d5873bc9483de5c0ddb2 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 00:15:08 +0200 Subject: [PATCH 08/38] chore: build error --- src/Uno.UI.Composition/Composition/Visual.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Uno.UI.Composition/Composition/Visual.cs b/src/Uno.UI.Composition/Composition/Visual.cs index 40f084f5ec0f..1761867dc47c 100644 --- a/src/Uno.UI.Composition/Composition/Visual.cs +++ b/src/Uno.UI.Composition/Composition/Visual.cs @@ -133,7 +133,9 @@ internal ICompositionTarget? CompositionTarget private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) { Compositor.InvalidateRender(this); +#if __SKIA__ InvalidatePaint(); // TODO: only repaint when "dependent" properties are changed +#endif } internal override object GetAnimatableProperty(string propertyName, string subPropertyName) From 310102830ac13823997ac2eaf55a7c109e7aeb3c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 01:26:00 +0200 Subject: [PATCH 09/38] chore: add fixes and consolidate more rendering logic inside Visual.skia.cs --- .../Composition/ContainerVisual.skia.cs | 14 +------ .../Composition/ShapeVisual.skia.cs | 16 ++++---- .../Composition/Visual.skia.cs | 39 ++++++++++++++++--- .../Given_ContainerVisual.cs | 12 +++--- 4 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs index 04eceb67e794..ebe61a456877 100644 --- a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs @@ -1,8 +1,6 @@ #nullable enable using System.Collections.Generic; using System.Linq; -using SkiaSharp; -using Uno.UI.Composition; namespace Microsoft.UI.Composition; @@ -18,7 +16,7 @@ partial void InitializePartial() Children.CollectionChanged += (s, e) => IsChildrenRenderOrderDirty = true; } - internal List GetChildrenInRenderOrder() + private protected override IEnumerable GetChildrenInRenderOrder() { if (IsChildrenRenderOrderDirty) { @@ -61,14 +59,4 @@ internal override bool SetMatrixDirty() return false; } - - internal override void Render(in DrawingSession parentSession) - { - base.Render(in parentSession); - - foreach (var child in GetChildrenInRenderOrder()) - { - child.Render(in parentSession); - } - } } diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 9d05d65bbcb9..71918883b9fa 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -1,21 +1,24 @@ #nullable enable +using SkiaSharp; using Uno.UI.Composition; namespace Microsoft.UI.Composition; public partial class ShapeVisual { - /// - internal override void Draw(in DrawingSession session) + private protected override void ApplyClipping(in SKCanvas canvas) { - // Note that Visual.Clip is already applied in Visual.Render -> Visual.BeginDrawing - + base.ApplyClipping(in canvas); if (ViewBox is { } viewBox) { - session.Canvas.ClipRect(viewBox.GetSKRect(), antialias: true); + canvas.ClipRect(viewBox.GetSKRect(), antialias: true); } + } + /// + internal override void Draw(in DrawingSession session) + { if (_shapes is { Count: not 0 } shapes) { foreach (var t in shapes) @@ -24,9 +27,6 @@ internal override void Draw(in DrawingSession session) } } - // The CornerRadiusClip doesn't affect the shapes of the ShapeVisual, only its children - CornerRadiusClip?.Apply(session.Canvas, this); - base.Draw(in session); } } diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 866358b59e4b..e166c917ba4d 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -27,6 +27,8 @@ public partial class Visual : global::Microsoft.UI.Composition.CompositionObject private bool _requiresRepaint = true; private SKPicture? _picture; + public object? Owner { get; set; } + /// true if wasn't dirty internal virtual bool SetMatrixDirty() { @@ -183,7 +185,7 @@ internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) /// Position a sub visual on the canvas and draw its content. /// /// The drawing session of the visual. - internal virtual void Render(in DrawingSession parentSession) + private void Render(in DrawingSession parentSession) { #if TRACE_COMPOSITION var indent = int.TryParse(Comment?.Split(new char[] { '-' }, 2, StringSplitOptions.TrimEntries).FirstOrDefault(), out var depth) @@ -207,8 +209,15 @@ internal virtual void Render(in DrawingSession parentSession) Draw(in recorderSession); _picture = _recorder.EndRecording(); } - session.Canvas.DrawPicture(_picture); + + // The CornerRadiusClip doesn't affect the visual itself, only its children + CornerRadiusClip?.Apply(parentSession.Canvas, this); + + foreach (var child in GetChildrenInRenderOrder()) + { + child.Render(in parentSession); + } } /// @@ -219,6 +228,27 @@ internal virtual void Draw(in DrawingSession session) { } + /// The canvas' TotalMatrix is assumed to already be set up to the local coordinates of the visual. + private protected virtual void ApplyClipping(in SKCanvas canvas) + { + // Apply the clipping defined on the element + // (Only the Clip property, clipping applied by parent for layout constraints reason it's managed by the ShapeVisual through the ViewBox) + // Note: The Clip is applied after the transformation matrix, so it's also transformed. + Clip?.Apply(canvas, this); + } + + private protected virtual IEnumerable GetChildrenInRenderOrder() => Array.Empty(); + internal IEnumerable GetChildrenInRenderOrderTestingOnly() => GetChildrenInRenderOrder(); + + /// + /// Identifies whether a Visual can paint things. For example, ContainerVisuals don't + /// paint on their own (even though they might contain other Visuals that do). + /// This is a temporary optimization used with to reduce unnecessary + /// SkPicture allocations. In the future, we should accurately set to + /// only be true when we really have something to paint (and that painting needs to be updated). + /// + internal virtual bool CanPaint => false; + private DrawingSession BeginDrawing(in DrawingSession parentSession) => BeginDrawing(parentSession.Surface, parentSession.Canvas, parentSession.Filters, parentSession.RootTransform); @@ -242,10 +272,7 @@ private DrawingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in Drawi canvas.SetMatrix((TotalMatrix * initialTransform).ToSKMatrix()); } - // Apply the clipping defined on the element - // (Only the Clip property, clipping applied by parent for layout constraints reason it's managed by the ShapeVisual through the ViewBox) - // Note: The Clip is applied after the transformation matrix, so it's also transformed. - Clip?.Apply(canvas, this); + ApplyClipping(canvas); var session = new DrawingSession(surface, canvas, in filters, in initialTransform); diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs index 694657a2d843..fe4470b2dcad 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_ContainerVisual.cs @@ -24,21 +24,21 @@ public void When_Children_Change() var shape = compositor.CreateShapeVisual(); containerVisual.Children.InsertAtTop(shape); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - var children = containerVisual.GetChildrenInRenderOrder(); + var children = containerVisual.GetChildrenInRenderOrderTestingOnly(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); - Assert.AreEqual(1, children.Count); + Assert.AreEqual(1, children.Count()); containerVisual.Children.InsertAtTop(compositor.CreateShapeVisual()); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - children = containerVisual.GetChildrenInRenderOrder(); + children = containerVisual.GetChildrenInRenderOrderTestingOnly(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); - Assert.AreEqual(2, children.Count); + Assert.AreEqual(2, children.Count()); containerVisual.Children.Remove(shape); Assert.IsTrue(containerVisual.IsChildrenRenderOrderDirty); - children = containerVisual.GetChildrenInRenderOrder(); + children = containerVisual.GetChildrenInRenderOrderTestingOnly(); Assert.IsFalse(containerVisual.IsChildrenRenderOrderDirty); - Assert.AreEqual(1, children.Count); + Assert.AreEqual(1, children.Count()); } #endif } From d49fd57b220f7c67220f9347cebd9c4556d4c02a Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 01:28:16 +0200 Subject: [PATCH 10/38] chore: renaming --- .../Composition/CompositionMaskBrush.skia.cs | 2 +- .../Composition/CompositionNineGridBrush.skia.cs | 2 +- .../Composition/CompositionShape.skia.cs | 6 +++--- .../Composition/CompositionSpriteShape.skia.cs | 8 ++++---- .../Composition/CompositionSurfaceBrush.skia.cs | 2 +- .../Composition/CompositionVisualSurface.skia.cs | 10 +++++----- .../Composition/RedirectVisual.skia.cs | 6 +++--- .../Composition/ShapeVisual.skia.cs | 8 ++++---- .../Composition/SpriteVisual.skia.cs | 4 ++-- .../Composition/Uno/DrawingSession.skia.cs | 4 ++-- .../Composition/Uno/IOnlineBrush.skia.cs | 2 +- .../Composition/Uno/ISkiaSurface.skia.cs | 2 +- src/Uno.UI.Composition/Composition/Visual.skia.cs | 14 +++++++------- .../UI/Xaml/Controls/TextBlock/TextVisual.skia.cs | 2 +- .../UI/Xaml/Documents/InlineCollection.skia.cs | 2 +- 15 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs index 63b95e52e735..9c9e171f5da7 100644 --- a/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs @@ -34,7 +34,7 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds) paint.Shader = SKShader.CreateCompose(_sourcePaint.Shader, _maskPaint.Shader, SKBlendMode.DstIn); } - void IOnlineBrush.Draw(in DrawingSession session, SKRect bounds) + void IOnlineBrush.Draw(in PaintingSession session, SKRect bounds) { _resultPaint ??= new SKPaint() { IsAntialias = true }; diff --git a/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs index 9b34215c0a87..29ba35ab69f4 100644 --- a/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs @@ -89,7 +89,7 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds) } } - void IOnlineBrush.Draw(in DrawingSession session, SKRect bounds) + void IOnlineBrush.Draw(in PaintingSession session, SKRect bounds) { SKRect sourceBounds; if (Source is ISizedBrush sizedBrush && sizedBrush.IsSized && sizedBrush.Size is Vector2 sourceSize) diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index 03dd478ab47a..c429d3021090 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -10,18 +10,18 @@ namespace Microsoft.UI.Composition; public partial class CompositionShape { - internal virtual void Render(in DrawingSession session) + internal virtual void Render(in PaintingSession session) { using var localSession = BeginDrawing(in session); Draw(in session); // We use the session on purpose here! } - internal virtual void Draw(in DrawingSession session) + internal virtual void Draw(in PaintingSession session) { } - private DrawingSession? BeginDrawing(in DrawingSession session) + private PaintingSession? BeginDrawing(in PaintingSession session) { var offset = Offset; var transform = this.GetTransform(); diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 99093f60d904..84487aba2d02 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -13,7 +13,7 @@ public partial class CompositionSpriteShape : CompositionShape private SKPaint? _strokePaint; private SKPaint? _fillPaint; - internal override void Draw(in DrawingSession session) + internal override void Draw(in PaintingSession session) { if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) { @@ -85,13 +85,13 @@ internal override void Draw(in DrawingSession session) } } - private SKPaint TryCreateAndClearStrokePaint(in DrawingSession session) + private SKPaint TryCreateAndClearStrokePaint(in PaintingSession session) => TryCreateAndClearPaint(in session, ref _strokePaint, true, CompositionConfiguration.UseBrushAntialiasing); - private SKPaint TryCreateAndClearFillPaint(in DrawingSession session) + private SKPaint TryCreateAndClearFillPaint(in PaintingSession session) => TryCreateAndClearPaint(in session, ref _fillPaint, false, CompositionConfiguration.UseBrushAntialiasing); - private static SKPaint TryCreateAndClearPaint(in DrawingSession session, ref SKPaint? paint, bool isStroke, bool isHighQuality = false) + private static SKPaint TryCreateAndClearPaint(in PaintingSession session, ref SKPaint? paint, bool isStroke, bool isHighQuality = false) { if (paint == null) { diff --git a/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs index 86a3d3b5e219..064d7f253e6c 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs @@ -125,7 +125,7 @@ internal override void UpdatePaint(SKPaint fillPaint, SKRect bounds) internal bool UsePaintColorToColorSurface { private get; set; } - void IOnlineBrush.Draw(in DrawingSession session, SKRect bounds) + void IOnlineBrush.Draw(in PaintingSession session, SKRect bounds) { if (Surface is ISkiaSurface skiaSurface) skiaSurface.UpdateSurface(session); diff --git a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs index 518b5686154e..54db86f237b1 100644 --- a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs @@ -16,7 +16,7 @@ namespace Microsoft.UI.Composition public partial class CompositionVisualSurface : CompositionObject, ICompositionSurface, ISkiaSurface { private SKSurface? _surface; - private DrawingSession? _drawingSession; + private PaintingSession? _drawingSession; SKSurface? ISkiaSurface.Surface { get => _surface; } @@ -41,7 +41,7 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) var info = new SKImageInfo((int)size.X, (int)size.Y, SKImageInfo.PlatformColorType, SKAlphaType.Premul); _surface = SKSurface.Create(info); canvas = _surface.Canvas; - _drawingSession = new DrawingSession(_surface, canvas, DrawingFilters.Default, Matrix4x4.Identity); + _drawingSession = new PaintingSession(_surface, canvas, DrawingFilters.Default, Matrix4x4.Identity); } canvas ??= _surface.Canvas; @@ -71,13 +71,13 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) bool? previousCompMode = Compositor.IsSoftwareRenderer; Compositor.IsSoftwareRenderer = true; - SourceVisual.Draw(_drawingSession.Value with { RootTransform = initialTransform }); + SourceVisual.Paint(_drawingSession.Value with { RootTransform = initialTransform }); Compositor.IsSoftwareRenderer = previousCompMode; } } - void ISkiaSurface.UpdateSurface(in DrawingSession session) + void ISkiaSurface.UpdateSurface(in PaintingSession session) { if (SourceVisual is not null && session.Canvas is not null) { @@ -88,7 +88,7 @@ void ISkiaSurface.UpdateSurface(in DrawingSession session) session.Canvas.ClipRect(new SKRect(SourceOffset.X, SourceOffset.Y, session.Canvas.DeviceClipBounds.Width, session.Canvas.DeviceClipBounds.Height)); } - SourceVisual.Draw(in session); + SourceVisual.Paint(in session); session.Canvas.RestoreToCount(save); } } diff --git a/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs b/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs index c3a3e1216af0..5f6b76286d5f 100644 --- a/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs @@ -7,10 +7,10 @@ namespace Microsoft.UI.Composition { public partial class RedirectVisual : ContainerVisual { - internal override void Draw(in DrawingSession session) + internal override void Paint(in PaintingSession session) { - base.Draw(in session); - Source?.Draw(session); + base.Paint(in session); + Source?.Paint(session); } } } diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 71918883b9fa..f70d32807588 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -17,16 +17,16 @@ private protected override void ApplyClipping(in SKCanvas canvas) } /// - internal override void Draw(in DrawingSession session) + internal override void Paint(in PaintingSession session) { if (_shapes is { Count: not 0 } shapes) { - foreach (var t in shapes) + foreach (var shape in shapes) { - t.Render(in session); + shape.Render(in session); } } - base.Draw(in session); + base.Paint(in session); } } diff --git a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs index f0ef81a762af..16499ed6d367 100644 --- a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs @@ -39,9 +39,9 @@ internal void SetPaintColor(Color? color) UpdatePaint(); } - internal override void Draw(in DrawingSession session) + internal override void Paint(in PaintingSession session) { - base.Draw(in session); + base.Paint(in session); if (Brush is IOnlineBrush onlineBrush && onlineBrush.IsOnline) { diff --git a/src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs index bbe5c1037c06..25d422c89851 100644 --- a/src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs @@ -10,9 +10,9 @@ namespace Uno.UI.Composition; // Accessing Surface.Canvas is slow due to SkiaSharp interop. // Avoid using .Surface.Canvas and use .Canvas right away. /// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. -internal record struct DrawingSession(SKSurface Surface, SKCanvas Canvas, in DrawingFilters Filters, in Matrix4x4 RootTransform) : IDisposable +internal record struct PaintingSession(SKSurface Surface, SKCanvas Canvas, in DrawingFilters Filters, in Matrix4x4 RootTransform) : IDisposable { - public static void PushOpacity(ref DrawingSession session, float opacity) + public static void PushOpacity(ref PaintingSession session, float opacity) { // We try to keep the filter ref as long as possible in order to share the same filter.OpacityColorFilter if (opacity is not 1.0f) diff --git a/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs b/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs index c74796a498ff..e495fdfe942e 100644 --- a/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs @@ -7,6 +7,6 @@ namespace Uno.UI.Composition internal interface IOnlineBrush { internal bool IsOnline { get; } - internal void Draw(in DrawingSession session, SKRect bounds = new()); + internal void Draw(in PaintingSession session, SKRect bounds = new()); } } diff --git a/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs b/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs index 6e76eb2c8f46..91f74ab6f305 100644 --- a/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs @@ -8,6 +8,6 @@ internal interface ISkiaSurface { internal SKSurface? Surface { get; } internal void UpdateSurface(bool recreateSurface = false); - internal void UpdateSurface(in DrawingSession session); + internal void UpdateSurface(in PaintingSession session); } } diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index e166c917ba4d..71a3ca9148be 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -185,7 +185,7 @@ internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) /// Position a sub visual on the canvas and draw its content. /// /// The drawing session of the visual. - private void Render(in DrawingSession parentSession) + private void Render(in PaintingSession parentSession) { #if TRACE_COMPOSITION var indent = int.TryParse(Comment?.Split(new char[] { '-' }, 2, StringSplitOptions.TrimEntries).FirstOrDefault(), out var depth) @@ -206,7 +206,7 @@ private void Render(in DrawingSession parentSession) _recorder ??= new SKPictureRecorder(); var recorderSession = session with { Canvas = _recorder.BeginRecording(new SKRect(float.NegativeInfinity, float.NegativeInfinity, float.PositiveInfinity, float.PositiveInfinity)) }; // To debug what exactly gets repainted, replace the following line with `Draw(in session);` - Draw(in recorderSession); + Paint(in recorderSession); _picture = _recorder.EndRecording(); } session.Canvas.DrawPicture(_picture); @@ -224,7 +224,7 @@ private void Render(in DrawingSession parentSession) /// Draws the content of this visual. /// /// The drawing session to use. - internal virtual void Draw(in DrawingSession session) + internal virtual void Paint(in PaintingSession session) { } @@ -249,10 +249,10 @@ private protected virtual void ApplyClipping(in SKCanvas canvas) /// internal virtual bool CanPaint => false; - private DrawingSession BeginDrawing(in DrawingSession parentSession) + private PaintingSession BeginDrawing(in PaintingSession parentSession) => BeginDrawing(parentSession.Surface, parentSession.Canvas, parentSession.Filters, parentSession.RootTransform); - private DrawingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 initialTransform) + private PaintingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 initialTransform) { if (ShadowState is { } shadow) { @@ -274,9 +274,9 @@ private DrawingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in Drawi ApplyClipping(canvas); - var session = new DrawingSession(surface, canvas, in filters, in initialTransform); + var session = new PaintingSession(surface, canvas, in filters, in initialTransform); - DrawingSession.PushOpacity(ref session, Opacity); + PaintingSession.PushOpacity(ref session, Opacity); return session; } diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs index 92f6122a1699..a1bbd063f5e2 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs @@ -27,7 +27,7 @@ public TextVisual(Compositor compositor, TextBlock owner) : base(compositor) _owner = new WeakReference(owner); } - internal override void Draw(in DrawingSession session) + internal override void Paint(in PaintingSession session) { if (_owner.TryGetTarget(out var owner)) { diff --git a/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs b/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs index 2b0c3039ceed..4fa5ae6f884f 100644 --- a/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs +++ b/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs @@ -468,7 +468,7 @@ internal void InvalidateMeasure() /// /// Renders a block-level inline collection, i.e. one that belongs to a TextBlock (or Paragraph, in the future). /// - internal void Draw(in DrawingSession session) + internal void Draw(in PaintingSession session) { var newDrawingState = (_selection, CaretAtEndOfSelection, RenderSelection, RenderCaret); var somethingChanged = _drawingValid is not { wentThroughDraw: true, wentThroughMeasure: true } || !_lastDrawingState.Equals(newDrawingState); From b0131f5b7db762c52dbbcdba3dc35e4f11ab4f8a Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 01:57:58 +0200 Subject: [PATCH 11/38] chore: only create an SkPicture for visuals that can actually paint something --- .../Composition/RedirectVisual.skia.cs | 2 ++ .../Composition/ShapeVisual.skia.cs | 2 ++ .../Composition/SpriteVisual.skia.cs | 2 ++ .../Composition/Visual.skia.cs | 22 ++++++++++++------- .../Controls/TextBlock/TextVisual.skia.cs | 4 ++++ 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs b/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs index 5f6b76286d5f..e63e8e8ddca0 100644 --- a/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs @@ -12,5 +12,7 @@ internal override void Paint(in PaintingSession session) base.Paint(in session); Source?.Paint(session); } + + internal override bool CanPaint => Source?.CanPaint ?? false; } } diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index f70d32807588..01e1ec3dee69 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -29,4 +29,6 @@ internal override void Paint(in PaintingSession session) base.Paint(in session); } + + internal override bool CanPaint => _shapes is { Count: not 0 }; } diff --git a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs index 16499ed6d367..9a78d451a035 100644 --- a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs @@ -67,5 +67,7 @@ internal override void Paint(in PaintingSession session) ); } } + + internal override bool CanPaint => true; } } diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 71a3ca9148be..1ce5f4e822ae 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -24,7 +24,7 @@ public partial class Visual : global::Microsoft.UI.Composition.CompositionObject private int _zIndex; private bool _matrixDirty = true; private Matrix4x4 _totalMatrix = Matrix4x4.Identity; - private bool _requiresRepaint = true; + private bool _requiresRepaint; private SKPicture? _picture; public object? Owner { get; set; } @@ -39,9 +39,12 @@ internal virtual bool SetMatrixDirty() internal void InvalidatePaint() { - _picture?.Dispose(); - _picture = null; - _requiresRepaint = true; + if (CanPaint) + { + _picture?.Dispose(); + _picture = null; + _requiresRepaint = true; + } } /// @@ -200,6 +203,7 @@ private void Render(in PaintingSession parentSession) } using var session = BeginDrawing(in parentSession); + if (_requiresRepaint) { _requiresRepaint = false; @@ -209,7 +213,11 @@ private void Render(in PaintingSession parentSession) Paint(in recorderSession); _picture = _recorder.EndRecording(); } - session.Canvas.DrawPicture(_picture); + + if (_picture is { }) + { + session.Canvas.DrawPicture(_picture); + } // The CornerRadiusClip doesn't affect the visual itself, only its children CornerRadiusClip?.Apply(parentSession.Canvas, this); @@ -224,9 +232,7 @@ private void Render(in PaintingSession parentSession) /// Draws the content of this visual. /// /// The drawing session to use. - internal virtual void Paint(in PaintingSession session) - { - } + internal virtual void Paint(in PaintingSession session) { } /// The canvas' TotalMatrix is assumed to already be set up to the local coordinates of the visual. private protected virtual void ApplyClipping(in SKCanvas canvas) diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs index a1bbd063f5e2..7e5d7fd3b7c9 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs @@ -34,5 +34,9 @@ internal override void Paint(in PaintingSession session) owner.Inlines.Draw(in session); } } + + // This might be optimized to be false when _owner.Text is empty + // and _owner is not focused (i.e. no caret) and maybe some other conditions + internal override bool CanPaint => true; } } From d91f45fb7096eb69d73ddf0bf3e031c3d260ce45 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 13:19:17 +0200 Subject: [PATCH 12/38] chore: typo --- src/Uno.UI.Composition/Composition/Visual.skia.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 1ce5f4e822ae..31d4b7bbf5a1 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -220,11 +220,11 @@ private void Render(in PaintingSession parentSession) } // The CornerRadiusClip doesn't affect the visual itself, only its children - CornerRadiusClip?.Apply(parentSession.Canvas, this); + CornerRadiusClip?.Apply(session.Canvas, this); foreach (var child in GetChildrenInRenderOrder()) { - child.Render(in parentSession); + child.Render(in session); } } From c368437047e4477c89453cf1baea7d957a92065d Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 13:36:22 +0200 Subject: [PATCH 13/38] chore: fix textbox invalidation --- src/Uno.UI.Composition/Composition/Visual.skia.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 31d4b7bbf5a1..7a951b0da5e9 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -44,6 +44,7 @@ internal void InvalidatePaint() _picture?.Dispose(); _picture = null; _requiresRepaint = true; + Compositor.InvalidateRender(this); } } From 96c626d71dbe8ba64b7f43ebaa3f192a8dd6245d Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 23 Apr 2024 19:07:36 +0200 Subject: [PATCH 14/38] chore: moved When_Source_Changed to a UI sample and more composition cleanup --- .../UITests.Shared/UITests.Shared.projitems | 1 + ...sitionNineGridBrush_Source_Changes.xaml.cs | 88 ++++++++++++ .../CompositionVisualSurface.skia.cs | 24 +--- .../Composition/Uno/VisualExtensions.cs | 16 --- .../Composition/Visual.skia.cs | 24 +++- .../Given_CompositionNineGridBrush.cs | 134 ------------------ .../Media/Imaging/RenderTargetBitmap.skia.cs | 4 +- src/Uno.UI/UI/Xaml/UIElement.cs | 13 +- 8 files changed, 112 insertions(+), 192 deletions(-) create mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs delete mode 100644 src/Uno.UI.Composition/Composition/Uno/VisualExtensions.cs delete mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_CompositionNineGridBrush.cs diff --git a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems index ef719c7479b0..4e855d666aba 100644 --- a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems +++ b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems @@ -8097,6 +8097,7 @@ Brushes_ImplicitConvert.xaml + DynamicBrushes_On_Shapes.xaml diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs new file mode 100644 index 000000000000..b8273301ddbd --- /dev/null +++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs @@ -0,0 +1,88 @@ +using System; +using Microsoft.UI.Composition; +using Uno.UI.Samples.Controls; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Hosting; +using Microsoft.UI.Xaml.Media; +using Microsoft.UI.Xaml.Media.Imaging; + +namespace UITests.Windows_UI_Xaml_Media.BrushesTests +{ + [Sample("Brushes")] + public sealed partial class CompositionNineGridBrush_Source_Changes : Page + { + public CompositionNineGridBrush_Source_Changes() + { + var compositor = ElementCompositionPreview.GetElementVisual(this).Compositor; + + var onlineSource = new Image + { + Width = 100, + Height = 100, + Stretch = Stretch.UniformToFill, + Source = new BitmapImage(new Uri("ms-appx:///Assets/test_image_100_100.png")) + }; + + var online = new Border + { + Width = 200, + Height = 200 + }; + + var offline = new Grid + { + Width = 200, + Height = 200 + }; + + var visualSurface = compositor.CreateVisualSurface(); + visualSurface.SourceVisual = ElementCompositionPreview.GetElementVisual(onlineSource); + visualSurface.SourceSize = new(200, 200); + + var onlineBrush = compositor.CreateSurfaceBrush(visualSurface); + var onlineNineGridBrush = compositor.CreateNineGridBrush(); + onlineNineGridBrush.Source = onlineBrush; + onlineNineGridBrush.SetInsets(35); + + online.Background = new TestBrush(onlineNineGridBrush); + + var surface = Microsoft.UI.Xaml.Media.LoadedImageSurface.StartLoadFromUri(new Uri("ms-appx:///Assets/test_image_100_100.png")); + + surface.LoadCompleted += new Windows.Foundation.TypedEventHandler((s, o) => + { + if (o.Status == Microsoft.UI.Xaml.Media.LoadedImageSourceLoadStatus.Success) + { + var offlineBrush = compositor.CreateSurfaceBrush(surface); + + var offlineNineGridBrush = compositor.CreateNineGridBrush(); + offlineNineGridBrush.Source = offlineBrush; + offlineNineGridBrush.SetInsets(35); + + offline.Background = new TestBrush(offlineNineGridBrush); + + this.Content = new StackPanel + { + Children = + { + onlineSource, + // we need to put the children in borders to work around our limited implementation of RenderTargetBitmap + // not taking the offsets of the Visuals into account. This way, the Child will not have any offset + // relative to its parent (the border) + new Border { Child = online }, + new Border { Child = offline } + } + }; + } + }); + } + + private class TestBrush : Microsoft.UI.Xaml.Media.XamlCompositionBrushBase + { + private CompositionBrush Brush; + + public TestBrush(CompositionBrush brush) => Brush = brush; + + protected override void OnConnected() => CompositionBrush = Brush; + } + } +} diff --git a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs index 54db86f237b1..fac47e7ad7fb 100644 --- a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs @@ -50,29 +50,9 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) { canvas.Clear(); - // similar logic to Visual.RenderRootVisual - var initialTransform = canvas.TotalMatrix.ToMatrix4x4(); - if (SourceVisual.Parent?.TotalMatrix is { } parentTotalMatrix) - { - Matrix4x4.Invert(parentTotalMatrix, out var invertedParentTotalMatrix); - initialTransform = invertedParentTotalMatrix; - } - - if (SourceOffset != default) - { - var translation = Matrix4x4.Identity with { M41 = -(SourceOffset.X), M42 = -(SourceOffset.Y) }; - initialTransform = translation * initialTransform; - } - - var totalOffset = SourceVisual.GetTotalOffset(); - var translation2 = Matrix4x4.Identity with { M41 = -(totalOffset.X + SourceVisual.AnchorPoint.X), M42 = -(totalOffset.Y + SourceVisual.AnchorPoint.Y) }; - initialTransform = translation2 * initialTransform; - - bool? previousCompMode = Compositor.IsSoftwareRenderer; + var previousCompMode = Compositor.IsSoftwareRenderer; Compositor.IsSoftwareRenderer = true; - - SourceVisual.Paint(_drawingSession.Value with { RootTransform = initialTransform }); - + SourceVisual.RenderRootVisual(_drawingSession.Value.Surface, SourceOffset); Compositor.IsSoftwareRenderer = previousCompMode; } } diff --git a/src/Uno.UI.Composition/Composition/Uno/VisualExtensions.cs b/src/Uno.UI.Composition/Composition/Uno/VisualExtensions.cs deleted file mode 100644 index 384aef92bd7f..000000000000 --- a/src/Uno.UI.Composition/Composition/Uno/VisualExtensions.cs +++ /dev/null @@ -1,16 +0,0 @@ -using System.Numerics; - -namespace Microsoft.UI.Composition; - -internal static class VisualExtensions -{ - internal static Vector3 GetTotalOffset(this Visual visual) - { - if (visual.IsTranslationEnabled && visual.Properties.TryGetVector3("Translation", out var translation) == CompositionGetValueStatus.Succeeded) - { - return visual.Offset + translation; - } - - return visual.Offset; - } -} diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 7a951b0da5e9..ff5853aab1c0 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -72,7 +72,7 @@ internal Matrix4x4 TotalMatrix var matrix = Parent?.TotalMatrix ?? Matrix4x4.Identity; // Set the position of the visual on the canvas (i.e. change coordinates system to the "XAML element" one) - var totalOffset = this.GetTotalOffset(); + var totalOffset = GetTotalOffset(); var offsetMatrix = new Matrix4x4( 1, 0, 0, 0, 0, 1, 0, 0, @@ -146,8 +146,8 @@ internal int ZIndex /// Render a visual as if it's the root visual. /// /// The surface on which this visual should be rendered. - /// A boolean that indicates if the location of the root visual should be ignored (so it will be rendered at 0,0). - internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) + /// The offset (from the origin) to render the Visual at. If null, the offset properties on the Visual like and are used + internal void RenderRootVisual(SKSurface surface, Vector2? adjustedInitialOffset = null) { if (this is { Opacity: 0 } or { IsVisible: false }) { @@ -168,18 +168,18 @@ internal void RenderRootVisual(SKSurface surface, bool ignoreLocation = false) initialTransform = invertedParentTotalMatrix; } - if (ignoreLocation) + if (adjustedInitialOffset is { } adjustedOffset) { canvas.Save(); - var totalOffset = this.GetTotalOffset(); - var translation = Matrix4x4.Identity with { M41 = -(totalOffset.X + AnchorPoint.X), M42 = -(totalOffset.Y + AnchorPoint.Y) }; + var totalOffset = GetTotalOffset(); + var translation = Matrix4x4.Identity with { M41 = -(adjustedOffset.X + totalOffset.X + AnchorPoint.X), M42 = -(adjustedOffset.Y + totalOffset.Y + AnchorPoint.Y) }; initialTransform = translation * initialTransform; } using var session = BeginDrawing(surface, canvas, DrawingFilters.Default, initialTransform); Render(in session); - if (ignoreLocation) + if (adjustedInitialOffset is { }) { canvas.Restore(); } @@ -235,6 +235,16 @@ private void Render(in PaintingSession parentSession) /// The drawing session to use. internal virtual void Paint(in PaintingSession session) { } + private Vector3 GetTotalOffset() + { + if (IsTranslationEnabled && Properties.TryGetVector3("Translation", out var translation) == CompositionGetValueStatus.Succeeded) + { + return Offset + translation; + } + + return Offset; + } + /// The canvas' TotalMatrix is assumed to already be set up to the local coordinates of the visual. private protected virtual void ApplyClipping(in SKCanvas canvas) { diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_CompositionNineGridBrush.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_CompositionNineGridBrush.cs deleted file mode 100644 index 2594d02c7c7d..000000000000 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Composition/Given_CompositionNineGridBrush.cs +++ /dev/null @@ -1,134 +0,0 @@ -using System; -using System.Threading.Tasks; -using Private.Infrastructure; -using Uno.UI.RuntimeTests.Helpers; -using Microsoft.UI.Composition; -using Microsoft.UI.Xaml; -using Microsoft.UI.Xaml.Controls; -using Microsoft.UI.Xaml.Hosting; -using Microsoft.UI.Xaml.Media; -using Microsoft.UI.Xaml.Media.Imaging; - -#if HAS_UNO -using Uno.UI.Dispatching; -#endif - -namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Composition; - -[TestClass] -public class Given_CompositionNineGridBrush -{ - [TestMethod] - [RunsOnUIThread] -#if !__SKIA__ - [Ignore("LoadedImageSurface is only implemented on skia")] -#else - [Ignore("Test is incorrectly authored")] -#endif - public async Task When_Source_Changes() - { -#if HAS_UNO - var expectedThreadId = -1; - NativeDispatcher.Main.Enqueue(() => expectedThreadId = Environment.CurrentManagedThreadId, NativeDispatcherPriority.High); - await TestServices.WindowHelper.WaitFor(() => expectedThreadId != -1); -#endif - - var compositor = ElementCompositionPreview.GetElementVisual(TestServices.WindowHelper.RootElement).Compositor; - - var onlineSource = new Image - { - Width = 100, - Height = 100, - Stretch = Stretch.UniformToFill, - Source = new BitmapImage(new Uri("ms-appx:///Assets/test_image_100_100.png")) - }; - - var online = new Border - { - Width = 200, - Height = 200 - }; - - var offline = new Grid - { - Width = 200, - Height = 200 - }; - - var visualSurface = compositor.CreateVisualSurface(); - visualSurface.SourceVisual = ElementCompositionPreview.GetElementVisual(onlineSource); - visualSurface.SourceSize = new(200, 200); - - var onlineBrush = compositor.CreateSurfaceBrush(visualSurface); - var onlineNineGridBrush = compositor.CreateNineGridBrush(); - onlineNineGridBrush.Source = onlineBrush; - onlineNineGridBrush.SetInsets(35); - - online.Background = new TestBrush(onlineNineGridBrush); - - var surface = Microsoft.UI.Xaml.Media.LoadedImageSurface.StartLoadFromUri(new Uri("ms-appx:///Assets/test_image_100_100.png")); - - bool loadCompleted = false; - surface.LoadCompleted += async (s, o) => - { -#if HAS_UNO - if (Environment.CurrentManagedThreadId != expectedThreadId) - { - Assert.Fail("LoadCompleted event is run on thread pool incorrectly"); - } -#endif - - if (o.Status == Microsoft.UI.Xaml.Media.LoadedImageSourceLoadStatus.Success) - { - var offlineBrush = compositor.CreateSurfaceBrush(surface); - - var offlineNineGridBrush = compositor.CreateNineGridBrush(); - offlineNineGridBrush.Source = offlineBrush; - offlineNineGridBrush.SetInsets(35); - - offline.Background = new TestBrush(offlineNineGridBrush); - - var result = await Render(onlineSource, online, offline); - await ImageAssert.AreEqualAsync(result.actual, result.expected); - - loadCompleted = true; - } - else - { - Assert.Fail(); - } - }; - - await TestServices.WindowHelper.WaitFor(() => loadCompleted); - } - - private class TestBrush : Microsoft.UI.Xaml.Media.XamlCompositionBrushBase - { - private CompositionBrush Brush; - - public TestBrush(CompositionBrush brush) => Brush = brush; - - protected override void OnConnected() => CompositionBrush = Brush; - } - - private async Task<(RawBitmap expected, RawBitmap actual)> Render(FrameworkElement source, FrameworkElement online, FrameworkElement offline) - { - await UITestHelper.Load(new StackPanel - { - Children = - { - source, - // we need to put the children in borders to work around our limited implementation of RenderTargetBitmap - // not taking the offsets of the Visuals into account. This way, the Child will not have any offset - // relative to its parent (the border) - new Border { Child = online }, - new Border { Child = offline } - } - }); - - var onlineImg = await UITestHelper.ScreenShot(online); - var offlineImg = await UITestHelper.ScreenShot(offline); - - return (onlineImg, offlineImg); - } -} diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs index 13194ebeac2b..cb8fc3df956f 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs @@ -1,6 +1,6 @@ #nullable enable using System; -using System.Runtime.InteropServices; +using System.Numerics; using Windows.Foundation; using Windows.Graphics.Display; using Microsoft.UI.Composition; @@ -58,7 +58,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle var canvas = surface.Canvas; canvas.Clear(SKColors.Transparent); canvas.Scale((float)dpi); - visual.RenderRootVisual(surface, ignoreLocation: true); + visual.RenderRootVisual(surface, adjustedInitialOffset: new Vector2(0, 0)); var img = surface.Snapshot(); diff --git a/src/Uno.UI/UI/Xaml/UIElement.cs b/src/Uno.UI/UI/Xaml/UIElement.cs index 70eece23b255..e314b4a18963 100644 --- a/src/Uno.UI/UI/Xaml/UIElement.cs +++ b/src/Uno.UI/UI/Xaml/UIElement.cs @@ -581,6 +581,7 @@ internal static Matrix3x2 GetTransform(UIElement from, UIElement to) #endif } +#if !__SKIA__ /// /// Applies to the given matrix the transformation needed to convert from parent to local element coordinates space. /// @@ -588,20 +589,9 @@ internal static Matrix3x2 GetTransform(UIElement from, UIElement to) [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void ApplyLayoutTransform(ref Matrix3x2 matrix) { - // This is equivalent to matrix * Matrix3x2.CreateTranslation(X, Y); -#if __SKIA__ - // On Skia, we want to consider Visual.Offset - // While arranging, it's equivalent to LayoutSlotWithMarginsAndAlignments PLUS UIElement.Translation. - // But also, if the user does ElementCompositionPreview.GetElementVisual(uiElement) and modifies - // the offset, we want to consider the user-modified value. - var totalOffset = Visual.GetTotalOffset(); - matrix.M31 += (float)totalOffset.X; - matrix.M32 += (float)totalOffset.Y; -#else var layoutSlot = LayoutSlotWithMarginsAndAlignments; matrix.M31 += (float)layoutSlot.X; matrix.M32 += (float)layoutSlot.Y; -#endif } /// @@ -664,6 +654,7 @@ internal void ApplyFlowDirectionTransform(ref Matrix3x2 matrix) } #endif } +#endif #if !__IOS__ && !__ANDROID__ && !__MACOS__ // This is the default implementation, but it can be customized per platform /// From ebb2e907f73ed892e349820cf9e42c2c89b67506 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 24 Apr 2024 13:20:19 +0200 Subject: [PATCH 15/38] chore: styling --- ...sitionNineGridBrush_Source_Changes.xaml.cs | 91 ++++++++++--------- ...ession.skia.cs => PaintingSession.skia.cs} | 2 +- .../Composition/Visual.skia.cs | 3 +- 3 files changed, 49 insertions(+), 47 deletions(-) rename src/Uno.UI.Composition/Composition/Uno/{DrawingSession.skia.cs => PaintingSession.skia.cs} (84%) diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs index b8273301ddbd..a49f03cb0fd3 100644 --- a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs +++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs @@ -15,65 +15,66 @@ public CompositionNineGridBrush_Source_Changes() { var compositor = ElementCompositionPreview.GetElementVisual(this).Compositor; - var onlineSource = new Image - { - Width = 100, - Height = 100, - Stretch = Stretch.UniformToFill, - Source = new BitmapImage(new Uri("ms-appx:///Assets/test_image_100_100.png")) - }; + var onlineSource = new Image + { + Width = 100, Height = 100, Stretch = Stretch.UniformToFill, Source = new BitmapImage(new Uri("ms-appx:///Assets/test_image_100_100.png")) + }; - var online = new Border - { - Width = 200, - Height = 200 - }; + var online = new Border + { + Width = 200, Height = 200 + }; - var offline = new Grid - { - Width = 200, - Height = 200 - }; + var offline = new Grid + { + Width = 200, Height = 200 + }; - var visualSurface = compositor.CreateVisualSurface(); - visualSurface.SourceVisual = ElementCompositionPreview.GetElementVisual(onlineSource); - visualSurface.SourceSize = new(200, 200); + var visualSurface = compositor.CreateVisualSurface(); + visualSurface.SourceVisual = ElementCompositionPreview.GetElementVisual(onlineSource); + visualSurface.SourceSize = new(200, 200); - var onlineBrush = compositor.CreateSurfaceBrush(visualSurface); - var onlineNineGridBrush = compositor.CreateNineGridBrush(); - onlineNineGridBrush.Source = onlineBrush; - onlineNineGridBrush.SetInsets(35); + var onlineBrush = compositor.CreateSurfaceBrush(visualSurface); + var onlineNineGridBrush = compositor.CreateNineGridBrush(); + onlineNineGridBrush.Source = onlineBrush; + onlineNineGridBrush.SetInsets(35); - online.Background = new TestBrush(onlineNineGridBrush); + online.Background = new TestBrush(onlineNineGridBrush); - var surface = Microsoft.UI.Xaml.Media.LoadedImageSurface.StartLoadFromUri(new Uri("ms-appx:///Assets/test_image_100_100.png")); + var surface = Microsoft.UI.Xaml.Media.LoadedImageSurface.StartLoadFromUri(new Uri("ms-appx:///Assets/test_image_100_100.png")); - surface.LoadCompleted += new Windows.Foundation.TypedEventHandler((s, o) => + surface.LoadCompleted += new Windows.Foundation.TypedEventHandler((s, o) => + { + if (o.Status == Microsoft.UI.Xaml.Media.LoadedImageSourceLoadStatus.Success) { - if (o.Status == Microsoft.UI.Xaml.Media.LoadedImageSourceLoadStatus.Success) - { - var offlineBrush = compositor.CreateSurfaceBrush(surface); + var offlineBrush = compositor.CreateSurfaceBrush(surface); - var offlineNineGridBrush = compositor.CreateNineGridBrush(); - offlineNineGridBrush.Source = offlineBrush; - offlineNineGridBrush.SetInsets(35); + var offlineNineGridBrush = compositor.CreateNineGridBrush(); + offlineNineGridBrush.Source = offlineBrush; + offlineNineGridBrush.SetInsets(35); - offline.Background = new TestBrush(offlineNineGridBrush); + offline.Background = new TestBrush(offlineNineGridBrush); - this.Content = new StackPanel + this.Content = new StackPanel + { + Children = { - Children = + onlineSource, + // we need to put the children in borders to work around our limited implementation of RenderTargetBitmap + // not taking the offsets of the Visuals into account. This way, the Child will not have any offset + // relative to its parent (the border) + new Border + { + Child = online + }, + new Border { - onlineSource, - // we need to put the children in borders to work around our limited implementation of RenderTargetBitmap - // not taking the offsets of the Visuals into account. This way, the Child will not have any offset - // relative to its parent (the border) - new Border { Child = online }, - new Border { Child = offline } + Child = offline } - }; - } - }); + } + }; + } + }); } private class TestBrush : Microsoft.UI.Xaml.Media.XamlCompositionBrushBase diff --git a/src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs similarity index 84% rename from src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs rename to src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs index 25d422c89851..ccead4d00f26 100644 --- a/src/Uno.UI.Composition/Composition/Uno/DrawingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs @@ -10,7 +10,7 @@ namespace Uno.UI.Composition; // Accessing Surface.Canvas is slow due to SkiaSharp interop. // Avoid using .Surface.Canvas and use .Canvas right away. /// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. -internal record struct PaintingSession(SKSurface Surface, SKCanvas Canvas, in DrawingFilters Filters, in Matrix4x4 RootTransform) : IDisposable +internal readonly record struct PaintingSession(SKSurface Surface, SKCanvas Canvas, in DrawingFilters Filters, in Matrix4x4 RootTransform) : IDisposable { public static void PushOpacity(ref PaintingSession session, float opacity) { diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index ff5853aab1c0..e8aa4665fb83 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -16,7 +16,8 @@ public partial class Visual : global::Microsoft.UI.Composition.CompositionObject { // Since painting (and recording) is done on the UI thread, we need a single SKPictureRecorder per UI thread. // If we move to a UI-thread-per-window model, then we need multiple recorders. - [ThreadStatic] private static SKPictureRecorder? _recorder; + [ThreadStatic] + private static SKPictureRecorder? _recorder; private CompositionClip? _clip; private RectangleClip? _cornerRadiusClip; From c15ab4b14c3610df1892e5d10113733fa38aac37 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 24 Apr 2024 14:21:12 +0200 Subject: [PATCH 16/38] chore: temporarily disable When_Drawn --- .../Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs index 2d12fff79f00..6f88cc991117 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs @@ -23,6 +23,7 @@ public class Given_AcrylicBrush #if __SKIA__ && HAS_UNO [TestMethod] [RunsOnUIThread] + [Ignore("Crashes")] public async Task When_Drawn() { var img = new Image From 81f252bcb874b9557655cc768cf828be0066490c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 24 Apr 2024 15:23:08 +0200 Subject: [PATCH 17/38] chore: formatting --- .../CompositionNineGridBrush_Source_Changes.xaml.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs index a49f03cb0fd3..8e08bd355c43 100644 --- a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs +++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Media/BrushesTests/CompositionNineGridBrush_Source_Changes.xaml.cs @@ -17,17 +17,22 @@ public CompositionNineGridBrush_Source_Changes() var onlineSource = new Image { - Width = 100, Height = 100, Stretch = Stretch.UniformToFill, Source = new BitmapImage(new Uri("ms-appx:///Assets/test_image_100_100.png")) + Width = 100, + Height = 100, + Stretch = Stretch.UniformToFill, + Source = new BitmapImage(new Uri("ms-appx:///Assets/test_image_100_100.png")) }; var online = new Border { - Width = 200, Height = 200 + Width = 200, + Height = 200 }; var offline = new Grid { - Width = 200, Height = 200 + Width = 200, + Height = 200 }; var visualSurface = compositor.CreateVisualSurface(); From 43c68f75696cff9e1494bcf18d5780fca56972f4 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 3 May 2024 01:01:36 +0300 Subject: [PATCH 18/38] chore: fix CompositionVisualSurface clipping --- .../Composition/CompositionVisualSurface.skia.cs | 8 +++++--- src/Uno.UI.Composition/Composition/Visual.skia.cs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs index fac47e7ad7fb..6357147bd770 100644 --- a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs @@ -64,11 +64,13 @@ void ISkiaSurface.UpdateSurface(in PaintingSession session) int save = session.Canvas.Save(); if (SourceOffset != default) { - session.Canvas.Translate(-SourceOffset.X, -SourceOffset.Y); - session.Canvas.ClipRect(new SKRect(SourceOffset.X, SourceOffset.Y, session.Canvas.DeviceClipBounds.Width, session.Canvas.DeviceClipBounds.Height)); + // clip to the left of and above the origin (in local coordinates). + // Note that this is applied before the SourceOffset translates the canvas' matrix, so + // when the translation happens, the drawing will be clipped by the SourceOffset. + session.Canvas.ClipRect(new SKRect(0, 0, int.MaxValue, int.MaxValue)); } - SourceVisual.Paint(in session); + SourceVisual.RenderRootVisual(session.Surface, SourceOffset); session.Canvas.RestoreToCount(save); } } diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index e8aa4665fb83..4948fb6ef92a 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -166,7 +166,7 @@ internal void RenderRootVisual(SKSurface surface, Vector2? adjustedInitialOffset if (Parent?.TotalMatrix is { } parentTotalMatrix) { Matrix4x4.Invert(parentTotalMatrix, out var invertedParentTotalMatrix); - initialTransform = invertedParentTotalMatrix; + initialTransform = invertedParentTotalMatrix * initialTransform; } if (adjustedInitialOffset is { } adjustedOffset) From 8333ce8cdccbd72ddc3f9258d1f52e2ca6d4a4dc Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 3 May 2024 01:06:00 +0300 Subject: [PATCH 19/38] chore: more Visual.skia.cs cleanup and refactoring --- .../Composition/Uno/PaintingSession.skia.cs | 10 +++++--- .../Composition/Visual.skia.cs | 24 +++++++------------ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs index ccead4d00f26..ecb3492af637 100644 --- a/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs @@ -1,6 +1,7 @@ #nullable enable using System; +using System.Diagnostics.Contracts; using System.Numerics; using SkiaSharp; @@ -12,14 +13,17 @@ namespace Uno.UI.Composition; /// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. internal readonly record struct PaintingSession(SKSurface Surface, SKCanvas Canvas, in DrawingFilters Filters, in Matrix4x4 RootTransform) : IDisposable { - public static void PushOpacity(ref PaintingSession session, float opacity) + [Pure] + public PaintingSession WithOpacity(float opacity) { // We try to keep the filter ref as long as possible in order to share the same filter.OpacityColorFilter if (opacity is not 1.0f) { - var filters = session.Filters; - session = session with { Filters = filters with { Opacity = filters.Opacity * opacity } }; + var filters = Filters; + return this with { Filters = filters with { Opacity = filters.Opacity * opacity } }; } + + return this; } /// diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 4948fb6ef92a..2dab43c6822d 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -161,7 +161,7 @@ internal void RenderRootVisual(SKSurface surface, Vector2? adjustedInitialOffset // so that when concatenated with this visual's TotalMatrix, the result is only the transforms // from this visual. // It's important to set the default to canvas.TotalMatrix not SKMatrix.Identity in case there's - // an initial global transformation set (e.g. if the renderer sets scaling for dpi) + // an initial global transformation set (e.g. if the renderer sets scaling for dpi or we're rendering from a VisualSurface) var initialTransform = canvas.TotalMatrix.ToMatrix4x4(); if (Parent?.TotalMatrix is { } parentTotalMatrix) { @@ -177,7 +177,7 @@ internal void RenderRootVisual(SKSurface surface, Vector2? adjustedInitialOffset initialTransform = translation * initialTransform; } - using var session = BeginDrawing(surface, canvas, DrawingFilters.Default, initialTransform); + using var session = new PaintingSession(surface, canvas, DrawingFilters.Default, initialTransform); Render(in session); if (adjustedInitialOffset is { }) @@ -204,7 +204,8 @@ private void Render(in PaintingSession parentSession) return; } - using var session = BeginDrawing(in parentSession); + using var session = parentSession.WithOpacity(Opacity); + SetupLocalCoordinatesAndClip(session); if (_requiresRepaint) { @@ -267,11 +268,10 @@ private protected virtual void ApplyClipping(in SKCanvas canvas) /// internal virtual bool CanPaint => false; - private PaintingSession BeginDrawing(in PaintingSession parentSession) - => BeginDrawing(parentSession.Surface, parentSession.Canvas, parentSession.Filters, parentSession.RootTransform); - - private PaintingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 initialTransform) + private void SetupLocalCoordinatesAndClip(in PaintingSession session) { + var canvas = session.Canvas; + var rootTransform = session.RootTransform; if (ShadowState is { } shadow) { canvas.SaveLayer(shadow.Paint); @@ -281,21 +281,15 @@ private PaintingSession BeginDrawing(SKSurface surface, SKCanvas canvas, in Draw canvas.Save(); } - if (initialTransform.IsIdentity) + if (rootTransform.IsIdentity) { canvas.SetMatrix(TotalMatrix.ToSKMatrix()); } else { - canvas.SetMatrix((TotalMatrix * initialTransform).ToSKMatrix()); + canvas.SetMatrix((TotalMatrix * rootTransform).ToSKMatrix()); } ApplyClipping(canvas); - - var session = new PaintingSession(surface, canvas, in filters, in initialTransform); - - PaintingSession.PushOpacity(ref session, Opacity); - - return session; } } From ae27470d3c02cacaecba4899c25c9d295f045da4 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 3 May 2024 01:31:05 +0300 Subject: [PATCH 20/38] chore: remove the repainting optimization until further notice since it breaks online brushes --- .../Composition/RedirectVisual.skia.cs | 2 - .../Composition/ShapeVisual.skia.cs | 2 - .../Composition/SpriteVisual.skia.cs | 2 - src/Uno.UI.Composition/Composition/Visual.cs | 3 -- .../Composition/Visual.skia.cs | 38 +------------------ .../Xaml/Controls/TextBlock/TextBlock.skia.cs | 1 - .../Controls/TextBlock/TextVisual.skia.cs | 4 -- 7 files changed, 2 insertions(+), 50 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs b/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs index e63e8e8ddca0..5f6b76286d5f 100644 --- a/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/RedirectVisual.skia.cs @@ -12,7 +12,5 @@ internal override void Paint(in PaintingSession session) base.Paint(in session); Source?.Paint(session); } - - internal override bool CanPaint => Source?.CanPaint ?? false; } } diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 01e1ec3dee69..f70d32807588 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -29,6 +29,4 @@ internal override void Paint(in PaintingSession session) base.Paint(in session); } - - internal override bool CanPaint => _shapes is { Count: not 0 }; } diff --git a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs index 9a78d451a035..16499ed6d367 100644 --- a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs @@ -67,7 +67,5 @@ internal override void Paint(in PaintingSession session) ); } } - - internal override bool CanPaint => true; } } diff --git a/src/Uno.UI.Composition/Composition/Visual.cs b/src/Uno.UI.Composition/Composition/Visual.cs index 1761867dc47c..48a1bcc188ce 100644 --- a/src/Uno.UI.Composition/Composition/Visual.cs +++ b/src/Uno.UI.Composition/Composition/Visual.cs @@ -133,9 +133,6 @@ internal ICompositionTarget? CompositionTarget private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) { Compositor.InvalidateRender(this); -#if __SKIA__ - InvalidatePaint(); // TODO: only repaint when "dependent" properties are changed -#endif } internal override object GetAnimatableProperty(string propertyName, string subPropertyName) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 2dab43c6822d..9c745a23c9a2 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -14,21 +14,12 @@ namespace Microsoft.UI.Composition; public partial class Visual : global::Microsoft.UI.Composition.CompositionObject { - // Since painting (and recording) is done on the UI thread, we need a single SKPictureRecorder per UI thread. - // If we move to a UI-thread-per-window model, then we need multiple recorders. - [ThreadStatic] - private static SKPictureRecorder? _recorder; - private CompositionClip? _clip; private RectangleClip? _cornerRadiusClip; private Vector2 _anchorPoint = Vector2.Zero; // Backing for scroll offsets private int _zIndex; private bool _matrixDirty = true; private Matrix4x4 _totalMatrix = Matrix4x4.Identity; - private bool _requiresRepaint; - private SKPicture? _picture; - - public object? Owner { get; set; } /// true if wasn't dirty internal virtual bool SetMatrixDirty() @@ -38,17 +29,6 @@ internal virtual bool SetMatrixDirty() return !matrixDirty; } - internal void InvalidatePaint() - { - if (CanPaint) - { - _picture?.Dispose(); - _picture = null; - _requiresRepaint = true; - Compositor.InvalidateRender(this); - } - } - /// /// This is the final transformation matrix from the origin to this Visual. /// @@ -205,22 +185,8 @@ private void Render(in PaintingSession parentSession) } using var session = parentSession.WithOpacity(Opacity); - SetupLocalCoordinatesAndClip(session); - - if (_requiresRepaint) - { - _requiresRepaint = false; - _recorder ??= new SKPictureRecorder(); - var recorderSession = session with { Canvas = _recorder.BeginRecording(new SKRect(float.NegativeInfinity, float.NegativeInfinity, float.PositiveInfinity, float.PositiveInfinity)) }; - // To debug what exactly gets repainted, replace the following line with `Draw(in session);` - Paint(in recorderSession); - _picture = _recorder.EndRecording(); - } - - if (_picture is { }) - { - session.Canvas.DrawPicture(_picture); - } + SetupLocalCoordinatesAndClip(in session); + Paint(in session); // The CornerRadiusClip doesn't affect the visual itself, only its children CornerRadiusClip?.Apply(session.Canvas, this); diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs index dd49450c51ed..6d615a272856 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs @@ -163,7 +163,6 @@ internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs arg private void InvalidateInlineAndRequireRepaint() { Inlines.InvalidateMeasure(); - _textVisual.InvalidatePaint(); } partial void OnInlinesChangedPartial() => InvalidateInlineAndRequireRepaint(); diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs index 7e5d7fd3b7c9..a1bbd063f5e2 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs @@ -34,9 +34,5 @@ internal override void Paint(in PaintingSession session) owner.Inlines.Draw(in session); } } - - // This might be optimized to be false when _owner.Text is empty - // and _owner is not focused (i.e. no caret) and maybe some other conditions - internal override bool CanPaint => true; } } From fc767b3421b9f952b38224df1f432824a4e606c6 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 3 May 2024 10:45:45 +0300 Subject: [PATCH 21/38] chore: add back changes from #16502 --- .../Composition/ShapeVisual.skia.cs | 29 +++++++++++++++++-- src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 5 ++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index f70d32807588..7eaf9f4b1d66 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -1,5 +1,6 @@ #nullable enable +using System.Numerics; using SkiaSharp; using Uno.UI.Composition; @@ -10,9 +11,9 @@ public partial class ShapeVisual private protected override void ApplyClipping(in SKCanvas canvas) { base.ApplyClipping(in canvas); - if (ViewBox is { } viewBox) + if (GetViewBoxPathInElementCoordinateSpace() is { } path) { - canvas.ClipRect(viewBox.GetSKRect(), antialias: true); + canvas.ClipPath(path, antialias: true); } } @@ -29,4 +30,28 @@ internal override void Paint(in PaintingSession session) base.Paint(in session); } + + internal SKPath? GetViewBoxPathInElementCoordinateSpace() + { + if (ViewBox is not { } viewBox) + { + return null; + } + + var shape = new SKPath(); + 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); + if (viewBox.IsAncestorClip) + { + Matrix4x4.Invert(TotalMatrix, out var totalMatrixInverted); + var childToParentTransform = Parent!.TotalMatrix * totalMatrixInverted; + if (!childToParentTransform.IsIdentity) + { + + shape.Transform(childToParentTransform.ToSKMatrix()); + } + } + + return shape; + } } diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index e69798a556b7..e0ac181ff723 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -492,9 +492,10 @@ private static (UIElement? element, Branch? stale) SearchDownForTopMostElementAt // The maximum region where the current element and its children might draw themselves // This is expressed in the window (absolute) coordinate space. - var clippingBounds = element.Visual.ViewBox?.GetRect() is { } rect - ? transformToElement.Transform(rect) + var clippingBounds = element.Visual.GetViewBoxPathInElementCoordinateSpace() is { } path + ? transformToElement.Transform(path.TightBounds.ToRect()) : Rect.Infinite; + if (element.Visual.Clip?.GetBounds(element.Visual) is { } clip) { clippingBounds = clippingBounds.IntersectWith(transformToElement.Transform(clip)) ?? default; From 29aadc9d49e71af3c9d959b04283560300fe47a5 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 3 May 2024 12:18:27 +0300 Subject: [PATCH 22/38] test: hittesting with ScaleTransform --- .../Tests/Windows_UI_Xaml/Given_UIElement.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs index 44ec7ebaf097..fd648ef33dcb 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs @@ -1567,6 +1567,54 @@ public async Task When_Element_Has_Translation_And_Visual_Has_Offset() #endif #endif +#if HAS_UNO + [TestMethod] + [RunsOnUIThread] + [RequiresFullWindow] +#if !__SKIA__ + [Ignore("Hittesting is only accurate in this case on skia.")] +#endif + [DataRow(true)] + [DataRow(false)] + public async Task When_ScaleTransform_HitTest(bool addClip) + { + var sut = new Rectangle + { + Width = 100, + Height = 100, + Fill = new SolidColorBrush(Microsoft.UI.Colors.Blue), + RenderTransform = new ScaleTransform + { + ScaleX = 2, + ScaleY = 2 + }, + // adding the clip here should do nothing since the clip matches the size of the drawing + Clip = addClip ? new RectangleGeometry { Rect = new Rect(0, 0, 100, 100) } : null + }; + + var rect = await UITestHelper.Load(sut); + + var (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 50, rect.Y + 50), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 150, rect.Y + 150), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 50, rect.Y + 150), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 150, rect.Y + 150), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 201, rect.Y + 201), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X, rect.Y - 1), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X - 1, rect.Y), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 200, rect.Y - 1), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 201, rect.Y), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + } +#endif + #if HAS_UNO && HAS_INPUT_INJECTOR #region Drag and Drop From f5c3a8139998a677f8915098c492178ca90a1d68 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 13:57:24 +0300 Subject: [PATCH 23/38] chore: CompositionObject._lock => CompositionObject._contextEntriesLock --- .../Composition/CompositionObject.Context.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs b/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs index ff7d85b76d55..d08d0d46b141 100644 --- a/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs +++ b/src/Uno.UI.Composition/Composition/CompositionObject.Context.cs @@ -7,12 +7,12 @@ namespace Microsoft.UI.Composition { public partial class CompositionObject { - private readonly object _lock = new object(); + private readonly object _contextEntriesLock = new object(); private List? _contextEntries; public void AddContext(CompositionObject context, string? propertyName) { - lock (_lock) + lock (_contextEntriesLock) { AddContextImpl(context, propertyName); } @@ -20,7 +20,7 @@ public void AddContext(CompositionObject context, string? propertyName) public void RemoveContext(CompositionObject context, string? propertyName) { - lock (_lock) + lock (_contextEntriesLock) { RemoveContextImpl(context, propertyName); } @@ -28,7 +28,7 @@ public void RemoveContext(CompositionObject context, string? propertyName) public void PropagateChanged() { - lock (_lock) + lock (_contextEntriesLock) { PropagateChangedImpl(); } From fb4e3b398082fc00687274267a0c853790a41292 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 14:00:13 +0300 Subject: [PATCH 24/38] chore: Draw => Paint --- .../Composition/CompositionMaskBrush.skia.cs | 2 +- .../Composition/CompositionNineGridBrush.skia.cs | 2 +- .../Composition/CompositionShape.skia.cs | 4 ++-- .../Composition/CompositionSpriteShape.skia.cs | 2 +- .../Composition/CompositionSurfaceBrush.skia.cs | 2 +- .../Composition/CompositionVisualSurface.skia.cs | 12 ++++++------ .../Composition/SpriteVisual.skia.cs | 2 +- .../Composition/Uno/IOnlineBrush.skia.cs | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs index 9c9e171f5da7..e9faf99f032e 100644 --- a/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs @@ -34,7 +34,7 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds) paint.Shader = SKShader.CreateCompose(_sourcePaint.Shader, _maskPaint.Shader, SKBlendMode.DstIn); } - void IOnlineBrush.Draw(in PaintingSession session, SKRect bounds) + void IOnlineBrush.Paint(in PaintingSession session, SKRect bounds) { _resultPaint ??= new SKPaint() { IsAntialias = true }; diff --git a/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs index 29ba35ab69f4..25434e41445b 100644 --- a/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs @@ -89,7 +89,7 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds) } } - void IOnlineBrush.Draw(in PaintingSession session, SKRect bounds) + void IOnlineBrush.Paint(in PaintingSession session, SKRect bounds) { SKRect sourceBounds; if (Source is ISizedBrush sizedBrush && sizedBrush.IsSized && sizedBrush.Size is Vector2 sourceSize) diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index c429d3021090..8dfe719eb679 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -14,10 +14,10 @@ internal virtual void Render(in PaintingSession session) { using var localSession = BeginDrawing(in session); - Draw(in session); // We use the session on purpose here! + Paint(in session); // We use the session on purpose here! } - internal virtual void Draw(in PaintingSession session) + internal virtual void Paint(in PaintingSession session) { } diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 84487aba2d02..e5945b5c57cc 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -13,7 +13,7 @@ public partial class CompositionSpriteShape : CompositionShape private SKPaint? _strokePaint; private SKPaint? _fillPaint; - internal override void Draw(in PaintingSession session) + internal override void Paint(in PaintingSession session) { if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) { diff --git a/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs index 064d7f253e6c..d480ed6b2962 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs @@ -125,7 +125,7 @@ internal override void UpdatePaint(SKPaint fillPaint, SKRect bounds) internal bool UsePaintColorToColorSurface { private get; set; } - void IOnlineBrush.Draw(in PaintingSession session, SKRect bounds) + void IOnlineBrush.Paint(in PaintingSession session, SKRect bounds) { if (Surface is ISkiaSurface skiaSurface) skiaSurface.UpdateSurface(session); diff --git a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs index 6357147bd770..749c9181f5ea 100644 --- a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs @@ -16,16 +16,16 @@ namespace Microsoft.UI.Composition public partial class CompositionVisualSurface : CompositionObject, ICompositionSurface, ISkiaSurface { private SKSurface? _surface; - private PaintingSession? _drawingSession; + private PaintingSession? _paintingSession; SKSurface? ISkiaSurface.Surface { get => _surface; } void ISkiaSurface.UpdateSurface(bool recreateSurface) { SKCanvas? canvas = null; - if (_surface is null || _drawingSession is null || recreateSurface) + if (_surface is null || _paintingSession is null || recreateSurface) { - _drawingSession?.Dispose(); + _paintingSession?.Dispose(); _surface?.Dispose(); Vector2 size = SourceSize switch @@ -41,7 +41,7 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) var info = new SKImageInfo((int)size.X, (int)size.Y, SKImageInfo.PlatformColorType, SKAlphaType.Premul); _surface = SKSurface.Create(info); canvas = _surface.Canvas; - _drawingSession = new PaintingSession(_surface, canvas, DrawingFilters.Default, Matrix4x4.Identity); + _paintingSession = new PaintingSession(_surface, canvas, DrawingFilters.Default, Matrix4x4.Identity); } canvas ??= _surface.Canvas; @@ -52,7 +52,7 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) var previousCompMode = Compositor.IsSoftwareRenderer; Compositor.IsSoftwareRenderer = true; - SourceVisual.RenderRootVisual(_drawingSession.Value.Surface, SourceOffset); + SourceVisual.RenderRootVisual(_paintingSession.Value.Surface, SourceOffset); Compositor.IsSoftwareRenderer = previousCompMode; } } @@ -79,7 +79,7 @@ private protected override void DisposeInternal() { base.Dispose(); - _drawingSession?.Dispose(); + _paintingSession?.Dispose(); _surface?.Dispose(); } diff --git a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs index 16499ed6d367..be694233ed76 100644 --- a/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/SpriteVisual.skia.cs @@ -45,7 +45,7 @@ internal override void Paint(in PaintingSession session) if (Brush is IOnlineBrush onlineBrush && onlineBrush.IsOnline) { - onlineBrush.Draw(session, new SKRect(left: 0, top: 0, right: Size.X, bottom: Size.Y)); + onlineBrush.Paint(session, new SKRect(left: 0, top: 0, right: Size.X, bottom: Size.Y)); return; } diff --git a/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs b/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs index e495fdfe942e..325d1567b883 100644 --- a/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs @@ -7,6 +7,6 @@ namespace Uno.UI.Composition internal interface IOnlineBrush { internal bool IsOnline { get; } - internal void Draw(in PaintingSession session, SKRect bounds = new()); + internal void Paint(in PaintingSession session, SKRect bounds = new()); } } From 553540f531bc3d3661c199621d9f003280c74856 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 14:02:25 +0300 Subject: [PATCH 25/38] chore: make GetChildrenInRenderOrder return an IList instead of IEnumerable --- src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs | 2 +- src/Uno.UI.Composition/Composition/Visual.skia.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs index ebe61a456877..28a3bf4fc977 100644 --- a/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ContainerVisual.skia.cs @@ -16,7 +16,7 @@ partial void InitializePartial() Children.CollectionChanged += (s, e) => IsChildrenRenderOrderDirty = true; } - private protected override IEnumerable GetChildrenInRenderOrder() + private protected override IList GetChildrenInRenderOrder() { if (IsChildrenRenderOrderDirty) { diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 9c745a23c9a2..9d692fa568c3 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -222,8 +222,8 @@ private protected virtual void ApplyClipping(in SKCanvas canvas) Clip?.Apply(canvas, this); } - private protected virtual IEnumerable GetChildrenInRenderOrder() => Array.Empty(); - internal IEnumerable GetChildrenInRenderOrderTestingOnly() => GetChildrenInRenderOrder(); + private protected virtual IList GetChildrenInRenderOrder() => Array.Empty(); + internal IList GetChildrenInRenderOrderTestingOnly() => GetChildrenInRenderOrder(); /// /// Identifies whether a Visual can paint things. For example, ContainerVisuals don't From d01b2a39db138d2b624df5b0ad9e3ec7fb1028d4 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 14:13:14 +0300 Subject: [PATCH 26/38] chore: iterate through ShapeVisual.Shapes with a regular for-loop instead of foreach --- src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 7eaf9f4b1d66..edad2d745a81 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -22,9 +22,9 @@ internal override void Paint(in PaintingSession session) { if (_shapes is { Count: not 0 } shapes) { - foreach (var shape in shapes) + for (var i = 0; i < shapes.Count; i++) { - shape.Render(in session); + shapes[i].Render(in session); } } From b129f7c46c323712739a1da21cdd00eae9411714 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 14:14:42 +0300 Subject: [PATCH 27/38] chore: adjustedInitialOffset => offsetOverride --- src/Uno.UI.Composition/Composition/Visual.skia.cs | 10 +++++----- .../UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 9d692fa568c3..cc14f0e2de11 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -127,8 +127,8 @@ internal int ZIndex /// Render a visual as if it's the root visual. /// /// The surface on which this visual should be rendered. - /// The offset (from the origin) to render the Visual at. If null, the offset properties on the Visual like and are used - internal void RenderRootVisual(SKSurface surface, Vector2? adjustedInitialOffset = null) + /// The offset (from the origin) to render the Visual at. If null, the offset properties on the Visual like and are used. + internal void RenderRootVisual(SKSurface surface, Vector2? offsetOverride = null) { if (this is { Opacity: 0 } or { IsVisible: false }) { @@ -149,18 +149,18 @@ internal void RenderRootVisual(SKSurface surface, Vector2? adjustedInitialOffset initialTransform = invertedParentTotalMatrix * initialTransform; } - if (adjustedInitialOffset is { } adjustedOffset) + if (offsetOverride is { } offset) { canvas.Save(); var totalOffset = GetTotalOffset(); - var translation = Matrix4x4.Identity with { M41 = -(adjustedOffset.X + totalOffset.X + AnchorPoint.X), M42 = -(adjustedOffset.Y + totalOffset.Y + AnchorPoint.Y) }; + var translation = Matrix4x4.Identity with { M41 = -(offset.X + totalOffset.X + AnchorPoint.X), M42 = -(offset.Y + totalOffset.Y + AnchorPoint.Y) }; initialTransform = translation * initialTransform; } using var session = new PaintingSession(surface, canvas, DrawingFilters.Default, initialTransform); Render(in session); - if (adjustedInitialOffset is { }) + if (offsetOverride is { }) { canvas.Restore(); } diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs index cb8fc3df956f..026480974ef6 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs @@ -58,7 +58,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle var canvas = surface.Canvas; canvas.Clear(SKColors.Transparent); canvas.Scale((float)dpi); - visual.RenderRootVisual(surface, adjustedInitialOffset: new Vector2(0, 0)); + visual.RenderRootVisual(surface, offsetOverride: new Vector2(0, 0)); var img = surface.Snapshot(); From e903b03daf0183aed64ca0ae47e14e782a691928 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 14:16:36 +0300 Subject: [PATCH 28/38] chore: unignore Given_AcrylicBrush.When_Drawn --- .../Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs index 6f88cc991117..2d12fff79f00 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media/Given_AcrylicBrush.cs @@ -23,7 +23,6 @@ public class Given_AcrylicBrush #if __SKIA__ && HAS_UNO [TestMethod] [RunsOnUIThread] - [Ignore("Crashes")] public async Task When_Drawn() { var img = new Image From 57f474432307b568911989149745f30f8ed6597b Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 14:17:16 +0300 Subject: [PATCH 29/38] chore: new Vector2(0, 0) => Vector2.Zero --- src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs index 026480974ef6..55523cd82dbf 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs @@ -58,7 +58,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle var canvas = surface.Canvas; canvas.Clear(SKColors.Transparent); canvas.Scale((float)dpi); - visual.RenderRootVisual(surface, offsetOverride: new Vector2(0, 0)); + visual.RenderRootVisual(surface, offsetOverride: Vector2.Zero); var img = surface.Snapshot(); From 3c096f0b96c91fcc2165cb54576d2dfb43b701ae Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 16:27:54 +0300 Subject: [PATCH 30/38] chore: move PaintingSession into Visual --- .../Composition/CompositionMaskBrush.skia.cs | 2 +- .../CompositionNineGridBrush.skia.cs | 2 +- .../Composition/CompositionShape.skia.cs | 40 ++++++------- .../CompositionSpriteShape.skia.cs | 8 +-- .../CompositionSurfaceBrush.skia.cs | 2 +- .../CompositionVisualSurface.skia.cs | 10 +--- .../Composition/Uno/IOnlineBrush.skia.cs | 3 +- .../Composition/Uno/ISkiaSurface.skia.cs | 3 +- .../Composition/Uno/PaintingSession.skia.cs | 25 +------- .../Uno/Visual.PaintingSession.skia.cs | 59 +++++++++++++++++++ .../Composition/Visual.skia.cs | 52 +++++++++------- .../Xaml/Documents/InlineCollection.skia.cs | 3 +- 12 files changed, 125 insertions(+), 84 deletions(-) create mode 100644 src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs diff --git a/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs index e9faf99f032e..39fa3c0bed23 100644 --- a/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionMaskBrush.skia.cs @@ -34,7 +34,7 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds) paint.Shader = SKShader.CreateCompose(_sourcePaint.Shader, _maskPaint.Shader, SKBlendMode.DstIn); } - void IOnlineBrush.Paint(in PaintingSession session, SKRect bounds) + void IOnlineBrush.Paint(in Visual.PaintingSession session, SKRect bounds) { _resultPaint ??= new SKPaint() { IsAntialias = true }; diff --git a/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs index 25434e41445b..a615f5c0b05c 100644 --- a/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionNineGridBrush.skia.cs @@ -89,7 +89,7 @@ internal override void UpdatePaint(SKPaint paint, SKRect bounds) } } - void IOnlineBrush.Paint(in PaintingSession session, SKRect bounds) + void IOnlineBrush.Paint(in Visual.PaintingSession session, SKRect bounds) { SKRect sourceBounds; if (Source is ISizedBrush sizedBrush && sizedBrush.IsSized && sizedBrush.Size is Vector2 sourceSize) diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index 8dfe719eb679..ae0cd7be8018 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -10,38 +10,34 @@ namespace Microsoft.UI.Composition; public partial class CompositionShape { - internal virtual void Render(in PaintingSession session) - { - using var localSession = BeginDrawing(in session); - - Paint(in session); // We use the session on purpose here! - } - - internal virtual void Paint(in PaintingSession session) - { - } - - private PaintingSession? BeginDrawing(in PaintingSession session) + internal virtual void Render(in Visual.PaintingSession session) { var offset = Offset; var transform = this.GetTransform(); - if (offset == Vector2.Zero && transform is { IsIdentity: true }) + if (offset != Vector2.Zero || transform is not { IsIdentity: true }) { - return default; // Use the session without saving it, nothing to dispose + session.Canvas.Save(); + + if (offset != Vector2.Zero) + { + session.Canvas.Translate(offset.X, offset.Y); + } + + // Intentionally not applying transform here. + // Derived classes should be responsible to call GetTransform and use it appropriately. + // For example, CompositionSpriteShape shouldn't "scale" the stroke thickness. } - session.Canvas.Save(); + Paint(in session); - if (offset != Vector2.Zero) + if (offset != Vector2.Zero || transform is not { IsIdentity: true }) { - session.Canvas.Translate(offset.X, offset.Y); + session.Canvas.Restore(); } + } - // Intentionally not applying transform here. - // Derived classes should be responsible to call GetTransform and use it appropriately. - // For example, CompositionSpriteShape shouldn't "scale" the stroke thickness. - - return session; + internal virtual void Paint(in Visual.PaintingSession session) + { } } diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index e5945b5c57cc..d7f68a315db7 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -13,7 +13,7 @@ public partial class CompositionSpriteShape : CompositionShape private SKPaint? _strokePaint; private SKPaint? _fillPaint; - internal override void Paint(in PaintingSession session) + internal override void Paint(in Visual.PaintingSession session) { if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) { @@ -85,13 +85,13 @@ internal override void Paint(in PaintingSession session) } } - private SKPaint TryCreateAndClearStrokePaint(in PaintingSession session) + private SKPaint TryCreateAndClearStrokePaint(in Visual.PaintingSession session) => TryCreateAndClearPaint(in session, ref _strokePaint, true, CompositionConfiguration.UseBrushAntialiasing); - private SKPaint TryCreateAndClearFillPaint(in PaintingSession session) + private SKPaint TryCreateAndClearFillPaint(in Visual.PaintingSession session) => TryCreateAndClearPaint(in session, ref _fillPaint, false, CompositionConfiguration.UseBrushAntialiasing); - private static SKPaint TryCreateAndClearPaint(in PaintingSession session, ref SKPaint? paint, bool isStroke, bool isHighQuality = false) + private static SKPaint TryCreateAndClearPaint(in Visual.PaintingSession session, ref SKPaint? paint, bool isStroke, bool isHighQuality = false) { if (paint == null) { diff --git a/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs index d480ed6b2962..cfab06833dc2 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSurfaceBrush.skia.cs @@ -125,7 +125,7 @@ internal override void UpdatePaint(SKPaint fillPaint, SKRect bounds) internal bool UsePaintColorToColorSurface { private get; set; } - void IOnlineBrush.Paint(in PaintingSession session, SKRect bounds) + void IOnlineBrush.Paint(in Visual.PaintingSession session, SKRect bounds) { if (Surface is ISkiaSurface skiaSurface) skiaSurface.UpdateSurface(session); diff --git a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs index 749c9181f5ea..ccff87e3bbbb 100644 --- a/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionVisualSurface.skia.cs @@ -16,16 +16,14 @@ namespace Microsoft.UI.Composition public partial class CompositionVisualSurface : CompositionObject, ICompositionSurface, ISkiaSurface { private SKSurface? _surface; - private PaintingSession? _paintingSession; SKSurface? ISkiaSurface.Surface { get => _surface; } void ISkiaSurface.UpdateSurface(bool recreateSurface) { SKCanvas? canvas = null; - if (_surface is null || _paintingSession is null || recreateSurface) + if (_surface is null || recreateSurface) { - _paintingSession?.Dispose(); _surface?.Dispose(); Vector2 size = SourceSize switch @@ -41,7 +39,6 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) var info = new SKImageInfo((int)size.X, (int)size.Y, SKImageInfo.PlatformColorType, SKAlphaType.Premul); _surface = SKSurface.Create(info); canvas = _surface.Canvas; - _paintingSession = new PaintingSession(_surface, canvas, DrawingFilters.Default, Matrix4x4.Identity); } canvas ??= _surface.Canvas; @@ -52,12 +49,12 @@ void ISkiaSurface.UpdateSurface(bool recreateSurface) var previousCompMode = Compositor.IsSoftwareRenderer; Compositor.IsSoftwareRenderer = true; - SourceVisual.RenderRootVisual(_paintingSession.Value.Surface, SourceOffset); + SourceVisual.RenderRootVisual(_surface, SourceOffset); Compositor.IsSoftwareRenderer = previousCompMode; } } - void ISkiaSurface.UpdateSurface(in PaintingSession session) + void ISkiaSurface.UpdateSurface(in Visual.PaintingSession session) { if (SourceVisual is not null && session.Canvas is not null) { @@ -79,7 +76,6 @@ private protected override void DisposeInternal() { base.Dispose(); - _paintingSession?.Dispose(); _surface?.Dispose(); } diff --git a/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs b/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs index 325d1567b883..d3b7651ae9e0 100644 --- a/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/IOnlineBrush.skia.cs @@ -1,5 +1,6 @@ #nullable enable +using Microsoft.UI.Composition; using SkiaSharp; namespace Uno.UI.Composition @@ -7,6 +8,6 @@ namespace Uno.UI.Composition internal interface IOnlineBrush { internal bool IsOnline { get; } - internal void Paint(in PaintingSession session, SKRect bounds = new()); + internal void Paint(in Visual.PaintingSession session, SKRect bounds = new()); } } diff --git a/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs b/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs index 91f74ab6f305..4c3cc6ff7ce5 100644 --- a/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/ISkiaSurface.skia.cs @@ -1,5 +1,6 @@ #nullable enable +using Microsoft.UI.Composition; using SkiaSharp; namespace Uno.UI.Composition @@ -8,6 +9,6 @@ internal interface ISkiaSurface { internal SKSurface? Surface { get; } internal void UpdateSurface(bool recreateSurface = false); - internal void UpdateSurface(in PaintingSession session); + internal void UpdateSurface(in Visual.PaintingSession session); } } diff --git a/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs index ecb3492af637..ea8aefe95ccb 100644 --- a/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs @@ -1,32 +1,9 @@ #nullable enable using System; -using System.Diagnostics.Contracts; using System.Numerics; +using Microsoft.UI.Composition; using SkiaSharp; namespace Uno.UI.Composition; - -// Accessing Surface.Canvas is slow due to SkiaSharp interop. -// Avoid using .Surface.Canvas and use .Canvas right away. -/// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. -internal readonly record struct PaintingSession(SKSurface Surface, SKCanvas Canvas, in DrawingFilters Filters, in Matrix4x4 RootTransform) : IDisposable -{ - [Pure] - public PaintingSession WithOpacity(float opacity) - { - // We try to keep the filter ref as long as possible in order to share the same filter.OpacityColorFilter - if (opacity is not 1.0f) - { - var filters = Filters; - return this with { Filters = filters with { Opacity = filters.Opacity * opacity } }; - } - - return this; - } - - /// - public void Dispose() - => Canvas.Restore(); -} diff --git a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs new file mode 100644 index 000000000000..ee3ccf4e986b --- /dev/null +++ b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs @@ -0,0 +1,59 @@ +using System; +using System.Numerics; +using SkiaSharp; +using Uno.UI.Composition; +namespace Microsoft.UI.Composition; + +public partial class Visual +{ + /// + /// ONLY CONSTRUCT THIS IN A USING BLOCK. C# doesn't have RAII, so we can't enforce this. + /// The closest thing we can do is make this PaintingSessionWrapper type only visible to the Visual + /// and limit the areas where a new PaintingSessionWrapper needs to be created. + /// + private readonly struct PaintingSessionWrapper(PaintingSession session, int saveCount) : IDisposable + { + public PaintingSession Session { get; } = session; + public void Dispose() => Session.Canvas.RestoreToCount(saveCount); + } + + private interface IPrivateSessionFactory + { + PaintingSessionWrapper CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform); + } + + /// + /// Represents the "context" in which a visual draws. + /// + /// + /// Accessing Surface.Canvas is slow due to SkiaSharp interop. + /// Avoid using .Surface.Canvas and use .Canvas right away. + /// + internal readonly struct PaintingSession + { + // This dance is done to make it so that only Visual can create a PaintingSession + public readonly struct SessionFactory : IPrivateSessionFactory + { + PaintingSessionWrapper IPrivateSessionFactory.CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) + { + var saveCount = visual.ShadowState is { } shadow ? canvas.SaveLayer(shadow.Paint) : canvas.Save(); + return new PaintingSessionWrapper(new PaintingSession(surface, canvas, filters, rootTransform), saveCount); + } + } + + private PaintingSession(SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) + { + Surface = surface; + Canvas = canvas; + Filters = filters; + RootTransform = rootTransform; + } + + public SKSurface Surface { get; init; } + public SKCanvas Canvas { get; init; } + public DrawingFilters Filters { get; init; } + + /// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. + public Matrix4x4 RootTransform { get; init; } + } +} diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index cc14f0e2de11..ff55c5701e92 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -157,8 +157,11 @@ internal void RenderRootVisual(SKSurface surface, Vector2? offsetOverride = null initialTransform = translation * initialTransform; } - using var session = new PaintingSession(surface, canvas, DrawingFilters.Default, initialTransform); - Render(in session); + using (var wrapper = + ((IPrivateSessionFactory)new PaintingSession.SessionFactory()).CreateInstance(this, surface, canvas, DrawingFilters.Default, initialTransform)) + { + Render(wrapper.Session); + } if (offsetOverride is { }) { @@ -184,16 +187,18 @@ private void Render(in PaintingSession parentSession) return; } - using var session = parentSession.WithOpacity(Opacity); - SetupLocalCoordinatesAndClip(in session); - Paint(in session); + using (var wrapper = CreateLocalSession(in parentSession)) + { + var session = wrapper.Session; + Paint(session); - // The CornerRadiusClip doesn't affect the visual itself, only its children - CornerRadiusClip?.Apply(session.Canvas, this); + // The CornerRadiusClip doesn't affect the visual itself, only its children + CornerRadiusClip?.Apply(session.Canvas, this); - foreach (var child in GetChildrenInRenderOrder()) - { - child.Render(in session); + foreach (var child in GetChildrenInRenderOrder()) + { + child.Render(in session); + } } } @@ -234,18 +239,21 @@ private protected virtual void ApplyClipping(in SKCanvas canvas) /// internal virtual bool CanPaint => false; - private void SetupLocalCoordinatesAndClip(in PaintingSession session) + /// + /// Creates a new set up with the local coordinates, + /// clipping and opacity. + /// + private PaintingSessionWrapper CreateLocalSession(in PaintingSession parentSession) { - var canvas = session.Canvas; - var rootTransform = session.RootTransform; - if (ShadowState is { } shadow) - { - canvas.SaveLayer(shadow.Paint); - } - else - { - canvas.Save(); - } + var surface = parentSession.Surface; + var canvas = parentSession.Canvas; + var rootTransform = parentSession.RootTransform; + // We try to keep the filter ref as long as possible in order to share the same filter.OpacityColorFilter + var filters = Opacity is 1.0f + ? parentSession.Filters + : parentSession.Filters with { Opacity = parentSession.Filters.Opacity * Opacity }; + + var session = ((IPrivateSessionFactory)new PaintingSession.SessionFactory()).CreateInstance(this, surface, canvas, filters, rootTransform); if (rootTransform.IsIdentity) { @@ -257,5 +265,7 @@ private void SetupLocalCoordinatesAndClip(in PaintingSession session) } ApplyClipping(canvas); + + return session; } } diff --git a/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs b/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs index 4fa5ae6f884f..f5dc80597aa1 100644 --- a/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs +++ b/src/Uno.UI/UI/Xaml/Documents/InlineCollection.skia.cs @@ -12,6 +12,7 @@ using Uno.UI.Composition; using Windows.Foundation; using Windows.UI.Text; +using Microsoft.UI.Composition; namespace Microsoft.UI.Xaml.Documents { @@ -468,7 +469,7 @@ internal void InvalidateMeasure() /// /// Renders a block-level inline collection, i.e. one that belongs to a TextBlock (or Paragraph, in the future). /// - internal void Draw(in PaintingSession session) + internal void Draw(in Visual.PaintingSession session) { var newDrawingState = (_selection, CaretAtEndOfSelection, RenderSelection, RenderCaret); var somethingChanged = _drawingValid is not { wentThroughDraw: true, wentThroughMeasure: true } || !_lastDrawingState.Equals(newDrawingState); From d67c05e966211674deb7586694fd820033b1d0dc Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 16:30:19 +0300 Subject: [PATCH 31/38] chore: remove forgotten CanPaint definition --- src/Uno.UI.Composition/Composition/Visual.skia.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index ff55c5701e92..626c56791855 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -230,15 +230,6 @@ private protected virtual void ApplyClipping(in SKCanvas canvas) private protected virtual IList GetChildrenInRenderOrder() => Array.Empty(); internal IList GetChildrenInRenderOrderTestingOnly() => GetChildrenInRenderOrder(); - /// - /// Identifies whether a Visual can paint things. For example, ContainerVisuals don't - /// paint on their own (even though they might contain other Visuals that do). - /// This is a temporary optimization used with to reduce unnecessary - /// SkPicture allocations. In the future, we should accurately set to - /// only be true when we really have something to paint (and that painting needs to be updated). - /// - internal virtual bool CanPaint => false; - /// /// Creates a new set up with the local coordinates, /// clipping and opacity. From faf4400e8df51febfbaa9f550483711d03cb4ddb Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 18:23:07 +0300 Subject: [PATCH 32/38] chore: formatting --- .../Tests/Windows_UI_Xaml/Given_UIElement.cs | 82 +++++++++---------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs index fd648ef33dcb..c9770a5fc32b 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs @@ -1568,51 +1568,51 @@ public async Task When_Element_Has_Translation_And_Visual_Has_Offset() #endif #if HAS_UNO - [TestMethod] - [RunsOnUIThread] - [RequiresFullWindow] + [TestMethod] + [RunsOnUIThread] + [RequiresFullWindow] #if !__SKIA__ - [Ignore("Hittesting is only accurate in this case on skia.")] + [Ignore("Hittesting is only accurate in this case on skia.")] #endif - [DataRow(true)] - [DataRow(false)] - public async Task When_ScaleTransform_HitTest(bool addClip) - { - var sut = new Rectangle + [DataRow(true)] + [DataRow(false)] + public async Task When_ScaleTransform_HitTest(bool addClip) { - Width = 100, - Height = 100, - Fill = new SolidColorBrush(Microsoft.UI.Colors.Blue), - RenderTransform = new ScaleTransform + var sut = new Rectangle { - ScaleX = 2, - ScaleY = 2 - }, - // adding the clip here should do nothing since the clip matches the size of the drawing - Clip = addClip ? new RectangleGeometry { Rect = new Rect(0, 0, 100, 100) } : null - }; - - var rect = await UITestHelper.Load(sut); - - var (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 50, rect.Y + 50), sut.XamlRoot); - Assert.AreEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 150, rect.Y + 150), sut.XamlRoot); - Assert.AreEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 50, rect.Y + 150), sut.XamlRoot); - Assert.AreEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 150, rect.Y + 150), sut.XamlRoot); - Assert.AreEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 201, rect.Y + 201), sut.XamlRoot); - Assert.AreNotEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X, rect.Y - 1), sut.XamlRoot); - Assert.AreNotEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X - 1, rect.Y), sut.XamlRoot); - Assert.AreNotEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 200, rect.Y - 1), sut.XamlRoot); - Assert.AreNotEqual(sut, element); - (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 201, rect.Y), sut.XamlRoot); - Assert.AreNotEqual(sut, element); - } + Width = 100, + Height = 100, + Fill = new SolidColorBrush(Microsoft.UI.Colors.Blue), + RenderTransform = new ScaleTransform + { + ScaleX = 2, + ScaleY = 2 + }, + // adding the clip here should do nothing since the clip matches the size of the drawing + Clip = addClip ? new RectangleGeometry { Rect = new Rect(0, 0, 100, 100) } : null + }; + + var rect = await UITestHelper.Load(sut); + + var (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 50, rect.Y + 50), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 150, rect.Y + 150), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 50, rect.Y + 150), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 150, rect.Y + 150), sut.XamlRoot); + Assert.AreEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 201, rect.Y + 201), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X, rect.Y - 1), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X - 1, rect.Y), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 200, rect.Y - 1), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + (element, _) = VisualTreeHelper.HitTest(new Windows.Foundation.Point(rect.X + 201, rect.Y), sut.XamlRoot); + Assert.AreNotEqual(sut, element); + } #endif #if HAS_UNO && HAS_INPUT_INJECTOR From 80b6b878ebeef0997b604b28168d78225e9dcfaa Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 20:28:32 +0300 Subject: [PATCH 33/38] chore: fix text rendering invalidation --- src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs index 6d615a272856..37df30a374d8 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs @@ -163,6 +163,7 @@ internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs arg private void InvalidateInlineAndRequireRepaint() { Inlines.InvalidateMeasure(); + _textVisual.Compositor.InvalidateRender(_textVisual); } partial void OnInlinesChangedPartial() => InvalidateInlineAndRequireRepaint(); From c72ad79f6a669e9fccc2c3acfb300bf4b08955b5 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Sat, 4 May 2024 23:09:15 +0300 Subject: [PATCH 34/38] chore: remove init from PaintingSession properties --- .../Composition/Uno/Visual.PaintingSession.skia.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs index ee3ccf4e986b..026213be311b 100644 --- a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs @@ -49,11 +49,11 @@ private PaintingSession(SKSurface surface, SKCanvas canvas, in DrawingFilters fi RootTransform = rootTransform; } - public SKSurface Surface { get; init; } - public SKCanvas Canvas { get; init; } - public DrawingFilters Filters { get; init; } + public SKSurface Surface { get; } + public SKCanvas Canvas { get; } + public DrawingFilters Filters { get; } /// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. - public Matrix4x4 RootTransform { get; init; } + public Matrix4x4 RootTransform { get; } } } From c8035d13c838d046d96bea1df10ede6714ca5603 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 6 May 2024 22:23:25 +0300 Subject: [PATCH 35/38] chore: remove PaintingSession.skia.cs --- .../Composition/Uno/PaintingSession.skia.cs | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs diff --git a/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs deleted file mode 100644 index ea8aefe95ccb..000000000000 --- a/src/Uno.UI.Composition/Composition/Uno/PaintingSession.skia.cs +++ /dev/null @@ -1,9 +0,0 @@ -#nullable enable - -using System; -using System.Numerics; -using Microsoft.UI.Composition; -using SkiaSharp; - -namespace Uno.UI.Composition; - From 27fa0b054f7b3cab48c214385ced3d0354cbe04f Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 6 May 2024 22:28:36 +0300 Subject: [PATCH 36/38] chore: remove PaintingSessionWrapper --- .../Uno/Visual.PaintingSession.skia.cs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs index 026213be311b..3cbeefdf17e6 100644 --- a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs @@ -6,20 +6,9 @@ namespace Microsoft.UI.Composition; public partial class Visual { - /// - /// ONLY CONSTRUCT THIS IN A USING BLOCK. C# doesn't have RAII, so we can't enforce this. - /// The closest thing we can do is make this PaintingSessionWrapper type only visible to the Visual - /// and limit the areas where a new PaintingSessionWrapper needs to be created. - /// - private readonly struct PaintingSessionWrapper(PaintingSession session, int saveCount) : IDisposable - { - public PaintingSession Session { get; } = session; - public void Dispose() => Session.Canvas.RestoreToCount(saveCount); - } - private interface IPrivateSessionFactory { - PaintingSessionWrapper CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform); + PaintingSession CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform); } /// @@ -29,20 +18,22 @@ private interface IPrivateSessionFactory /// Accessing Surface.Canvas is slow due to SkiaSharp interop. /// Avoid using .Surface.Canvas and use .Canvas right away. /// - internal readonly struct PaintingSession + internal readonly struct PaintingSession: IDisposable { // This dance is done to make it so that only Visual can create a PaintingSession public readonly struct SessionFactory : IPrivateSessionFactory { - PaintingSessionWrapper IPrivateSessionFactory.CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) + PaintingSession IPrivateSessionFactory.CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) { - var saveCount = visual.ShadowState is { } shadow ? canvas.SaveLayer(shadow.Paint) : canvas.Save(); - return new PaintingSessionWrapper(new PaintingSession(surface, canvas, filters, rootTransform), saveCount); + return new PaintingSession(visual, surface, canvas, filters, rootTransform); } } - private PaintingSession(SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) + private readonly int _saveCount; + + private PaintingSession(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) { + _saveCount = visual.ShadowState is { } shadow ? canvas.SaveLayer(shadow.Paint) : canvas.Save();; Surface = surface; Canvas = canvas; Filters = filters; @@ -55,5 +46,7 @@ private PaintingSession(SKSurface surface, SKCanvas canvas, in DrawingFilters fi /// The transform matrix to the root visual of this drawing session (which isn't necessarily the identity matrix due to scaling (DPI) and/or RenderTargetBitmap. public Matrix4x4 RootTransform { get; } + + public void Dispose() => Canvas.RestoreToCount(_saveCount); } } From 9c5aab1f042264ab7f925800f1c571870aaf2cbc Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 8 May 2024 11:41:16 +0300 Subject: [PATCH 37/38] chore: remove repetitively creating new SessionFactory boxes --- .../Composition/Uno/Visual.PaintingSession.skia.cs | 4 ++-- src/Uno.UI.Composition/Composition/Visual.skia.cs | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs index 3cbeefdf17e6..12470a37552a 100644 --- a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs @@ -8,7 +8,7 @@ public partial class Visual { private interface IPrivateSessionFactory { - PaintingSession CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform); + PaintingSession CreateInstance(in Visual visual, in SKSurface surface, in SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform); } /// @@ -23,7 +23,7 @@ private interface IPrivateSessionFactory // This dance is done to make it so that only Visual can create a PaintingSession public readonly struct SessionFactory : IPrivateSessionFactory { - PaintingSession IPrivateSessionFactory.CreateInstance(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) + PaintingSession IPrivateSessionFactory.CreateInstance(in Visual visual, in SKSurface surface, in SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) { return new PaintingSession(visual, surface, canvas, filters, rootTransform); } diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 626c56791855..4d39e4e15ee0 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -14,6 +14,8 @@ namespace Microsoft.UI.Composition; public partial class Visual : global::Microsoft.UI.Composition.CompositionObject { + private static readonly IPrivateSessionFactory _factory = new PaintingSession.SessionFactory(); + private CompositionClip? _clip; private RectangleClip? _cornerRadiusClip; private Vector2 _anchorPoint = Vector2.Zero; // Backing for scroll offsets @@ -157,10 +159,9 @@ internal void RenderRootVisual(SKSurface surface, Vector2? offsetOverride = null initialTransform = translation * initialTransform; } - using (var wrapper = - ((IPrivateSessionFactory)new PaintingSession.SessionFactory()).CreateInstance(this, surface, canvas, DrawingFilters.Default, initialTransform)) + using (var session = _factory.CreateInstance(this, surface, canvas, DrawingFilters.Default, initialTransform)) { - Render(wrapper.Session); + Render(session); } if (offsetOverride is { }) @@ -187,9 +188,8 @@ private void Render(in PaintingSession parentSession) return; } - using (var wrapper = CreateLocalSession(in parentSession)) + using (var session = CreateLocalSession(in parentSession)) { - var session = wrapper.Session; Paint(session); // The CornerRadiusClip doesn't affect the visual itself, only its children @@ -234,7 +234,7 @@ private protected virtual void ApplyClipping(in SKCanvas canvas) /// Creates a new set up with the local coordinates, /// clipping and opacity. /// - private PaintingSessionWrapper CreateLocalSession(in PaintingSession parentSession) + private PaintingSession CreateLocalSession(in PaintingSession parentSession) { var surface = parentSession.Surface; var canvas = parentSession.Canvas; @@ -244,7 +244,7 @@ private PaintingSessionWrapper CreateLocalSession(in PaintingSession parentSessi ? parentSession.Filters : parentSession.Filters with { Opacity = parentSession.Filters.Opacity * Opacity }; - var session = ((IPrivateSessionFactory)new PaintingSession.SessionFactory()).CreateInstance(this, surface, canvas, filters, rootTransform); + var session = _factory.CreateInstance(this, surface, canvas, filters, rootTransform); if (rootTransform.IsIdentity) { From 284ea6339c21cb32cbb69273375932e08511ed18 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 8 May 2024 15:04:06 +0300 Subject: [PATCH 38/38] chore: formatting --- .../Composition/Uno/Visual.PaintingSession.skia.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs index 12470a37552a..4ed3e57dac44 100644 --- a/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs +++ b/src/Uno.UI.Composition/Composition/Uno/Visual.PaintingSession.skia.cs @@ -18,7 +18,7 @@ private interface IPrivateSessionFactory /// Accessing Surface.Canvas is slow due to SkiaSharp interop. /// Avoid using .Surface.Canvas and use .Canvas right away. /// - internal readonly struct PaintingSession: IDisposable + internal readonly struct PaintingSession : IDisposable { // This dance is done to make it so that only Visual can create a PaintingSession public readonly struct SessionFactory : IPrivateSessionFactory @@ -33,7 +33,7 @@ PaintingSession IPrivateSessionFactory.CreateInstance(in Visual visual, in SKSur private PaintingSession(Visual visual, SKSurface surface, SKCanvas canvas, in DrawingFilters filters, in Matrix4x4 rootTransform) { - _saveCount = visual.ShadowState is { } shadow ? canvas.SaveLayer(shadow.Paint) : canvas.Save();; + _saveCount = visual.ShadowState is { } shadow ? canvas.SaveLayer(shadow.Paint) : canvas.Save(); Surface = surface; Canvas = canvas; Filters = filters;