From 913ee223e6ceb11d90af9bc7d1364b29a0a7fa42 Mon Sep 17 00:00:00 2001 From: Sam Bent Date: Wed, 19 Jan 2022 03:22:52 -0800 Subject: [PATCH] fix hang when margin is not a multiple of DPI rounding quantum (#5841) --- .../System/Windows/Controls/ItemsControl.cs | 32 ++++++++++++ .../Controls/VirtualizingStackPanel.cs | 49 ++++++++++++------- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs index fc05da1d0e8..3503c30130f 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs @@ -3078,6 +3078,38 @@ internal static ElementViewportPosition GetElementViewportPosition(FrameworkElem return ElementViewportPosition.None; } + // this version also returns the element's layout rectangle (in viewport's coordinates). + // VirtualizingStackPanel needs this, to determine the element's scroll offset. + internal static ElementViewportPosition GetElementViewportPosition(FrameworkElement viewPort, + UIElement element, + FocusNavigationDirection axis, + bool fullyVisible, + bool ignorePerpendicularAxis, + out Rect elementRect, + out Rect layoutRect) + { + ElementViewportPosition position = GetElementViewportPosition( + viewPort, + element, + axis, + fullyVisible, + false, + out elementRect); + + if (position == ElementViewportPosition.None) + { + layoutRect = Rect.Empty; + } + else + { + Visual parent = VisualTreeHelper.GetParent(element) as Visual; + Debug.Assert(element != viewPort && element.IsArrangeValid && parent != null, "GetElementViewportPosition called in unsupported situation"); + layoutRect = CorrectCatastrophicCancellation(parent.TransformToAncestor(viewPort)).TransformBounds(element.PreviousArrangeRect); + } + + return position; + } + // in large virtualized hierarchical lists (TreeView or grouping), the transform // returned by element.TransformToAncestor(viewport) is vulnerable to catastrophic // cancellation. If element is at the top of the viewport, but embedded in diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs index af4a5157080..e64de005bf7 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs @@ -1189,6 +1189,7 @@ private FrameworkElement ComputeFirstContainerInViewport( if (fe.IsVisible) { Rect elementRect; + Rect layoutRect; // get the vp-position of the element, ignoring the secondary axis // (DevDiv2 1136036, 1203626 show two different cases why we @@ -1201,7 +1202,8 @@ private FrameworkElement ComputeFirstContainerInViewport( direction, false /*fullyVisible*/, !isVSP45Compat /*ignorePerpendicularAxis*/, - out elementRect); + out elementRect, + out layoutRect); if (elementPosition == ElementViewportPosition.PartiallyInViewport || elementPosition == ElementViewportPosition.CompletelyInViewport) @@ -1287,20 +1289,35 @@ private FrameworkElement ComputeFirstContainerInViewport( { if (direction == FocusNavigationDirection.Down) { - firstContainerOffsetFromViewport = elementRect.Y; - if (!isVSP45Compat) - { - firstContainerOffsetFromViewport -= fe.Margin.Top; - } - } + if (isVSP45Compat) + { + firstContainerOffsetFromViewport = elementRect.Y; + } + else + { + // include the leading margin in the offset. Simply subtracting + // the margin doesn't work when layout rounding is in effect, as + // we can't deduce how rounding affected the arrangement of the + // element and its margin. Instead, just use the layout rect directly. + firstContainerOffsetFromViewport = layoutRect.Top; + } + } else // (direction == FocusNavigationDirection.Right) { - firstContainerOffsetFromViewport = elementRect.X; - if (!isVSP45Compat) - { - firstContainerOffsetFromViewport -= fe.Margin.Left; - } - } + + if (isVSP45Compat) + { + firstContainerOffsetFromViewport = elementRect.X; + } + else + { + // include the leading margin in the offset. Simply subtracting + // the margin doesn't work when layout rounding is in effect, as + // we can't deduce how rounding affected the arrangement of the + // element and its margin. Instead, just use the layout rect directly. + firstContainerOffsetFromViewport = layoutRect.Left; + } + } } else if (findTopContainer && isTopContainer) { @@ -1931,7 +1948,7 @@ public ScrollViewer ScrollOwner } set { - if (_scrollData == null) EnsureScrollData(); + EnsureScrollData(); if (value != _scrollData._scrollOwner) { ResetScrolling(this); @@ -9510,10 +9527,6 @@ private int ItemCount private void EnsureScrollData() { if (_scrollData == null) { _scrollData = new ScrollData(); } - else - { - Debug.Assert(_scrollData._scrollOwner != null, "Scrolling an unconnected VSP"); - } } private static void ResetScrolling(VirtualizingStackPanel element)