perf(canvas): sub-pixel node culling at low zoom#636
Merged
softmarshmallow merged 1 commit intomainfrom Apr 7, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughAdds sub-pixel LOD culling to Renderer::frame: computes min_world_size = 0.5 / zoom and filters visible layer indices by per-node absolute render bounds (width/height vs min_world_size) in both full-viewport and R-tree query paths; nodes without bounds remain included. Documentation updated to mark the feature implemented. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Renderer
participant GeometryCache
participant FramePlan
participant Compositor
Client->>Renderer: request frame(zoom, ...)
Renderer->>GeometryCache: lookup absolute_render_bounds(node)
GeometryCache-->>Renderer: bounds (or None)
Renderer->>FramePlan: compute min_world_size = 0.5 / zoom
Renderer->>FramePlan: filter layer indices by bounds vs min_world_size
FramePlan-->>Compositor: viewport & indices (culled)
Compositor->>GPU: skip draw calls for culled indices / submit draws
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/wg/feat-2d/optimization.md (2)
1357-1361: Nit: Word variant consistency.Static analysis flagged mixed usage of "pre-compute" variants. Consider using "Precomputes" (unhyphenated) for consistency with common technical writing style.
Drop leaf nodes from the frame plan when both projected dimensions - fall below a threshold (0.5 px). Pre-computes + fall below a threshold (0.5 px). Precomputes `min_world_size = 0.5 / zoom` and filters indices during🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wg/feat-2d/optimization.md` around lines 1357 - 1361, The phrase "Pre-computes" in this paragraph is using a hyphenated variant; update it to "Precomputes" for consistency with the surrounding technical style and ensure other occurrences in this section use the same unhyphenated form (references: min_world_size, Renderer::frame(), absolute_render_bounds, DenseNodeMap, frame plan, projected dimensions).
1375-1385: Minor: Benchmark numbers differ from PR summary.The benchmark results in this table differ slightly from those in the PR description:
Metric PR Summary This Doc baseline_nocache_fit draw_us 7,417 (−16%) 7,248 (−18%) fl_16ms MAX 85,990 (−22%) 82,731 (−25%) This could be from different benchmark runs. Consider aligning these values if one set is more authoritative, or note that results may vary between runs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wg/feat-2d/optimization.md` around lines 1375 - 1385, The documented benchmark table contains numbers that conflict with the PR summary for specific metrics (baseline_nocache_fit draw_us and fl_16ms MAX); locate the results in the doc table and reconcile them by either updating the table to use the authoritative numbers from the PR (or vice versa) or add a short note stating that benchmarks vary between runs and which run is authoritative, referencing the affected metrics (baseline_nocache_fit draw_us, fl_16ms MAX) and the implementation location Renderer::frame() in runtime/scene.rs so readers know which code the numbers apply to.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/wg/feat-2d/optimization.md`:
- Around line 1357-1361: The phrase "Pre-computes" in this paragraph is using a
hyphenated variant; update it to "Precomputes" for consistency with the
surrounding technical style and ensure other occurrences in this section use the
same unhyphenated form (references: min_world_size, Renderer::frame(),
absolute_render_bounds, DenseNodeMap, frame plan, projected dimensions).
- Around line 1375-1385: The documented benchmark table contains numbers that
conflict with the PR summary for specific metrics (baseline_nocache_fit draw_us
and fl_16ms MAX); locate the results in the doc table and reconcile them by
either updating the table to use the authoritative numbers from the PR (or vice
versa) or add a short note stating that benchmarks vary between runs and which
run is authoritative, referencing the affected metrics (baseline_nocache_fit
draw_us, fl_16ms MAX) and the implementation location Renderer::frame() in
runtime/scene.rs so readers know which code the numbers apply to.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b712cb76-89a0-46de-b223-87b75f2f8cbb
📒 Files selected for processing (2)
crates/grida-canvas/src/runtime/scene.rsdocs/wg/feat-2d/optimization.md
At low zoom levels (e.g. 0.02 fit-zoom on 136K-node scenes), most nodes project to sub-pixel screen size. Skip these during frame plan building to eliminate downstream draw calls, picture cache lookups, and GPU rasterization for invisible results. Pre-computes min_world_size = 0.5 / zoom and filters indices in Renderer::frame() using absolute_render_bounds from the geometry cache (O(1) per node via DenseNodeMap). Applies to both the full-viewport fast path and the R-tree query path. Measured on Apple M2 Pro, 136K-node fixture: - draw_us: -16% to -43% depending on scenario - fl_16ms MAX: -22% (110ms → 86ms) - No regressions on pan/zoom cached frames or effects-heavy scenes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
softmarshmallow
force-pushed
the
feature/funny-shockley
branch
from
e663b27 to
bfd5ce1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/grida-canvas/src/runtime/scene.rs (1)
2146-2224: Add targeted boundary tests for the new culling threshold.Please add focused tests around
0.5pxthreshold behavior (<,==,>),zoom == 1.0gating, and missing-bounds inclusion. This logic is performance-critical and easy to regress silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-canvas/src/runtime/scene.rs` around lines 2146 - 2224, Add focused unit tests around the sub-pixel culling logic: create tests that exercise SUBPIXEL_CULL_THRESHOLD/min_world_size behavior by setting zoom and render-bounds so nodes are just below, exactly equal to, and just above the 0.5px projected threshold and assert passes_size_cull returns false/true as expected; add a test where zoom == 1.0 to ensure min_world_size gating disables size culling and all nodes are included; include a case where a node has no render bounds (or no layer entry) to assert it is conservatively included; finally, exercise the full-viewport fast-path by making layer_count == rtree_size and the scene_envelope fully contained in bounds to ensure the code path that returns 0..n is taken and still respects size-cull when min_world_size > 0.0 (use the functions/fields scene_cache.layers.layers, scene_cache.layer_index.size(), scene_cache.scene_envelope(), passes_size_cull, and self.scene_cache.intersects to set up and verify each scenario).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/wg/feat-2d/optimization.md`:
- Line 1358: The doc uses inconsistent capitalization/hyphenation for the term
"precompute"; replace the instance "Pre-computes" (found in the sentence
containing "fall below a threshold (0.5 px). Pre-computes") with the
standardized form "precompute" to match other occurrences (e.g.,
"precompute"/"precomputed") and ensure consistent wording across the document.
---
Nitpick comments:
In `@crates/grida-canvas/src/runtime/scene.rs`:
- Around line 2146-2224: Add focused unit tests around the sub-pixel culling
logic: create tests that exercise SUBPIXEL_CULL_THRESHOLD/min_world_size
behavior by setting zoom and render-bounds so nodes are just below, exactly
equal to, and just above the 0.5px projected threshold and assert
passes_size_cull returns false/true as expected; add a test where zoom == 1.0 to
ensure min_world_size gating disables size culling and all nodes are included;
include a case where a node has no render bounds (or no layer entry) to assert
it is conservatively included; finally, exercise the full-viewport fast-path by
making layer_count == rtree_size and the scene_envelope fully contained in
bounds to ensure the code path that returns 0..n is taken and still respects
size-cull when min_world_size > 0.0 (use the functions/fields
scene_cache.layers.layers, scene_cache.layer_index.size(),
scene_cache.scene_envelope(), passes_size_cull, and self.scene_cache.intersects
to set up and verify each scenario).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9e926b5-36df-46ef-96bd-3c613aea176e
📒 Files selected for processing (2)
crates/grida-canvas/src/runtime/scene.rsdocs/wg/feat-2d/optimization.md
| fixture, ~38% of visible leaves have both dimensions below 0.5 px | ||
| at zoom 0.02. Culling them reduces `draw_us` by 6–18% and GPU | ||
| `mid_flush_us` by up to 24%. | ||
| fall below a threshold (0.5 px). Pre-computes |
There was a problem hiding this comment.
Use consistent wording for “precompute”.
Line 1358 uses “Pre-computes”; elsewhere the doc uses “precompute/precomputed”. Please standardize to one form for consistency.
✏️ Suggested doc edit
- fall below a threshold (0.5 px). Pre-computes
+ fall below a threshold (0.5 px). Precomputes📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fall below a threshold (0.5 px). Pre-computes | |
| fall below a threshold (0.5 px). Precomputes |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1358-~1358: Do not mix variants of the same word (‘pre-compute’ and ‘precompute’) within a single text.
Context: ...ns fall below a threshold (0.5 px). Pre-computes min_world_size = 0.5 / zoom and f...
(EN_WORD_COHERENCY)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/wg/feat-2d/optimization.md` at line 1358, The doc uses inconsistent
capitalization/hyphenation for the term "precompute"; replace the instance
"Pre-computes" (found in the sentence containing "fall below a threshold (0.5
px). Pre-computes") with the standardized form "precompute" to match other
occurrences (e.g., "precompute"/"precomputed") and ensure consistent wording
across the document.
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.
Summary
min_world_size = 0.5 / zoomand filters indices inRenderer::frame()usingabsolute_render_bounds(O(1) per node viaDenseNodeMap)Benchmark (Apple M2 Pro, 136K-node fixture)
No regressions on pan/zoom cached frames or effects-heavy scenes (bench.grida 72K nodes, 7110 effects).
Test plan
cargo test -p cg— all 900+ tests passcargo check -p cg -p grida-canvas-wasm -p grida-dev— all crates compilecargo clippy -p cg --no-deps— no new warnings🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Documentation