Grida Canvas CG: save layer optimizations & UX improvements #479
Conversation
… enable caching for all crates
WalkthroughThis PR refactors blend-mode isolation in the painter by centralizing bounds computation into world space, updates the editor's selection and hover API with explicit modes and sources, improves undo/redo handling in text-editing contexts, reorganizes optimization documentation toward bounds-aware rendering, and consolidates UI components across the viewport. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
softmarshmallow
changed the title
…rement target calculations
…lick with mod keys to NOT trigger rename
…hover state handling for title bars
…triggered hover actions, and implement consistent state validation on window blur
…ne-based logic for hit testing
… bounds for `save_layer`, adaptive resolution during zoom, and interaction overlays at full resolution
…n and updating drawing methods for shapes and text layers
…ws in painter module
softmarshmallow
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Start with local bounds (0,0 based) | ||
| let mut local_bounds = shape.rect; | ||
|
|
||
| // Expand for drop shadows in local space (drawn inside blend mode isolation) | ||
| for shadow in &effects.shadows { |
There was a problem hiding this comment.
compute_blend_mode_bounds seeds the save_layer bounds from shape.rect and only expands for drop shadows. with_blendmode now uses these bounds for SaveLayerRec, so any stroke drawn afterward that extends outside the fill rect (center/outside strokes or large widths) will be clipped whenever a non–PassThrough blend mode is applied. Previously the isolation layer covered the whole canvas, so strokes rendered fully; with the new bounds a rectangle with an Outside stroke and Multiply blend loses its outer stroke pixels at the layer edges.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/grida-canvas/src/painter/painter_debug_node.rs (1)
143-170: Fixdraw_line_nodeto useidentity_transformfor blend mode bounds calculation.The
draw_line_nodefunction incorrectly passes&node.transform.matrixtowith_blendmode, while all other draw functions (draw_rect_node,draw_image_node,draw_ellipse_node,draw_vector_node,draw_path_node,draw_polygon_node) pass&identity_transform. Since the function already applies the transform viawith_transform(&node.transform.matrix), thecompute_blend_mode_boundscalculation should use identity transform for local space bounds, not the node's transform matrix. This inconsistency will cause incorrect blend mode isolation bounds.
🧹 Nitpick comments (3)
docs/wg/feat-2d/optimization.md (1)
148-148: Minor: "offscreen" should be hyphenated as "off-screen" per standard usage.Static analysis flagged this. While not critical, consider using "off-screen" for consistency with common technical writing standards.
🔎 Suggested fix
-- **Critical for blend mode isolation**: Blend modes require `save_layer` for isolation semantics. Using tight bounds reduces offscreen buffer size dramatically. +- **Critical for blend mode isolation**: Blend modes require `save_layer` for isolation semantics. Using tight bounds reduces off-screen buffer size dramatically.crates/grida-canvas/src/painter/painter.rs (1)
111-190: Well-structured bounds computation with clear TODO for future optimization.The
compute_blend_mode_boundshelper centralizes the bounds calculation logic. The 3x sigma expansion for Gaussian blur coverage is the standard practice. The TODO comment about moving to the geometry stage is a good architectural note.One observation on spread handling at line 152:
let expansion = ds.spread.abs();Using
abs()here treats negative spread (which should contract the shadow) the same as positive spread for bounds calculation. This is actually correct for bounds safety—even contracted shadows need the blur expansion—but the comment says "expand or contract" which might be misleading.Consider clarifying the comment to explain why we use
abs():🔎 Optional clarification
- // Apply spread (expand or contract) + // Apply spread to bounds (use abs since even contracted shadows need bounds for blur) if ds.spread != 0.0 { let expansion = ds.spread.abs();editor/grida-canvas/editor.ts (1)
4623-4653: Well-designed undo/redo override for content-editable contexts.The implementation correctly prevents browser's native undo/redo from conflicting with the editor's document history when editing text. The generic event type parameter ensures compatibility with both native and React keyboard events.
Minor consideration: On Windows/Linux,
Ctrl+Yis a common redo shortcut alongsideCtrl+Shift+Z. If users on those platforms expectCtrl+Yto work within content-editable areas, you may want to add support for it:🔎 Optional: Add Ctrl+Y support for redo
public explicitlyOverrideInputUndoRedo(event: { key: string; metaKey: boolean; ctrlKey: boolean; shiftKey: boolean; preventDefault: () => void; stopPropagation: () => void; }): boolean { // Check if this is Cmd+Z (undo) or Cmd+Shift+Z (redo) const isCmdOrCtrl = event.metaKey || event.ctrlKey; const isZKey = event.key === "z" || event.key === "Z"; + const isYKey = event.key === "y" || event.key === "Y"; - if (!isCmdOrCtrl || !isZKey) { + if (!isCmdOrCtrl || !(isZKey || isYKey)) { return false; } // Prevent browser's native undo/redo behavior event.preventDefault(); event.stopPropagation(); // Execute the appropriate editor command - if (event.shiftKey) { + if (event.shiftKey || isYKey) { // Redo: Cmd+Shift+Z or Ctrl+Y this._editor.doc.redo(); } else { // Undo: Cmd+Z this._editor.doc.undo(); } return true; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/grida-canvas-wasm/lib/bin/grida_canvas_wasm.wasmis excluded by!**/*.wasm
📒 Files selected for processing (22)
.github/workflows/test-crates.ymlcrates/grida-canvas-wasm/package.jsoncrates/grida-canvas/src/painter/painter.rscrates/grida-canvas/src/painter/painter_debug_node.rsdocs/wg/feat-2d/optimization.mdeditor/grida-canvas-react-starter-kit/starterkit-hierarchy/node-hierarchy-tree-item.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas-react/viewport/surface.tsxeditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/viewport/ui/text-editor.tsxeditor/grida-canvas/ASSERTIONS.mdeditor/grida-canvas/action.tseditor/grida-canvas/editor.i.tseditor/grida-canvas/editor.tseditor/grida-canvas/reducers/document.reducer.tseditor/grida-canvas/reducers/index.tseditor/grida-canvas/reducers/methods/hover.tseditor/grida-canvas/reducers/tools/target.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxpackages/grida-canvas-io-figma/lib.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript 5 as the main language for most apps
Use Lucide or Radix Icons for icons
Files:
editor/grida-canvas/reducers/index.tseditor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas/reducers/methods/hover.tseditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/grida-canvas/action.tseditor/grida-canvas-react/viewport/ui/text-editor.tsxeditor/grida-canvas-react-starter-kit/starterkit-hierarchy/node-hierarchy-tree-item.tsxeditor/grida-canvas/reducers/tools/target.tseditor/grida-canvas-react/viewport/surface.tsxeditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas/reducers/document.reducer.tspackages/grida-canvas-io-figma/lib.tseditor/grida-canvas/editor.i.tseditor/grida-canvas/editor.ts
{editor/**/*.{ts,tsx},packages/grida-canvas-*/**/*.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
Use DOM (plain DOM as canvas) for website builder canvas, bound with React
Files:
editor/grida-canvas/reducers/index.tseditor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas/reducers/methods/hover.tseditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/grida-canvas/action.tseditor/grida-canvas-react/viewport/ui/text-editor.tsxeditor/grida-canvas-react-starter-kit/starterkit-hierarchy/node-hierarchy-tree-item.tsxeditor/grida-canvas/reducers/tools/target.tseditor/grida-canvas-react/viewport/surface.tsxeditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas/reducers/document.reducer.tspackages/grida-canvas-io-figma/lib.tseditor/grida-canvas/editor.i.tseditor/grida-canvas/editor.ts
editor/grida-*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use /editor/grida-* directories to isolate domain-specific modules; promote well-defined modules to /packages
Files:
editor/grida-canvas/reducers/index.tseditor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas/reducers/methods/hover.tseditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/grida-canvas/action.tseditor/grida-canvas-react/viewport/ui/text-editor.tsxeditor/grida-canvas-react-starter-kit/starterkit-hierarchy/node-hierarchy-tree-item.tsxeditor/grida-canvas/reducers/tools/target.tseditor/grida-canvas-react/viewport/surface.tsxeditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas/reducers/document.reducer.tseditor/grida-canvas/editor.i.tseditor/grida-canvas/editor.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use React.js 19 for web applications
Files:
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/grida-canvas-react/viewport/ui/text-editor.tsxeditor/grida-canvas-react-starter-kit/starterkit-hierarchy/node-hierarchy-tree-item.tsxeditor/grida-canvas-react/viewport/surface.tsxeditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react/viewport/hotkeys.tsx
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Use Rust 2024 edition for wasm builds and graphics core
Use Skia graphics backend for 2D graphics, bound with skia-safe
Rust crates in /crates directory are under rapid development and serve as the new rendering backend; ensure high quality implementations
Files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rs
crates/grida-canvas/**/*.rs
📄 CodeRabbit inference engine (crates/grida-canvas/AGENTS.md)
crates/grida-canvas/**/*.rs: UseNodeId(u64) for internal structs (NodeRecs, SceneGraph, caches) in the rendering engine for high-performance operations
UseUserNodeId(String) for public APIs that accept or return node IDs for stability and serialization
Handle NodeId to UserNodeId conversion viaIdConverterduring .grida file loading
Auto-generate IDs (ID=0) inNodeRepositoryfor factory-created nodes
Maintain bidirectional mapping between NodeId and UserNodeId at the application layer for API boundary management
Useskia-safecrate for painting operations in the rendering engine
Usemath2crate for geometry and common math operations
Runcargo fmtto maintain code formatting standards
Runcargo clippy --no-deps --all-targets --all-featuresfor linting to check style, performance, and correctness suggestions
Files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rs
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Documentation source of truth is in the ./docs directory; deployment is handled by apps/docs
Files:
docs/wg/feat-2d/optimization.md
docs/{wg,reference}/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Only actively maintain docs/wg/** and docs/reference/** directories; see docs/AGENTS.md for contribution scope
Files:
docs/wg/feat-2d/optimization.md
editor/scaffolds/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use /editor/scaffolds for feature-specific larger components, pages, and editors
Files:
editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx
packages/grida-canvas-*/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Packages under /packages/grida-canvas-* power the canvas; some are published to npm, refer to individual package README
Files:
packages/grida-canvas-io-figma/lib.ts
🧠 Learnings (28)
📓 Common learnings
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Applies to packages/grida-canvas-*/**/*.{ts,tsx,js,jsx} : Packages under /packages/grida-canvas-* power the canvas; some are published to npm, refer to individual package README
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Run cargo check for Rust crates checking
Applied to files:
.github/workflows/test-crates.yml
📚 Learning: 2025-12-01T00:22:06.800Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:06.800Z
Learning: Applies to crates/grida-canvas-fonts/**/*.rs : Run all tests with: `cargo test`
Applied to files:
.github/workflows/test-crates.yml
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Run cargo test for Rust crates testing
Applied to files:
.github/workflows/test-crates.yml
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Applies to {editor/**/*.{ts,tsx},packages/grida-canvas-*/**/*.{ts,tsx}} : Use DOM (plain DOM as canvas) for website builder canvas, bound with React
Applied to files:
editor/grida-canvas/reducers/index.tseditor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas-react/viewport/surface.tsxeditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding new parameters to the halftone tool, add state with useState, include in useEffect dependency array, pass to renderHalftone() function, use in rendering logic, and add UI control
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas/reducers/methods/hover.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react/viewport/hotkeys.tsx
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use React hooks for state management (imageSrc, shape, grid, maxRadius, gamma, jitter, opacity, color, customShapeImage, imageDataRef, sizeRef)
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/grida-canvas-react/use-mixed-properties.tseditor/grida-canvas/reducers/methods/hover.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use Canvas 2D API with path commands for rendering geometric shapes (circle, square, triangle, etc.)
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxpackages/grida-canvas-io-figma/lib.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding new shape types, update the Shape type union, add cases in drawShape() function, add cases in shapeToSVG() function, and add SelectItem in UI
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxeditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxpackages/grida-canvas-io-figma/lib.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : For SVG export, convert circles to <circle> elements, rectangles to <rect> elements, and polygons to <polygon> elements with calculated points
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsxpackages/grida-canvas-io-figma/lib.ts
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Use `NodeId` (u64) for internal structs (NodeRecs, SceneGraph, caches) in the rendering engine for high-performance operations
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rsdocs/wg/feat-2d/optimization.md
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Use `skia-safe` crate for painting operations in the rendering engine
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rs
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Auto-generate IDs (ID=0) in `NodeRepository` for factory-created nodes
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Handle NodeId to UserNodeId conversion via `IdConverter` during .grida file loading
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rs
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Use `UserNodeId` (String) for public APIs that accept or return node IDs for stability and serialization
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Maintain bidirectional mapping between NodeId and UserNodeId at the application layer for API boundary management
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rs
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Use `math2` crate for geometry and common math operations
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rs
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Run `cargo fmt` to maintain code formatting standards
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rscrates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Applies to crates/**/*.rs : Use Skia graphics backend for 2D graphics, bound with skia-safe
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rs
📚 Learning: 2025-12-20T08:11:16.201Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.201Z
Learning: Applies to crates/grida-canvas/**/*.rs : Run `cargo clippy --no-deps --all-targets --all-features` for linting to check style, performance, and correctness suggestions
Applied to files:
crates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/painter/painter.rscrates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/main.rs : Update `grida-canvas-wasm.d.ts` TypeScript definitions file when new APIs are introduced via `main.rs`
Applied to files:
crates/grida-canvas-wasm/package.jsoneditor/grida-canvas-react/viewport/surface.tsxpackages/grida-canvas-io-figma/lib.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/+(grida-canvas-wasm.js|grida-canvas-wasm.wasm) : Include WASM artifacts (`grida-canvas-wasm.js` and `grida-canvas-wasm.wasm`) in git for faster CI builds
Applied to files:
crates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/*.{js,ts,jsx,tsx} : Use Web Workers for heavy graphics operations to improve performance and responsiveness
Applied to files:
crates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/Cargo.toml : Use the target `wasm32-unknown-emscripten` when building Rust code for WebAssembly compilation
Applied to files:
crates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Applies to packages/grida-canvas-*/**/*.{ts,tsx,js,jsx} : Packages under /packages/grida-canvas-* power the canvas; some are published to npm, refer to individual package README
Applied to files:
crates/grida-canvas-wasm/package.jsoneditor/grida-canvas-react/viewport/surface.tsxpackages/grida-canvas-io-figma/lib.ts
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/Cargo.toml : Set environment variables `CC=emcc`, `CXX=em++`, and `AR=emar` when building Rust code for WebAssembly targets
Applied to files:
crates/grida-canvas-wasm/package.json
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Applies to editor/components/ui/**/*.{ts,tsx} : Use /editor/components/ui for shadcn UI components
Applied to files:
editor/grida-canvas-react/viewport/surface.tsx
📚 Learning: 2025-12-14T23:30:42.112Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T23:30:42.112Z
Learning: Applies to editor/grida-*/**/*.{ts,tsx} : Use /editor/grida-* directories to isolate domain-specific modules; promote well-defined modules to <root>/packages
Applied to files:
editor/grida-canvas-react/viewport/surface.tsxeditor/grida-canvas/editor.ts
🧬 Code graph analysis (6)
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsx (1)
editor/grida-canvas-react/provider.tsx (1)
useToolState(499-502)
editor/grida-canvas-react/use-mixed-properties.ts (2)
packages/grida-canvas-io-figma/lib.ts (1)
blendMode(1271-1275)packages/grida-canvas-cg/lib.ts (1)
LayerBlendMode(100-100)
editor/grida-canvas/reducers/methods/hover.ts (2)
editor/grida-canvas/editor.i.ts (1)
IEditorState(1393-1412)editor/grida-canvas/reducers/tools/target.ts (1)
getRayTarget(84-248)
editor/grida-canvas/action.ts (2)
editor/grida-canvas/editor.i.ts (1)
NodeID(136-136)packages/grida-canvas-schema/grida.ts (1)
NodeID(1101-1101)
editor/grida-canvas-react/viewport/surface.tsx (1)
editor/grida-canvas-react/viewport/ui/floating-bar.tsx (2)
FloatingBarContentWrapper(43-56)FloatingBar(11-36)
editor/grida-canvas/reducers/document.reducer.ts (3)
editor/grida-canvas/action.ts (1)
EditorSelectAction(235-244)editor/grida-canvas/reducers/methods/selection.ts (2)
self_selectNode(33-91)self_clearSelection(93-115)editor/grida-canvas/reducers/methods/hover.ts (1)
__validateHoverState(11-26)
🪛 LanguageTool
docs/wg/feat-2d/optimization.md
[grammar] ~148-~148: Ensure spelling is correct
Context: ...n semantics. Using tight bounds reduces offscreen buffer size dramatically. - **Coordinat...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~305-~305: The name of this brand needs to be capitalized.
Context: ...cache invalidation mechanism based on zoom is introduced. --- ## Text & Glyph Op...
(ON_SKYPE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cargo test
🔇 Additional comments (58)
editor/grida-canvas-react/viewport/ui/surface-distribution-overlay.tsx (3)
5-9: LGTM!The import addition is correctly structured and consistent with the existing provider imports.
186-187: LGTM!The pointer event handler is correctly attached to the gap overlay div, which appropriately uses
pointer-events-autoto enable hover detection while the parent remains non-interactive.
124-137: The tool type values "insert" and "draw" are valid and confirmed in the type definitions. The defensive check is unnecessary because thetoolproperty is always initialized with{ type: "cursor" }in the default state, ensuring it can never be undefined. The same pattern of accessingtool.typedirectly is used consistently throughout the codebase (e.g., insurface.tsxlines 1058 and 1235-1236) without defensive checks. The code is correct as written.Likely an incorrect or invalid review comment.
crates/grida-canvas-wasm/package.json (1)
4-4: Version bump looks appropriate for canary iteration.packages/grida-canvas-io-figma/lib.ts (1)
1723-1725: Formatting cleanup looks good.The multi-line conditional is collapsed to a single expression while preserving the same fallback behavior.
.github/workflows/test-crates.yml (1)
63-64: Good cache optimization for Rust builds.The
shared-key: test-nativeis more descriptive, andcache-all-crates: truewill cache all dependencies including transitive ones, improving CI build times.editor/grida-canvas/ASSERTIONS.md (1)
49-51: Well-documented UX assertion for history system precedence.The assertion clearly explains the problem (browser intercepting undo/redo in content edit mode), the expected behavior (editor history takes precedence), and the appropriate scope limitation (canvas content inputs only). This aligns with the related code changes for undo/redo interception.
editor/grida-canvas-react/viewport/hotkeys.tsx (3)
567-568: Improved selection pattern using query-then-select.The two-step approach (
querySelectAll→select) provides better control and allows checking target availability before selection.
632-638: Smart fallback for enter key behavior.The logic now queries for children first with
querySelectAll(">"), and only toggles content edit mode when no children exist. This prevents attempting to select non-existent children.
650-651: Consistent adoption of querySelectAll pattern for navigation hotkeys.The parent (
..), next sibling (~+), and previous sibling (~-) selections now use the same query-then-select pattern for consistency.Also applies to: 663-664, 676-677
editor/grida-canvas-react/viewport/surface.tsx (4)
205-218: Good centralized blur handler for modifier key reset.This prevents stuck modifier states when the window loses focus, which is a common issue with keyboard-dependent tools. The handler properly cleans up event listener on unmount.
632-640: Improved title bar structure with wrapper divs.The padding-bottom approach ensures pointer events work correctly in the gap between the title bar and the node, which is better UX than margin-based spacing.
Also applies to: 660-666
704-708: Explicit selection mode improves clarity.Using
editor.commands.select([node.id], "toggle")andeditor.commands.select([node.id], "reset")makes the intent clear compared to implicit behavior.
797-802: Good UX: Prevent accidental rename with modifier keys.Only triggering rename when no modifier keys are pressed prevents conflicts with other double-click behaviors (e.g., Shift+double-click for selection, Cmd+double-click for shortcuts).
editor/grida-canvas-react-starter-kit/starterkit-hierarchy/node-hierarchy-tree-item.tsx (3)
79-96: Good component extraction for shared button behavior.
NodeActionButtoncleanly encapsulates the hover-reveal pattern with analwaysVisibleoverride, reducing duplication and making the visibility logic explicit.
151-157: Container visibility logic is correct.The logic ensures the actions container:
- Is always visible when
node.lockedor!node.active(so users see the non-default state)- Is hidden by default but visible on hover for normal nodes
- Is hidden during rename mode
159-184: Individual button visibility is well-designed.
- Lock button:
alwaysVisible={node.locked}— visible when locked so users know the node is protected- Active button:
alwaysVisible={!node.active}— visible when inactive so users know the node is hiddenThis provides appropriate visual feedback for non-default states.
editor/grida-canvas/reducers/index.ts (1)
109-119: Theignores_root_with_childrenconfiguration field removal is properly documented and intentional. No active code dispatches this field to the action, and all previous usages have been removed or commented out. The scene-based logic replacement (single mode vs normal mode) is consistently documented across the codebase.editor/grida-canvas/reducers/methods/hover.ts (3)
5-26: Well-structured hover state validation.The
__validateHoverStatefunction provides a clean invariant enforcement mechanism. The two conditions correctly handle:
- Clearing orphaned sources when
hovered_node_idis null- Clearing sources when UI-triggered hovers have no corresponding node
55-61: Title-bar hover preservation logic is sound.The conditional check
draft.hovered_node_source !== "title-bar"correctly prevents hit-testing from overwriting UI-triggered hovers, which is the intended behavior for title bars that have no geometry in hit-testing.
63-85: Measurement target handling looks correct.The separation of normal hover computation from measurement target computation is a good design choice. The merged config approach (
...draft.pointer_hit_testing_config, ...MEASUREMENT_HIT_TESTING_CONFIG) properly combines base settings with measurement-specific overrides.editor/grida-canvas-react/viewport/ui/floating-bar.tsx (2)
67-67: Improved truncation behavior withw-full min-w-0.The change from
max-w-full w-mintow-full min-w-0is correct for flex containers.min-w-0is essential to allow the element to shrink below its content size, enabling proper text truncation with thetruncateclass.
43-56: Component renamed fromFloatingBarContenttoFloatingBarContentWrapper.This is a breaking change if external consumers import
FloatingBarContent. All internal usages have been updated—surface.tsx imports and usesFloatingBarContentWrapper(lines 64, 725, 727), and no references to the old component name remain in the codebase.editor/grida-canvas/reducers/tools/target.ts (1)
10-25: Good consolidation of top/root node resolution.The
getTopNodeIdfunction cleanly handles both scene-context and non-scene-context cases, with a sensible fallback tonode_idwhen no root can be determined.editor/grida-canvas/action.ts (2)
237-243: Selection mode enhancement is well-designed.The new
modeparameter with"reset" | "add" | "toggle"options provides flexible selection behavior while defaulting to"reset"for backward compatibility. The JSDoc documentation clearly explains the default.
253-276: Good separation of hover action sources with deprecation path.The split into
EditorTitleBarHoverActionandEditorUITriggeredHoverActionprovides clear semantics for different hover sources. The deprecatedEditorHoverActiontype alias maintains backward compatibility for consumers.editor/grida-canvas/reducers/document.reducer.ts (3)
277-307: Title-bar hover handling is correctly implemented.The enter/leave logic properly:
- Sets both
hovered_node_idandhovered_node_sourceon enter- Only clears hover on leave when both target and source match
- Validates state consistency after both operations
The conditional clear on leave prevents interference between different hover sources.
308-338: UI hover (hierarchy-tree) handling mirrors title-bar pattern.The implementation correctly uses
"hierarchy-tree"as the source, which aligns with the action type documentation inaction.ts. The pattern is consistent with the title-bar case.
268-270: Selection mode integration is clean.The destructuring with default
mode = "reset"and delegation toself_selectNode(draft, mode, ...selection)properly integrates the new mode-based selection behavior.crates/grida-canvas/src/painter/painter_debug_node.rs (2)
35-62: Consistent blend mode bounds pattern applied.The introduction of
identity_transformfor debug rendering correctly indicates that transforms are already applied and bounds should be in local space. The pattern is well-documented with inline comments.
282-326: Polygon delegation pattern is efficient.
draw_regular_polygon_nodeanddraw_regular_star_polygon_nodecorrectly delegate todraw_polygon_nodeafter converting to the base polygon representation. This avoids code duplication while leveraging the updated blend mode pattern.docs/wg/feat-2d/optimization.md (2)
140-151: Excellent documentation ofsave_layeroptimization strategy.This section clearly explains the critical importance of providing tight bounds for
save_layercalls. The documentation of coordinate space consistency and future optimization opportunities is valuable for maintainers.
167-289: Comprehensive zoom & interaction optimization section.The new section provides detailed, actionable strategies for interactive rendering during zoom operations. The progressive refinement, crossfading, and text-first refinement approaches are well-explained with practical implementation details.
editor/grida-canvas-react/viewport/ui/text-editor.tsx (1)
90-99: Undo/redo override integration looks correct.The early return pattern properly delegates Cmd+Z/Cmd+Shift+Z handling to the editor's history system. The event is still passed to
stopPropagationfor the Escape key path.Ensure
explicitlyOverrideInputUndoRedoreturnstruewhen it handles the event and prevents default:#!/bin/bash # Find the implementation of explicitlyOverrideInputUndoRedo ast-grep --pattern 'explicitlyOverrideInputUndoRedo($_) { $$$ }'editor/grida-canvas/editor.i.ts (6)
388-407: LGTM - Clean documentation for removed feature.The comments clearly explain the removal of
ignores_root_with_childrenand the rationale for the new scene-based logic. TheMEASUREMENT_HIT_TESTING_CONFIGcorrectly retains only theignores_locked: falseoverride, allowing measurement to locked reference elements.
752-764: LGTM - Comprehensive deprecation note.The multi-line comment block thoroughly documents the removal reason, the replacement logic (single mode vs normal mode), and preserves context for future reference. Good practice to keep this as a commented-out block for historical context.
1029-1041: LGTM - Well-typed hover source tracking.The
hovered_node_sourcefield with literal union type"hit-test" | "title-bar" | "hierarchy-tree" | nullprovides excellent type safety and self-documenting code. The JSDoc clearly explains the purpose and behavior of each source type.
1445-1445: LGTM - Correctly initializes new field in reset state.The
hovered_node_source: nullinitialization aligns with the field's default value documented inISceneSurfaceState.
2747-2801: LGTM - Clean API separation between query and action.The new
querySelectAll(pure query) andselect(action dispatcher with mode) methods provide a well-designed separation of concerns. The mode parameter with"reset" | "add" | "toggle"options covers all common selection patterns. The documentation is thorough with good examples.
3664-3689: Implementation correctly resets all documented states on blur.The
onblurimplementation ineditor.ts(lines 3757-3787) fully aligns with the documented behavior. It properly clears the title bar hover state, resets all surface configurations (raycast targeting, measurement, and all 8 modifier-dependent settings), and resets the tool to cursor. The code includes clear comments explaining the necessity of modifier resets when the window blurs.crates/grida-canvas/src/painter/painter.rs (7)
200-228: LGTM - Clean blend mode isolation with bounds optimization.The
with_blendmodemethod correctly:
- Passes through without isolation for
PassThrough(fast path)- Computes world-space bounds for proper
save_layersizing- Uses the computed bounds to limit offscreen buffer size (~100x improvement as noted)
The signature change to accept
shape,effects, andtransformis a good design that enables proper bounds calculation while maintaining clean separation.
343-384: LGTM - Precise shadow bounds calculation for text.The bounds calculation properly accounts for:
- Text paragraph dimensions
- Shadow offset (dx, dy)
- Spread expansion
- Blur expansion (3x sigma)
- Union with original bounds
This ensures the
save_layeris sized correctly to capture the entire shadow effect.
585-589: LGTM - Consistent local-space bounds for glass effect.The layer bounds are correctly computed relative to the translated origin (0,0 based after the
translatecall), ensuring the glass effect's backdrop filter operates on the correct region.
860-898: LGTM - Shape layer uses unified blend mode wrapper.The shape layer drawing now correctly:
- Wraps in
with_blendmodewith shape, effects, and transform for proper bounds- Applies transform inside the blend mode isolation
- Handles clip path, effects, fills, noise, and strokes in correct order
The nesting order (blend mode → transform → clip → effects → content) is correct per the spec.
901-1002: LGTM - Text layer with comprehensive effect handling.The text layer drawing is well-structured:
- Blend mode isolation wraps everything
- Transform applied inside isolation
- Paragraph caching and layout calculation
- Effect ordering: backdrop blur → drop shadows → content → inner shadows
- Layer blur wraps the entire effect sequence
- Clip path handled correctly
The y_offset calculation for vertical alignment is preserved and correctly propagated to all effect methods.
1005-1074: LGTM - Vector layer with unified blend mode handling.The vector layer correctly:
- Uses
with_blendmodewrapper with proper parameters- Separates fill and stroke rendering for correct layering
- Applies noise effects between fills and strokes
- Constructs
StrokeOptionswith all required fieldsThe pattern matches the shape layer implementation, ensuring consistency across layer types.
393-415: Verify inner shadow bounds expansion logic.For inner shadows, the code only expands bounds for blur but skips offset/spread expansion with the comment "inner shadows are clipped to shape, but blur needs expansion."
This is correct for the clipping aspect, but I want to verify: since inner shadows are rendered inside the shape and clipped, the blur expansion here is for the
save_layerthat applies the inner shadow image filter. The blur can bleed inward, so expansion is needed to avoid clipping the blur itself.#!/bin/bash # Check how inner_shadow_image_filter is implemented to confirm blur handling rg -n -A 15 'fn inner_shadow_image_filter' crates/grida-canvas/src/painter/editor/grida-canvas/editor.ts (4)
784-830: LGTM!Clean separation of querying logic from selection dispatch. The scene-scoped filtering prevents accidental cross-scene selection (e.g., when using CMD+A on an empty scene), and the Set-based deduplication handles overlapping selector results correctly.
832-870: LGTM!The explicit mode parameter (
"reset" | "add" | "toggle") provides clear control over selection behavior. The early return for empty arrays is a sensible no-op that prevents unnecessary state updates.
3715-3737: LGTM!The hover action type split (
hover/uivshover/title-bar) enables source-aware hover state management. This separation is essential for theonblurhandler to selectively clear only the title-bar hover state without affecting UI-triggered hovers.
3739-3788: Solid defensive implementation for window blur handling.The method correctly addresses the edge case where modifier key
keyupevents are not fired when the window loses focus. The selective clearing of title-bar hover state (lines 3763-3768) while leaving UI hover unaffected demonstrates good understanding of the hover source separation.The
hovered_node_sourceproperty is properly defined ineditor.state.ISceneSurfaceStatewith type"hit-test" | "title-bar" | "hierarchy-tree" | null, initialized correctly in the default state, and consistently managed by the title-bar and hierarchy-tree hover reducers. The property access in theonblurmethod is safe and correct.editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx (4)
48-48: LGTM: BlendModeDropdown import added.The import is properly placed and follows the established pattern for control imports.
330-330: LGTM: blend_mode property extracted.The property is correctly added to the destructured properties from the mixed properties editor, following the established pattern.
365-365: LGTM: has_stylable condition correctly identifies stylable nodes.The condition appropriately filters out
template_instancenodes and determines when the Appearance section should be displayed. The use of.some()ensures the section appears when at least one node in the selection is stylable, which is the correct behavior for mixed selections.
496-506: LGTM: Appearance section properly configured for blend mode control.The section is correctly hidden when no stylable nodes are present, and the BlendModeDropdown is properly wired with the value from mixed properties and the change handler. The placement and structure are consistent with the single-node implementation at lines 1131-1141.
editor/grida-canvas-react/use-mixed-properties.ts (3)
382-382: LGTM: blend_mode action properly exposed.The action is correctly added to the actions object and follows the established naming pattern.
418-418: LGTM: Dependency correctly included.The
blendModecallback is properly added to theuseMemodependencies array, ensuring the actions object updates when the callback changes.
338-345: The implementation is correct and consistent with the codebase's patterns.The assignment
instance.doc.getNodeById(id).blend_mode = valueuses theblend_modesetter defined in the editor (lines 4839-4845), which properly dispatches a state change action through the document's dispatch system. This approach:
- Tracks changes for undo/redo via the "node/change/*" dispatch action
- Triggers necessary re-renders through the reducer state management
- Follows the same pattern as other simple properties like
activeandlocked(lines 60, 69)While
cursorandcorner_radiususe explicit command methods,blend_modeintentionally follows the simpler property pattern because it doesn't require special transformation logic. NochangeNodePropertyBlendModecommand exists or is needed—the setter-based dispatch approach is the established pattern for properties of this type.
| const scene = context.document.nodes[ | ||
| context.scene_id | ||
| ] as grida.program.nodes.SceneNode; | ||
| // Only filter in single mode | ||
| return scene.constraints.children === "single"; |
There was a problem hiding this comment.
Unchecked type assertion on scene node.
The code casts context.document.nodes[context.scene_id] to SceneNode without verifying the node exists or is actually a scene node. If scene_id is invalid or the node was deleted, this could cause runtime errors.
🔎 Suggested defensive check
- const scene = context.document.nodes[
- context.scene_id
- ] as grida.program.nodes.SceneNode;
- // Only filter in single mode
- return scene.constraints.children === "single";
+ const scene = context.document.nodes[context.scene_id];
+ if (!scene || scene.type !== "scene") {
+ return false;
+ }
+ // Only filter in single mode
+ return (scene as grida.program.nodes.SceneNode).constraints.children === "single";📝 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.
| const scene = context.document.nodes[ | |
| context.scene_id | |
| ] as grida.program.nodes.SceneNode; | |
| // Only filter in single mode | |
| return scene.constraints.children === "single"; | |
| const scene = context.document.nodes[context.scene_id]; | |
| if (!scene || scene.type !== "scene") { | |
| return false; | |
| } | |
| // Only filter in single mode | |
| return (scene as grida.program.nodes.SceneNode).constraints.children === "single"; |
1 participant
Release Notes
New Features
Bug Fixes
Refactor