Skip to content

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

Open
h3xds1nz wants to merge 1 commit into
dotnet:mainfrom
h3xds1nz:fix-datagrid-rounding-hang
Open

Fix DataGrid hang during scroll to cell when UseLayoutRounding is enabled#9983
h3xds1nz wants to merge 1 commit into
dotnet:mainfrom
h3xds1nz:fix-datagrid-rounding-hang

Conversation

@h3xds1nz
Copy link
Copy Markdown
Member

@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

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 Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed Status:Proposed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Layout rounding can cause DataGrid to hang

2 participants