grid, daemon: ship scrolls over written rows and DL/IL as directives #455
No reviewers
Labels
No labels
priority/P0
priority/P1
priority/P2
release/v0.1.0
status/blocked
status/planned
type/bug
type/design
type/test-gap
type/tracker
No milestone
No project
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
natsukium/felis!455
Loading…
Reference in a new issue
No description provided.
Delete branch "scroll-447"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The daemon now sends
GridMsg::Scrolledfor a scroll over rows already written in the same frame, and forIL/DL. The directive is followed by only the rows the shift does not reproduce, so the daemon no longer resends the whole scrolled region as rows. The wire format is unchanged, and so isfelis.proto.What changed
scroll_region_up/downalways queue the directive, andDamage::shift_bandmoves the band's dirty bits along with the rows and marks the rows the scroll vacated. A row written before the scroll therefore stays owed at its new position.IL/DLrotate the band from the cursor row to the bottom margin and send the same directive. They used to callmark_all(). The grid's effect queue folds consecutive shifts of one band into one entry. That entry carries the range of the scroll sequence it covers,first_seq..=last_seq. A scrollback push also folds past a trailing directive, so a line-feed flood drains as two effects.accept_scrollfell back to replaying the band when a subscriber still owed rows in it. Now it shifts the subscriber's ownDamagethe same way. It folds consecutive shifts of one band, and it drops a directive once the whole band is owed anyway. The 1024 queue cap stays.offer_scrollreplays every row when a folded entry spans a full replay, so the shift is never applied twice.catch_up_toreplays every row when the grid's damage would merge in while a scroll is still queued in the grid.grid_cycle_for, the path that answers a pull, now drains effects first, which keeps that second case rare during a flood.ScreenBuffer::write_row_cellscompared the incoming cells with the row's storage past its watermark and skipped the write when they matched. That storage is where a directive leaves the rotated-out cells. So on the client, a line that matched the line that had just scrolled out showed blank.mainhas this bug for pure scrolls today: a loop printing\nyshows it once the screen is full.ESC[1;Nr ESC[H ESC[M ESC[r, and the two DECSTBMs in it turned every^Einto a full-screen replay.damage_correctness.rsused to count a scrolled band as covered. It now applies the queued directives to the prior screen and checks every row that is not marked, so the test no longer passes trivially whenever a band scrolled.proptest_grid.rsnow starts from a non-empty screen.proptest_damage.rsmodelsshift_band/all_dirty_in_range.every_mirror_ends_each_compose_holding_the_grids_cells. It drives two subscribers into realShadowScreens, each pulling at its own random cadence, through random output (DECSTBM,IL/DL, DECSLRM, the alt screen, scrollback clears, BCE), drain cadences, resizes and scrollback browsing. After every compose it requires each shadow to hold exactly the grid's cells. I ran it at 512 cases in the suite and at 30 000 in release.docs/reference/ipc.md(theScrolled/RowDeltabullets and the send-side rules).docs/explanation/rendering/damage-tracking.md: the mechanism, plus records for the decisions below.docs/explanation/rendering/pipeline.mdcoalescing sentence, which described a merge the code did not do.docs/reference/testing.md.CHANGELOG.md.Open questions from the issue
Damagelearnedshift_band(plusall_dirty_in_range/is_dirty). The grid calls it right afterrotate_region, and the subscriber stream calls it inaccept_scroll.damage_correctness.rs(now strict) and the row-cache property both pass with a directive and rows in the same cycle.Scrolledhas no column bounds, and no measured workload scrolls a DECSLRM band every frame. Recorded with a "Revisit if".DL/ILstill userotate_regionwithout a scrollback push, andPtyEffect::Scrolledis only parked in the queue. A scrollback push folds past a trailing directive, which commutes with it because the directive touches no placement.Released client (v0.1.0)
v0.1.0is not a tag on the remote. I compared againstv0.1.0-rc.6(4b9af186), the code released as 0.1.0. Between rc.6 andmain, the client's apply path (ShadowScreen::apply/scroll,ScreenBuffer::apply_scroll_directiveandwrite_row_cells) differs only by an unrelatedclear_damage.RowDeltaentries override the rows in order. So a stream in which "every row not resent equals the shifted mirror" converges, and nothing on the client assumes the band is clean afterwards.write_row_cellsskip described above. Sending directives far more often would have hit it on everyyesline. The daemon property test checks every row entry against that rc.6 behaviour: it fails if a row's storage equals the incoming cells while the row reads differently. The test fails when the default-cell entry is removed and passes with it.Measurements
Setup: Linux, NVIDIA, release build, 212x93 window. The client was instrumented to count, per pull cycle, the rows and bytes it receives in
RowDeltaand theScrolleddirectives. Base isorigin/mainat35cd16b8. There were 3 runs per workload per build, and the table gives medians of per-cycle values. One-minute load average during the runs:The row and directive counts do not depend on load.
Scrolledper cycle (base → new)less,jevery 30 msprintf "\n...")vim -u NONE,^Eevery 30 msseq -wflood\rbefore each line feed marked. The last one is a pre-existing overdraw from cursor-only moves, filed as #454.Parse-thread cost, felis-grid Criterion benches on the same host with a load average of about 3–5. Base → new:
scroll_region/full_screen_scroll_up_1: 55.0 → 19.9 nspartial_region_scroll_up_1: 111 → 65.6 nsmixed_print_scroll_full: 131.7 → 118.2 nsflood_scroll_up_1: 42.1 → 34.5 nsascii_throughput/crlf_lines: 101 → 98 µsbare_lf_staircase: 134 → 113 µsThe full-screen and partial-region cases gain because a directive now folds into the tail entry of the queue instead of adding one entry per line.
Review
Independent review with
pi(sol) overorigin/main...HEAD:ipc.mdsaid vacated rows that are default blanks are left out. In fact every vacated row is sent again. Fixed.CHANGELOG.md.shift_band.Closes #447
a53fc7c0f776ec65ab54