-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix ListView/GridView scroll performance when automation peers are created #9182
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 ListView/GridView scroll performance when automation peers are created #9182
Conversation
@dotnet-policy-service agree |
Tested? Code changes look good to me. Would it make sense to have a reference to something holding the cached row number so that it doesn't have to be searched for each cell in a row? |
e6be461
to
1c5f446
Compare
@miloush Yeah, tested, it works as expected. It would definitely make more sense to cache the row index for all the cells, however I don't see an easy way how to do it here, given that we would load the index lazily. |
@miloush I have actually thought of a much simpler solution that didn't occur to me before, we can just retrieve the index from the realized item in ItemGenerator. That way the change is pretty much a one-liner. I have also thought about your caching proposal, and my original solution does actually ruin the UAI performance with multiple columns indeed. This solution on the other hand is rather smooth. |
For one liner I think it would appreciated if the PR didn't contain all the other stylistic changes. |
Doesn't the IndexFromContainer do the same linear search? |
Agreed, I'll push a commit with only the one-liner. No, ItemGenerator contains always just the realized items (block) and it does a linear search but only in the bunch, so in case of virtualization that's only a handful few that are currently realized (it has a starting index). I use it a lot in other projects to draw over controls so I'm aware of the performance. I ran it with the build of the new PresentationFramework as well just to be sure before committing. |
Hello, i'm following this conversation since the start because i ran into this issue. |
Thanks @h3xds1nz. |
@pchaurasia14 Thank you. |
@h3xds1nz I was trying to check your sample repro by replacing the locally built PresentationFramework.dll and could experience similar scrolling performance. Couldn't observe something noticeable. I might be missing a step here, Could you please share the exact steps to verify? |
@singhashish-wpf Thanks for checking, I've included sample videos for you with the repro that I've originally recorded. First I'm regularly scrolling when peers are not being created. Notice what happens in the first/original video when Same goes for the scrollbar being laggy after I reach the bottom. By the way, the fact that automation peers are created after interacting in with dropdowns, popups, even though there's no client subbed, that's a long-standing issue since the beginning, I believe there's a tracking for this as well in this repo but that's just a side note. |
…GridView's item rowIndex
ce98a71
to
d212d66
Compare
I have rebased and cleaned up the commit history. |
@h3xds1nz Thank you for your contributions. |
@harshit7962 Thanks for including it, finally can use UIA again in the upcoming project. |
Fixes #9181
Description
I was running some tests with ListView (using GridView) and when I loaded
1 000 000
items (yes, an obscure use case but that's why we got virtualization, no?), the scrolling performance would degrade and keyboard scrolling felt almost unusable.During a quick rundown, I've discovered the culprit is in override of
GetChildrenCore
ofGridViewItemAutomationPeer
, which performs IndexOf (linear) search in theItems
collection of the underlyingItemsSource
to find the respective item's Row.Reproduction Steps
The easiest step to reproduce is to add a ListView with GridView plus a ComboBox, after opening the ComboBox's dropdown (clicking it),
Accessibility.dll
gets loaded and the party gets started. You can find a demo repository on Github - here.Behavior
The scrolling performance becomes worse to unusable with increasing number of items.
Known Workarounds
The "workaround" is to subclass
GridView
, overridingGetAutomationPeer
to return NULL. Not really a solution though.Risk
None.
Microsoft Reviewers: Open in CodeFlow