[DataGrid] Scrollbar not visible and performance issue

[ArchieCoder]

Describe the bug

My grid in the video contains 3 rows and the number of columns expands fast (every half second). The first 4 columns are fixed.

https://user-images.githubusercontent.com/1608424/164730612-4d2b9047-fc36-4d23-9a4b-37ac705d8010.mp4

Problem 1: Scrollbar not visible

• Despite the fact that I explicitly set the HorizontalScrollbarVisibility=Visible, the scrollbar appears on demand.
• It's actually not obvious where is the hit area for the Scrollbar to appear

Problem 2: Performance

• When I scrolled when there are ~1000 columns, the control is almost unusable.

Regression

No response

Reproducible in sample app?

• [ ] This bug can be reproduced in the sample app.

Steps to reproduce

1- I created a sample app to reproduce the bugs.
2- Open / Compile / Run
3- Let the grid has t1000

Expected behavior

• Scrollbars should be visible when explicitly visible
• I expect a DataGrid to support more than 1000 columns even if data are added every second

Screenshots

Video provided in the description[

Sample project to see the bug Sample.zip ](url)

Windows Build Number

• [ ] Windows 10 1809 (Build 17763)
• [ ] Windows 10 1903 (Build 18362)
• [ ] Windows 10 1909 (Build 18363)
• [ ] Windows 10 2004 (Build 19041)
• [ ] Windows 10 20H2 (Build 19042)
• [ ] Windows 10 21H1 (Build 19043)
• [X] Windows 11 21H2 (Build 22000)
• [ ] Other (specify)

Other Windows Build number

No response

App minimum and target SDK version

• [ ] Windows 10, version 1809 (Build 17763)
• [X] Windows 10, version 1903 (Build 18362)
• [ ] Windows 10, version 1909 (Build 18363)
• [ ] Windows 10, version 2004 (Build 19041)
• [ ] Other (specify)

Other SDK version

No response

Visual Studio Version

2022

Visual Studio Build Number

17.1.4

Device form factor

Desktop

Nuget packages

Microsoft.Toolkit.Uwp.UI.Controls.DataGrid 7.1.2

Additional context

No response

Help us help you

No.

[ghost]

Hello ArchieCoder, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

[tutkus]

Hi @michael-hawker, I can work on it. Please assigne to me if noone else has booked it yet.

[michael-hawker]

@tutkus thanks! Please feel free to take a look; I'll assign it to you.

[tutkus]

@michael-hawker I spent a bit of time with the example attached to the issue. I run profiler and figured out that the problem most lilkly is with Panels: DataGridColumnHeadersPresenter, DataGridRowsPresenter, DataGridCellsPresenter. Each of them in order to prepare layout goes through all columns in: ArrangeOverride and MeasureOverride. Based on my investigation this is a root casue of performance issue.

I was thinking about to run the computation only on these columns which are displaed already and ignore everything what out of currentlly showing area.

Not sure what side effect that change might have as I'm not very familar with the control itself.

[michael-hawker]

Thanks for the investigation @tutkus. I would imagine the original architecture of the DataGrid assumed that data would expand in rows vs. columns being used for data as in @ArchieCoder's scenario.

@ArchieCoder can you explain why your time is in columns vs. transposing the grid or using a chart? Are the rows always a fixed/static/smaller number in your scenarios?

I'd be a bit hesitant to make a sweeping change to the behavior here as I'm not sure what unintended consequences that could lead to, though presumably evaluating only visible things should be fine as long as the underlying data is still there. @tutkus mind using the permalink feature on GitHub here to call out where those loops are in a comment here? Is there anyway we could cache/share that logic between them all and not re-calculate in each layout cycle?

If we do find an acceptable solution, this is the type of thing if we make a big change it could be handy to get some vetting on by the platform team who contributed this code. FYI @RBrid @ryandemopoulos.

[ArchieCoder]

@michael-hawker This is a specification that I got from the client. You can see the desired result using the Telerik DataGrid. In the meantime, the DataGrid works up to 1000 columns, but after it has its limitation. I tried to convince the client that data grid in general is made for vertical expansion. A column is added each time a new data entry comes in.

[Arlodotexe]

The performance issue smells of broken virtualization. Has anyone taken a look to see if that's the issue?

[tutkus]

Give me couple of days so I could implement my ideas, next i will push to my fork and share the link to my repo here so you could have a look. Next you will decide what to do farther.

[tutkus]

I started with simple case in: DataGridColumnHeadersPresenter. This is commit in to my fork: https://github.com/tutkus/WindowsCommunityToolkit/commit/fa73a913de4047e1f35dee417b5af5992b41dcf7

What I did is:

1. MeasureOverride handles only these columns which are in the visible area on the screen (ShouldDisplay method)
2. ShouldDisplay were extracted from DataGridCellsPresenter and converted to extention method and moved to Extentions class
3. EnsureColumnHeaderClip creates RectangleGeometry and sets it on the ColumnHeader only when the Rect is different then the one already set on the Column Header

With these changes I managed to improve performance however only for scrolling header case with my own test case (I needed to have some banchmark). The grid itself given with the example still performs very bed.

My next gole is to focus on DataGridCellsPresenter.

[yaira2]

The performance issue smells of broken virtualization. Has anyone taken a look to see if that's the issue?

@Arlodotexe one of the reasons we don't use this control is because it doesn't support virtualization. I don't think it was ever supported but we'd love to switch if this was resolved.