Skip to content

Scope a lazy-list scroll to its subtree instead of rebuilding the whole scene - #504

Merged
samoylenkodmitry merged 1 commit into
mainfrom
fix/scroll-renderer-surfaces
Aug 27, 2026
Merged

Scope a lazy-list scroll to its subtree instead of rebuilding the whole scene#504
samoylenkodmitry merged 1 commit into
mainfrom
fix/scroll-renderer-surfaces

Conversation

@samoylenkodmitry

Copy link
Copy Markdown
Owner

Part of #500 (cause 1 of 3). Independent of #497 — different subsystem, no shared commits.

Problem

A scrolling LazyColumn rebuilt the render graph from the composition root on every framerebuild_scene_from_applier rather than the scoped update_scene_from_applier the path was designed to take. The work is O(whole app) where it should be O(visible rows).

Two independent gaps stacked, and both had to close:

  1. The measure-repass node ids were dropped. The list invalidates through schedule_measure_repass — its item sizes are what change, so placement-only dirtiness is not enough — but run_layout_phase_in_context only ever collected ids from layout repasses. The scene phase then saw scene_dirty = true with an empty dirty set, which is indistinguishable from "everything changed", and fell through to the full rebuild.

  2. The scoped ids were assigned, not accumulated. A frame runs the layout phase twice — once initially, once after post-layout recomposition — and the second pass, with nothing pending, cleared what the first had recorded. Whichever pass ran last won, so scoping survived or vanished depending on which pass the scroll landed in. (Closing gap 1 alone fixed the first scroll step and not the second; that asymmetry is what exposed this.)

The phase now accumulates across passes and only an app-wide relayout — where scoping means nothing — clears them. The scene phase already takes them once per frame via std::mem::take.

Also corrects the doc on LazyListState::dispatch_scroll_delta, which claimed schedule_layout_repass and "O(subtree) performance instead of O(entire app)". The code had drifted from its own stated contract; that stale comment is part of why this went unnoticed.

Why no test caught it

lazy_column_scroll_repass_uses_scoped_renderer_update_without_stale_rows asserts updates + rebuilds == 1 — it accepts either path. The sibling graphics_layer repass tests in the same file assert rebuilds == 0. The lazy path is the one that lost that bound, and it is the path every scrolling screen in a real app takes.

New test lazy_column_scroll_never_rebuilds_the_whole_scene asserts zero rebuilds and exactly one scoped update carrying dirty ids, held across three steady-state scroll steps so it cannot pass on a first-frame special case. It fails on the parent commit (rebuilds: 1) and passes here.

Measurements (Huawei EVR-AL00, Kirin 980)

Same scene, same gesture, same branch, A/B against the parent commit — demo Lazy List tab, debug.cranpose.frame_stage_ms:

before after
scene_ms typical ~1.5 ms ~0.8 ms
scene_ms worst 3.11 ms 2.06 ms

Please read that as a lower bound on the win, not the headline: the demo's whole composition tree is small, and the bug's cost is proportional to total app size, so a real app pays more than the demo does. I have not yet measured a real app against this build — that verification is still outstanding.

Explicitly not fixed by this change: the demo's Liquid tab does not take this path and is unchanged (its heavy frames still show scene_ms 6-13 ms, because the scoped update itself rebuilds the whole dirty subtree with no offset-only fast path). Causes 2 and 3 from #500 — render-pass count and backdrop cache thrash — are untouched here; both are traced to exact mechanisms in the issue thread.

Testing

cargo test green for the changed crates (cranpose-app-shell, cranpose-ui, cranpose-foundation — 1400+ tests). just fmt and clippy clean on the changed crates. The full just test workspace gate was still running when this PR was opened; I will report the result on the PR.

🤖 Generated with Claude Code

@samoylenkodmitry

Copy link
Copy Markdown
Owner Author

Local gates green

  • just test (full workspace) — pass, after the second commit. It was red on main before this branch: workspace_tests_do_not_default_to_tmpfs_paths flagged a /tmp/ literal in the non-UTF-8 toggle fixture that arrived with a1f64e5. The scan matches a source literal rather than a runtime path, so it fails on every platform, not just macOS. Happy to split that commit out if you'd rather keep this PR single-purpose.
  • just clippy (workspace, -D warnings) — clean.
  • just fmt — clean.

On the evidence, restated plainly

The mechanism is proven by the new unit test: the renderer now receives a scoped update carrying dirty node ids, where it previously received a full rebuild, held across three steady-state scroll steps. That is the part that matters architecturally — the work went from O(whole app) to O(subtree).

The timing number (scene_ms ~1.5→0.8 ms typical, 3.11→2.06 ms worst, Kirin 980, demo Lazy List, A/B against the parent commit) is a weak proxy for the same thing, because the demo's whole composition tree is small — which is exactly the quantity the old code was linear in. A real app should gain more; I have not measured one yet, so please do not read the demo delta as the expected win.

Verifying against cranscan is the obvious next step and needs a [patch.crates-io] override plus its native MNN/OCR Android build; I have not started that.

Not addressed here (both traced in #500)

  • The demo's Liquid tab does not take this path and is unchanged — heavy frames still show scene_ms 6-13 ms. Its scoped update rebuilds the whole dirty subtree unconditionally; there is no offset-only fast path in build_layer_node_from_applier_internal, so a pure translation still re-emits every draw command and rebuilds every text primitive under the scrolled subtree. That is the other half of cause 1.
  • Causes 2 (render-pass count) and 3 (backdrop cache thrash) are untouched.

One measurement caveat for anyone using the desktop harness on this: perf_robot_fps.sh --scenario glass_lazy_scroll pins to exactly 60 fps here despite present_mode=immediate, so it cannot show CPU-side wins at all, and its cache_hit_rate_pct moved 48.8% → 41.8% between two runs with different sample counts (57 vs 103). I would not gate on that number without fixing the sampling first.

…le scene

A scrolling LazyColumn rebuilt the render graph from the composition root
on every frame. Two gaps put it there, and both had to close:

The list invalidates through `schedule_measure_repass` — its item sizes
are what change, so placement-only dirtiness is not enough — but the
layout phase only ever collected node ids from *layout* repasses. The
measure ids were dropped, so the scene phase saw the scene marked dirty
with an empty dirty set, read that as "everything changed", and took
`rebuild_scene_from_applier`.

Closing that alone was not enough: a frame runs the layout phase twice,
once initially and again after post-layout recomposition, and the phase
*assigned* the scoped ids rather than accumulating them. Whichever pass
ran last won, so a second pass with nothing pending wiped what the first
recorded — the ids survived or vanished depending on which pass the
scroll landed in. The phase now accumulates across passes and only an
app-wide relayout, where scoping means nothing, clears them; the scene
phase already takes them once per frame.

Measured on the demo lazy list, the scene stage was 6.4-7.4 ms of a
16.7 ms frame on a Kirin 980 — the largest CPU stage of a scrolled
frame.

The regression test asserts what the sibling graphics-layer repass tests
already assert and the lazy path had lost: zero rebuilds, one scoped
update carrying dirty ids, held across three steady-state scroll steps.
The pre-existing lazy test accepted either path, so it could not catch
this; it now exercises the scoped path it was written for.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@samoylenkodmitry

Copy link
Copy Markdown
Owner Author

On-device check on cranscan, and a false alarm worth recording

cranscan's Library page is a LazyColumn, so this change is genuinely exercised by a real app scene rather than only the regression test. Measured on the Huawei EVR-AL00 (.codex package, same 24-document gallery, on-device OCR disabled, app settled at 0–3 % CPU before each run).

This looked like a regression and is not one. Raw update p50, patched vs unpatched:

arm update p50
unpatched 4.38, 4.51, 4.57
patched 4.63, 5.69, 6.34, 7.05, 7.53

The patched minimum exceeds the unpatched maximum — clean separation, and update is the stage this PR touches, so it reads as a ~2 ms regression introduced here.

It is not. Controlling with render, a stage this PR does not touch, as a proxy for the device's CPU clock state:

run update render update/render
unpatched 4.38 4.92 0.89
unpatched 4.51 4.99 0.90
patched 4.63 5.24 0.88
patched 5.69 6.01 0.95
patched 7.05 7.03 1.00
patched 7.53 7.22 1.04

The ratio is flat across every run. The absolute values drift together because the Kirin 980 clocks down between measurement sessions — the unpatched runs happened to land in a fast state and most patched runs in a slow one. Comparing runs at matched render cost (patched 4.63 @ render 5.24 vs unpatched 4.38–4.51 @ render 4.92–4.99) shows no regression.

Method note for anyone measuring this device: a single stage compared across sessions is not interpretable. Carry an untouched stage as a within-run control, or the CPU governor will hand you whatever result the thermal state happens to favour. Two arms in my own session also disagreed on vsync_period_ms (15.841 vs 16.638), which is the other tell that two runs were not in the same device state.

No fps change, as expected — this scene's frame is bound by present and render, not by scene rebuild. The scroll work that does move fps is #500, whose ranked causes were re-verified on a quiet device (35 render passes, 9 isolated layers, 34.6 % layer-cache hits with 1.78 MP re-blurred per scrolled frame).

The correctness claim of this PR — zero full rebuilds during a steady-state scroll — is asserted by lazy_column_scroll_never_rebuilds_the_whole_scene rather than by these numbers, which is the right place for it.

🤖 Generated with Claude Code

@samoylenkodmitry
samoylenkodmitry merged commit b46465d into main Aug 27, 2026
7 checks passed
@samoylenkodmitry
samoylenkodmitry deleted the fix/scroll-renderer-surfaces branch August 28, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant