Skip to content

feat(canvas): add MarkdownNode with direct Skia rendering #620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
softmarshmallow merged 6 commits into from
Apr 2, 2026
Merged

feat(canvas): add MarkdownNode with direct Skia rendering #620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8aa374b
feat(html): support CSS margin import via tree surgery
softmarshmallow Apr 1, 2026
3c9d530
fmt
softmarshmallow Apr 1, 2026
ca12e17
feat(html): merge inline elements into AttributedText nodes
softmarshmallow Apr 1, 2026
77115e8
feat(markdown): add comprehensive kitchen-sink markdown fixture
softmarshmallow Apr 2, 2026
5aa598f
feat(canvas): add MarkdownNode with direct Skia rendering
softmarshmallow Apr 2, 2026
13a4170
chore
softmarshmallow Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/csscascade/src/adapter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@ impl HtmlDocument {
// ---------------------------------------------------------------------------

impl HtmlElement {
/// Wrap a DOM [`NodeId`] as an [`HtmlElement`].
///
/// # Safety (logical)
/// The caller must ensure the node is actually an element node.
/// Calling methods on a non-element HtmlElement will panic.
pub fn from_node_id(id: NodeId) -> Self {
Self(id)
}
Comment on lines +139 to +146
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid exposing an unchecked public constructor for HtmlElement.

On Line 144, from_node_id lets callers construct HtmlElement from any NodeId, including non-element nodes; that violates the type invariant and defers failure to panics in element_data() (Line 168). Prefer a checked public API (or make unchecked construction non-public).

Suggested safer API shape 
 impl HtmlElement {
-    pub fn from_node_id(id: NodeId) -> Self {
-        Self(id)
-    }
+    pub fn try_from_node_id(id: NodeId) -> Option<Self> {
+        matches!(dom().node(id).data, DemoNodeData::Element(_)).then_some(Self(id))
+    }
+
+    pub(crate) fn from_node_id_unchecked(id: NodeId) -> Self {
+        Self(id)
+    }
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/csscascade/src/adapter.rs` around lines 139 - 146, The public
constructor HtmlElement::from_node_id exposes an unchecked creation path that
can wrap non-element NodeId values (causing later panics in element_data);
change the API to either (a) make from_node_id non-public (e.g., pub(crate) or
remove pub) and provide a checked public constructor that validates the node
kind and returns Option<HtmlElement>/Result<HtmlElement, Error>, or (b) keep
from_node_id but mark it unsafe and document the precondition, and implement a
new safe method (e.g., try_from_node_id or HtmlElement::from_node_id_checked)
that calls the node-kind check used by element_data and returns None/Err on
failure; update call sites to use the safe checked API or the unsafe constructor
accordingly and reference HtmlElement::from_node_id and element_data when making
these changes.


/// Returns the underlying DOM [`NodeId`].
pub fn node_id(&self) -> NodeId {
self.0
Expand Down
1 change: 1 addition & 0 deletions crates/grida-canvas/examples/tool_io_grida.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,5 +465,6 @@ fn classify_node(node: &Node) -> &'static str {
Node::TextSpan(_) => "tspan",
Node::AttributedText(_) => "attributed_text",
Node::Error(_) => "error",
Node::Markdown(_) => "markdown",
}
}
1 change: 1 addition & 0 deletions crates/grida-canvas/examples/tool_io_svg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ fn classify_node(node: &Node) -> &'static str {
Node::AttributedText(_) => "attributed_text",
Node::Tray(_) => "tray",
Node::Error(_) => "error",
Node::Markdown(_) => "markdown",
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/grida-canvas/src/cache/compositor/promotion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ fn has_expensive_effects(layer: &PainterPictureLayer) -> bool {
PainterPictureLayer::Shape(shape) => &shape.effects,
PainterPictureLayer::Text(text) => &text.effects,
PainterPictureLayer::Vector(vec) => &vec.effects,
PainterPictureLayer::Markdown(md) => &md.effects,
};
effects.has_expensive_effects()
}
Expand All @@ -110,6 +111,7 @@ fn has_context_dependent_effects(layer: &PainterPictureLayer) -> bool {
PainterPictureLayer::Shape(shape) => &shape.effects,
PainterPictureLayer::Text(text) => &text.effects,
PainterPictureLayer::Vector(vec) => &vec.effects,
PainterPictureLayer::Markdown(md) => &md.effects,
};
effects.backdrop_blur.as_ref().is_some_and(|b| b.active)
|| effects.glass.as_ref().is_some_and(|g| g.active)
Expand Down
5 changes: 5 additions & 0 deletions crates/grida-canvas/src/cg/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2064,6 +2064,11 @@ impl AttributedStringBuilder {
self
}

/// Returns `true` if no text has been pushed yet.
pub fn is_empty(&self) -> bool {
self.text.is_empty()
}

/// Build the final [`AttributedString`].
///
/// Panics if no text was pushed.
Expand Down
Loading
Loading