Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DataGrid hang during scroll to cell when UseLayoutRounding is enabled #9983

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Oct 22, 2024

Fixes #9944

Description

Fixes an infinite loop (an application hang) when scrolling to a Cell right after layout refresh. This only happens when UseLayoutRounding 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 simple Math.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 for DataGrid.SelectAllCommand. The layout computation logic does not sit well with negative numbers.

CellsPanelHorizontalOffset = DataGridHelper.GetParentCellsPanelHorizontalOffset(cell);

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 with FrameworkElement's Width.

By the way, the repro to do two layout passes can be simplified to (with DataGrid being read only):

myDataGrid.MouseUp += (sender, e) =>
{
    myDataGrid.Items.Refresh();
    DataGridColumn currentColumn = myDataGrid.CurrentColumn ?? myDataGrid.Columns[1];
    myDataGrid.CurrentCell = new DataGridCellInfo(myDataGrid.SelectedItem, currentColumn);
};

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 called GetParentCellsPanelHorizontalOffset 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

@h3xds1nz h3xds1nz requested review from a team as code owners October 22, 2024 21:04
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layout rounding can cause DataGrid to hang
1 participant