Trim per-frame preview render work on dense layouts - #10
Merged
Merged
Conversation
Measured that renderScene, not fill calls, spent per-frame time on repeated data-dependent work. Two changes, both verified visually identical: - Hoist the outer-tube-limit radius into a useMemo (view-independent), so its per-point Map.get + sqrt loop no longer runs on every pan/zoom frame. - Add a no-overrides fast path: when there are no per-hole edits or selection (the common large layout), skip the per-point Map.get / Set.has lookups and draw plain dots directly; the override path is unchanged. Rendering was already within frame budget (~5ms for 52k holes); these keep it there without the redundant per-frame passes. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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
1 participant
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.
Follow-up to the layout-worker work: reduce redundant per-frame work in the preview render.
What I found (measured, not assumed)
Benchmarking the real render path on a 52,471-point layout was clarifying:
renderSceneruns in ~5 ms/frame — already within the 60fps budget. Rendering was not the bottleneck.Path2D-batched fill was not faster than plain per-holearc+fill(Chrome optimises the latter well), so I did not ship batching.Changes (both verified visually identical)
useMemoin the preview — it's view-independent, so it no longer recomputes on every frame, only when the layout / hidden set changes.renderScene: when there are no per-hole edits and no selection, skip the per-pointMap.get/Set.hasand draw plain dots directly. The modified/selected path is unchanged.Honest note on magnitude
For typical layouts (hundreds–few thousand holes) everything here is already sub-millisecond; these help only the extreme near-cap layouts. The larger one-time cost on a layout change is point-key computation done in several modules (~9 ms each at 52k) — a separate key-sharing refactor could halve that, but it only matters near the cap, so I've left it out rather than add cross-module coupling for a rare case.
Verification
pnpm typecheck✅ ·pnpm test(58) ✅ ·pnpm build✅🤖 Generated with Claude Code