cf248 [Grida Canvas Experimental WASM] - Export as PDF / SVG
#400
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughThis update introduces comprehensive support for exporting scenes as PDF and SVG in addition to existing image formats (PNG, JPEG, etc.) across the Grida Canvas codebase. It modularizes export logic by format, adds new export interfaces and plugin integration in both Rust and TypeScript, and provides new example and test programs demonstrating PDF and SVG export capabilities. Field renaming and method encapsulation are applied for stroke alignment in line nodes. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor
participant ExportProvider (Image/PDF/SVG)
participant Grida2D/Surface
Editor->>ExportProvider: exportNodeAs(node_id, format)
ExportProvider->>Grida2D/Surface: exportNodeAs(node_id, { format })
Grida2D/Surface-->>ExportProvider: Exported data (Uint8Array or string)
ExportProvider-->>Editor: Exported data (Uint8Array or string)
sequenceDiagram
participant Scene
participant ExportDispatcher
participant ExportAsImage/PDF/SVG
participant Renderer
participant SkiaBackend (PDF/SVG/Image)
ExportDispatcher->>ExportAsImage/PDF/SVG: Match export format
ExportAsImage/PDF/SVG->>Renderer: Prepare scene and camera
Renderer->>SkiaBackend: Render scene to target format
SkiaBackend-->>Renderer: Encoded data (image/pdf/svg)
Renderer-->>ExportDispatcher: Exported data
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)warning: failed to write cache, path: /usr/local/registry/index/index.crates.io-1949cf8c6b5b557f/.cache/as/yn/async-trait, error: Permission denied (os error 13) Caused by: 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
cf248 [Grida Canvas Experimental WASM] - Export as PDFcf248 [Grida Canvas Experimental WASM] - Export as PDF / SVG
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (12)
crates/grida-canvas/examples/skpdf.rs (1)
1-30: LGTM: Clean example demonstrating PDF creation with Skia.This is a well-structured example that clearly demonstrates:
- PDF document creation with Skia
- Basic drawing operations (rectangle and line)
- Proper resource management (page lifecycle)
The code is educational and follows good practices.
Consider adding error handling for the file creation:
- let mut file = File::create("goldens/skia_output.pdf").expect("failed to create pdf"); + let mut file = File::create("goldens/skia_output.pdf") + .map_err(|e| format!("Failed to create PDF file: {}", e))?;But this is optional for an example program.
crates/grida-canvas/src/export/export_as_pdf.rs (1)
13-17: Document or utilize the unused_optionsparameter.The
_optionsparameter is currently unused. If it's intended for future use, add a TODO comment. Otherwise, consider removing it from the function signature.pub fn export_node_as_pdf( scene: &Scene, rect: Rectangle, - _options: ExportAsPDF, + _options: ExportAsPDF, // TODO: Implement PDF export options (e.g., compression, metadata) ) -> Option<Exported> {crates/grida-canvas/src/export/export_as_image.rs (1)
23-29: Remove unnecessary clone of format parameter.The
formatparameter doesn't need to be cloned sinceIntocan consume the value.pub fn export_node_as_image( scene: &Scene, size: ExportSize, rect: Rectangle, format: ExportAsImage, ) -> Option<Exported> { - let skfmt: EncodedImageFormat = format.clone().into(); + let skfmt: EncodedImageFormat = format.into();editor/scaffolds/sidecontrol/controls/export.tsx (1)
82-96: Simplify redundant switch statement.All cases in the switch statement execute identical code. This can be simplified.
const (format: "SVG" | "PDF" | "PNG" | "JPEG") => { - let task: Promise<Blob>; - - switch (format) { - case "JPEG": - case "PNG": - task = exportHandler(editor.exportNodeAs(node_id, format), format); - break; - case "PDF": - task = exportHandler(editor.exportNodeAs(node_id, format), format); - break; - case "SVG": - task = exportHandler(editor.exportNodeAs(node_id, format), format); - break; - } + const task = exportHandler(editor.exportNodeAs(node_id, format), format);crates/grida-canvas/src/window/application.rs (2)
75-89: Consider enhancing the clipboard implementation.The current clipboard implementation is very basic and only stores data in memory without system clipboard integration. Consider:
- Adding size limits to prevent memory issues
- Implementing actual system clipboard integration
- Adding a method to clear clipboard data
pub struct Clipboard { pub(crate) data: Option<Vec<u8>>, + pub(crate) format: Option<String>, } impl Default for Clipboard { fn default() -> Self { - Self { data: None } + Self { data: None, format: None } } } impl Clipboard { - pub(crate) fn set_data(&mut self, data: Vec<u8>) { + pub(crate) fn set_data(&mut self, data: Vec<u8>, format: &str) { + // Limit clipboard size to 50MB + if data.len() > 50 * 1024 * 1024 { + eprintln!("Clipboard data too large: {} bytes", data.len()); + return; + } self.data = Some(data); + self.format = Some(format.to_string()); + } + + pub(crate) fn clear(&mut self) { + self.data = None; + self.format = None; } }
184-192: Update clipboard usage to track data format.When storing PNG data in the clipboard, the format should be tracked for proper handling.
ApplicationCommand::TryCopyAsPNG => { if let Some(id) = self.devtools_selection.clone() { let exported = self.export_node_as(&id, ExportAs::png()); if let Some(exported) = exported { - self.clipboard.set_data(exported.data().to_vec()); + self.clipboard.set_data(exported.data().to_vec(), "image/png"); return true; } } }Note: This assumes the
set_datamethod is updated as suggested in the previous comment.crates/grida-canvas/src/export/mod.rs (1)
96-113: Consider usingmatchfor cleaner format dispatch.Replace the if-else chain with a match expression for better readability and exhaustiveness checking.
- if format.is_format_pdf() { - let format: ExportAsPDF = match format { - ExportAs::PDF(pdf_format) => pdf_format, - _ => unreachable!(), - }; - return export_node_as_pdf(scene, rect, format); - } else if format.is_format_svg() { - let format: ExportAsSVG = match format { - ExportAs::SVG(svg_format) => svg_format, - _ => unreachable!(), - }; - return export_node_as_svg(scene, rect, format); - } else if format.is_format_image() { - let format: ExportAsImage = format.clone().try_into().unwrap(); - return export_node_as_image(scene, size, rect, format); - } else { - return None; - } + match format { + ExportAs::PDF(pdf_format) => export_node_as_pdf(scene, rect, pdf_format), + ExportAs::SVG(svg_format) => export_node_as_svg(scene, rect, svg_format), + ExportAs::PNG(_) | ExportAs::JPEG(_) | ExportAs::WEBP(_) | ExportAs::BMP(_) => { + let Ok(image_format) = format.try_into() else { + return None; + }; + export_node_as_image(scene, size, rect, image_format) + } + }editor/grida-canvas/editor.ts (2)
146-165: Consider refactoring repetitive exporter initialization.The initialization logic for the three exporters follows the same pattern and could be consolidated.
private initializeExporter<T>( plugin: WithEditorInstance<T> | undefined, setter: (exporter: T) => void ) { if (plugin) { setter(resolveWithEditorInstance(this, plugin)); } } // In constructor: this.initializeExporter(plugins?.export_as_image, (e) => this._m_exporter_image = e); this.initializeExporter(plugins?.export_as_pdf, (e) => this._m_exporter_pdf = e); this.initializeExporter(plugins?.export_as_svg, (e) => this._m_exporter_svg = e);
1956-1982: Improve error messages for better debugging.The error messages could be more specific to help with debugging.
case "PNG": case "JPEG": { if (!this.exporterImage) { - throw new Error("Exporter is not bound"); + throw new Error(`Image exporter for ${format} is not bound`); } return this.exporterImage.exportNodeAsImage(node_id, format); } case "PDF": { if (!this.exporterPdf) { - throw new Error("Exporter is not bound"); + throw new Error("PDF exporter is not bound"); } return this.exporterPdf.exportNodeAsPDF(node_id); } case "SVG": { if (!this.exporterSvg) { - throw new Error("Exporter is not bound"); + throw new Error("SVG exporter is not bound"); } return this.exporterSvg.exportNodeAsSVG(node_id); } } - throw new Error("Not implemented"); + throw new Error(`Unsupported export format: ${format}`);crates/grida-canvas/src/export/types.rs (3)
44-52: Consider enforcing constraint restrictions at the type level.The comments indicate that PDF and SVG don't support constraints, but this isn't enforced by the type system. Users could still try to set constraints that would be ignored.
Consider either:
- Making the constraint restriction explicit in the API documentation
- Using phantom types or sealed traits to prevent constraint usage at compile time
- Adding validation that returns an error if constraints are provided for PDF/SVG formats
90-102: Use a descriptive error type instead of unit type.The
TryFromimplementation returns()as the error type, which doesn't provide useful information about why the conversion failed.+#[derive(Debug, Clone, Copy)] +pub struct NotAnImageFormatError; + +impl std::fmt::Display for NotAnImageFormatError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Format is not an image format (PNG, JPEG, WEBP, or BMP)") + } +} + +impl std::error::Error for NotAnImageFormatError {} + impl TryFrom<ExportAs> for ExportAsImage { - type Error = (); + type Error = NotAnImageFormatError; - fn try_from(value: ExportAs) -> Result<Self, ()> { + fn try_from(value: ExportAs) -> Result<Self, Self::Error> { match value { ExportAs::PNG(x) => Ok(Self::PNG(x)), ExportAs::JPEG(x) => Ok(Self::JPEG(x)), ExportAs::WEBP(x) => Ok(Self::WEBP(x)), ExportAs::BMP(x) => Ok(Self::BMP(x)), - _ => Err(()), + _ => Err(NotAnImageFormatError), } } }
71-88: Consider the maintenance burden of duplicating format variants.The
ExportAsImageenum duplicates the image format variants fromExportAs. This could lead to maintenance issues if new image formats are added.Consider using a more maintainable approach such as:
- Using a single enum with methods to check format categories
- Using const generics or associated types to group formats
- Implementing the constraint getter directly on
ExportAswith pattern matchingThe current approach works but requires updating multiple places when adding new formats.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockcrates/grida-canvas-wasm/lib/bin/grida_canvas_wasm.wasmis excluded by!**/*.wasmcrates/grida-canvas/goldens/pdf.pdfis excluded by!**/*.pdfcrates/grida-canvas/goldens/skpdf.pdfis excluded by!**/*.pdfcrates/grida-canvas/goldens/sksvg.svgis excluded by!**/*.svgcrates/grida-canvas/goldens/svg.svgis excluded by!**/*.svg
📒 Files selected for processing (28)
crates/grida-canvas-wasm/lib/index.ts(2 hunks)crates/grida-canvas-wasm/package.json(1 hunks)crates/grida-canvas/Cargo.toml(1 hunks)crates/grida-canvas/examples/lines.rs(1 hunks)crates/grida-canvas/examples/pdf.rs(1 hunks)crates/grida-canvas/examples/skpdf.rs(1 hunks)crates/grida-canvas/examples/sksvg.rs(1 hunks)crates/grida-canvas/examples/svg.rs(1 hunks)crates/grida-canvas/src/cache/geometry.rs(1 hunks)crates/grida-canvas/src/export/export_as_image.rs(1 hunks)crates/grida-canvas/src/export/export_as_pdf.rs(1 hunks)crates/grida-canvas/src/export/export_as_svg.rs(1 hunks)crates/grida-canvas/src/export/mod.rs(4 hunks)crates/grida-canvas/src/export/types.rs(5 hunks)crates/grida-canvas/src/io/io_figma.rs(1 hunks)crates/grida-canvas/src/node/factory.rs(1 hunks)crates/grida-canvas/src/node/schema.rs(2 hunks)crates/grida-canvas/src/painter/layer.rs(1 hunks)crates/grida-canvas/src/painter/painter.rs(1 hunks)crates/grida-canvas/src/runtime/scene.rs(1 hunks)crates/grida-canvas/src/window/application.rs(4 hunks)crates/grida-canvas/src/window/application_native.rs(1 hunks)crates/grida-canvas/src/window/command.rs(1 hunks)crates/grida-canvas/tests/export_as_pdf.rs(1 hunks)editor/grida-canvas/backends/wasm.ts(1 hunks)editor/grida-canvas/editor.ts(7 hunks)editor/grida-canvas/index.ts(1 hunks)editor/scaffolds/sidecontrol/controls/export.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
crates/grida-canvas/src/export/export_as_svg.rs (5)
packages/grida-canvas-schema/grida.ts (1)
Scene(454-483)crates/grida-canvas/src/export/types.rs (1)
svg(117-119)crates/grida-canvas/src/runtime/camera.rs (1)
new_from_bounds(56-73)crates/grida-canvas/src/runtime/scene.rs (1)
new_from_raster(77-80)crates/grida-canvas/src/export/mod.rs (1)
data(28-37)
crates/grida-canvas-wasm/lib/index.ts (2)
library/worker/src/worker.py (1)
format(34-40)crates/grida-canvas/src/export/mod.rs (1)
data(28-37)
crates/grida-canvas/examples/lines.rs (4)
packages/grida-canvas-cg/lib.ts (3)
AffineTransform(65-68)LinearGradientPaint(317-322)GradientStop(331-339)crates/grida-canvas/src/node/factory.rs (1)
new(9-11)crates/grida-canvas/src/node/schema.rs (1)
new(296-302)crates/grida-canvas/src/window/application_native_demo.rs (1)
run_demo_window(11-13)
🪛 Biome (1.9.4)
editor/scaffolds/sidecontrol/controls/export.tsx
[error] 71-79: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
⏰ 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: Cursor BugBot
🔇 Additional comments (22)
crates/grida-canvas/src/window/command.rs (1)
1-1: LGTM! Adding Clone trait for improved command handling.Adding the
Clonetrait toApplicationCommandis a good improvement that enables more flexible command handling and supports the clipboard management enhancements mentioned in the PR.crates/grida-canvas-wasm/package.json (1)
4-4: LGTM! Version bump reflects new export capabilities.The version increment to 0.0.10 appropriately reflects the addition of PDF and SVG export functionality mentioned in the PR objectives.
crates/grida-canvas/Cargo.toml (1)
11-11: LGTM! Adding PDF and SVG features for new export capabilities.Adding the "pdf" and "svg" features to the skia-safe dependency correctly enables the required Skia backends for the new PDF and SVG export functionality.
crates/grida-canvas/src/node/factory.rs (1)
92-92: LGTM! Field encapsulation improves API design.The renaming from
stroke_alignto_data_stroke_alignis part of the broader refactoring to encapsulate stroke alignment handling in line nodes. The underscore prefix correctly indicates this is now an internal field accessed via theget_stroke_align()method.crates/grida-canvas/src/painter/layer.rs (1)
379-379: LGTM! Method-based access aligns with encapsulation refactoring.The change from direct field access to
n.get_stroke_align()is consistent with the broader refactoring to encapsulate stroke alignment handling in line nodes. This provides better API design and centralized logic for stroke alignment computation.crates/grida-canvas/src/io/io_figma.rs (1)
1113-1113: LGTM: Consistent field renaming for stroke alignment encapsulation.This change aligns with the broader refactoring to encapsulate stroke alignment handling for line nodes. The underscore prefix follows Rust conventions for internal data fields.
crates/grida-canvas/src/painter/painter.rs (1)
454-454: LGTM: Method call replaces direct field access for better encapsulation.This change is consistent with the stroke alignment refactoring across the codebase. Using
get_stroke_align()method provides better encapsulation than direct field access.crates/grida-canvas/src/cache/geometry.rs (1)
485-490: LGTM: Consistent method call for stroke alignment in geometry calculations.This change maintains consistency with the stroke alignment refactoring pattern. The
get_stroke_align()method call ensures uniform handling of line node stroke alignment in render bounds calculations.crates/grida-canvas/examples/sksvg.rs (1)
1-26: LGTM! Clean and educational SVG example.This example effectively demonstrates direct usage of Skia's SVG canvas with clear, straightforward code. The drawing operations (blue rectangle + red diagonal line) provide a good foundation for understanding SVG canvas manipulation, and the file I/O with proper error handling is appropriate for example code.
editor/grida-canvas/index.ts (1)
1712-1713: LGTM! Consistent interface extension for new export formats.The new export format overloads maintain consistency with the existing API design. The return types are appropriate -
Promise<Uint8Array>for binary PDF data andPromise<string>for SVG text content.crates/grida-canvas/tests/export_as_pdf.rs (1)
5-57: LGTM! Comprehensive PDF export test.This test provides excellent coverage of the PDF export pipeline:
- Creates a realistic scene with proper node setup
- Tests the complete export workflow from scene to PDF
- Validates both the presence of data and correct PDF format using magic bytes
- Uses appropriate assertions for robust testing
The test design effectively validates the new PDF export functionality.
crates/grida-canvas/src/export/export_as_svg.rs (2)
27-32: Question the raster backend choice for SVG export.Using a raster backend for SVG export seems counterintuitive since SVG is inherently vector-based. This approach might impact the scalability and quality benefits of SVG format.
Could you clarify if this is intentional for consistency with the renderer architecture, or if there's a vector-based approach that would be more appropriate for SVG export?
Otherwise, the implementation looks solid with proper resource management and correct SVG canvas handling.
40-42: LGTM! Proper resource cleanup and data conversion.The resource cleanup with
renderer.free()and the conversion to byte vector for consistent return type handling is well implemented.crates/grida-canvas/src/node/schema.rs (3)
61-61: LGTM! Added PartialEq for comparison support.Adding the
PartialEqtrait toStrokeAlignenum enables equality comparisons, which is useful for the encapsulation logic.
628-628: LGTM! Clear field renaming for encapsulation.The underscore prefix in
_data_stroke_alignclearly indicates this is internal data that shouldn't be accessed directly, supporting the encapsulation design.
634-639: LGTM! Excellent encapsulation with clear documentation.The
get_stroke_align()method provides a clean interface that enforces the business rule (lines always use center alignment) while preserving the underlying data for path conversion scenarios. The documentation clearly explains the design rationale.crates/grida-canvas/src/runtime/scene.rs (1)
543-543: Good practice: Explicitly ignoring unused return value.Using
_makes it clear that theDrawResultis intentionally ignored.editor/grida-canvas/backends/wasm.ts (1)
61-92: LGTM! Clean implementation of PDF and SVG export providers.The new export providers follow the established pattern from
CanvasWasmImageExportInterfaceProviderand correctly handle the different return types (binary for PDF, string for SVG).crates/grida-canvas-wasm/lib/index.ts (2)
17-23: Well-structured union type for export formats.The discriminated union pattern with distinct format types allows for type-safe handling of different export options and their specific constraints.
245-245: Good improvement to error messaging.The dynamic error message provides better context for debugging export failures.
crates/grida-canvas/src/export/mod.rs (1)
1-3: Good modularization of export logic.Separating export functionality into dedicated modules for each format type improves code organization and maintainability.
editor/grida-canvas/editor.ts (1)
50-63: Clean implementation of the plugin initialization pattern.The
WithEditorInstancepattern provides good flexibility for plugins that need access to the editor instance during initialization.
| let ok = self.app.command(cmd.clone()); | ||
|
|
||
| if ok { | ||
| match cmd.clone() { | ||
| ApplicationCommand::TryCopyAsPNG => { | ||
| { | ||
| // save to file | ||
| use std::io::Write; | ||
| let path = format!("clipboard.png"); | ||
| let mut file = std::fs::File::create(path).unwrap(); | ||
| file.write_all(self.app.clipboard.data.as_ref().unwrap()) | ||
| .unwrap(); | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and file path management.
The file I/O operations have several concerns:
- Hardcoded file path: The path "clipboard.png" is relative and could cause issues depending on the working directory.
- Poor error handling: Using
unwrap()will panic on file creation or write failures. - No data validation: The clipboard data is written without validation.
Consider this improvement:
- ApplicationCommand::TryCopyAsPNG => {
- {
- // save to file
- use std::io::Write;
- let path = format!("clipboard.png");
- let mut file = std::fs::File::create(path).unwrap();
- file.write_all(self.app.clipboard.data.as_ref().unwrap())
- .unwrap();
- }
- }
+ ApplicationCommand::TryCopyAsPNG => {
+ if let Some(data) = &self.app.clipboard.data {
+ use std::io::Write;
+ let path = std::env::current_dir()
+ .unwrap_or_else(|_| std::path::PathBuf::from("."))
+ .join("clipboard.png");
+
+ match std::fs::File::create(&path) {
+ Ok(mut file) => {
+ if let Err(e) = file.write_all(data) {
+ eprintln!("Failed to write clipboard data: {}", e);
+ }
+ }
+ Err(e) => {
+ eprintln!("Failed to create clipboard file: {}", e);
+ }
+ }
+ }
+ }🤖 Prompt for AI Agents
In crates/grida-canvas/src/window/application_native.rs around lines 191 to 207,
improve the file I/O by replacing the hardcoded relative path with a more robust
path, such as using a temporary directory or a configurable location. Replace
all unwrap() calls with proper error handling using match or if let to
gracefully handle file creation and write errors. Before writing, validate that
clipboard data exists and is valid to avoid panics or writing invalid data.
| /// Render the current scene onto the provided canvas. This is useful for | ||
| /// exporting the scene using alternate backends such as PDF. | ||
| pub fn render_to_canvas(&self, canvas: &Canvas, width: f32, height: f32) { | ||
| let frame = self.frame(self.camera.rect(), 1.0, true); | ||
| let background = self.scene.as_ref().and_then(|s| s.background_color); | ||
| F438 | let _ = self.draw_nocache(canvas, &frame, background, width, height); | |
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation and input validation for the new public method.
The method lacks documentation and input validation. Consider:
- Adding a doc comment explaining the method's purpose and parameters
- Validating that width and height are positive values
+ /// Render the current scene onto the provided canvas. This is useful for
+ /// exporting the scene using alternate backends such as PDF or SVG.
+ ///
+ /// # Arguments
+ /// * `canvas` - The target canvas to render onto
+ /// * `width` - The width of the canvas in pixels (must be positive)
+ /// * `height` - The height of the canvas in pixels (must be positive)
pub fn render_to_canvas(&self, canvas: &Canvas, width: f32, height: f32) {
+ if width <= 0.0 || height <= 0.0 {
+ eprintln!("Warning: Invalid canvas dimensions: {}x{}", width, height);
+ return;
+ }
let frame = self.frame(self.camera.rect(), 1.0, true);
let background = self.scene.as_ref().and_then(|s| s.background_color);
let _ = self.draw_nocache(canvas, &frame, background, width, height);
}🤖 Prompt for AI Agents
In crates/grida-canvas/src/runtime/scene.rs around lines 548 to 554, the public
method render_to_canvas lacks documentation and input validation. Add a doc
comment describing the method's purpose and its parameters (canvas, width,
height). Also, add checks to ensure width and height are positive values,
returning early or handling errors if they are not.
| // Calculate page size based on the node bounds | ||
| let width = rect.width; | ||
| let height = rect.height; | ||
|
|
||
| // Begin a new page | ||
| let mut page = doc.begin_page(SkSize::new(width, height), None); | ||
| let canvas = page.canvas(); | ||
|
|
||
| // Create a camera that focuses on the specific node bounds | ||
| let camera = Camera2D::new_from_bounds(rect); | ||
|
|
||
| // Create a renderer with a temporary backend | ||
| let mut renderer = Renderer::new( | ||
| Backend::new_from_raster(width as i32, height as i32), | ||
| None, | ||
| camera, | ||
| ); | ||
|
|
||
| // Load the scene | ||
| renderer.load_scene(scene.clone()); | ||
|
|
||
| // Render the scene to the PDF canvas | ||
| renderer.render_to_canvas(canvas, width, height); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and consider error handling.
The function should validate input dimensions and handle potential PDF creation failures more gracefully.
// Calculate page size based on the node bounds
let width = rect.width;
let height = rect.height;
+
+ // Validate dimensions
+ if width <= 0.0 || height <= 0.0 || !width.is_finite() || !height.is_finite() {
+ eprintln!("Invalid PDF dimensions: {}x{}", width, height);
+ return None;
+ }
// Begin a new page
let mut page = doc.begin_page(SkSize::new(width, height), None);📝 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.
| // Calculate page size based on the node bounds | |
| let width = rect.width; | |
| let height = rect.height; | |
| // Begin a new page | |
| let mut page = doc.begin_page(SkSize::new(width, height), None); | |
| let canvas = page.canvas(); | |
| // Create a camera that focuses on the specific node bounds | |
| let camera = Camera2D::new_from_bounds(rect); | |
| // Create a renderer with a temporary backend | |
| let mut renderer = Renderer::new( | |
| Backend::new_from_raster(width as i32, height as i32), | |
| None, | |
| camera, | |
| ); | |
| // Load the scene | |
| renderer.load_scene(scene.clone()); | |
| // Render the scene to the PDF canvas | |
| renderer.render_to_canvas(canvas, width, height); | |
| // Calculate page size based on the node bounds | |
| let width = rect.width; | |
| let height = rect.height; | |
| // Validate dimensions | |
| if width <= 0.0 || height <= 0.0 || !width.is_finite() || !height.is_finite() { | |
| eprintln!("Invalid PDF dimensions: {}x{}", width, height); | |
| return None; | |
| } | |
| // Begin a new page | |
| let mut page = doc.begin_page(SkSize::new(width, height), None); | |
| let canvas = page.canvas(); | |
| // Create a camera that focuses on the specific node bounds | |
| let camera = Camera2D::new_from_bounds(rect); | |
| // Create a renderer with a temporary backend | |
| let mut renderer = Renderer::new( | |
| Backend::new_from_raster(width as i32, height as i32), | |
| None, | |
| camera, | |
| ); | |
| // Load the scene | |
| renderer.load_scene(scene.clone()); | |
| // Render the scene to the PDF canvas | |
| renderer.render_to_canvas(canvas, width, height); |
🤖 Prompt for AI Agents
In crates/grida-canvas/src/export/export_as_pdf.rs between lines 24 and 46, the
code lacks validation for input dimensions and does not handle potential errors
during PDF creation. Add checks to ensure width and height are positive and
valid before proceeding. Implement error handling around PDF page creation and
rendering steps to gracefully manage failures, returning or logging errors as
appropriate.
| let mut r = Renderer::new( | ||
| Backend::new_from_raster(size.width as i32, size.height as i32), | ||
| None, | ||
| camera, | ||
| ); | ||
|
|
||
| r.load_scene(scene.clone()); | ||
| let image = r.snapshot(); | ||
|
|
||
| #[allow(deprecated)] | ||
| let Some(data) = image.encode_to_data(skfmt) else { | ||
| return None; | ||
| }; | ||
|
|
||
| // 2. export node | ||
| match format { | ||
| ExportAsImage::PNG(_) => Some(Exported::PNG(data.to_vec())), | ||
| ExportAsImage::JPEG(_) => Some(Exported::JPEG(data.to_vec())), | ||
| ExportAsImage::WEBP(_) => Some(Exported::WEBP(data.to_vec())), | ||
| ExportAsImage::BMP(_) => Some(Exported::BMP(data.to_vec())), | ||
| } |
There was a problem hiding this comment.
Fix comment numbering and add renderer cleanup.
The function has duplicate comment numbers and is missing renderer cleanup.
- // 2. create a renderer
+ // 1. create a renderer
let mut r = Renderer::new(
Backend::new_from_raster(size.width as i32, size.height as i32),
None,
camera,
);
r.load_scene(scene.clone());
let image = r.snapshot();
#[allow(deprecated)]
let Some(data) = image.encode_to_data(skfmt) else {
+ r.free();
return None;
};
- // 2. export node
+ // 2. export node
match format {
ExportAsImage::PNG(_) => Some(Exported::PNG(data.to_vec())),
ExportAsImage::JPEG(_) => Some(Exported::JPEG(data.to_vec())),
ExportAsImage::WEBP(_) => Some(Exported::WEBP(data.to_vec())),
ExportAsImage::BMP(_) => Some(Exported::BMP(data.to_vec())),
- }
+ };
+
+ r.free();
+ result
}Note: You'll need to store the result in a variable before freeing the renderer.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/grida-canvas/src/export/export_as_image.rs around lines 34 to 54, fix
the duplicate comment numbering by renumbering comments sequentially and add
explicit cleanup for the renderer after capturing the snapshot. Store the
snapshot result in a variable before freeing or dropping the renderer to ensure
proper resource management without losing the image data.
| #[allow(deprecated)] | ||
| let Some(data) = image.encode_to_data(skfmt) else { | ||
| return None; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider replacing deprecated API.
The code uses a deprecated encode_to_data method. Please verify if there's a non-deprecated alternative available in the current skia-safe version.
🌐 Web query:
What is the non-deprecated alternative to encode_to_data in skia-safe for encoding images?
💡 Result:
The non-deprecated alternative to encode_to_data in the skia-safe crate for encoding images is encode_to_data_with_context. The reason for this change is that support for encoding GPU-backed images without an explicit GPU context was removed; you now need to provide a context if the image is GPU-backed[4][8].
Recommended Usage
-
encode_to_data_with_contextallows you to specify an optionalDirectContext, anEncodedImageFormat, and an optional quality value for the encoding process:image.encode_to_data_with_context( context, // Option<DirectContext> image_format, // EncodedImageFormat quality // Option<u32> )
-
If your image is CPU-backed, you can pass
Nonefor the context. For GPU-backed images, provide the relevant GPU context[4][8].
Deprecated Functions
encode_to_dataandencode_to_data_with_qualityare now deprecated. These methods do not support encoding GPU-backed images without a context and may be removed in future releases[4][8].
Supported Formats
- Commonly supported output formats are PNG, JPEG, and (if enabled) WEBP. Some additional formats such as GIF, ICO, and BMP may be available depending on platform and build configuration[7].
Summary Table
| Method | Deprecated | Requires Context | Recommended For |
|---|---|---|---|
encode_to_data |
Yes | No (but now deprecated for GPU images) | CPU images only (not recommended) |
encode_to_data_with_quality |
Yes | No (but now deprecated for GPU images) | CPU images only (not recommended) |
encode_to_data_with_context |
No | Yes (GPU images), No (CPU images) | All images (recommended) |
Use encode_to_data_with_context for all new code to future-proof your image encoding in skia-safe[4][8].
Citations:
- 1: https://github.com/rust-skia/rust-skia/blob/master/skia-safe/src/core/image.rs
- 2: https://learn.microsoft.com/en-us/dotnet/api/skiasharp.skimage.encode?view=skiasharp-2.88
- 3: https://skia.googlesource.com/skia/+/b3923b715c50/RELEASE_NOTES.md
- 4: https://rust-skia.github.io/doc/skia_safe/type.Image.html
- 5: Which image formats are supported on which platforms? rust-skia/rust-skia#389
- 6: https://issues.skia.org/40045064
- 7: https://docs.rs/crate/skia-safe/latest
- 8: https://rust-skia.github.io/doc/skia_safe/struct.RCHandle.html
- 9: https://app.unpkg.com/canvaskit-wasm@0.28.1/files/CHANGELOG.md
- 10: https://chromium.googlesource.com/skia/+/a560c4796f5b83a2e55cf564dc847ad6498164b0/docs/SkImage_Reference.bmh
Replace deprecated encode_to_data with encode_to_data_with_context
The use of the deprecated encode_to_data method should be replaced with the new encode_to_data_with_context, which supports both CPU- and GPU-backed images by requiring an explicit context (or None for CPU).
• File: crates/grida-canvas/src/export/export_as_image.rs
Lines: 43–46
Suggested change:
- #[allow(deprecated)]
- let Some(data) = image.encode_to_data(skfmt) else {
- return None;
- };
+ let Some(data) = image.encode_to_data_with_context(
+ None, // CPU images: no GPU context
+ skfmt, // EncodedImageFormat
+ None, // Optional quality (use default)
+ ) else {
+ return None;
+ };This removes the #[allow(deprecated)] and future-proofs encoding by using the non-deprecated API.
🤖 Prompt for AI Agents
In crates/grida-canvas/src/export/export_as_image.rs around lines 43 to 46,
replace the deprecated call to encode_to_data with encode_to_data_with_context
by passing None as the context argument for CPU-backed images. Remove the
#[allow(deprecated)] attribute and update the code to use
image.encode_to_data_with_context(None, skfmt) to future-proof the encoding
process.
| opacity: 1.0, | ||
| }); | ||
| line_outside.stroke_width = 8.0; | ||
| line_outside._data_stroke_align = StrokeAlign::Outside; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider using a public API for stroke alignment.
The code accesses a private field _data_stroke_align. This suggests that stroke alignment should be exposed through a proper public API method rather than direct field access.
🏁 Script executed:
#!/bin/bash
# Check if there's a public method for setting stroke alignment
ast-grep --pattern 'impl $_ {
$$$
fn set_stroke_align($$$) {
$$$
}
$$$
}' Length of output: 87
I couldn’t find a set_stroke_align method—let’s search the entire crate for any public API around StrokeAlign.
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of StrokeAlign in public methods or functions
rg "StrokeAlign" -n crates/grida-canvas
rg "stroke_align" -n crates/grida-canvasLength of output: 15848
Use public stroke_align field instead of private _data_stroke_align
Replace direct access to the private field with the existing public API on the node type.
• File: crates/grida-canvas/examples/lines.rs
Line 54
Diff:
- line_outside._data_stroke_align = StrokeAlign::Outside;
+ line_outside.stroke_align = StrokeAlign::Outside;📝 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.
| line_outside._data_stroke_align = StrokeAlign::Outside; | |
| line_outside.stroke_align = StrokeAlign::Outside; |
🤖 Prompt for AI Agents
In crates/grida-canvas/examples/lines.rs at line 54, replace the direct
assignment to the private field _data_stroke_align with the public stroke_align
field provided by the node type. Change the code to set
line_outside.stroke_align = StrokeAlign::Outside instead of accessing the
private field.
| }; | ||
| rect_gradient.corner_radius = RectangularCornerRadius::all(20.0); | ||
| rect_gradient.fill = Paint::LinearGradient(LinearGradientPaint { | ||
| transform: AffineTransform::from_rotatation(45.0), |
There was a problem hiding this comment.
Fix typo in method name.
- transform: AffineTransform::from_rotatation(45.0),
+ transform: AffineTransform::from_rotation(45.0),🤖 Prompt for AI Agents
In crates/grida-canvas/examples/pdf.rs at line 91, there is a typo in the method
name 'from_rotatation'. Correct the method name to 'from_rotation' to fix the
typo and ensure the code compiles and runs as expected.
| }; | ||
| return export_node_as_svg(scene, rect, format); | ||
| } else if format.is_format_image() { | ||
| let format: ExportAsImage = format.clone().try_into().unwrap(); |
There was a problem hiding this comment.
Handle conversion error instead of using unwrap().
The unwrap() call could panic if the conversion fails. Handle the error case gracefully.
- let format: ExportAsImage = format.clone().try_into().unwrap();
+ let Ok(format) = format.clone().try_into() else {
+ return None;
+ };🤖 Prompt for AI Agents
In crates/grida-canvas/src/export/mod.rs at line 109, replace the use of
unwrap() on the try_into() conversion with proper error handling. Instead of
unwrapping, use a match statement or the ? operator to handle the Result
returned by try_into(), returning or propagating an error if the conversion
fails to avoid panics.
| let format: ExportAsPDF = match format { | ||
| ExportAs::PDF(pdf_format) => pdf_format, | ||
| _ => unreachable!(), | ||
| }; | ||
| return export_node_as_pdf(scene, rect, format); |
There was a problem hiding this comment.
Replace unreachable!() with proper error handling.
Using unreachable!() can cause runtime panics if the format check logic changes or has bugs. Return None or a proper error instead.
- let format: ExportAsPDF = match format {
- ExportAs::PDF(pdf_format) => pdf_format,
- _ => unreachable!(),
- };
+ let ExportAs::PDF(format) = format else {
+ return None;
+ };📝 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 format: ExportAsPDF = match format { | |
| ExportAs::PDF(pdf_format) => pdf_format, | |
| _ => unreachable!(), | |
| }; | |
| return export_node_as_pdf(scene, rect, format); | |
| let ExportAs::PDF(format) = format else { | |
| return None; | |
| }; | |
| return export_node_as_pdf(scene, rect, format); |
🤖 Prompt for AI Agents
In crates/grida-canvas/src/export/mod.rs around lines 97 to 101, replace the use
of unreachable!() in the match statement with proper error handling to avoid
runtime panics. Instead of assuming the match arm is unreachable, return an
appropriate error or None when the format does not match ExportAs::PDF, ensuring
the function handles unexpected input gracefully.
| ExportAs::SVG(svg_format) => svg_format, | ||
| _ => unreachable!(), | ||
| }; | ||
| return export_node_as_svg(scene, rect, format); |
There was a problem hiding this comment.
Replace unreachable!() with proper error handling.
Same issue as above - avoid unreachable!() for safer code.
- let format: ExportAsSVG = match format {
- ExportAs::SVG(svg_format) => svg_format,
- _ => unreachable!(),
- };
+ let ExportAs::SVG(format) = format else {
+ return None;
+ };📝 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.
| ExportAs::SVG(svg_format) => svg_format, | |
| _ => unreachable!(), | |
| }; | |
| return export_node_as_svg(scene, rect, format); | |
| let ExportAs::SVG(format) = format else { | |
| return None; | |
| }; | |
| return export_node_as_svg(scene, rect, format); |
🤖 Prompt for AI Agents
In crates/grida-canvas/src/export/mod.rs around lines 104 to 107, replace the
use of unreachable!() with proper error handling to avoid panics. Instead of
assuming the code path is impossible, return a Result with an appropriate error
or handle the unexpected case gracefully to ensure safer and more robust code.
There was a problem hiding this comment.
Bug: Unintended PNG File Creation
Accidentally committed debug code saves clipboard PNG data to a file named "clipboard.png" in the current directory when the TryCopyAsPNG command succeeds. This code uses unwrap() and performs unconsented file I/O, and should be removed.
crates/grida-canvas/src/window/application_native.rs#L189-L206
grida/crates/grida-canvas/src/window/application_native.rs
Lines 189 to 206 in e4077ac
Was this report helpful? Give feedback by reacting with 👍 or 👎
Grida Canvas WASM Now supports exporting node to PDF/SVG.
Experimental playground: https://canary.grida.co/canvas/experimental/wasm
day-248-grida-canvas-core-graphics-pt5-wasm-export-as-pdf-svg.mp4
sequential export is not working. will fix in new pr.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests