Grida Canvas - Pixel Preview#509
Conversation
…ategy - Introduced PixelPreviewStrategy enum for accurate and stable rendering options. - Implemented pixel preview scaling in the renderer, allowing for 1x and 2x pixelated views. - Added new example scripts for visual testing of pixel preview stability. - Updated application API to support pixel preview configuration. - Enhanced editor state management to include pixel preview settings and shortcuts.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request introduces comprehensive pixel preview functionality to the Grida canvas rendering system. It adds a new rendering mode that downscales to an offscreen surface at a specified scale, applies mathematical planning to determine optimal render parameters, and includes configuration, WASM FFI bindings, runtime APIs, UI controls, and keyboard shortcuts across the editor stack. Changes
Sequence DiagramsequenceDiagram
participant User
participant Menu as Hotkey/Menu
participant Editor as Editor Surface API
participant Reducer as Reducer Chain
participant Runtime as Runtime (WASM)
participant Renderer as Renderer
participant Canvas as Canvas Output
User->>Menu: Toggle Pixel Preview<br/>(Ctrl+Shift+Alt+P)
Menu->>Editor: surfaceTogglePixelPreview()
Editor->>Reducer: dispatch(surface/pixel-preview action)
Reducer->>Reducer: update pixelpreview state
Editor->>Editor: subscribe: pixelpreview changed
Editor->>Runtime: __runtime_renderer_set_pixel_preview_scale(scale)
Runtime->>Renderer: set_pixel_preview_scale(scale)
Runtime->>Renderer: set_pixel_preview_strategy_stable(true)
Renderer->>Renderer: update config
Runtime->>Canvas: queue redraw
Renderer->>Renderer: compute_pixel_preview_plan()
Renderer->>Renderer: render to offscreen at scaled resolution
Renderer->>Renderer: upsample to canvas with nearest-neighbor
Canvas-->>User: display updated preview
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd07b7bc32
ℹ️ 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".
| // Present: upscale to the main canvas using nearest-neighbor sampling. | ||
| Self::clear_and_paint_background(canvas, background_color, width, height); | ||
|
|
There was a problem hiding this comment.
Avoid double-painting background in pixel preview
When Pixel Preview is enabled, the background is already painted into the offscreen surface (clear_and_paint_background on off_canvas earlier in this block). Calling clear_and_paint_background again on the main canvas before drawing the offscreen image applies the background twice. This is visible when background_color has alpha < 1 (e.g., semi‑transparent canvas backgrounds), where the preview becomes darker/more opaque than the normal render. To keep colors consistent, only paint the background once (either keep it in the offscreen render and clear the main canvas to transparent, or skip the offscreen background and paint only on the main canvas).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.ts`:
- Around line 176-185: Add the missing canvas import to the type declarations so
the canvas namespace used by grida-canvas-wasm.d.ts is resolved: update the file
that extends canvas.CanvasModule to include an import for the canvas namespace
from "../modules/canvas-bindings" (so the new methods
_runtime_renderer_set_pixel_preview_scale and
_runtime_renderer_set_pixel_preview_stable are correctly typed), mirroring the
existing imports for fonts, markdown, and svg.
In `@crates/grida-canvas/examples/golden_pixel_preview_stability.rs`:
- Around line 145-153: The write to "goldens/pixel_preview_stability.png" can
fail if the goldens directory doesn't exist; before calling std::fs::write (the
code that writes data.as_bytes()), ensure the directory exists by calling
std::fs::create_dir_all on the path created from
concat!(env!("CARGO_MANIFEST_DIR"), "/goldens") and handle or expect its Result
so the subsequent image/data encode and std::fs::write succeed reliably.
In `@editor/grida-canvas-hosted/playground/uxhost-menu.tsx`:
- Around line 494-528: Duplicate keyboard shortcut hints appear for multiple
DropdownMenuRadioItem entries in the Pixel preview group; update the
DropdownMenuRadioItem render logic so
keyboardShortcutText("workbench.surface.view.toggle-pixel-preview") is only
rendered once (for example on the "Disabled" item or moved to the parent label)
instead of being included on both the "Disabled" and "1x" items; locate
DropdownMenuRadioGroup and its DropdownMenuRadioItem children (values
"disabled", "1x", "2x") and remove the extra keyboardShortcutText invocation so
the toggle shortcut is shown a single time while leaving
instance.surface.surfaceConfigurePixelPreviewScale and pixelpreview handling
unchanged.
In `@editor/grida-canvas/editor.ts`:
- Around line 3256-3261: The subscription to this.doc.subscribeWithSelector for
state.pixelpreview only reacts to changes and doesn't apply the current value on
mount; call the current pixelpreview state once during initialization and pass
it to __runtime_renderer_set_pixel_preview_scale so the runtime matches restored
sessions. In other words, after (or before) setting up
this.doc.subscribeWithSelector, read this.doc.getState().pixelpreview (or invoke
the selector directly) and call
this.__runtime_renderer_set_pixel_preview_scale(...) with that value to sync
initial state.
🧹 Nitpick comments (3)
crates/grida-canvas/examples/golden_pixel_preview_stability.rs (1)
135-142: Avoid recreating the border paint in every iteration.Move it outside the loop for a tiny allocation/CPU win.
♻️ Suggested refactor
- for (idx, (label, img)) in frames.iter().enumerate() { + let mut border = SkPaint::default(); + border.set_color(Color::from_argb(255, 220, 220, 220)); + border.set_style(skia_safe::paint::Style::Stroke); + border.set_stroke_width(1.0); + + for (idx, (label, img)) in frames.iter().enumerate() { let r = idx / zooms.len(); let c = idx % zooms.len(); let x = padding + c as f32 * (cell_w + padding); let y = padding + r as f32 * (cell_h + label_h + padding); canvas.draw_str(label, (x, y + 12.0), &font, &text_paint); canvas.draw_image(img, (x, y + label_h), None); - let mut border = SkPaint::default(); - border.set_color(Color::from_argb(255, 220, 220, 220)); - border.set_style(skia_safe::paint::Style::Stroke); - border.set_stroke_width(1.0); canvas.draw_rect( Rect::from_xywh(x, y + label_h, cell_w, cell_h), &border, ); }crates/grida-canvas/examples/golden_pixel_preview.rs (1)
97-99: Unsafe raw pointer dereference for surface access.The unsafe block for obtaining a mutable reference from a raw pointer follows the existing pattern in this codebase. However, consider whether
backendcould expose a safe accessor method that encapsulates this unsafety internally.crates/grida-canvas/src/runtime/pixel_preview.rs (1)
89-89: Consider using a named constant for the enablement threshold.The condition
zoom > pixel_preview_scale as f32 + f32::EPSILONusesf32::EPSILONwhich is approximately1.19e-7. This effectively requireszoom > scalewith minimal tolerance. If the intent is to require a meaningful zoom difference (e.g., at least 0.1 or 1.0), consider using a named constant for clarity.Example clarification
// If the intent is "zoom must be strictly greater than scale": const ENABLE_THRESHOLD_EPS: f32 = 1.0e-3; // or whatever tolerance is appropriate let enabled = pixel_preview_scale != 0 && zoom > pixel_preview_scale as f32 + ENABLE_THRESHOLD_EPS;
|
|
||
| _runtime_renderer_set_pixel_preview_scale( | ||
| state: GridaCanvasApplicationPtr, | ||
| scale: number | ||
| ): void; | ||
|
|
||
| _runtime_renderer_set_pixel_preview_stable( | ||
| state: GridaCanvasApplicationPtr, | ||
| stable: boolean | ||
| ): void; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and inspect both d.ts files
echo "=== Finding canvas-bindings.d.ts ==="
fd -a 'canvas-bindings\.d\.ts' crates/grida-canvas-wasm/
echo "=== Finding grida-canvas-wasm.d.ts ==="
fd -a 'grida-canvas-wasm\.d\.ts' crates/grida-canvas-wasm/
echo "=== Checking for pixel preview APIs in canvas-bindings.d.ts ==="
rg -n "runtime_renderer_set_pixel_preview_(scale|stable)" crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.ts
echo "=== Checking for pixel preview APIs in grida-canvas-wasm.d.ts ==="
fd -a 'grida-canvas-wasm\.d\.ts' crates/grida-canvas-wasm/lib -x cat {}Repository: gridaco/grida
Length of output: 1419
🏁 Script executed:
#!/bin/bash
# Check the structure of canvas-bindings.d.ts to see where these methods are defined
echo "=== Viewing canvas-bindings.d.ts around the new APIs ==="
sed -n '160,190p' crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.ts
echo "=== Checking for CanvasModule interface definition ==="
rg -n "interface CanvasModule" crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.ts -A 100 | head -120
echo "=== Checking what canvas.CanvasModule imports from ==="
rg -n "import.*CanvasModule\|export.*CanvasModule" crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.tsRepository: gridaco/grida
Length of output: 4216
🏁 Script executed:
#!/bin/bash
# Verify the import path for canvas in grida-canvas-wasm.d.ts
echo "=== Full import section of grida-canvas-wasm.d.ts ==="
head -20 crates/grida-canvas-wasm/lib/bin/grida-canvas-wasm.d.ts
# Check export namespace of canvas-bindings.d.ts
echo "=== Checking canvas-bindings.d.ts export structure ==="
head -10 crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.tsRepository: gridaco/grida
Length of output: 1210
🏁 Script executed:
#!/bin/bash
# Check if canvas is imported in grida-canvas-wasm.d.ts
echo "=== Complete grida-canvas-wasm.d.ts file ==="
cat crates/grida-canvas-wasm/lib/bin/grida-canvas-wasm.d.ts
echo "=== Check if canvas-bindings.d.ts exports canvas namespace ==="
grep -n "declare namespace canvas" crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.tsRepository: gridaco/grida
Length of output: 1095
🏁 Script executed:
#!/bin/bash
# Check if canvas-bindings is imported anywhere in the lib folder
echo "=== Searching for canvas-bindings imports ==="
rg -n "import.*canvas.*bindings\|from.*canvas.*bindings" crates/grida-canvas-wasm/lib/
echo "=== Check the complete structure around the interface extension ==="
sed -n '20,35p' crates/grida-canvas-wasm/lib/bin/grida-canvas-wasm.d.ts
echo "=== List all .d.ts files in lib directory ==="
find crates/grida-canvas-wasm/lib -name "*.d.ts" -type fRepository: gridaco/grida
Length of output: 884
🏁 Script executed:
#!/bin/bash
# Check c-api.d.ts and api.d.ts
echo "=== Check c-api.d.ts ==="
cat crates/grida-canvas-wasm/lib/c-api.d.ts
echo "=== Check api.d.ts ==="
cat crates/grida-canvas-wasm/lib/api.d.tsRepository: gridaco/grida
Length of output: 7241
Add missing canvas import to grida-canvas-wasm.d.ts to expose new pixel preview APIs.
The new _runtime_renderer_set_pixel_preview_scale and _runtime_renderer_set_pixel_preview_stable methods are defined in canvas-bindings.d.ts but grida-canvas-wasm.d.ts extends canvas.CanvasModule without importing the canvas namespace. Add the import alongside the existing fonts, markdown, and svg imports:
import type { canvas } from "../modules/canvas-bindings";Without this import, the canvas namespace is unresolved and TypeScript consumers won't have proper type definitions for the new APIs.
🤖 Prompt for AI Agents
In `@crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.ts` around lines 176 -
185, Add the missing canvas import to the type declarations so the canvas
namespace used by grida-canvas-wasm.d.ts is resolved: update the file that
extends canvas.CanvasModule to include an import for the canvas namespace from
"../modules/canvas-bindings" (so the new methods
_runtime_renderer_set_pixel_preview_scale and
_runtime_renderer_set_pixel_preview_stable are correctly typed), mirroring the
existing imports for fonts, markdown, and svg.
| let image = surface.image_snapshot(); | ||
| let data = image | ||
| .encode(None, skia_safe::EncodedImageFormat::PNG, None) | ||
| .expect("encode"); | ||
| std::fs::write( | ||
| concat!(env!("CARGO_MANIFEST_DIR"), "/goldens/pixel_preview_stability.png"), | ||
| data.as_bytes(), | ||
| ) | ||
| .expect("write png"); |
There was a problem hiding this comment.
Ensure the goldens/ directory exists before writing.
Fresh clones or cleaned worktrees can fail this write if the directory is missing.
✅ Proposed fix
- let image = surface.image_snapshot();
- let data = image
- .encode(None, skia_safe::EncodedImageFormat::PNG, None)
- .expect("encode");
- std::fs::write(
- concat!(env!("CARGO_MANIFEST_DIR"), "/goldens/pixel_preview_stability.png"),
- data.as_bytes(),
- )
- .expect("write png");
+ let image = surface.image_snapshot();
+ let data = image
+ .encode(None, skia_safe::EncodedImageFormat::PNG, None)
+ .expect("encode");
+ let out_dir = concat!(env!("CARGO_MANIFEST_DIR"), "/goldens");
+ std::fs::create_dir_all(out_dir).expect("create goldens dir");
+ std::fs::write(
+ concat!(env!("CARGO_MANIFEST_DIR"), "/goldens/pixel_preview_stability.png"),
+ data.as_bytes(),
+ )
+ .expect("write png");📝 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.
| let image = surface.image_snapshot(); | |
| let data = image | |
| .encode(None, skia_safe::EncodedImageFormat::PNG, None) | |
| .expect("encode"); | |
| std::fs::write( | |
| concat!(env!("CARGO_MANIFEST_DIR"), "/goldens/pixel_preview_stability.png"), | |
| data.as_bytes(), | |
| ) | |
| .expect("write png"); | |
| let image = surface.image_snapshot(); | |
| let data = image | |
| .encode(None, skia_safe::EncodedImageFormat::PNG, None) | |
| .expect("encode"); | |
| let out_dir = concat!(env!("CARGO_MANIFEST_DIR"), "/goldens"); | |
| std::fs::create_dir_all(out_dir).expect("create goldens dir"); | |
| std::fs::write( | |
| concat!(env!("CARGO_MANIFEST_DIR"), "/goldens/pixel_preview_stability.png"), | |
| data.as_bytes(), | |
| ) | |
| .expect("write png"); |
🤖 Prompt for AI Agents
In `@crates/grida-canvas/examples/golden_pixel_preview_stability.rs` around lines
145 - 153, The write to "goldens/pixel_preview_stability.png" can fail if the
goldens directory doesn't exist; before calling std::fs::write (the code that
writes data.as_bytes()), ensure the directory exists by calling
std::fs::create_dir_all on the path created from
concat!(env!("CARGO_MANIFEST_DIR"), "/goldens") and handle or expect its Result
so the subsequent image/data encode and std::fs::write succeed reliably.
| <DropdownMenuSub> | ||
| <DropdownMenuSubTrigger className="text-xs"> | ||
| Pixel preview | ||
| </DropdownMenuSubTrigger> | ||
| <DropdownMenuSubContent className="min-w-40"> | ||
| <DropdownMenuRadioGroup | ||
| value={pixelpreview} | ||
| onValueChange={(v) => { | ||
| instance.surface.surfaceConfigurePixelPreviewScale( | ||
| v as "disabled" | "1x" | "2x" | ||
| ); | ||
| }} | ||
| > | ||
| <DropdownMenuRadioItem value="disabled" className="text-xs"> | ||
| Disabled | ||
| <DropdownMenuShortcut> | ||
| {keyboardShortcutText( | ||
| "workbench.surface.view.toggle-pixel-preview" | ||
| )} | ||
| </DropdownMenuShortcut> | ||
| </DropdownMenuRadioItem> | ||
| <DropdownMenuRadioItem value="1x" className="text-xs"> | ||
| 1x | ||
| <DropdownMenuShortcut> | ||
| {keyboardShortcutText( | ||
| "workbench.surface.view.toggle-pixel-preview" | ||
| )} | ||
| </DropdownMenuShortcut> | ||
| </DropdownMenuRadioItem> | ||
| <DropdownMenuRadioItem value="2x" className="text-xs"> | ||
| 2x | ||
| </DropdownMenuRadioItem> | ||
| </DropdownMenuRadioGroup> | ||
| </DropdownMenuSubContent> | ||
| </DropdownMenuSub> |
There was a problem hiding this comment.
Avoid duplicate shortcut hints in the Pixel preview radio group.
The same toggle shortcut is shown for both “Disabled” and “1x”, which can imply different shortcuts per option even though the action toggles a single state. Consider showing it once (e.g., only on “Disabled”).
🛠 Suggested tweak
<DropdownMenuRadioItem value="1x" className="text-xs">
1x
- <DropdownMenuShortcut>
- {keyboardShortcutText(
- "workbench.surface.view.toggle-pixel-preview"
- )}
- </DropdownMenuShortcut>
</DropdownMenuRadioItem>📝 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.
| <DropdownMenuSub> | |
| <DropdownMenuSubTrigger className="text-xs"> | |
| Pixel preview | |
| </DropdownMenuSubTrigger> | |
| <DropdownMenuSubContent className="min-w-40"> | |
| <DropdownMenuRadioGroup | |
| value={pixelpreview} | |
| onValueChange={(v) => { | |
| instance.surface.surfaceConfigurePixelPreviewScale( | |
| v as "disabled" | "1x" | "2x" | |
| ); | |
| }} | |
| > | |
| <DropdownMenuRadioItem value="disabled" className="text-xs"> | |
| Disabled | |
| <DropdownMenuShortcut> | |
| {keyboardShortcutText( | |
| "workbench.surface.view.toggle-pixel-preview" | |
| )} | |
| </DropdownMenuShortcut> | |
| </DropdownMenuRadioItem> | |
| <DropdownMenuRadioItem value="1x" className="text-xs"> | |
| 1x | |
| <DropdownMenuShortcut> | |
| {keyboardShortcutText( | |
| "workbench.surface.view.toggle-pixel-preview" | |
| )} | |
| </DropdownMenuShortcut> | |
| </DropdownMenuRadioItem> | |
| <DropdownMenuRadioItem value="2x" className="text-xs"> | |
| 2x | |
| </DropdownMenuRadioItem> | |
| </DropdownMenuRadioGroup> | |
| </DropdownMenuSubContent> | |
| </DropdownMenuSub> | |
| <DropdownMenuSub> | |
| <DropdownMenuSubTrigger className="text-xs"> | |
| Pixel preview | |
| </DropdownMenuSubTrigger> | |
| <DropdownMenuSubContent className="min-w-40"> | |
| <DropdownMenuRadioGroup | |
| value={pixelpreview} | |
| onValueChange={(v) => { | |
| instance.surface.surfaceConfigurePixelPreviewScale( | |
| v as "disabled" | "1x" | "2x" | |
| ); | |
| }} | |
| > | |
| <DropdownMenuRadioItem value="disabled" className="text-xs"> | |
| Disabled | |
| <DropdownMenuShortcut> | |
| {keyboardShortcutText( | |
| "workbench.surface.view.toggle-pixel-preview" | |
| )} | |
| </DropdownMenuShortcut> | |
| </DropdownMenuRadioItem> | |
| <DropdownMenuRadioItem value="1x" className="text-xs"> | |
| 1x | |
| </DropdownMenuRadioItem> | |
| <DropdownMenuRadioItem value="2x" className="text-xs"> | |
| 2x | |
| </DropdownMenuRadioItem> | |
| </DropdownMenuRadioGroup> | |
| </DropdownMenuSubContent> | |
| </DropdownMenuSub> |
🤖 Prompt for AI Agents
In `@editor/grida-canvas-hosted/playground/uxhost-menu.tsx` around lines 494 -
528, Duplicate keyboard shortcut hints appear for multiple DropdownMenuRadioItem
entries in the Pixel preview group; update the DropdownMenuRadioItem render
logic so keyboardShortcutText("workbench.surface.view.toggle-pixel-preview") is
only rendered once (for example on the "Disabled" item or moved to the parent
label) instead of being included on both the "Disabled" and "1x" items; locate
DropdownMenuRadioGroup and its DropdownMenuRadioItem children (values
"disabled", "1x", "2x") and remove the extra keyboardShortcutText invocation so
the toggle shortcut is shown a single time while leaving
instance.surface.surfaceConfigurePixelPreviewScale and pixelpreview handling
unchanged.
| this.doc.subscribeWithSelector( | ||
| (state) => state.pixelpreview, | ||
| (_, v) => { | ||
| this.__runtime_renderer_set_pixel_preview_scale(v); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Sync the initial pixel preview state on mount.
The subscription only applies changes after mount; if the state is already "1x" or "2x" (e.g., restored session), the runtime stays at default until toggled.
🔧 Suggested fix
this.doc.subscribeWithSelector(
(state) => state.debug,
(_, v) => {
this._m_wasm_canvas_scene?.setDebug(v);
this._m_wasm_canvas_scene?.redraw();
}
);
+ // Ensure initial pixel preview state is applied on mount.
+ this.__runtime_renderer_set_pixel_preview_scale(
+ this.doc.state.pixelpreview
+ );
+
this.doc.subscribeWithSelector(
(state) => state.pixelpreview,
(_, v) => {
this.__runtime_renderer_set_pixel_preview_scale(v);
}
);📝 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.
| this.doc.subscribeWithSelector( | |
| (state) => state.pixelpreview, | |
| (_, v) => { | |
| this.__runtime_renderer_set_pixel_preview_scale(v); | |
| } | |
| ); | |
| // Ensure initial pixel preview state is applied on mount. | |
| this.__runtime_renderer_set_pixel_preview_scale( | |
| this.doc.state.pixelpreview | |
| ); | |
| this.doc.subscribeWithSelector( | |
| (state) => state.pixelpreview, | |
| (_, v) => { | |
| this.__runtime_renderer_set_pixel_preview_scale(v); | |
| } | |
| ); |
🤖 Prompt for AI Agents
In `@editor/grida-canvas/editor.ts` around lines 3256 - 3261, The subscription to
this.doc.subscribeWithSelector for state.pixelpreview only reacts to changes and
doesn't apply the current value on mount; call the current pixelpreview state
once during initialization and pass it to
__runtime_renderer_set_pixel_preview_scale so the runtime matches restored
sessions. In other words, after (or before) setting up
this.doc.subscribeWithSelector, read this.doc.getState().pixelpreview (or invoke
the selector directly) and call
this.__runtime_renderer_set_pixel_preview_scale(...) with that value to sync
initial state.
day-314-grida-canvas-pixel-preview.mp4
New Features
Documentation