Fix DataGrid hang during scroll to cell when UseLayoutRounding is enabled #9983
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #9944
Description
Fixes an infinite loop (an application hang) when scrolling to a
Cell
right after layout refresh. This only happens whenUseLayoutRounding
is enabled and you hit the jackpot (e.g. with the repro in the issue, x1.25 on the pixels).As I've described originally in the issue, with some small adjustments:
Because of layout rounding,
GetParentCellsPanelHorizontalOffset
can return negative (e.g.-0.4
), which happens here. Doing simpleMath.Max(0d, result)
on the result fixes the hang and prevents breaking the binding for the button in row header that's used in all styles forDataGrid.SelectAllCommand
. The layout computation logic does not sit well with negative numbers.wpf/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/DataGrid.cs
Line 8522 in 58c3b2e
In my humble opinion, this is the right thing to do to constrain it at zero, it fixes the immediate problem but also the other options will pass zero or greater;
RowHeaderWidth
is also constrained withFrameworkElement
'sWidth
.By the way, the repro to do two layout passes can be simplified to (with
DataGrid
being read only):Customer Impact
Customers will enjoy
DataGrid
without the application randomly hanging.Regression
Unlikely.
Testing
All the other layout calculations that depend on this I've checked seem to be outputting the same before and after fix (the final output, not the infinite loop one), visually I couldn't see any difference either. I've tested it on several different DPIs and everything seems to be in order.
Risk
Low-to-medium; while it makes sense that it shouldn't be negative due to the binding (and it never will be unless you
UseLayoutRounding
with magical dimensions), we shall not take that as a holy grail. Though the functions calledGetParentCellsPanelHorizontalOffset
are only called by this method where I've done the fix, it made sense to me to make it here where it's being decided and not somewhere in the actual calculation.Microsoft Reviewers: Open in CodeFlow