From aae69559b2ed2d02f0874b3ba085e6c134cd4422 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 9 Apr 2024 19:38:58 +0200 Subject: [PATCH 01/37] fix(shapes): make Shape.IsViewHit true if either Fill or Stroke are null --- src/Uno.UI/UI/Xaml/Shapes/Shape.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs index 65af65289059..8742c0746f28 100644 --- a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs +++ b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs @@ -169,7 +169,7 @@ public DoubleCollection StrokeDashArray #endregion internal override bool IsViewHit() - => Fill != null; // Do not invoke base.IsViewHit(): We don't have to have de FrameworkElement.Background to be hit testable! + => Fill != null || Stroke != null; // Do not invoke base.IsViewHit(): We don't have to have de FrameworkElement.Background to be hit testable! protected override void OnBackgroundChanged(DependencyPropertyChangedEventArgs e) { From 9fd40b4e1118e68b5862a61b66048a42117f1328 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 10 Apr 2024 11:04:16 +0200 Subject: [PATCH 02/37] feat: use skia's SkPaths to hit-test --- .../Composition/CompositionShape.skia.cs | 3 +++ .../CompositionSpriteShape.skia.cs | 20 ++++++++++++++-- .../Composition/ShapeVisual.skia.cs | 24 +++++++++++++++++++ .../Xaml/Controls/TextBlock/TextBlock.skia.cs | 3 +++ src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 5 ++++ src/Uno.UI/UI/Xaml/UIElement.skia.cs | 4 ++++ 6 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index ae0cd7be8018..d87b05279fee 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -3,6 +3,7 @@ using SkiaSharp; using System; using System.Numerics; +using Windows.Foundation; using Uno.Extensions; using Uno.UI.Composition; @@ -40,4 +41,6 @@ internal virtual void Render(in Visual.PaintingSession session) internal virtual void Paint(in Visual.PaintingSession session) { } + + internal virtual bool HitTest(Point point) => false; } diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 238aec0524cb..d6750450492e 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -2,6 +2,7 @@ using System; using System.Linq; +using Windows.Foundation; using SkiaSharp; using Uno; using Uno.Extensions; @@ -13,9 +14,15 @@ public partial class CompositionSpriteShape : CompositionShape { private SKPaint? _strokePaint; private SKPaint? _fillPaint; + + internal SKPath? LastDrawnStrokePath { get; private set; } + internal SKPath? LastDrawnFillPath { get; private set; } internal override void Paint(in Visual.PaintingSession session) { + LastDrawnStrokePath = null; + LastDrawnFillPath = null; + if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) { var transform = this.GetTransform(); @@ -33,7 +40,7 @@ internal override void Paint(in Visual.PaintingSession session) if (FillBrush is { } fill) { var fillPaint = TryCreateAndClearFillPaint(in session); - + if (Compositor.TryGetEffectiveBackgroundColor(this, out var colorFromTransition)) { fillPaint.Color = colorFromTransition.ToSKColor(); @@ -42,6 +49,8 @@ internal override void Paint(in Visual.PaintingSession session) { fill.UpdatePaint(fillPaint, geometryWithTransformations.Bounds); } + + LastDrawnFillPath = geometryWithTransformations; if (fill is CompositionBrushWrapper wrapper) { @@ -92,10 +101,11 @@ internal override void Paint(in Visual.PaintingSession session) // So, to get a correct stroke geometry, we must apply the transformations first. // Get the stroke geometry, after scaling has been applied. - using var strokeGeometry = strokePaint.GetFillPath(geometryWithTransformations); + var strokeGeometry = strokePaint.GetFillPath(geometryWithTransformations); stroke.UpdatePaint(fillPaint, strokeGeometry.Bounds); + LastDrawnStrokePath = strokeGeometry; session.Canvas.DrawPath(strokeGeometry, fillPaint); } } @@ -145,5 +155,11 @@ private static SKPaint TryCreateAndClearPaint(in Visual.PaintingSession session, return paint; } + + internal override bool HitTest(Point point) + { + return (LastDrawnStrokePath?.Contains((float)point.X, (float)point.Y) ?? false) + || (LastDrawnFillPath?.Contains((float)point.X, (float)point.Y) ?? false); + } } } diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 29666222fc02..24accab2ea77 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -1,6 +1,7 @@ #nullable enable using System.Numerics; +using Windows.Foundation; using SkiaSharp; using Uno.UI.Composition; @@ -54,4 +55,27 @@ internal override void Paint(in PaintingSession session) return shape; } + + /// This does NOT take the clipping into account. + internal bool HitTest(Point point) + { + if (_shapes is null) + { + return false; + } + + foreach (var shape in _shapes) + { + if (shape.HitTest(point)) + { + return true; + } + } + + // Do not check the child visuals. On WinUI, if you add a child visual (e.g. using ContainerVisual.Children.InsertAtTop), + // the child doesn't factor at all in hit-testing. The children of the UIElement that owns this visual will be checked + // separately in VisualTreeHelper.HitTest + + return false; + } } 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 d36a49f20d06..154674a45916 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs @@ -89,6 +89,9 @@ protected override Size MeasureOverride(Size availableSize) return desiredSize; } + // the entire body of the text block is considered hit-testable + internal override bool HitTest(Point point) => TransformToVisual((UIElement)this.GetParent()).Inverse.TransformBounds(LayoutSlotWithMarginsAndAlignments).Contains(point); + partial void OnIsTextSelectionEnabledChangedPartial() { if (_inlines is { }) diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index f26a9c1be91e..94382bc6a3a6 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -659,9 +659,14 @@ internal static (UIElement? element, Branch? stale) SearchDownForTopMostElementA } } +#if __SKIA__ + // We still need to check the renderingBounds in case the element is clipped. + if (element.HitTest(transformToElement.Inverse.TransformPoint(testPosition)) && renderingBounds.Contains(testPosition)) +#else // We didn't find any child at the given position, validate that element can be touched (i.e. not HitTestability.Invisible), // and the position is in actual bounds (which might be different than the clipping bounds) if (elementHitTestVisibility == HitTestability.Visible && renderingBounds.Contains(testPosition)) +#endif { TRACE($"> LEAF! ({element.GetDebugName()} is the OriginalSource) | stale branch: {stale?.ToString() ?? "-- none --"}"); return (element, stale); diff --git a/src/Uno.UI/UI/Xaml/UIElement.skia.cs b/src/Uno.UI/UI/Xaml/UIElement.skia.cs index 058f7a59b856..a8adcbe674b2 100644 --- a/src/Uno.UI/UI/Xaml/UIElement.skia.cs +++ b/src/Uno.UI/UI/Xaml/UIElement.skia.cs @@ -97,6 +97,10 @@ internal ShapeVisual Visual private protected virtual ShapeVisual CreateElementVisual() => Compositor.GetSharedCompositor().CreateShapeVisual(); + /// The point being tested, in element coordinates (i.e. top-left of element is (0,0) if not RTL) + /// This does NOT take the clipping into account. + internal virtual bool HitTest(Point point) => Visual.HitTest(point); + internal void AddChild(UIElement child, int? index = null) { if (child == null) From 78250ef369a8728dac54742cb3d44d9b533bce28 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 10 Apr 2024 11:04:45 +0200 Subject: [PATCH 03/37] test: add tests for shapes hit-testing --- .../Windows_UI_Xaml_Shapes/When_Shape.cs | 288 +++++++++++++++++- 1 file changed, 281 insertions(+), 7 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs index 75da23a4bb83..feace29512b3 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs @@ -1,26 +1,24 @@ #if __IOS__ || __MACOS__ || __SKIA__ -using System; using System.Collections.Generic; -using System.Runtime.CompilerServices; -using System.Text; using System.Threading.Tasks; -using Microsoft.VisualStudio.TestTools.UnitTesting; using Private.Infrastructure; using Uno.Extensions; -using Windows.Devices.Perception; using Windows.Foundation; +using Microsoft.UI; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Media; using Microsoft.UI.Xaml.Shapes; +using Uno.UI.Controls.Legacy; +using Uno.UI.RuntimeTests.Helpers; namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Shapes { [TestClass] + [RunsOnUIThread] public class When_Shape { [TestMethod] - [RunsOnUIThread] #if __MACOS__ [Ignore("Currently fails on macOS, part of #9282! epic")] #endif @@ -58,7 +56,6 @@ public async Task When_Shape_Stretch_None() #if __SKIA__ // This needs the netstd layouter + non-legacy shapes layout [TestMethod] - [RunsOnUIThread] public async Task When_Shape_Stretch_UniformToFill() { var topLevelGrid = new Grid(); @@ -93,6 +90,283 @@ public async Task When_Shape_Stretch_UniformToFill() Assert.IsTrue(MathEx.ApproxEqual(100, SUT.LayoutSlotWithMarginsAndAlignments.Height, 1E-3)); } #endif + +#if HAS_UNO + [TestMethod] +#if !__SKIA__ + [Ignore("Only skia accurately hittests shapes")] +#endif + public async Task When_Shapes_HitTesting() + { + // copied from HitTest_Shapes + var root = new Grid + { + Name = "Root", + Background = Colors.Transparent, + Width = 200, + HorizontalAlignment = HorizontalAlignment.Left, + VerticalAlignment = VerticalAlignment.Top, + RowSpacing = 10, + ColumnDefinitions = + { + new ColumnDefinition(), + new ColumnDefinition() + }, + RowDefinitions = + { + new RowDefinition(), + new RowDefinition(), + new RowDefinition(), + new RowDefinition(), + new RowDefinition(), + new RowDefinition() + }, + Children = + { + new Rectangle + { + Name = "RectangleFilled", + Width = 50, + Height = 50, + Fill = Colors.Red.WithOpacity(0.5), + Stroke = Colors.Red, + StrokeThickness = 6 + }.Apply(r => r.GridPosition(0, 0)), + new Rectangle + { + Name = "RectangleUnfilled", + Width = 50, + Height = 50, + Fill = null, + Stroke = Colors.Red, + StrokeThickness = 6 + }.Apply(r => r.GridPosition(0, 1)), + new Ellipse + { + Name = "EllipseFilled", + Width = 50, + Height = 50, + Fill = Colors.Orange.WithOpacity(0.5), + Stroke = Colors.Orange, + StrokeThickness = 6 + }.Apply(r => r.GridPosition(1, 0)), + new Ellipse + { + Name = "EllipseUnfilled", + Width = 50, + Height = 50, + Fill = null, + Stroke = Colors.Orange, + StrokeThickness = 6 + }.Apply(r => r.GridPosition(1, 1)), + new Line + { + Name = "LineFilled", + Width = 50, + Height = 50, + Stroke = Colors.Yellow, + StrokeThickness = 20, + X1 = 0, + Y1 = 0, + X2 = 50, + Y2 = 50 + }.Apply(r => r.GridPosition(2, 0)), + new Line + { + Name = "LineUnfilled", + Width = 50, + Height = 50, + Stroke = null, + StrokeThickness = 20, + X1 = 0, + Y1 = 0, + X2 = 50, + Y2 = 50 + }.Apply(r => r.GridPosition(2, 1)), + new Path + { + Name = "PathFilled", + Width = 50, + Height = 50, + Fill = Colors.Green.WithOpacity(0.5), + Stroke = Colors.Green, + StrokeThickness = 6, + Data = new GeometryGroup + { + Children = + { + new EllipseGeometry + { + Center = new Point(50, 50), + RadiusX = 50, + RadiusY = 50 + } + } + } + }.Apply(r => r.GridPosition(3, 0)), + new Path + { + Name = "PathUnfilled", + Width = 50, + Height = 50, + Fill = null, + Stroke = Colors.Green, + StrokeThickness = 6, + Data = new GeometryGroup + { + Children = + { + new EllipseGeometry + { + Center = new Point(50, 50), + RadiusX = 50, + RadiusY = 50 + } + } + } + }.Apply(r => r.GridPosition(3, 1)), + new Polygon + { + Name = "PolygonFilled", + Width = 50, + Height = 50, + Fill = Colors.Blue.WithOpacity(0.5), + Stroke = Colors.Blue, + StrokeThickness = 6, + Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) + }.Apply(r => r.GridPosition(4, 0)), + new Polygon + { + Name = "PolygonUnfilled", + Width = 50, + Height = 50, + Fill = null, + Stroke = Colors.Blue, + StrokeThickness = 6, + Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) + }.Apply(r => r.GridPosition(4, 1)), + new Polyline + { + Name = "PolylineFilled", + Width = 50, + Height = 50, + Fill = Colors.Purple.WithOpacity(0.5), + Stroke = Colors.Purple, + StrokeThickness = 6, + Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) + }.Apply(r => r.GridPosition(5, 0)), + new Polyline + { + Name = "PolylineUnfilled", + Width = 50, + Height = 50, + Fill = null, + Stroke = Colors.Purple, + StrokeThickness = 6, + Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) + }.Apply(r => r.GridPosition(5, 1)), + } + }; + + await UITestHelper.Load(root); + + // points are relative to root + var pointToTarget = new Dictionary + { + { new(48, 32), "RectangleFilled" }, + { new(31, 27), "RectangleFilled" }, + { new(50, 3), "RectangleFilled" }, + { new(74, 25), "RectangleFilled" }, + { new(58, 45), "RectangleFilled" }, + { new(77, 25), "Root" }, + + { new(130, 30), "RectangleUnfilled" }, + { new(150, 3), "RectangleUnfilled" }, + { new(172, 27), "RectangleUnfilled" }, + { new(150, 47), "RectangleUnfilled" }, + { new(150, 29), "Root" }, + + { new(50, 85), "EllipseFilled" }, + { new(28, 91), "EllipseFilled" }, + { new(59, 105), "EllipseFilled" }, + { new(47, 90), "EllipseFilled" }, + { new(65, 107), "Root" }, + { new(30, 102), "Root" }, + + { new(128, 91), "EllipseUnfilled" }, + { new(159, 105), "EllipseUnfilled" }, + { new(150, 85), "Root" }, + { new(147, 90), "Root" }, + { new(165, 107), "Root" }, + { new(130, 102), "Root" }, + + { new(51, 155), "LineFilled" }, + { new(42, 130), "LineFilled" }, + { new(35, 157), "Root" }, + { new(60, 134), "Root" }, + + // LineUnfilled + { new(151, 155), "Root" }, + { new(142, 130), "Root" }, + { new(135, 157), "Root" }, + { new(160, 134), "Root" }, + + { new(31, 209), "PathFilled" }, + { new(49, 186), "PathFilled" }, + { new(49, 219), "PathFilled" }, + { new(67, 210), "PathFilled" }, + { new(65, 223), "PathFilled" }, + { new(31, 189), "Root" }, + + { new(131, 209), "PathUnfilled" }, + { new(149, 186), "PathUnfilled" }, + { new(149, 219), "Root" }, + { new(167, 210), "Root" }, + { new(165, 223), "Root" }, + { new(131, 189), "Root" }, + + { new(26, 271), "PolygonFilled" }, + { new(45, 288), "PolygonFilled" }, + { new(39, 256), "PolygonFilled" }, + { new(62, 278), "PolygonFilled" }, + { new(36, 270), "PolygonFilled" }, + { new(50, 282), "PolygonFilled" }, + { new(60, 258), "Root" }, + + { new(126, 271), "PolygonUnfilled" }, + { new(145, 288), "PolygonUnfilled" }, + { new(139, 256), "PolygonUnfilled" }, + { new(162, 278), "PolygonUnfilled" }, + { new(136, 270), "Root" }, + { new(150, 282), "Root" }, + { new(160, 258), "Root" }, + + { new(27, 326), "PolylineFilled" }, + { new(51, 349), "PolylineFilled" }, + { new(35, 326), "PolylineFilled" }, + { new(49, 339), "PolylineFilled" }, + { new(63, 327), "Root" }, + { new(54, 315), "Root" }, + + { new(127, 326), "PolylineUnfilled" }, + { new(151, 349), "PolylineUnfilled" }, + { new(135, 326), "Root" }, + { new(149, 339), "Root" }, + { new(163, 327), "Root" }, + { new(154, 315), "Root" }, + }; + + // This may or may not fail if the window is not in view, since it depends on a render cycle occuring after loading, + // so that the shapes can be hit-tested correctly. + await Task.Delay(100); + var transform = root.TransformToVisual(null); + foreach (var (point, expectedName) in pointToTarget) + { + var actualTarget = (FrameworkElement)VisualTreeHelper.HitTest(transform.TransformPoint(point), TestServices.WindowHelper.XamlRoot).element; + Assert.AreEqual(expectedName, actualTarget.Name); + } + } +#endif } } #endif From 9dc88a824f55270083bffbaf67e59624a61d9c04 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 10 Apr 2024 14:39:02 +0200 Subject: [PATCH 04/37] test: add testing for border, cornerradius, and background hittesting --- .../Windows_UI_Xaml_Controls/Given_Border.cs | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Border.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Border.cs index a9837de534a4..8af63367f579 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Border.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Border.cs @@ -765,6 +765,131 @@ public async Task When_CornerRadius() await ImageAssert.AreSimilarAsync(bitmap2, bitmap2Expected, imperceptibilityThreshold: 0.7); } +#if HAS_UNO + [TestMethod] +#if !__SKIA__ + [Ignore("Only skia accurately hittests CorderRadius")] +#endif + [DataRow(true, true)] + [DataRow(false, true)] + [DataRow(false, false)] + public async Task When_Border_CorderRadius_HitTesting(bool addBorderChild, bool addGridBackground) + { + var borderPressedCount = 0; + var rectanglePressedCount = 0; + var gridPressedCount = 0; + var root = new Grid + { + Background = addGridBackground ? new SolidColorBrush(Colors.Transparent) : null, + Children = + { + new Border + { + Height = 150, + Width = 150, + CornerRadius = 50, + BorderBrush = Colors.Red, + BorderThickness = new Thickness(20), + Child = !addBorderChild ? null : new Rectangle + { + Width = 150, + Height = 150, + Fill = Colors.Green + }.Apply(r => r.PointerPressed += (_, args) => + { + rectanglePressedCount++; + args.Handled = true; + }) + }.Apply(b => b.PointerPressed += (_, args) => + { + borderPressedCount++; + args.Handled = true; + }) + } + }.Apply(g => g.PointerPressed += (_, _) => gridPressedCount++); + + await UITestHelper.Load(root); + + var injector = InputInjector.TryCreate() ?? throw new InvalidOperationException("Failed to init the InputInjector"); + using var mouse = injector.GetMouse(); + + // points are relative to root + var pointToTarget = new Dictionary + { + { new Point(12, 11), addGridBackground ? "grid" : "" }, + { new Point(7, 21), addGridBackground ? "grid" : "" }, + { new Point(7, 127), addGridBackground ? "grid" : "" }, + { new Point(19, 138), addGridBackground ? "grid" : "" }, + { new Point(9, 143), addGridBackground ? "grid" : "" }, + { new Point(125, 144), addGridBackground ? "grid" : "" }, + { new Point(140, 134), addGridBackground ? "grid" : "" }, + { new Point(145, 27), addGridBackground ? "grid" : "" }, + { new Point(134, 13), addGridBackground ? "grid" : "" }, + + { new Point(140, 77), "border" }, + { new Point(122, 135), "border" }, + { new Point(74, 143), "border" }, + { new Point(19, 125), "border" }, + { new Point(10, 80), "border" }, + { new Point(24, 18), "border" }, + + // This is actually what WinUI reports.The inner radius of the corners doesn't clip in hit-testing, but is itself hit-testable. + // So if there's a child in the border, you can click it "through" the thickness of the border, but if there is no child, + // the border will be clicked. + { new Point(24, 25), addBorderChild ? "rectangle" : "border" }, + { new Point(20, 125), addBorderChild ? "rectangle" : "border" }, + { new Point(122, 126), addBorderChild ? "rectangle" : "border" }, + { new Point(121, 22), addBorderChild ? "rectangle" : "border" }, + { new Point(29, 123), addBorderChild ? "rectangle" : "border" }, + { new Point(118, 123), addBorderChild ? "rectangle" : "border" }, + { new Point(117, 29), addBorderChild ? "rectangle" : "border" }, + { new Point(27, 33), addBorderChild ? "rectangle" : "border" }, + + { new Point(35, 39), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(33, 112), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(73, 126), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(113, 119), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(127, 73), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(116, 37), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(74, 26), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + { new Point(33, 35), addBorderChild ? "rectangle" : addGridBackground ? "grid" : "" }, + }; + + var expectedBorderPressedCount = 0; + var expectedRectanglePressedCount = 0; + var expectedGridPressedCount = 0; + + Assert.AreEqual(expectedBorderPressedCount, borderPressedCount); + Assert.AreEqual(expectedRectanglePressedCount, rectanglePressedCount); + Assert.AreEqual(expectedGridPressedCount, gridPressedCount); + + await Task.Delay(100); // wait for the renderer + foreach (var (point, actualTarget) in pointToTarget) + { + mouse.MoveTo(root.TransformToVisual(null).TransformPoint(point), 1); + mouse.Press(); + mouse.Release(); + + switch (actualTarget) + { + case "grid": + expectedGridPressedCount++; + break; + case "border": + expectedBorderPressedCount++; + break; + case "rectangle": + expectedRectanglePressedCount++; + break; + } + + Assert.AreEqual(expectedBorderPressedCount, borderPressedCount); + Assert.AreEqual(expectedRectanglePressedCount, rectanglePressedCount); + Assert.AreEqual(expectedGridPressedCount, gridPressedCount); + } + } +#endif + [TestMethod] public async Task Border_AntiAlias() { From a07167d59af59edb92075fa5f3358d72648f0675 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 10 Apr 2024 14:39:14 +0200 Subject: [PATCH 05/37] test: RTL hittesting --- .../Windows_UI_Xaml/Given_FlowDirection.cs | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_FlowDirection.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_FlowDirection.cs index adb3a4c0729b..75915658ff55 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_FlowDirection.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_FlowDirection.cs @@ -1,10 +1,12 @@ using System; +using System.Collections.Generic; using System.Threading.Tasks; using Private.Infrastructure; using Uno.UI.RuntimeTests.Helpers; using Windows.Foundation; using Windows.Foundation.Metadata; using Windows.UI; +using Windows.UI.Input.Preview.Injection; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Controls.Primitives; @@ -199,6 +201,100 @@ public async Task When_RTL() Assert.AreEqual($"-1,0,0,1,{m31},{m32}", redTransformToRoot.Matrix.ToString()); } +#if HAS_UNO + [TestMethod] + [RunsOnUIThread] + public async Task When_RTL_HitTesting() + { + // copied from When_RTL + var rootGrid = CreateGrid(Colors.WhiteSmoke); + + var grid = CreateRTLGrid200x200WithTwoRowsAndTwoColumns(); + + rootGrid.Children.Add(grid); + + var red = CreateRectangle(Stretch.Fill, Colors.Red); + Grid.SetRow(red, 0); + Grid.SetColumn(red, 0); + + var green = CreateRectangle(Stretch.Fill, Colors.Green); + Grid.SetRow(green, 0); + Grid.SetColumn(green, 1); + + var blue = CreateRectangle(Stretch.Fill, Colors.Blue); + Grid.SetRow(blue, 1); + Grid.SetColumn(blue, 0); + + var yellow = CreateRectangle(Stretch.Fill, Colors.Yellow); + Grid.SetRow(yellow, 1); + Grid.SetColumn(yellow, 1); + + grid.Children.Add(red); + grid.Children.Add(green); + grid.Children.Add(blue); + grid.Children.Add(yellow); + + TestServices.WindowHelper.WindowContent = rootGrid; + await TestServices.WindowHelper.WaitForLoaded(rootGrid); + + var injector = InputInjector.TryCreate() ?? throw new InvalidOperationException("Failed to init the InputInjector"); + using var mouse = injector.GetMouse(); + + // points are relative to rootGrid + var pointToTarget = new Dictionary + { + { new Point(50, 50), "green" }, + { new Point(150, 50), "red" }, + { new Point(50, 150), "yellow" }, + { new Point(150, 150), "blue" }, + }; + + var greenPresses = 0; + var redPresses = 0; + var yellowPresses = 0; + var bluePresses = 0; + + green.PointerPressed += (_, _) => greenPresses++; + red.PointerPressed += (_, _) => redPresses++; + yellow.PointerPressed += (_, _) => yellowPresses++; + blue.PointerPressed += (_, _) => bluePresses++; + + var expectedGreenPresses = 0; + var expectedRedPresses = 0; + var expectedYellowPresses = 0; + var expectedBluePresses = 0; + + await Task.Delay(100); // wait for the renderer + foreach (var (point, target) in pointToTarget) + { + mouse.MoveTo(rootGrid.TransformToVisual(null).TransformPoint(point), 1); + mouse.Press(); + mouse.Release(); + + switch (target) + { + case "green": + expectedGreenPresses++; + break; + case "red": + expectedRedPresses++; + break; + case "yellow": + expectedYellowPresses++; + break; + case "blue": + expectedBluePresses++; + break; + } + + Assert.AreEqual(expectedGreenPresses, greenPresses); + Assert.AreEqual(expectedRedPresses, redPresses); + Assert.AreEqual(expectedYellowPresses, yellowPresses); + Assert.AreEqual(expectedBluePresses, bluePresses); + } + } +#endif + [TestMethod] [RunsOnUIThread] [RequiresFullWindow] From 2cf97a9a36523a81ee2f4f52ee0774b14bdc9d32 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 16:48:39 +0300 Subject: [PATCH 06/37] chore: formatting --- .../Composition/CompositionSpriteShape.skia.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index d6750450492e..1c2891c1e9b0 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -14,7 +14,7 @@ public partial class CompositionSpriteShape : CompositionShape { private SKPaint? _strokePaint; private SKPaint? _fillPaint; - + internal SKPath? LastDrawnStrokePath { get; private set; } internal SKPath? LastDrawnFillPath { get; private set; } @@ -40,7 +40,7 @@ internal override void Paint(in Visual.PaintingSession session) if (FillBrush is { } fill) { var fillPaint = TryCreateAndClearFillPaint(in session); - + if (Compositor.TryGetEffectiveBackgroundColor(this, out var colorFromTransition)) { fillPaint.Color = colorFromTransition.ToSKColor(); @@ -49,7 +49,7 @@ internal override void Paint(in Visual.PaintingSession session) { fill.UpdatePaint(fillPaint, geometryWithTransformations.Bounds); } - + LastDrawnFillPath = geometryWithTransformations; if (fill is CompositionBrushWrapper wrapper) From ebe1ab6848c299f5c08bb70cd8267f8ea9ee924c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 16:50:57 +0300 Subject: [PATCH 07/37] chore: fix after rebase --- src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index 94382bc6a3a6..a5f31fd549dd 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -661,7 +661,7 @@ internal static (UIElement? element, Branch? stale) SearchDownForTopMostElementA #if __SKIA__ // We still need to check the renderingBounds in case the element is clipped. - if (element.HitTest(transformToElement.Inverse.TransformPoint(testPosition)) && renderingBounds.Contains(testPosition)) + if (element.HitTest(transformToElement.Inverse().Transform(testPosition)) && renderingBounds.Contains(testPosition)) #else // We didn't find any child at the given position, validate that element can be touched (i.e. not HitTestability.Invisible), // and the position is in actual bounds (which might be different than the clipping bounds) From bb2f30fb9d8598a682c7d7aaed0d753c98488e9c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 16:53:48 +0300 Subject: [PATCH 08/37] chore: extend hittesting logic to BorderVisual --- src/Uno.UI.Composition/Composition/BorderVisual.skia.cs | 3 +++ src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs index f0faa3476ba1..e7b29c88187a 100644 --- a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs @@ -2,6 +2,7 @@ using System; using System.Numerics; +using Windows.Foundation; using SkiaSharp; using Uno.UI.Composition; @@ -290,4 +291,6 @@ private unsafe void UpdateBorderPath(SKRect innerArea, SKRect outerArea, SKPoint _borderPath.Close(); } } + + internal override bool HitTest(Point point) => (_borderShape?.HitTest(point) ?? false) || (_backgroundShape?.HitTest(point) ?? false) || base.HitTest(point); } diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 24accab2ea77..8254ad7e9636 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -57,7 +57,7 @@ internal override void Paint(in PaintingSession session) } /// This does NOT take the clipping into account. - internal bool HitTest(Point point) + internal virtual bool HitTest(Point point) { if (_shapes is null) { From 3b7b0d4589a575d06bb639a53e15090ae63d6a87 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 22:05:27 +0300 Subject: [PATCH 09/37] chore: fix SetObjectProperty typo --- src/Uno.UI.Composition/Composition/CompositionObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionObject.cs b/src/Uno.UI.Composition/Composition/CompositionObject.cs index 21370bc7d503..4c59be5c6f64 100644 --- a/src/Uno.UI.Composition/Composition/CompositionObject.cs +++ b/src/Uno.UI.Composition/Composition/CompositionObject.cs @@ -303,7 +303,7 @@ private protected void SetObjectProperty(ref T field, T value, [CallerMemberN // Is this valid even for non-composition objects like interface? var fieldCO = field as CompositionObject; var valueCO = value as CompositionObject; - if (fieldCO != null || value != null) + if (fieldCO != null || valueCO != null) { OnCompositionPropertyChanged(fieldCO, valueCO, propertyName); } From 2830fbd560d12e05cc88a9604422c50d17bbd4a2 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 22:09:48 +0300 Subject: [PATCH 10/37] chore: make SkiaGeometrySource2D's SKPath read-only --- .../Composition/SkiaGeometrySource2D.skia.cs | 19 +++++++++++++------ src/Uno.UI/Media/PathStreamGeometryContext.cs | 14 +++++++------- src/Uno.UI/Media/StreamGeometry.cs | 13 ++++--------- src/Uno.UI/UI/Xaml/Media/Geometry.skia.cs | 2 +- src/Uno.UI/UI/Xaml/Shapes/Ellipse.skia.cs | 5 +++-- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs b/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs index f8c460b4ec06..65a52055ba2f 100644 --- a/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs +++ b/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs @@ -6,18 +6,25 @@ namespace Microsoft.UI.Composition { - internal class SkiaGeometrySource2D : IGeometrySource2D + internal class SkiaGeometrySource2D : CompositionObject, IGeometrySource2D { - public SkiaGeometrySource2D() - { - Geometry = new SKPath(); - } + private SKPath _geometry; +#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. public SkiaGeometrySource2D(SKPath source) +#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. { Geometry = source ?? throw new ArgumentNullException(nameof(source)); } - public SKPath Geometry { get; set; } + /// + /// DO NOT MODIFY THIS SKPath. CREATE A NEW SKPath INSTEAD. + /// This can lead to nasty invalidation bugs where the SKPath changes without notifying anyone. + /// + public SKPath Geometry + { + get => _geometry; + set => SetObjectProperty(ref _geometry, value); + } } } diff --git a/src/Uno.UI/Media/PathStreamGeometryContext.cs b/src/Uno.UI/Media/PathStreamGeometryContext.cs index d8fdea896730..9841224c91da 100644 --- a/src/Uno.UI/Media/PathStreamGeometryContext.cs +++ b/src/Uno.UI/Media/PathStreamGeometryContext.cs @@ -21,7 +21,7 @@ using Path = Android.Graphics.Path; using Uno.UI; #elif __SKIA__ -using Path = Microsoft.UI.Composition.SkiaGeometrySource2D; +using Path = SkiaSharp.SKPath; #else using Path = System.Object; #endif @@ -46,7 +46,7 @@ public override void BeginFigure(Point startPoint, bool isFilled) #elif __ANDROID__ bezierPath.MoveTo((float)startPoint.X, (float)startPoint.Y); #elif __SKIA__ - bezierPath.Geometry.MoveTo(new SkiaSharp.SKPoint((float)startPoint.X, (float)startPoint.Y)); + bezierPath.MoveTo(new SkiaSharp.SKPoint((float)startPoint.X, (float)startPoint.Y)); #endif _points.Add(startPoint); @@ -61,7 +61,7 @@ public override void LineTo(Point point, bool isStroked, bool isSmoothJoin) #elif __ANDROID__ bezierPath.LineTo((float)point.X, (float)point.Y); #elif __SKIA__ - bezierPath.Geometry.LineTo((float)point.X, (float)point.Y); + bezierPath.LineTo((float)point.X, (float)point.Y); #endif _points.Add(point); @@ -76,7 +76,7 @@ public override void BezierTo(Point point1, Point point2, Point point3, bool isS #elif __ANDROID__ bezierPath.CubicTo((float)point1.X, (float)point1.Y, (float)point2.X, (float)point2.Y, (float)point3.X, (float)point3.Y); #elif __SKIA__ - bezierPath.Geometry.CubicTo((float)point1.X, (float)point1.Y, (float)point2.X, (float)point2.Y, (float)point3.X, (float)point3.Y); + bezierPath.CubicTo((float)point1.X, (float)point1.Y, (float)point2.X, (float)point2.Y, (float)point3.X, (float)point3.Y); #endif _points.Add(point3); } @@ -97,7 +97,7 @@ public override void QuadraticBezierTo(Point point1, Point point2, bool isStroke #elif __ANDROID__ bezierPath.QuadTo((float)point1.X, (float)point1.Y, (float)point2.X, (float)point2.Y); #elif __SKIA__ - bezierPath.Geometry.QuadTo((float)point1.X, (float)point1.Y, (float)point2.X, (float)point2.Y); + bezierPath.QuadTo((float)point1.X, (float)point1.Y, (float)point2.X, (float)point2.Y); #endif _points.Add(point2); @@ -182,7 +182,7 @@ public override void ArcTo(Point point, Size size, double rotationAngle, bool is sweepAngle -= 360; } - bezierPath.Geometry.ArcTo( + bezierPath.ArcTo( new SkiaSharp.SKRect((float)circle.Left, (float)circle.Top, (float)circle.Right, (float)circle.Bottom), (float)startAngle, (float)sweepAngle, @@ -248,7 +248,7 @@ public override void SetClosedState(bool closed) #elif __ANDROID__ bezierPath.Close(); #elif __SKIA__ - bezierPath.Geometry.Close(); + bezierPath.Close(); #elif __WASM__ // TODO: In most cases, the path is handled by the browser. // But it might still be possible to hit this code path on Wasm? diff --git a/src/Uno.UI/Media/StreamGeometry.cs b/src/Uno.UI/Media/StreamGeometry.cs index a998a2ef43d4..6d4115af8bd2 100644 --- a/src/Uno.UI/Media/StreamGeometry.cs +++ b/src/Uno.UI/Media/StreamGeometry.cs @@ -23,7 +23,8 @@ #elif __ANDROID__ using Android.Graphics; #elif __SKIA__ -using Path = Microsoft.UI.Composition.SkiaGeometrySource2D; +using Microsoft.UI.Composition; +using Path = SkiaSharp.SKPath; using SkiaSharp; using Uno.UI.UI.Xaml.Media; #else @@ -50,16 +51,10 @@ internal void Close(Path bezierPath_) } #if __SKIA__ - internal override Path GetGeometrySource2D() - { - bezierPath.Geometry.FillType = FillRule.ToSkiaFillType(); - return bezierPath; - } - internal override SKPath GetSKPath() { - bezierPath.Geometry.FillType = FillRule.ToSkiaFillType(); - return bezierPath.Geometry; + bezierPath.FillType = FillRule.ToSkiaFillType(); + return bezierPath; } #endif diff --git a/src/Uno.UI/UI/Xaml/Media/Geometry.skia.cs b/src/Uno.UI/UI/Xaml/Media/Geometry.skia.cs index 89ef20bf1fc0..ec2e7b9f3879 100644 --- a/src/Uno.UI/UI/Xaml/Media/Geometry.skia.cs +++ b/src/Uno.UI/UI/Xaml/Media/Geometry.skia.cs @@ -11,6 +11,6 @@ partial class Geometry // this class doesn't have public constructors in UWP, which makes it not-inheritable either way. internal virtual SKPath GetSKPath() => throw new NotSupportedException($"Geometry {this} is not supported"); - internal virtual SkiaGeometrySource2D GetGeometrySource2D() => new SkiaGeometrySource2D(GetSKPath()); + internal virtual SkiaGeometrySource2D GetGeometrySource2D() => new SkiaGeometrySource2D(new SKPath(GetSKPath())); } } diff --git a/src/Uno.UI/UI/Xaml/Shapes/Ellipse.skia.cs b/src/Uno.UI/UI/Xaml/Shapes/Ellipse.skia.cs index fef442ec43fb..7509a2e2b0ec 100644 --- a/src/Uno.UI/UI/Xaml/Shapes/Ellipse.skia.cs +++ b/src/Uno.UI/UI/Xaml/Shapes/Ellipse.skia.cs @@ -25,8 +25,9 @@ protected override Size ArrangeOverride(Size finalSize) private SkiaGeometrySource2D GetGeometry(Rect renderingArea) { - var geometry = new SkiaGeometrySource2D(); - geometry.Geometry.AddOval(new SKRect((float)renderingArea.X, (float)renderingArea.Y, (float)renderingArea.Right, (float)renderingArea.Bottom)); + var path = new SKPath(); + path.AddOval(new SKRect((float)renderingArea.X, (float)renderingArea.Y, (float)renderingArea.Right, (float)renderingArea.Bottom)); + var geometry = new SkiaGeometrySource2D(path); return geometry; } From 95f2fa5d4faaf2fc9a62b3168b3e5073c0867a52 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 23:13:02 +0300 Subject: [PATCH 11/37] chore: remove TransformableObjectExtensions --- .../Uno/TransformableObjectExtensions.cs | 62 ------------------- src/Uno.UI.Composition/Composition/Visual.cs | 2 +- .../Composition/Visual.skia.cs | 29 ++++++++- 3 files changed, 29 insertions(+), 64 deletions(-) delete mode 100644 src/Uno.UI.Composition/Composition/Uno/TransformableObjectExtensions.cs diff --git a/src/Uno.UI.Composition/Composition/Uno/TransformableObjectExtensions.cs b/src/Uno.UI.Composition/Composition/Uno/TransformableObjectExtensions.cs deleted file mode 100644 index e1fe73e687cb..000000000000 --- a/src/Uno.UI.Composition/Composition/Uno/TransformableObjectExtensions.cs +++ /dev/null @@ -1,62 +0,0 @@ -#nullable enable -using System; -using System.Linq; -using System.Numerics; -using Uno.UI.Composition; - -namespace Uno.Extensions; - -internal static class TransformableObjectExtensions -{ - /// - /// Gets the total transformation matrix applied on the object. - /// The resulting matrix only contains transformation related properties, it does **not** include the Offset. - /// - public static Matrix3x2 GetTransform(this I2DTransformableObject transformableObject) - { - var transform = transformableObject.TransformMatrix; - - if (transformableObject.Scale != Vector2.One) - { - transform *= Matrix3x2.CreateScale(transformableObject.Scale, transformableObject.CenterPoint); - } - - if (transformableObject.RotationAngle is not 0) - { - transform *= Matrix3x2.CreateRotation(transformableObject.RotationAngle, transformableObject.CenterPoint); - } - - return transform; - } - - /// - /// Gets the total transformation matrix applied on the object. - /// The resulting matrix only contains transformation related properties, it does **not** include the Offset. - /// - public static Matrix4x4 GetTransform(this I3DTransformableObject transformableObject) - { - var transform = transformableObject.TransformMatrix; - - var scale = transformableObject.Scale; - if (scale != Vector3.One) - { - transform *= Matrix4x4.CreateScale(scale, transformableObject.CenterPoint); - } - - var orientation = transformableObject.Orientation; - if (orientation != Quaternion.Identity) - { - transform *= Matrix4x4.CreateFromQuaternion(orientation); - } - - var rotation = transformableObject.RotationAngle; - if (rotation is not 0) - { - transform *= Matrix4x4.CreateTranslation(-transformableObject.CenterPoint); - transform *= Matrix4x4.CreateFromAxisAngle(transformableObject.RotationAxis, rotation); - transform *= Matrix4x4.CreateTranslation(transformableObject.CenterPoint); - } - - return transform; - } -} diff --git a/src/Uno.UI.Composition/Composition/Visual.cs b/src/Uno.UI.Composition/Composition/Visual.cs index f29297871c26..3852014b15ba 100644 --- a/src/Uno.UI.Composition/Composition/Visual.cs +++ b/src/Uno.UI.Composition/Composition/Visual.cs @@ -10,7 +10,7 @@ namespace Microsoft.UI.Composition { - public partial class Visual : CompositionObject, I3DTransformableObject + public partial class Visual : CompositionObject { private Vector2 _size; private Vector3 _offset; diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 0402ef1a8c50..d5f25810743d 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -113,7 +113,7 @@ internal Matrix4x4 TotalMatrix matrix = offsetMatrix * matrix; // Apply the rending transformation matrix (i.e. change coordinates system to the "rendering" one) - if (this.GetTransform() is { IsIdentity: false } transform) + if (GetTransform() is { IsIdentity: false } transform) { matrix = transform * matrix; } @@ -123,6 +123,33 @@ internal Matrix4x4 TotalMatrix } return _totalMatrix; + + Matrix4x4 GetTransform() + { + var transform = TransformMatrix; + + var scale = Scale; + if (scale != Vector3.One) + { + transform *= Matrix4x4.CreateScale(scale, CenterPoint); + } + + var orientation = Orientation; + if (orientation != Quaternion.Identity) + { + transform *= Matrix4x4.CreateFromQuaternion(orientation); + } + + var rotation = RotationAngle; + if (rotation is not 0) + { + transform *= Matrix4x4.CreateTranslation(-CenterPoint); + transform *= Matrix4x4.CreateFromAxisAngle(RotationAxis, rotation); + transform *= Matrix4x4.CreateTranslation(CenterPoint); + } + + return transform; + } } } From 24c420d615f09dcdce31823816160de608411fc9 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Tue, 30 Jul 2024 23:13:24 +0300 Subject: [PATCH 12/37] chore: remove empty CompositionPath.skia.cs partial --- .../Composition/CompositionPath.skia.cs | 11 ------ .../Composition/CompositionShape.skia.cs | 34 ++++++++++++++++++- .../CompositionSpriteShape.skia.cs | 2 +- 3 files changed, 34 insertions(+), 13 deletions(-) delete mode 100644 src/Uno.UI.Composition/Composition/CompositionPath.skia.cs diff --git a/src/Uno.UI.Composition/Composition/CompositionPath.skia.cs b/src/Uno.UI.Composition/Composition/CompositionPath.skia.cs deleted file mode 100644 index 0526a545b110..000000000000 --- a/src/Uno.UI.Composition/Composition/CompositionPath.skia.cs +++ /dev/null @@ -1,11 +0,0 @@ -#nullable enable - -using SkiaSharp; -using System; - -namespace Microsoft.UI.Composition -{ - public partial class CompositionPath - { - } -} diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index d87b05279fee..572c6046071c 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -11,10 +11,42 @@ namespace Microsoft.UI.Composition; public partial class CompositionShape { + private Matrix3x2 _combinedTransformMatrix; + + protected Matrix3x2 CombinedTransformMatrix + { + get => _combinedTransformMatrix; + private set => SetProperty(ref _combinedTransformMatrix, value); + } + + private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) + { + base.OnPropertyChangedCore(propertyName, isSubPropertyChange); + + switch (propertyName) + { + case nameof(TransformMatrix) or nameof(Scale) or nameof(RotationAngle) or nameof(CenterPoint): + var transform = TransformMatrix; + + if (Scale != Vector2.One) + { + transform *= Matrix3x2.CreateScale(Scale, CenterPoint); + } + + if (RotationAngle is not 0) + { + transform *= Matrix3x2.CreateRotation(RotationAngle, CenterPoint); + } + + CombinedTransformMatrix = transform; + break; + } + } + internal virtual void Render(in Visual.PaintingSession session) { var offset = Offset; - var transform = this.GetTransform(); + var transform = CombinedTransformMatrix; if (offset != Vector2.Zero || transform is not { IsIdentity: true }) { diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 1c2891c1e9b0..49ac73ef0baf 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -25,7 +25,7 @@ internal override void Paint(in Visual.PaintingSession session) if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) { - var transform = this.GetTransform(); + var transform = CombinedTransformMatrix; SKPath geometryWithTransformations; if (transform.IsIdentity) { From 9f7f314e78b74a00caecc48054cad05812301336 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 00:00:43 +0300 Subject: [PATCH 13/37] chore: fix invalid CombinedTransformMatrix initial value --- src/Uno.UI.Composition/Composition/CompositionShape.skia.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index 572c6046071c..bd7b7bc4f663 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -11,7 +11,7 @@ namespace Microsoft.UI.Composition; public partial class CompositionShape { - private Matrix3x2 _combinedTransformMatrix; + private Matrix3x2 _combinedTransformMatrix = Matrix3x2.Identity; protected Matrix3x2 CombinedTransformMatrix { From 4cc9871cdb79d72faffa30da35e861c809ba19b9 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 00:01:19 +0300 Subject: [PATCH 14/37] chore: make SkiaGeometrySource2D read-only --- .../Composition/SkiaGeometrySource2D.skia.cs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs b/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs index 65a52055ba2f..e1872a244684 100644 --- a/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs +++ b/src/Uno.UI.Composition/Composition/SkiaGeometrySource2D.skia.cs @@ -6,25 +6,17 @@ namespace Microsoft.UI.Composition { - internal class SkiaGeometrySource2D : CompositionObject, IGeometrySource2D + internal class SkiaGeometrySource2D : IGeometrySource2D { - private SKPath _geometry; - -#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. public SkiaGeometrySource2D(SKPath source) -#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. { Geometry = source ?? throw new ArgumentNullException(nameof(source)); } /// - /// DO NOT MODIFY THIS SKPath. CREATE A NEW SKPath INSTEAD. + /// DO NOT MODIFY THIS SKPath. CREATE A NEW SkiaGeometrySource2D INSTEAD. /// This can lead to nasty invalidation bugs where the SKPath changes without notifying anyone. /// - public SKPath Geometry - { - get => _geometry; - set => SetObjectProperty(ref _geometry, value); - } + public SKPath Geometry { get; } } } From ef85c295a21cc2881eeeb0124cae888bea24e541 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 00:03:49 +0300 Subject: [PATCH 15/37] chore: cache CompositionSpriteShape calculations geometry calculations --- .../CompositionSpriteShape.skia.cs | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 49ac73ef0baf..85b2a3ed77cf 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -14,6 +14,7 @@ public partial class CompositionSpriteShape : CompositionShape { private SKPaint? _strokePaint; private SKPaint? _fillPaint; + private SKPath? _geometryWithTransformations; internal SKPath? LastDrawnStrokePath { get; private set; } internal SKPath? LastDrawnFillPath { get; private set; } @@ -23,20 +24,8 @@ internal override void Paint(in Visual.PaintingSession session) LastDrawnStrokePath = null; LastDrawnFillPath = null; - if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) + if (_geometryWithTransformations is { } geometryWithTransformations) { - var transform = CombinedTransformMatrix; - SKPath geometryWithTransformations; - if (transform.IsIdentity) - { - geometryWithTransformations = geometry; - } - else - { - geometryWithTransformations = new SKPath(); - geometry.Transform(transform.ToSKMatrix(), geometryWithTransformations); - } - if (FillBrush is { } fill) { var fillPaint = TryCreateAndClearFillPaint(in session); @@ -156,6 +145,37 @@ private static SKPaint TryCreateAndClearPaint(in Visual.PaintingSession session, return paint; } + private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) + { + base.OnPropertyChangedCore(propertyName, isSubPropertyChange); + + switch (propertyName) + { + case nameof(Geometry) or nameof(CombinedTransformMatrix): + if (Geometry?.BuildGeometry() is SkiaGeometrySource2D { Geometry: { } geometry }) + { + var transform = CombinedTransformMatrix; + SKPath geometryWithTransformations; + if (transform.IsIdentity) + { + geometryWithTransformations = geometry; + } + else + { + geometryWithTransformations = new SKPath(); + geometry.Transform(transform.ToSKMatrix(), geometryWithTransformations); + } + + _geometryWithTransformations = geometryWithTransformations; + } + else + { + _geometryWithTransformations = null; + } + break; + } + } + internal override bool HitTest(Point point) { return (LastDrawnStrokePath?.Contains((float)point.X, (float)point.Y) ?? false) From 2f4e384ae45c56073bfd90ec8b988a8ace5ae262 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 00:20:32 +0300 Subject: [PATCH 16/37] chore: reimplement Shape.HitTest without depending on painting --- .../CompositionSpriteShape.skia.cs | 56 +++++++++++-------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 85b2a3ed77cf..3be12c572a2e 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -1,12 +1,9 @@ #nullable enable -using System; -using System.Linq; using Windows.Foundation; using SkiaSharp; using Uno; using Uno.Extensions; -using Uno.UI.Composition; namespace Microsoft.UI.Composition { @@ -16,19 +13,14 @@ public partial class CompositionSpriteShape : CompositionShape private SKPaint? _fillPaint; private SKPath? _geometryWithTransformations; - internal SKPath? LastDrawnStrokePath { get; private set; } - internal SKPath? LastDrawnFillPath { get; private set; } - internal override void Paint(in Visual.PaintingSession session) { - LastDrawnStrokePath = null; - LastDrawnFillPath = null; - if (_geometryWithTransformations is { } geometryWithTransformations) { if (FillBrush is { } fill) { - var fillPaint = TryCreateAndClearFillPaint(in session); + var fillPaint = TryCreateAndClearFillPaint(); + fillPaint.ColorFilter = session.Filters.OpacityColorFilter; if (Compositor.TryGetEffectiveBackgroundColor(this, out var colorFromTransition)) { @@ -39,8 +31,6 @@ internal override void Paint(in Visual.PaintingSession session) fill.UpdatePaint(fillPaint, geometryWithTransformations.Bounds); } - LastDrawnFillPath = geometryWithTransformations; - if (fill is CompositionBrushWrapper wrapper) { fill = wrapper.WrappedBrush; @@ -63,8 +53,10 @@ internal override void Paint(in Visual.PaintingSession session) if (StrokeBrush is { } stroke && StrokeThickness > 0) { - var fillPaint = TryCreateAndClearFillPaint(in session); - var strokePaint = TryCreateAndClearStrokePaint(in session); + var fillPaint = TryCreateAndClearFillPaint(); + var strokePaint = TryCreateAndClearStrokePaint(); + fillPaint.ColorFilter = session.Filters.OpacityColorFilter; + strokePaint.ColorFilter = session.Filters.OpacityColorFilter; // Set stroke thickness strokePaint.StrokeWidth = StrokeThickness; @@ -94,19 +86,18 @@ internal override void Paint(in Visual.PaintingSession session) stroke.UpdatePaint(fillPaint, strokeGeometry.Bounds); - LastDrawnStrokePath = strokeGeometry; session.Canvas.DrawPath(strokeGeometry, fillPaint); } } } - private SKPaint TryCreateAndClearStrokePaint(in Visual.PaintingSession session) - => TryCreateAndClearPaint(in session, ref _strokePaint, true, CompositionConfiguration.UseBrushAntialiasing); + private SKPaint TryCreateAndClearStrokePaint() + => TryCreateAndClearPaint(ref _strokePaint, true, CompositionConfiguration.UseBrushAntialiasing); - private SKPaint TryCreateAndClearFillPaint(in Visual.PaintingSession session) - => TryCreateAndClearPaint(in session, ref _fillPaint, false, CompositionConfiguration.UseBrushAntialiasing); + private SKPaint TryCreateAndClearFillPaint() + => TryCreateAndClearPaint(ref _fillPaint, false, CompositionConfiguration.UseBrushAntialiasing); - private static SKPaint TryCreateAndClearPaint(in Visual.PaintingSession session, ref SKPaint? paint, bool isStroke, bool isHighQuality = false) + private static SKPaint TryCreateAndClearPaint(ref SKPaint? paint, bool isStroke, bool isHighQuality = false) { if (paint == null) { @@ -140,8 +131,6 @@ private static SKPaint TryCreateAndClearPaint(in Visual.PaintingSession session, } } - paint.ColorFilter = session.Filters.OpacityColorFilter; - return paint; } @@ -178,8 +167,27 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool internal override bool HitTest(Point point) { - return (LastDrawnStrokePath?.Contains((float)point.X, (float)point.Y) ?? false) - || (LastDrawnFillPath?.Contains((float)point.X, (float)point.Y) ?? false); + point = CombinedTransformMatrix.Inverse().Transform(point); + + if (_geometryWithTransformations is { } geometryWithTransformations) + { + if (FillBrush is { } && geometryWithTransformations.Contains((float)point.X, (float)point.Y)) + { + return true; + } + + if (StrokeBrush is { } stroke && StrokeThickness > 0) + { + var strokePaint = TryCreateAndClearStrokePaint(); + strokePaint.StrokeWidth = StrokeThickness; + var strokeGeometry = strokePaint.GetFillPath(geometryWithTransformations); + if (strokeGeometry.Contains((float)point.X, (float)point.Y)) + { + return true; + } + } + } + return false; } } } From 297f0f7b0ed10c6beb9427632deb48b321a8a08c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 00:28:20 +0300 Subject: [PATCH 17/37] chore: refactor BorderVisual to work with the new changes --- .../Composition/BorderVisual.skia.cs | 79 ++++++++++--------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs index e7b29c88187a..344daa3a177d 100644 --- a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs @@ -13,9 +13,6 @@ namespace Microsoft.UI.Composition; /// internal class BorderVisual(Compositor compositor) : ShapeVisual(compositor) { - [ThreadStatic] // this should be tied to the compositor - private static CompositionPathGeometry? _sharedPathGeometry; - // state set from outside and used inside the class private CornerRadius _cornerRadius; private Thickness _borderThickness; @@ -28,12 +25,17 @@ internal class BorderVisual(Compositor compositor) : ShapeVisual(compositor) private CompositionSpriteShape? _backgroundShape; private CompositionSpriteShape? _borderShape; private CompositionClip? _backgroundClip; - private SKPath? _borderPath; - private SKPath? _backgroundPath; // state set here but affects children private RectangleClip? _childClipCausedByCornerRadius; - internal CompositionSpriteShape? BackgroundShape => _backgroundShape; + internal CompositionSpriteShape? BackgroundShape + { + get => _backgroundShape; + private set => SetProperty(ref _backgroundShape, value); + } + + // We do this instead of a direct SetProperty call so that SetProperty automatically gets an accurate propertyName + private CompositionSpriteShape? BorderShape { set => SetProperty(ref _borderShape, value); } public CornerRadius CornerRadius { @@ -82,8 +84,11 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool _borderPathValid = false; // to update _borderPath if previously skipped if (BorderBrush is not null && _borderShape is null) { - // we need this to track get notified on brush updates. - SetProperty(ref _borderShape, Compositor.CreateSpriteShape()); + // we need this to get notified on brush updates + // (BorderBrush internals change -> BorderShape is notified through FillBrush -> render invalidation) + BorderShape = Compositor.CreateSpriteShape(); + + _borderShape!.Geometry = Compositor.CreatePathGeometry(); #if DEBUG _borderShape!.Comment = "#borderShape"; #endif @@ -97,8 +102,11 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool _backgroundPathValid = false; // to update _backgroundPath if previously skipped if (BackgroundBrush is not null && _backgroundShape is null) { - // we need this to track get notified on brush updates. - SetProperty(ref _backgroundShape, Compositor.CreateSpriteShape()); + // we need this to get notified on brush updates. + // (BackgroundBrush internals change -> BackgroundShape is notified through FillBrush -> render invalidation) + BackgroundShape = Compositor.CreateSpriteShape(); + + _backgroundShape!.Geometry = Compositor.CreatePathGeometry(); #if DEBUG _backgroundShape!.Comment = "#backgroundShape"; #endif @@ -113,16 +121,10 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool internal override void Paint(in PaintingSession session) { - _sharedPathGeometry ??= Compositor.CreatePathGeometry(); - _sharedPathGeometry.Path ??= new CompositionPath(new SkiaGeometrySource2D()); - var geometrySource = (SkiaGeometrySource2D)_sharedPathGeometry.Path.GeometrySource; - UpdatePathsAndCornerClip(); - if (_backgroundShape is { } backgroundShape && _backgroundPath is { } backgroundPath) + if (_backgroundShape is { } backgroundShape) { - backgroundShape.Geometry = _sharedPathGeometry; // will only do something the first time - geometrySource.Geometry = backgroundPath; // changing Geometry doesn't raise OnPropertyChanged or invalidate render. session.Canvas.Save(); // it's necessary to clip the background because not all backgrounds are simple rounded rectangles with a solid color. // E.g. effect brushes will draw outside the intended area if they're not clipped. @@ -133,12 +135,7 @@ internal override void Paint(in PaintingSession session) base.Paint(in session); - if (_borderShape is { } borderShape && _borderPath is { } borderPath) - { - borderShape.Geometry = _sharedPathGeometry; // will only do something the first time - geometrySource.Geometry = borderPath; // changing Geometry doesn't raise OnPropertyChanged or invalidate render. - _borderShape?.Render(in session); - } + _borderShape?.Render(in session); } private protected override void ApplyPostPaintingClipping(in SKCanvas canvas) @@ -255,8 +252,8 @@ private void UpdatePathsAndCornerClip() private unsafe void UpdateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackground, SKSize innerArea, SKSize outerArea, SKPoint* outerRadii, SKPoint* innerRadii) { - _backgroundPath ??= new SKPath(); - _backgroundPath.Reset(); + var backgroundPath = new SKPath(); + backgroundPath.Reset(); var roundRect = new SKRoundRect(); var rect = useInnerBorderBoundsAsAreaForBackground ? new SKRect(0, 0, innerArea.Width, innerArea.Height) @@ -265,32 +262,42 @@ private unsafe void UpdateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackg roundRect.Handle, &rect, useInnerBorderBoundsAsAreaForBackground ? innerRadii : outerRadii); - _backgroundPath.AddRoundRect(roundRect); - _backgroundPath.Close(); + backgroundPath.AddRoundRect(roundRect); + backgroundPath.Close(); + + // Unfortunately, this will cause an unnecessary render invalidation + ((CompositionPathGeometry)_backgroundShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(backgroundPath)); } private unsafe void UpdateBorderPath(SKRect innerArea, SKRect outerArea, SKPoint* outerRadii, SKPoint* innerRadii) { - _borderPath ??= new SKPath(); - _borderPath.Reset(); + var borderPath = new SKPath(); - // It's important to set this every time, since borderPathGeometry.Reset will reset it. - _borderPath.FillType = SKPathFillType.EvenOdd; + borderPath.FillType = SKPathFillType.EvenOdd; // The order here (outer then inner) is important because of the SKPathFillType. { var outerRect = new SKRoundRect(); UnoSkiaApi.sk_rrect_set_rect_radii(outerRect.Handle, &outerArea, outerRadii); - _borderPath.AddRoundRect(outerRect); - _borderPath.Close(); + borderPath.AddRoundRect(outerRect); + borderPath.Close(); } { var innerRect = new SKRoundRect(); UnoSkiaApi.sk_rrect_set_rect_radii(innerRect.Handle, &innerArea, innerRadii); - _borderPath.AddRoundRect(innerRect); - _borderPath.Close(); + borderPath.AddRoundRect(innerRect); + borderPath.Close(); } + + // Unfortunately, this will cause an unnecessary render invalidation + ((CompositionPathGeometry)_borderShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(borderPath)); } - internal override bool HitTest(Point point) => (_borderShape?.HitTest(point) ?? false) || (_backgroundShape?.HitTest(point) ?? false) || base.HitTest(point); + internal override bool HitTest(Point point) + { + UpdatePathsAndCornerClip(); + return (_borderShape?.HitTest(point) ?? false) || + (_backgroundShape?.HitTest(point) ?? false) || + base.HitTest(point); + } } From 873d11c219ff08d4052d9c4d1c6de65c5d75b8bd Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 12:53:38 +0300 Subject: [PATCH 18/37] chore: formatting --- .../Composition/BorderVisual.skia.cs | 10 ++++------ .../Composition/CompositionShape.skia.cs | 2 +- .../Composition/CompositionSpriteShape.skia.cs | 2 +- src/Uno.UI.Composition/Composition/Visual.skia.cs | 2 +- .../Tests/Windows_UI_Xaml_Shapes/When_Shape.cs | 2 +- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs index 344daa3a177d..11dd2a149716 100644 --- a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs @@ -87,7 +87,7 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool // we need this to get notified on brush updates // (BorderBrush internals change -> BorderShape is notified through FillBrush -> render invalidation) BorderShape = Compositor.CreateSpriteShape(); - + _borderShape!.Geometry = Compositor.CreatePathGeometry(); #if DEBUG _borderShape!.Comment = "#borderShape"; @@ -105,7 +105,7 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool // we need this to get notified on brush updates. // (BackgroundBrush internals change -> BackgroundShape is notified through FillBrush -> render invalidation) BackgroundShape = Compositor.CreateSpriteShape(); - + _backgroundShape!.Geometry = Compositor.CreatePathGeometry(); #if DEBUG _backgroundShape!.Comment = "#backgroundShape"; @@ -264,7 +264,7 @@ private unsafe void UpdateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackg useInnerBorderBoundsAsAreaForBackground ? innerRadii : outerRadii); backgroundPath.AddRoundRect(roundRect); backgroundPath.Close(); - + // Unfortunately, this will cause an unnecessary render invalidation ((CompositionPathGeometry)_backgroundShape!.Geometry!).Path = new CompositionPath(new SkiaGeometrySource2D(backgroundPath)); } @@ -296,8 +296,6 @@ private unsafe void UpdateBorderPath(SKRect innerArea, SKRect outerArea, SKPoint internal override bool HitTest(Point point) { UpdatePathsAndCornerClip(); - return (_borderShape?.HitTest(point) ?? false) || - (_backgroundShape?.HitTest(point) ?? false) || - base.HitTest(point); + return (_borderShape?.HitTest(point) ?? false) || (_backgroundShape?.HitTest(point) ?? false) || base.HitTest(point); } } diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index bd7b7bc4f663..06390f8b5d83 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -18,7 +18,7 @@ protected Matrix3x2 CombinedTransformMatrix get => _combinedTransformMatrix; private set => SetProperty(ref _combinedTransformMatrix, value); } - + private protected override void OnPropertyChangedCore(string? propertyName, bool isSubPropertyChange) { base.OnPropertyChangedCore(propertyName, isSubPropertyChange); diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 3be12c572a2e..6e9d9a936d83 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -175,7 +175,7 @@ internal override bool HitTest(Point point) { return true; } - + if (StrokeBrush is { } stroke && StrokeThickness > 0) { var strokePaint = TryCreateAndClearStrokePaint(); diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index d5f25810743d..216a801973f0 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -123,7 +123,7 @@ internal Matrix4x4 TotalMatrix } return _totalMatrix; - + Matrix4x4 GetTransform() { var transform = TransformMatrix; diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs index feace29512b3..c2a03347cc8e 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs @@ -271,7 +271,7 @@ public async Task When_Shapes_HitTesting() await UITestHelper.Load(root); // points are relative to root - var pointToTarget = new Dictionary + var pointToTarget = new Dictionary { { new(48, 32), "RectangleFilled" }, { new(31, 27), "RectangleFilled" }, From 0844dac2ada1a5ce1a0212b4447f7f20d8a01cd0 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 31 Jul 2024 18:40:27 +0300 Subject: [PATCH 19/37] chore: formatting --- .../Windows_UI_Xaml_Shapes/When_Shape.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs index c2a03347cc8e..5eb8d722732f 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Shapes/When_Shape.cs @@ -102,7 +102,7 @@ public async Task When_Shapes_HitTesting() var root = new Grid { Name = "Root", - Background = Colors.Transparent, + Background = Microsoft.UI.Colors.Transparent, Width = 200, HorizontalAlignment = HorizontalAlignment.Left, VerticalAlignment = VerticalAlignment.Top, @@ -128,8 +128,8 @@ public async Task When_Shapes_HitTesting() Name = "RectangleFilled", Width = 50, Height = 50, - Fill = Colors.Red.WithOpacity(0.5), - Stroke = Colors.Red, + Fill = Microsoft.UI.Colors.Red.WithOpacity(0.5), + Stroke = Microsoft.UI.Colors.Red, StrokeThickness = 6 }.Apply(r => r.GridPosition(0, 0)), new Rectangle @@ -138,7 +138,7 @@ public async Task When_Shapes_HitTesting() Width = 50, Height = 50, Fill = null, - Stroke = Colors.Red, + Stroke = Microsoft.UI.Colors.Red, StrokeThickness = 6 }.Apply(r => r.GridPosition(0, 1)), new Ellipse @@ -146,8 +146,8 @@ public async Task When_Shapes_HitTesting() Name = "EllipseFilled", Width = 50, Height = 50, - Fill = Colors.Orange.WithOpacity(0.5), - Stroke = Colors.Orange, + Fill = Microsoft.UI.Colors.Orange.WithOpacity(0.5), + Stroke = Microsoft.UI.Colors.Orange, StrokeThickness = 6 }.Apply(r => r.GridPosition(1, 0)), new Ellipse @@ -156,7 +156,7 @@ public async Task When_Shapes_HitTesting() Width = 50, Height = 50, Fill = null, - Stroke = Colors.Orange, + Stroke = Microsoft.UI.Colors.Orange, StrokeThickness = 6 }.Apply(r => r.GridPosition(1, 1)), new Line @@ -164,7 +164,7 @@ public async Task When_Shapes_HitTesting() Name = "LineFilled", Width = 50, Height = 50, - Stroke = Colors.Yellow, + Stroke = Microsoft.UI.Colors.Yellow, StrokeThickness = 20, X1 = 0, Y1 = 0, @@ -188,8 +188,8 @@ public async Task When_Shapes_HitTesting() Name = "PathFilled", Width = 50, Height = 50, - Fill = Colors.Green.WithOpacity(0.5), - Stroke = Colors.Green, + Fill = Microsoft.UI.Colors.Green.WithOpacity(0.5), + Stroke = Microsoft.UI.Colors.Green, StrokeThickness = 6, Data = new GeometryGroup { @@ -210,7 +210,7 @@ public async Task When_Shapes_HitTesting() Width = 50, Height = 50, Fill = null, - Stroke = Colors.Green, + Stroke = Microsoft.UI.Colors.Green, StrokeThickness = 6, Data = new GeometryGroup { @@ -230,8 +230,8 @@ public async Task When_Shapes_HitTesting() Name = "PolygonFilled", Width = 50, Height = 50, - Fill = Colors.Blue.WithOpacity(0.5), - Stroke = Colors.Blue, + Fill = Microsoft.UI.Colors.Blue.WithOpacity(0.5), + Stroke = Microsoft.UI.Colors.Blue, StrokeThickness = 6, Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) }.Apply(r => r.GridPosition(4, 0)), @@ -241,7 +241,7 @@ public async Task When_Shapes_HitTesting() Width = 50, Height = 50, Fill = null, - Stroke = Colors.Blue, + Stroke = Microsoft.UI.Colors.Blue, StrokeThickness = 6, Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) }.Apply(r => r.GridPosition(4, 1)), @@ -250,8 +250,8 @@ public async Task When_Shapes_HitTesting() Name = "PolylineFilled", Width = 50, Height = 50, - Fill = Colors.Purple.WithOpacity(0.5), - Stroke = Colors.Purple, + Fill = Microsoft.UI.Colors.Purple.WithOpacity(0.5), + Stroke = Microsoft.UI.Colors.Purple, StrokeThickness = 6, Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) }.Apply(r => r.GridPosition(5, 0)), @@ -261,7 +261,7 @@ public async Task When_Shapes_HitTesting() Width = 50, Height = 50, Fill = null, - Stroke = Colors.Purple, + Stroke = Microsoft.UI.Colors.Purple, StrokeThickness = 6, Points = new PointCollection(new [] { new Point(0, 0), new Point(0, 50), new Point(50, 50) }) }.Apply(r => r.GridPosition(5, 1)), From 0668fab2d84df6a2a50e3fcd2d30d3d85b364d1c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Thu, 1 Aug 2024 13:56:38 +0300 Subject: [PATCH 20/37] chore: fix scrollviewer scrolling --- .../ScrollContentPresenter.Managed.cs | 8 ------- .../ScrollContentPresenter.cs | 24 +++++++++++++++++++ .../ScrollViewer/ScrollViewer.Managed.cs | 6 ++++- .../Controls/ScrollViewer/ScrollViewer.cs | 13 ++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs index 95a5f566ac93..68c8d3be978e 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs @@ -65,15 +65,7 @@ partial void InitializePartial() _strategy.Initialize(this); - // Mouse wheel support - PointerWheelChanged += PointerWheelScroll; - - // Touch scroll support ManipulationMode = ManipulationModes.TranslateX | ManipulationModes.TranslateY; // Updated in PrepareTouchScroll! - ManipulationStarting += PrepareTouchScroll; - ManipulationStarted += TouchScrollStarted; - ManipulationDelta += UpdateTouchScroll; - ManipulationCompleted += CompleteTouchScroll; } /// diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index 1855b752c39f..645419e321ec 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -235,6 +235,30 @@ internal override bool IsViewHit() => new Rect(default, RenderSize); #endif + internal void HookScrollEvents(ScrollViewer sv) + { + // Mouse wheel support + sv.PointerWheelChanged += PointerWheelScroll; + + // Touch scroll support + sv.ManipulationStarting += PrepareTouchScroll; + sv.ManipulationStarted += TouchScrollStarted; + sv.ManipulationDelta += UpdateTouchScroll; + sv.ManipulationCompleted += CompleteTouchScroll; + } + + internal void UnhookScrollEvents(ScrollViewer sv) + { + // Mouse wheel support + sv.PointerWheelChanged -= PointerWheelScroll; + + // Touch scroll support + sv.ManipulationStarting -= PrepareTouchScroll; + sv.ManipulationStarted -= TouchScrollStarted; + sv.ManipulationDelta -= UpdateTouchScroll; + sv.ManipulationCompleted -= CompleteTouchScroll; + } + private void PointerWheelScroll(object sender, Input.PointerRoutedEventArgs e) { var properties = e.GetCurrentPoint(null).Properties; diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs index 9d507d5b7d2f..fb259ce23571 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs @@ -24,7 +24,11 @@ private bool ChangeViewNative(double? horizontalOffset, double? verticalOffset, => (_presenter as ScrollContentPresenter)?.Set(horizontalOffset, verticalOffset, disableAnimation: disableAnimation) ?? true; private partial void OnLoadedPartial() { } - private partial void OnUnloadedPartial() { } + + private partial void OnUnloadedPartial() + { + Presenter?.UnhookScrollEvents(this); + } #region Over scroll support /// diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs index 31afe45d0222..e4e6e9068ee5 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs @@ -1012,10 +1012,23 @@ protected override void OnApplyTemplate() } #endif +#if UNO_HAS_MANAGED_SCROLL_PRESENTER + Presenter?.UnhookScrollEvents(this); +#endif if (scpTemplatePart is ScrollContentPresenter presenter) { presenter.ScrollOwner = this; Presenter = presenter; + + // Note: the way WinUI does scrolling is very different, and doesn't use + // PointerWheelChanged changes, etc. + // We can either subscribe on the ScrollViewer or the SCP directly, but due to + // the way hit-testing works (see #16201), the SCP will not receive any pointer + // events. On WinUI, this is also the case: pointer presses are received on the SV, + // not on the SCP. +#if UNO_HAS_MANAGED_SCROLL_PRESENTER + presenter.HookScrollEvents(this); +#endif } else { From 631da28c698ef2ab98ffb17c2600bf3985de532c Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Thu, 1 Aug 2024 14:22:41 +0300 Subject: [PATCH 21/37] chore: fix popuppanel hit-testing --- src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs index ad96ac44f360..23f3b0ca1a7c 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs @@ -295,17 +295,15 @@ private void ApplyLightDismissOverlayMode() /// private Brush GetPanelBackground() { + // In all cases, we need the background to not be null so that it can receive pointer events, or else it + // will fail hit-testing. if (ShouldShowLightDismissOverlay) { - return LightDismissOverlayBackground; - } - else if (IsLightDismissEnabled) - { - return SolidColorBrushHelper.Transparent; + return LightDismissOverlayBackground ?? SolidColorBrushHelper.Transparent; } else { - return null; + return SolidColorBrushHelper.Transparent; } } From 10aa77ca5fd51393ec86f5690c77c33e46c19dcc Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Thu, 1 Aug 2024 15:38:53 +0300 Subject: [PATCH 22/37] chore: formatting --- .../ScrollContentPresenter/ScrollContentPresenter.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index 645419e321ec..98363e7c6b57 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -244,9 +244,9 @@ internal void HookScrollEvents(ScrollViewer sv) sv.ManipulationStarting += PrepareTouchScroll; sv.ManipulationStarted += TouchScrollStarted; sv.ManipulationDelta += UpdateTouchScroll; - sv.ManipulationCompleted += CompleteTouchScroll; + sv.ManipulationCompleted += CompleteTouchScroll; } - + internal void UnhookScrollEvents(ScrollViewer sv) { // Mouse wheel support @@ -256,7 +256,7 @@ internal void UnhookScrollEvents(ScrollViewer sv) sv.ManipulationStarting -= PrepareTouchScroll; sv.ManipulationStarted -= TouchScrollStarted; sv.ManipulationDelta -= UpdateTouchScroll; - sv.ManipulationCompleted -= CompleteTouchScroll; + sv.ManipulationCompleted -= CompleteTouchScroll; } private void PointerWheelScroll(object sender, Input.PointerRoutedEventArgs e) From 5dd383b51c7c50942291b7b59b2e71a14e86a132 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 2 Aug 2024 12:09:21 +0300 Subject: [PATCH 23/37] chore: build error --- .../Controls/ScrollContentPresenter/ScrollContentPresenter.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index 98363e7c6b57..fc213b3683db 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -235,6 +235,7 @@ internal override bool IsViewHit() => new Rect(default, RenderSize); #endif +#if UNO_HAS_MANAGED_SCROLL_PRESENTER internal void HookScrollEvents(ScrollViewer sv) { // Mouse wheel support @@ -258,6 +259,7 @@ internal void UnhookScrollEvents(ScrollViewer sv) sv.ManipulationDelta -= UpdateTouchScroll; sv.ManipulationCompleted -= CompleteTouchScroll; } +#endif private void PointerWheelScroll(object sender, Input.PointerRoutedEventArgs e) { From b298f453ae306b944470918797b9222a2fc4ce4b Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 2 Aug 2024 14:33:22 +0300 Subject: [PATCH 24/37] chore: fix touch scrolling firing Released --- .../ScrollContentPresenter.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index fc213b3683db..36e988787c88 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -242,22 +242,20 @@ internal void HookScrollEvents(ScrollViewer sv) sv.PointerWheelChanged += PointerWheelScroll; // Touch scroll support - sv.ManipulationStarting += PrepareTouchScroll; - sv.ManipulationStarted += TouchScrollStarted; - sv.ManipulationDelta += UpdateTouchScroll; - sv.ManipulationCompleted += CompleteTouchScroll; + ManipulationStarting += PrepareTouchScroll; + ManipulationStarted += TouchScrollStarted; + ManipulationDelta += UpdateTouchScroll; + ManipulationCompleted += CompleteTouchScroll; } internal void UnhookScrollEvents(ScrollViewer sv) { - // Mouse wheel support sv.PointerWheelChanged -= PointerWheelScroll; - - // Touch scroll support - sv.ManipulationStarting -= PrepareTouchScroll; - sv.ManipulationStarted -= TouchScrollStarted; - sv.ManipulationDelta -= UpdateTouchScroll; - sv.ManipulationCompleted -= CompleteTouchScroll; + + ManipulationStarting -= PrepareTouchScroll; + ManipulationStarted -= TouchScrollStarted; + ManipulationDelta -= UpdateTouchScroll; + ManipulationCompleted -= CompleteTouchScroll; } #endif From 87c58eb444774544006b17965068b9f4a76df468 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 2 Aug 2024 16:53:09 +0300 Subject: [PATCH 25/37] chore: formatting --- .../ScrollContentPresenter/ScrollContentPresenter.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index 36e988787c88..c37b5b09fa88 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -251,11 +251,11 @@ internal void HookScrollEvents(ScrollViewer sv) internal void UnhookScrollEvents(ScrollViewer sv) { sv.PointerWheelChanged -= PointerWheelScroll; - + ManipulationStarting -= PrepareTouchScroll; - ManipulationStarted -= TouchScrollStarted; - ManipulationDelta -= UpdateTouchScroll; - ManipulationCompleted -= CompleteTouchScroll; + ManipulationStarted -= TouchScrollStarted; + ManipulationDelta -= UpdateTouchScroll; + ManipulationCompleted -= CompleteTouchScroll; } #endif From 503ee415ccb571e0a05a16d8c5af5b754b17058b Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 00:21:24 +0300 Subject: [PATCH 26/37] chore: build errors --- src/Uno.UI.Composition/Composition/CompositionShape.skia.cs | 2 +- src/Uno.UI.Composition/Composition/Visual.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs index 06390f8b5d83..6f49851c17e6 100644 --- a/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionShape.skia.cs @@ -13,7 +13,7 @@ public partial class CompositionShape { private Matrix3x2 _combinedTransformMatrix = Matrix3x2.Identity; - protected Matrix3x2 CombinedTransformMatrix + private protected Matrix3x2 CombinedTransformMatrix { get => _combinedTransformMatrix; private set => SetProperty(ref _combinedTransformMatrix, value); diff --git a/src/Uno.UI.Composition/Composition/Visual.cs b/src/Uno.UI.Composition/Composition/Visual.cs index 3852014b15ba..f29297871c26 100644 --- a/src/Uno.UI.Composition/Composition/Visual.cs +++ b/src/Uno.UI.Composition/Composition/Visual.cs @@ -10,7 +10,7 @@ namespace Microsoft.UI.Composition { - public partial class Visual : CompositionObject + public partial class Visual : CompositionObject, I3DTransformableObject { private Vector2 _size; private Vector3 _offset; From b05fe071e53e9f12a6d3025b3a77412ea35d5402 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 16:40:10 +0300 Subject: [PATCH 27/37] chore: fix shapes hit-testing on non-skia --- src/Uno.UI/UI/Xaml/Shapes/Shape.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs index 8742c0746f28..1849f6bfdf84 100644 --- a/src/Uno.UI/UI/Xaml/Shapes/Shape.cs +++ b/src/Uno.UI/UI/Xaml/Shapes/Shape.cs @@ -168,8 +168,13 @@ public DoubleCollection StrokeDashArray ); #endregion + // Do not invoke base.IsViewHit(): We don't have to have de FrameworkElement.Background to be hit testable! internal override bool IsViewHit() - => Fill != null || Stroke != null; // Do not invoke base.IsViewHit(): We don't have to have de FrameworkElement.Background to be hit testable! + => Fill != null +#if __SKIA__ // we only add this condition for skia because it's the only platform with proper hit-testing support for shapes. If we add it for other platforms, we get a different but still inaccurate behaviour, so we prefer to keep the behaviour as is. + || Stroke != null +#endif + ; protected override void OnBackgroundChanged(DependencyPropertyChangedEventArgs e) { From 1bd2f463aaf41b47b6131daca9a055304cbe5ec1 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 17:21:05 +0300 Subject: [PATCH 28/37] chore: reduce BorderVisual shapes invalidations --- .../Composition/BorderVisual.skia.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs index 11dd2a149716..b385a3dfbe91 100644 --- a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs @@ -86,14 +86,15 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool { // we need this to get notified on brush updates // (BorderBrush internals change -> BorderShape is notified through FillBrush -> render invalidation) - BorderShape = Compositor.CreateSpriteShape(); - - _borderShape!.Geometry = Compositor.CreatePathGeometry(); + var borderShape = Compositor.CreateSpriteShape(); + borderShape.Geometry = Compositor.CreatePathGeometry(); #if DEBUG - _borderShape!.Comment = "#borderShape"; + borderShape.Comment = "#borderShape"; #endif + borderShape.FillBrush = BorderBrush; + BorderShape = borderShape; } - if (_borderShape is { }) + else if (_borderShape is { }) { _borderShape.FillBrush = BorderBrush; } @@ -104,14 +105,17 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool { // we need this to get notified on brush updates. // (BackgroundBrush internals change -> BackgroundShape is notified through FillBrush -> render invalidation) - BackgroundShape = Compositor.CreateSpriteShape(); + var backgroundShape = Compositor.CreateSpriteShape(); - _backgroundShape!.Geometry = Compositor.CreatePathGeometry(); + backgroundShape.Geometry = Compositor.CreatePathGeometry(); #if DEBUG - _backgroundShape!.Comment = "#backgroundShape"; + backgroundShape.Comment = "#backgroundShape"; #endif + backgroundShape.FillBrush = BackgroundBrush; + + BackgroundShape = backgroundShape; } - if (_backgroundShape is { }) + else if (_backgroundShape is { }) { _backgroundShape.FillBrush = BackgroundBrush; } From 4270267a8c405141ae417f4c95b3303dcb3263db Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 17:33:37 +0300 Subject: [PATCH 29/37] chore: remove unnecessary reset --- src/Uno.UI.Composition/Composition/BorderVisual.skia.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs index b385a3dfbe91..669016443c47 100644 --- a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs @@ -257,7 +257,6 @@ private void UpdatePathsAndCornerClip() private unsafe void UpdateBackgroundPath(bool useInnerBorderBoundsAsAreaForBackground, SKSize innerArea, SKSize outerArea, SKPoint* outerRadii, SKPoint* innerRadii) { var backgroundPath = new SKPath(); - backgroundPath.Reset(); var roundRect = new SKRoundRect(); var rect = useInnerBorderBoundsAsAreaForBackground ? new SKRect(0, 0, innerArea.Width, innerArea.Height) From 26f6deac8de32ec8ef7de12ee5952f3291cc6772 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 17:35:07 +0300 Subject: [PATCH 30/37] chore: adjust TryCreateAndClearStrokePaint signature --- .../CompositionSpriteShape.skia.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 6e9d9a936d83..8fb10e2ecf78 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -19,8 +19,7 @@ internal override void Paint(in Visual.PaintingSession session) { if (FillBrush is { } fill) { - var fillPaint = TryCreateAndClearFillPaint(); - fillPaint.ColorFilter = session.Filters.OpacityColorFilter; + var fillPaint = TryCreateAndClearFillPaint(session.Filters.OpacityColorFilter); if (Compositor.TryGetEffectiveBackgroundColor(this, out var colorFromTransition)) { @@ -53,10 +52,8 @@ internal override void Paint(in Visual.PaintingSession session) if (StrokeBrush is { } stroke && StrokeThickness > 0) { - var fillPaint = TryCreateAndClearFillPaint(); - var strokePaint = TryCreateAndClearStrokePaint(); - fillPaint.ColorFilter = session.Filters.OpacityColorFilter; - strokePaint.ColorFilter = session.Filters.OpacityColorFilter; + var fillPaint = TryCreateAndClearFillPaint(session.Filters.OpacityColorFilter); + var strokePaint = TryCreateAndClearStrokePaint(session.Filters.OpacityColorFilter); // Set stroke thickness strokePaint.StrokeWidth = StrokeThickness; @@ -91,13 +88,13 @@ internal override void Paint(in Visual.PaintingSession session) } } - private SKPaint TryCreateAndClearStrokePaint() - => TryCreateAndClearPaint(ref _strokePaint, true, CompositionConfiguration.UseBrushAntialiasing); + private SKPaint TryCreateAndClearStrokePaint(SKColorFilter? colorFilter) + => TryCreateAndClearPaint(ref _strokePaint, true, colorFilter, CompositionConfiguration.UseBrushAntialiasing); - private SKPaint TryCreateAndClearFillPaint() - => TryCreateAndClearPaint(ref _fillPaint, false, CompositionConfiguration.UseBrushAntialiasing); + private SKPaint TryCreateAndClearFillPaint(SKColorFilter? colorFilter) + => TryCreateAndClearPaint(ref _fillPaint, false, colorFilter, CompositionConfiguration.UseBrushAntialiasing); - private static SKPaint TryCreateAndClearPaint(ref SKPaint? paint, bool isStroke, bool isHighQuality = false) + private static SKPaint TryCreateAndClearPaint(ref SKPaint? paint, bool isStroke, SKColorFilter? colorFilter, bool isHighQuality = false) { if (paint == null) { @@ -130,6 +127,8 @@ private static SKPaint TryCreateAndClearPaint(ref SKPaint? paint, bool isStroke, paint.PathEffect = null; } } + + paint.ColorFilter = colorFilter; return paint; } @@ -178,7 +177,7 @@ internal override bool HitTest(Point point) if (StrokeBrush is { } stroke && StrokeThickness > 0) { - var strokePaint = TryCreateAndClearStrokePaint(); + var strokePaint = TryCreateAndClearStrokePaint(null); strokePaint.StrokeWidth = StrokeThickness; var strokeGeometry = strokePaint.GetFillPath(geometryWithTransformations); if (strokeGeometry.Contains((float)point.X, (float)point.Y)) From 55816aff720d6b41ce4d412e6a4ae867f3bdbaab Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 17:50:52 +0300 Subject: [PATCH 31/37] chore: small perf optimization --- .../Composition/CompositionSpriteShape.skia.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 8fb10e2ecf78..4c4a1c12520c 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -166,10 +166,10 @@ private protected override void OnPropertyChangedCore(string? propertyName, bool internal override bool HitTest(Point point) { - point = CombinedTransformMatrix.Inverse().Transform(point); - if (_geometryWithTransformations is { } geometryWithTransformations) { + point = CombinedTransformMatrix.Inverse().Transform(point); + if (FillBrush is { } && geometryWithTransformations.Contains((float)point.X, (float)point.Y)) { return true; From 382fd69d03fce782c05ccb4a8fa85c7e72e74d4d Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 17:54:00 +0300 Subject: [PATCH 32/37] chore: move logic to correct partial --- .../ScrollContentPresenter.Managed.cs | 23 ++++++++++++++++++ .../ScrollContentPresenter.cs | 24 ------------------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs index 68c8d3be978e..2a67040fee84 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs @@ -68,6 +68,29 @@ partial void InitializePartial() ManipulationMode = ManipulationModes.TranslateX | ManipulationModes.TranslateY; // Updated in PrepareTouchScroll! } + internal void HookScrollEvents(ScrollViewer sv) + { + // Mouse wheel support + sv.PointerWheelChanged += PointerWheelScroll; + + // Touch scroll support + // Note: Events are hooked on the SCP itself, not the ScrollViewer + ManipulationStarting += PrepareTouchScroll; + ManipulationStarted += TouchScrollStarted; + ManipulationDelta += UpdateTouchScroll; + ManipulationCompleted += CompleteTouchScroll; + } + + internal void UnhookScrollEvents(ScrollViewer sv) + { + sv.PointerWheelChanged -= PointerWheelScroll; + + ManipulationStarting -= PrepareTouchScroll; + ManipulationStarted -= TouchScrollStarted; + ManipulationDelta -= UpdateTouchScroll; + ManipulationCompleted -= CompleteTouchScroll; + } + /// protected override void OnContentChanged(object oldValue, object newValue) { diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index c37b5b09fa88..1855b752c39f 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -235,30 +235,6 @@ internal override bool IsViewHit() => new Rect(default, RenderSize); #endif -#if UNO_HAS_MANAGED_SCROLL_PRESENTER - internal void HookScrollEvents(ScrollViewer sv) - { - // Mouse wheel support - sv.PointerWheelChanged += PointerWheelScroll; - - // Touch scroll support - ManipulationStarting += PrepareTouchScroll; - ManipulationStarted += TouchScrollStarted; - ManipulationDelta += UpdateTouchScroll; - ManipulationCompleted += CompleteTouchScroll; - } - - internal void UnhookScrollEvents(ScrollViewer sv) - { - sv.PointerWheelChanged -= PointerWheelScroll; - - ManipulationStarting -= PrepareTouchScroll; - ManipulationStarted -= TouchScrollStarted; - ManipulationDelta -= UpdateTouchScroll; - ManipulationCompleted -= CompleteTouchScroll; - } -#endif - private void PointerWheelScroll(object sender, Input.PointerRoutedEventArgs e) { var properties = e.GetCurrentPoint(null).Properties; From 97eb0256142f8cff96815d96b81833a6988fb022 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 18:12:29 +0300 Subject: [PATCH 33/37] chore: adjust comments --- src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index a5f31fd549dd..f89ea6102355 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -659,12 +659,12 @@ internal static (UIElement? element, Branch? stale) SearchDownForTopMostElementA } } + // We didn't find any child at the given position, validate that element can be touched, + // and the position is in actual bounds(which might be different than the clipping bounds) #if __SKIA__ - // We still need to check the renderingBounds in case the element is clipped. if (element.HitTest(transformToElement.Inverse().Transform(testPosition)) && renderingBounds.Contains(testPosition)) #else - // We didn't find any child at the given position, validate that element can be touched (i.e. not HitTestability.Invisible), - // and the position is in actual bounds (which might be different than the clipping bounds) + if (elementHitTestVisibility == HitTestability.Visible && renderingBounds.Contains(testPosition)) #endif { From 2550f6df82de9e7b34fbde367a72a809fc9d37ab Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 18:13:35 +0300 Subject: [PATCH 34/37] chore: move event subscription logic back inside SCP --- .../ScrollContentPresenter.Managed.cs | 26 +++++++++++++++++++ .../ScrollContentPresenter.cs | 12 +++++++++ .../Controls/ScrollViewer/ScrollViewer.cs | 13 ---------- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs index 2a67040fee84..2de1b51c5d31 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs @@ -3,6 +3,7 @@ using Microsoft.UI.Xaml.Input; using Windows.Devices.Input; using Windows.Foundation; +using Uno.UI.Xaml; #if HAS_UNO_WINUI using _PointerDeviceType = global::Microsoft.UI.Input.PointerDeviceType; @@ -70,6 +71,13 @@ partial void InitializePartial() internal void HookScrollEvents(ScrollViewer sv) { + // Note: the way WinUI does scrolling is very different, and doesn't use + // PointerWheelChanged changes, etc. + // We can either subscribe on the ScrollViewer or the SCP directly, but due to + // the way hit-testing works (see #16201), the SCP will not receive any pointer + // events. On WinUI, this is also the case: pointer presses are received on the SV, + // not on the SCP. + // Mouse wheel support sv.PointerWheelChanged += PointerWheelScroll; @@ -91,6 +99,24 @@ internal void UnhookScrollEvents(ScrollViewer sv) ManipulationCompleted -= CompleteTouchScroll; } + private protected override void OnLoaded() + { + base.OnLoaded(); + if (Scroller is { } sv) + { + HookScrollEvents(sv); + } + } + + private protected override void OnUnloaded() + { + base.OnUnloaded(); + if (Scroller is { } sv) + { + UnhookScrollEvents(sv); + } + } + /// protected override void OnContentChanged(object oldValue, object newValue) { diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs index 1855b752c39f..a7daa2bafc8c 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.cs @@ -44,10 +44,22 @@ public object ScrollOwner { if (_scroller is { } oldScroller) { +#if UNO_HAS_MANAGED_SCROLL_PRESENTER + if (oldScroller.Target is ScrollViewer oldScrollerTarget) + { + UnhookScrollEvents(oldScrollerTarget); + } +#endif WeakReferencePool.ReturnWeakReference(this, oldScroller); } _scroller = WeakReferencePool.RentWeakReference(this, value); +#if UNO_HAS_MANAGED_SCROLL_PRESENTER + if (IsInLiveTree && value is ScrollViewer newTarget) + { + HookScrollEvents(newTarget); + } +#endif } } #endregion diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs index e4e6e9068ee5..31afe45d0222 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs @@ -1012,23 +1012,10 @@ protected override void OnApplyTemplate() } #endif -#if UNO_HAS_MANAGED_SCROLL_PRESENTER - Presenter?.UnhookScrollEvents(this); -#endif if (scpTemplatePart is ScrollContentPresenter presenter) { presenter.ScrollOwner = this; Presenter = presenter; - - // Note: the way WinUI does scrolling is very different, and doesn't use - // PointerWheelChanged changes, etc. - // We can either subscribe on the ScrollViewer or the SCP directly, but due to - // the way hit-testing works (see #16201), the SCP will not receive any pointer - // events. On WinUI, this is also the case: pointer presses are received on the SV, - // not on the SCP. -#if UNO_HAS_MANAGED_SCROLL_PRESENTER - presenter.HookScrollEvents(this); -#endif } else { From 43b6fb5d8207c7d6adc4f065e7642652d163f939 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 5 Aug 2024 21:19:45 +0300 Subject: [PATCH 35/37] chore: formatting --- .../Composition/CompositionSpriteShape.skia.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs index 4c4a1c12520c..ec7c5cedce72 100644 --- a/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs +++ b/src/Uno.UI.Composition/Composition/CompositionSpriteShape.skia.cs @@ -127,7 +127,7 @@ private static SKPaint TryCreateAndClearPaint(ref SKPaint? paint, bool isStroke, paint.PathEffect = null; } } - + paint.ColorFilter = colorFilter; return paint; From acd07a36a63949876cdca12a943b20e5604e0b0e Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 14 Aug 2024 12:26:32 +0300 Subject: [PATCH 36/37] chore: make SCP methods private --- .../ScrollContentPresenter.Managed.cs | 4 ++-- .../UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs index 2de1b51c5d31..0c585caf63bc 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollContentPresenter/ScrollContentPresenter.Managed.cs @@ -69,7 +69,7 @@ partial void InitializePartial() ManipulationMode = ManipulationModes.TranslateX | ManipulationModes.TranslateY; // Updated in PrepareTouchScroll! } - internal void HookScrollEvents(ScrollViewer sv) + private void HookScrollEvents(ScrollViewer sv) { // Note: the way WinUI does scrolling is very different, and doesn't use // PointerWheelChanged changes, etc. @@ -89,7 +89,7 @@ internal void HookScrollEvents(ScrollViewer sv) ManipulationCompleted += CompleteTouchScroll; } - internal void UnhookScrollEvents(ScrollViewer sv) + private void UnhookScrollEvents(ScrollViewer sv) { sv.PointerWheelChanged -= PointerWheelScroll; diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs index fb259ce23571..9d507d5b7d2f 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.Managed.cs @@ -24,11 +24,7 @@ private bool ChangeViewNative(double? horizontalOffset, double? verticalOffset, => (_presenter as ScrollContentPresenter)?.Set(horizontalOffset, verticalOffset, disableAnimation: disableAnimation) ?? true; private partial void OnLoadedPartial() { } - - private partial void OnUnloadedPartial() - { - Presenter?.UnhookScrollEvents(this); - } + private partial void OnUnloadedPartial() { } #region Over scroll support /// From f3de6ac5b008ffc3e0b5ee1ded3e72708c962a8f Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Wed, 14 Aug 2024 12:40:34 +0300 Subject: [PATCH 37/37] chore: make BackgroundShape private --- src/Uno.UI.Composition/Composition/BorderVisual.skia.cs | 9 +++------ src/Uno.UI.Composition/Composition/Compositor.skia.cs | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs index 669016443c47..1babb79005ab 100644 --- a/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/BorderVisual.skia.cs @@ -28,15 +28,12 @@ internal class BorderVisual(Compositor compositor) : ShapeVisual(compositor) // state set here but affects children private RectangleClip? _childClipCausedByCornerRadius; - internal CompositionSpriteShape? BackgroundShape - { - get => _backgroundShape; - private set => SetProperty(ref _backgroundShape, value); - } - // We do this instead of a direct SetProperty call so that SetProperty automatically gets an accurate propertyName + private CompositionSpriteShape? BackgroundShape { set => SetProperty(ref _backgroundShape, value); } private CompositionSpriteShape? BorderShape { set => SetProperty(ref _borderShape, value); } + internal bool IsMyBackgroundShape(CompositionSpriteShape shape) => _backgroundShape == shape; + public CornerRadius CornerRadius { private get => _cornerRadius; diff --git a/src/Uno.UI.Composition/Composition/Compositor.skia.cs b/src/Uno.UI.Composition/Composition/Compositor.skia.cs index 2f39f70674e7..e5a7bc1b03de 100644 --- a/src/Uno.UI.Composition/Composition/Compositor.skia.cs +++ b/src/Uno.UI.Composition/Composition/Compositor.skia.cs @@ -53,7 +53,7 @@ internal bool TryGetEffectiveBackgroundColor(CompositionSpriteShape shape, out C { foreach (var transition in _backgroundTransitions) { - if (transition.Visual.BackgroundShape == shape) + if (transition.Visual.IsMyBackgroundShape(shape)) { color = transition.CurrentColor; return true;