fix(ListView): flicker when repositioning item#23168
Conversation
There was a problem hiding this comment.
Pull request overview
This PR targets a single-frame flicker in ListViewBase virtualization where newly materialized containers briefly render at (0,0) (top-left) after an item move triggers a refresh, by attempting to force layout/arrange immediately after FillLayout adds new lines.
Changes:
- Changed
FillLayoutto return the number of newly materialized lines. - In non-scroll update paths, attempted to force an immediate layout pass when new lines were added via
OwnerPanel.UpdateLayout().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If any new lines are materialized from FillLayout, we need to force an immediate layout. | ||
| // This is because the newly materialized lines are not arranged, and if a render pass happens to occur | ||
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | ||
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), | ||
| // as the user will notice a single frame of broken view between two correct frames. | ||
| OwnerPanel.UpdateLayout(); |
There was a problem hiding this comment.
OwnerPanel.UpdateLayout() is very likely a no-op here: this method is only called with isScroll: false from MeasureOverride, i.e. while the visual tree root is already in a layout pass. UIElement.UpdateLayout() early-returns when _isLayoutingVisualTreeRoot (or _isInUpdateLayout) is set, so this won't reliably force an arrange before the next render and may not address the flicker.
Consider instead explicitly arranging the newly materialized elements (similar to the scroll path calling ArrangeElements(...)), or deferring the UpdateLayout() call to run after the current layout cycle (e.g., via dispatcher) so it can actually execute.
| // If any new lines are materialized from FillLayout, we need to force an immediate layout. | |
| // This is because the newly materialized lines are not arranged, and if a render pass happens to occur | |
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | |
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), | |
| // as the user will notice a single frame of broken view between two correct frames. | |
| OwnerPanel.UpdateLayout(); | |
| // If any new lines are materialized from FillLayout, we need to trigger another layout pass. | |
| // Calling UpdateLayout() directly here is unreliable because this path is reached during measure, | |
| // while the visual tree root is already processing layout. Defer the request so it runs once | |
| // the current layout cycle has completed and the newly materialized children can be arranged | |
| // before the next render. | |
| _ = OwnerPanel.DispatcherQueue.TryEnqueue(() => OwnerPanel.UpdateLayout()); |
| // If any new lines are materialized from FillLayout, we need to force an immediate layout. | ||
| // This is because the newly materialized lines are not arranged, and if a render pass happens to occur | ||
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | ||
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), | ||
| // as the user will notice a single frame of broken view between two correct frames. | ||
| OwnerPanel.UpdateLayout(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Even if OwnerPanel.UpdateLayout() does run, it forces a synchronous layout of the entire visual tree root. That can be very expensive and risks layout reentrancy side-effects for what appears to be a panel-local arrange issue. Prefer a more targeted approach (e.g., directly arranging the materialized containers or only invalidating/arranging the owner panel) so this doesn’t introduce a perf regression on collection moves.
| // If any new lines are materialized from FillLayout, we need to force an immediate layout. | |
| // This is because the newly materialized lines are not arranged, and if a render pass happens to occur | |
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | |
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), | |
| // as the user will notice a single frame of broken view between two correct frames. | |
| OwnerPanel.UpdateLayout(); | |
| } | |
| } | |
| // If any new lines are materialized from FillLayout, we need to force an immediate arrange. | |
| // This is because the newly materialized lines are not arranged, and if a render pass happens to occur | |
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | |
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), | |
| // as the user will notice a single frame of broken view between two correct frames. | |
| // | |
| // Avoid UpdateLayout() here because it forces a synchronous layout of the visual tree root. | |
| // We only need to refresh the owner panel's arrange so newly materialized children receive a valid slot. | |
| ArrangeOwnerPanel(); | |
| } | |
| } | |
| private void ArrangeOwnerPanel() | |
| { | |
| var arrangeRect = new Rect(0, 0, OwnerPanel.ActualWidth, OwnerPanel.ActualHeight); | |
| if (arrangeRect.Width <= 0 || arrangeRect.Height <= 0) | |
| { | |
| return; | |
| } | |
| OwnerPanel.InvalidateArrange(); | |
| OwnerPanel.Arrange(arrangeRect); | |
| } |
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | ||
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), |
There was a problem hiding this comment.
There are a few grammar/wording issues in this new comment that make it harder to read/grep: "all item" -> "all items", "listview" -> "ListView", and "atm" is informal (prefer "currently").
| // before the next layout pass, the listview will render with all item clustered at the top-left of the panel. | |
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented atm), | |
| // before the next layout pass, the ListView will render with all items clustered at the top-left of the panel. | |
| // This is particularly noticeable from the full refresh caused by NotifyCollectionChangedAction.Move (not properly implemented currently), |
ajpinedam
left a comment
ajpinedam
left a comment
There was a problem hiding this comment.
Being a fix for a visual issue I imagine it's hard/impossible to be tested
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-23168/wasm-skia-net9/index.html |
|
|
GitHub Issue: closes unoplatform/kahua-private#452
PR Type:
🐞 Bugfix
What changed? 🚀
When moving an item in a
ListView,NotifyCollectionChangedAction.Movetriggers a full panel refresh.FillLayoutmaterializes new lines, but they are not arranged until the next layout pass. If a render pass occurs between materialization and arrangement, items appear clustered at(0,0)for one frame — visible as a brief flicker of broken layout.This PR makes
FillLayoutreturn the number of lines it added, and forces an immediateOwnerPanel.UpdateLayout()after non-scroll refresh paths when new lines were materialized, ensuring items are arranged before the next render pass.PR Checklist ✅
Screenshots Compare Test Runresults.