[release/3.1] Port VSP Freeze fix from 4.8 #5008
Merged
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.
Addresses #2490
This is a port of a servicing fix in .NET 4.7/4.8.
Issue: Scrolling a TreeView hangs.
Discussion:
The customer's data is highly non-uniform, in the sense that a given node's children govern subtrees whose sizes are quite different. This means the estimates VSP computes can differ wildly from the truth, and are subject to frequent (and sometimes violent) revision as more of the tree de-virtualizes. This is not necessarily bad, but it stresses the algorithms in ways that uniform trees don't. This stress uncovers two problems:
VSP stores "effective offsets", intended to support correct layout after bottom-up size changes (e.g. a deep node gets larger because of new content), which shouldn't move the visible items. When VSP revises its estimates, it also changes its effective offsets. These are intended to stay in effect until the user scrolls, moving the visible items. But VSP used an indirect way to detect this (namely the local viewport offset). The non-uniform churn exposed a flaw - the viewport offset after a scroll can be the same as it was before (by coincidence, if estimates in nearby nodes change in just the wrong way). If so, the layout algorithm will choose the same node as before to be at the top of the viewport, and the "anchor" logic that checks whether the scroll ended up in the right place will ask for a remeasure, which merely repeats the same futile actions. Infinite loop.
A violent enough change can cause the "anchor" logic to think that the anchor node's scroll offset is outside the extent of the full tree. This is because the former is computed bottom-up, and can include revised estimates that have not yet propagated to the latter (computed top-down). When this happens, the anchor logic simply accepts the current state, and the TreeView scrolls to the wrong place (sometimes by a lot - I've seen examples that were off by 1000s of nodes).
The fix for (1) is to keep a "scroll generation" counter, incremented each time the top-level scroll offset changes. Effective offsets remember the generation at which they were created, and are only applied during layout requests in the same generation.
The fix for (2) is to detect the bad situation and ask for a remeasure.
The customer's real app (but sadly not the sample repro they provided) suffers from a third problem.
3. When UseLayoutRounding=true, the anchor logic can fail because it uses unrounded size estimates of never-been-devirtualized items, inconsistent with the layout algorithm that rounds everything. As usual, this kind of failure leads to an infinite-remeasure hang, depends sensitively on the size and shape of the data and the history of scrolling and virtualization, and seems to be more likely in large non-uniform datasets.
The fix for (3) is to apply layout rounding to the estimated sizes.