Repository navigation
Reduce MarkupControl memory allocations and add scroll offset notifications - #93
Conversation
|
Thanks for this, and for working through the headers, formatting and docs across the six commits. The allocation work is the kind of change that is easy to get subtly wrong and you kept the public surface intact throughout. Verified on
The event tests cover the values but not that it fires from real scrolling. The second file drives keys through the input dispatch and a wheel through the control's mouse path, then asserts the offset survives a re-render, that successive notifications chain, and that scrolling against the top raises nothing.
|
WriteTextRowClipped computes its own clip bounds and had a single caller and no tests. It paints the cell buffer directly, so a wrong column there is silent: a CJK line at a window edge loses or gains a column and nothing fails. The 25 cases paint the same cells through it and through WriteCellsClipped at every clip offset, with wide glyphs straddling each edge, and compare the buffers. The arithmetic is right as written. The -1 and +1 are wider bounds than needed rather than corrections: either can be removed with every case still passing, because the clamp and the per-cell check in WriteCellsClippedCore absorb the difference. Shifting the write position by one column fails 24 of the 25, so the tests do hold the painting itself. The offset tests drive the real paths. VerticalScrollOffsetChanged exists so a virtualizing consumer can follow the viewport, and that consumer never sets the offset itself: it scrolls, or the user does. So these send keys through the input dispatch and a wheel through the control's mouse path, inside a hosted window, and assert the reported offset is the one the panel still has after the frame is composed.
|
Thanks, this is great work. Merged. One thing I left open, worth a follow-up note rather than a change: I mutated both adjustments in So they read as wider bounds rather than corrections. If that is what you intended, a line saying so would stop someone later reading them as an off-by-one and "fixing" it. If they are load-bearing for a case the tests miss, that case is worth adding. |
|
This is out in 2.10.8. I measured the scroll path after merging, since I had only eyeballed it. 600 scrolls over 400 markup lines mixing CJK, emoji, combining marks and per-word tags, headless, three alternating runs each way:
45% faster and 27% less allocated, with the parse cache warm in both, so this is the painting itself getting cheaper rather than fewer parses. MarkupCacheDemo is visibly smoother to scroll now. |
Summary
This PR reduces memory allocations in MarkupControl rendering and adds scroll offset notifications needed by virtualization consumers.
Changes
Type
Testing
Commands run:
Code quality
Screenshots are not applicable: this changes parsing/cache behavior and adds a notification API without changing visual design.
I use this new API in my application for virtual scrolling and memory optimization during long-running sessions.