From d9aa625a5cc634c3eaa827697eeeac4efa11303e Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Thu, 15 Aug 2024 15:32:33 +0300 Subject: [PATCH] fix(skia): make UIElement.HitTest strictly stronger than UIElement.IsViewHit --- .../Composition/ShapeVisual.skia.cs | 4 ++-- .../Composition/Visual.skia.cs | 4 ++++ .../ScrollPresenter/ScrollPresenter.Uno.cs | 2 ++ src/Uno.UI/UI/Xaml/Controls/Border/Border.cs | 12 +----------- .../Xaml/Controls/Border/IBorderInfoProvider.cs | 2 ++ .../ContentPresenter/ContentPresenter.cs | 2 +- src/Uno.UI/UI/Xaml/Controls/Page/Page.cs | 2 ++ src/Uno.UI/UI/Xaml/Controls/Panel/Panel.cs | 2 +- src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs | 3 +++ .../Xaml/Controls/TextBlock/TextVisual.skia.cs | 2 ++ src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 16 ++++++++-------- 11 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs index 8254ad7e9636..43a9310d2016 100644 --- a/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs +++ b/src/Uno.UI.Composition/Composition/ShapeVisual.skia.cs @@ -56,8 +56,8 @@ internal override void Paint(in PaintingSession session) return shape; } - /// This does NOT take the clipping into account. - internal virtual bool HitTest(Point point) + /// + internal override bool HitTest(Point point) { if (_shapes is null) { diff --git a/src/Uno.UI.Composition/Composition/Visual.skia.cs b/src/Uno.UI.Composition/Composition/Visual.skia.cs index 216a801973f0..f2173a2a9358 100644 --- a/src/Uno.UI.Composition/Composition/Visual.skia.cs +++ b/src/Uno.UI.Composition/Composition/Visual.skia.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Numerics; +using Windows.Foundation; using SkiaSharp; using Uno.Extensions; using Uno.UI.Composition; @@ -344,6 +345,9 @@ private PaintingSession CreateLocalSession(in PaintingSession parentSession) return session; } + /// This does NOT take the clipping into account. + internal virtual bool HitTest(Point point) => false; + [Flags] internal enum VisualFlags : byte { diff --git a/src/Uno.UI/Microsoft/UI/Xaml/Controls/ScrollPresenter/ScrollPresenter.Uno.cs b/src/Uno.UI/Microsoft/UI/Xaml/Controls/ScrollPresenter/ScrollPresenter.Uno.cs index 48ded86c05d9..e471ae760442 100644 --- a/src/Uno.UI/Microsoft/UI/Xaml/Controls/ScrollPresenter/ScrollPresenter.Uno.cs +++ b/src/Uno.UI/Microsoft/UI/Xaml/Controls/ScrollPresenter/ScrollPresenter.Uno.cs @@ -23,6 +23,8 @@ partial void InitializePartial() private protected override ShapeVisual CreateElementVisual() => Compositor.GetSharedCompositor().CreateBorderVisual(); #endif + internal override bool IsViewHit() => ((IBorderInfoProvider)this).IsViewHit(); + Brush IBorderInfoProvider.Background => Background; BackgroundSizing IBorderInfoProvider.BackgroundSizing => BackgroundSizing.InnerBorderEdge; diff --git a/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs b/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs index b630b2a75dfe..0ed96d837bb2 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Border/Border.cs @@ -322,17 +322,7 @@ protected override void OnBackgroundChanged(DependencyPropertyChangedEventArgs e internal override bool CanHaveChildren() => true; - internal override bool IsViewHit() => IsViewHitImpl(this); - - internal static bool IsViewHitImpl(FrameworkElement element) - { - _Debug.Assert(element is Panel - || element is Border - || element is ContentPresenter - ); - - return element.Background != null; - } + internal override bool IsViewHit() => ((IBorderInfoProvider)this).IsViewHit(); #if !UNO_HAS_BORDER_VISUAL private void UpdateBorder() => BorderRenderer.Update(); diff --git a/src/Uno.UI/UI/Xaml/Controls/Border/IBorderInfoProvider.cs b/src/Uno.UI/UI/Xaml/Controls/Border/IBorderInfoProvider.cs index a08502db2466..99fc9a3c29cb 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Border/IBorderInfoProvider.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Border/IBorderInfoProvider.cs @@ -41,4 +41,6 @@ internal partial interface IBorderInfoProvider #if UNO_HAS_BORDER_VISUAL BorderVisual BorderVisual { get; } #endif + + bool IsViewHit() => Background != null || BorderBrush != null; } diff --git a/src/Uno.UI/UI/Xaml/Controls/ContentPresenter/ContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ContentPresenter/ContentPresenter.cs index 1faa59ef1a22..d1eee2e74a5f 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ContentPresenter/ContentPresenter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ContentPresenter/ContentPresenter.cs @@ -1383,7 +1383,7 @@ protected override Size MeasureOverride(Size size) internal override bool CanHaveChildren() => true; - internal override bool IsViewHit() => Border.IsViewHitImpl(this); + internal override bool IsViewHit() => ((IBorderInfoProvider)this).IsViewHit(); /// /// Registers the provided native element in the native shell diff --git a/src/Uno.UI/UI/Xaml/Controls/Page/Page.cs b/src/Uno.UI/UI/Xaml/Controls/Page/Page.cs index 14f2e19ed004..3dbe1a2aef78 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Page/Page.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Page/Page.cs @@ -32,6 +32,8 @@ private void UpdateBorder() } #endif + internal override bool IsViewHit() => ((IBorderInfoProvider)this).IsViewHit(); + protected internal virtual void OnNavigatedFrom(NavigationEventArgs e) { } protected internal virtual void OnNavigatedTo(NavigationEventArgs e) { } diff --git a/src/Uno.UI/UI/Xaml/Controls/Panel/Panel.cs b/src/Uno.UI/UI/Xaml/Controls/Panel/Panel.cs index 64fa4c034f4f..414e45a81b29 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Panel/Panel.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Panel/Panel.cs @@ -259,7 +259,7 @@ private protected void OnBackgroundSizingChangedInnerPanel(DependencyPropertyCha #endif } - internal override bool IsViewHit() => Border.IsViewHitImpl(this); + internal override bool IsViewHit() => ((IBorderInfoProvider)this).IsViewHit(); #if !UNO_HAS_BORDER_VISUAL private void UpdateBorder() => _borderRenderer.Update(); diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs index dd3d0545b4a4..210b930a939b 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs @@ -296,4 +296,7 @@ internal override bool IsViewHit() return Popup?.IsLightDismissEnabled ?? false; } + + // This shouldn't be needed, see the above comment in IsViewHit + internal override bool HitTest(Point point) => IsViewHit(); } diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs index a1bbd063f5e2..0f0d7210a503 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextVisual.skia.cs @@ -34,5 +34,7 @@ internal override void Paint(in PaintingSession session) owner.Inlines.Draw(in session); } } + + internal override bool HitTest(Point point) => new Rect(Offset.X, Offset.Y, Size.X, Size.Y).Contains(point); } } diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index f89ea6102355..520c457d8a8c 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -501,12 +501,6 @@ internal static (UIElement? element, Branch? stale) SearchDownForTopMostElementA clippingBounds = clippingBounds.IntersectWith(transformToElement.Transform(clip)) ?? default; } TRACE($"- clipping (absolute): {clippingBounds.ToDebugString()}"); - - // The region where the current element draws itself. - // Be aware that children might be out of this rendering bounds if no clipping defined. - // This is expressed in the window (absolute) coordinate space. - var renderingBounds = transformToElement.Transform(new Rect(new Point(), element.LayoutSlotWithMarginsAndAlignments.Size)).IntersectWith(clippingBounds) ?? Rect.Empty; - TRACE($"- rendering (absolute): {renderingBounds.ToDebugString()}"); #else // First compute the transformation between the element and its parent coordinate space var matrix = Matrix3x2.Identity; @@ -662,12 +656,18 @@ 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__ - if (element.HitTest(transformToElement.Inverse().Transform(testPosition)) && renderingBounds.Contains(testPosition)) + if (element.HitTest(transformToElement.Inverse().Transform(testPosition))) #else - if (elementHitTestVisibility == HitTestability.Visible && renderingBounds.Contains(testPosition)) #endif { + // UIElement.HitTest should be a stronger condition than elementHitTestVisibility == HitTestability.Visible + // On WASM we match the native-element hit-testability (i.e. the `pointer-events` css property) to the + // HitTestVisibility of the corresponding managed element, so if we implement UIElement.HitTest for WASM + // in the future, this assertion makes sure that we're not regressing (e.g. imagine + // elementHitTestVisibility != Visible, but element.HitTest returns true). + global::System.Diagnostics.Debug.Assert(elementHitTestVisibility == HitTestability.Visible); + TRACE($"> LEAF! ({element.GetDebugName()} is the OriginalSource) | stale branch: {stale?.ToString() ?? "-- none --"}"); return (element, stale); }