Keep layout from looping and the scroll offset from leaving its range - #123
Merged
Merged
Conversation
Two reports from our users, one root: `_updateDisplayRenderParagraphs` calls itself when layout has corrected the scroll offset, and that recursion had no limit. It does not always converge. `applyContentDimensions` starts a ballistic activity while the offset is out of range, the activity moves the offset, and the next cycle sees the same mismatch. Paging through a file froze the application hard; the stack taken from the running process was thousands of frames of this one method. The estimate behind it cannot be made exact: lines outside the viewport are counted as one grid line each, while a wrapped line takes more. So the number of cycles is bounded, the way `RenderViewport` bounds itself with `_maxLayoutCycles`. Stopping early costs at most one imprecise frame. The rebuild is also skipped when there is nothing above to build. A gap at the top appears when the offset is merely out of range — a hair above the first line — and rebuilding cannot close it if line 0 is already the first one shown. Layout kept asking for layout, the ballistic activity that would have returned the offset to zero was restarted by every frame, and the editor shuddered at the top of the file. Finally, `makePositionVisible` no longer leaves the scrollable range, and the distance to a line below the viewport is counted from the last displayed line rather than the first — counting from `first` overshot by a whole screen. Landing out of range is what hands the offset to the spring simulation in the first place.
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this fixes
_updateDisplayRenderParagraphscalls itself when layout has corrected thescroll offset, and that recursion has no limit. It does not always converge:
applyContentDimensionsstarts a ballistic activity while the offset is out ofrange, the activity moves the offset, and the next cycle sees the same mismatch.
Two failures our users hit, both taken from a running process rather than
guessed:
PageUp/PageDownpresses. Thestack pulled from the VM service was thousands of frames of this one method.
a point above zero (we logged −0.7), layout kept asking for layout, and the
ballistic activity that would have returned the offset to zero was restarted
by every frame and never got to run.
The change
RenderViewportbounds itselfwith
_maxLayoutCycles. The estimate behind the recursion cannot be madeexact — lines outside the viewport are counted as one grid line each, while a
wrapped line takes more — so it needs a bound rather than a better guess.
Stopping early costs at most one imprecise frame.
already the first one shown, rebuilding cannot close a gap at the top.
makePositionVisibleno longer leaves the scrollable range, and the distanceto a line below the viewport is counted from the last displayed line rather
than the first — counting from
firstovershot by a whole screen. Landing outof range is what hands the offset to the spring simulation in the first place.
Found while building a file manager on
re_editor.Related: #122 — that one removes the source of the accumulating correction. This one keeps layout terminating whatever the estimate, since with word wrap the estimate cannot be exact.