perf(screen): collapse non-clear ResetPosition cursor walk-up into one CSI#1285
Merged
ArthurSonzogni merged 3 commits intoJun 4, 2026
Merged
Conversation
…ed CSI In Screen::ResetPosition(std::string&, bool), the non-clear (else) branch previously emitted one \x1B[1A per row to walk the cursor to the top. Replace that per-row loop with a single parameterized CSI cursor-up (\x1B[<dimy_-1>A), guarded by if (dimy_ > 1). The leading \r (MOVE_LEFT) is preserved and the final cursor position is identical. For a 50-row screen this reduces the per-frame output from 197 bytes to 6 bytes (~32x). The clear branch is left unchanged because each row needs its own \x1B[2K erase; a comment documents the rationale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add src/ftxui/screen/screen_test.cpp covering the collapsed non-clear cursor walk-up in Screen::ResetPosition: - dimy_ > 1 emits exactly "\r\x1B[<dimy-1>A" (single CSI cursor-up) - dimy_ == 1 emits only "\r" - output is byte-for-byte equivalent to the old per-row \x1B[1A walk - 50-row screen output is >=10x smaller (6 bytes vs 197) - the clear branch remains per-row (each row keeps its CLEAR_LINE) Register the new test in cmake/ftxui_test.cmake. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce test Add a Changelog 'Next' > Screen entry describing the per-frame escape-byte reduction in the non-clear Screen::ResetPosition. Also fix ResetPositionNonClearEquivalentToPerRowWalk: the collapsed single-CSI form (\x1B[<n>A) is intentionally NOT byte-equal to the old per-row walk (\x1B[1A x N), so equivalence is now checked by terminal effect (total rows moved up) instead of string equality. All 339 ctest tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Thanks! That looks useful! let's wait for the CQ. |
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.
Problem
In the steady-state redraw path,
Screen::ResetPosition(std::string&, bool)(the non-clearbranch) walks the cursor back to the top-left by emitting one\x1B[1A(cursor-up) escape per row. On a tall terminal this writes a large, redundant burst of escape bytes every single frame. The cursor only needs to be repositioned once — the per-row walk encodes the same movement far more verbosely than necessary.The change
Collapse the per-row walk-up into a single parameterized CSI cursor-up, guarded for the single-row case:
The leading
\rand the final cursor position are unchanged. On-screen output is identical.The
clearbranch is intentionally left per-row: each row needs its own\x1B[2K(CLEAR_LINE) erase, so it cannot be collapsed. A code comment notes this.Before / after (bytes per frame, non-clear path)
Tests
Adds
src/ftxui/screen/screen_test.cpp(registered incmake/ftxui_test.cmake):dimy_ > 1equals"\r\x1B[" + (dimy_-1) + "A"; equals"\r"fordimy_ == 1.clearbranch remains per-row.ctestpasses in full: 339/339 tests pass, including the new ones.