Skip to content
3 changes: 2 additions & 1 deletion crates/grida-canvas/examples/golden_htmlcss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ fn fonts() -> FontRepository {

fn render_to_png(html: &str, width: f32, name: &str, out_dir: &Path) {
let fonts = fonts();
let picture = htmlcss::render(html, width, 600.0, &fonts).expect("render failed");
let picture =
htmlcss::render(html, width, 600.0, &fonts, &htmlcss::NoImages).expect("render failed");
let cull = picture.cull_rect();
let w = cull.width().max(1.0) as i32;
let h = cull.height().max(1.0) as i32;
Expand Down
11 changes: 8 additions & 3 deletions crates/grida-canvas/src/cache/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,9 +746,14 @@ fn resolve_layout(
h.max(MIN_SIZE)
} else {
let styled_html = crate::htmlcss::markdown_to_styled_html(&n.markdown);
crate::htmlcss::measure_content_height(&styled_html, width, fonts)
.unwrap_or(0.0)
.max(MIN_SIZE)
crate::htmlcss::measure_content_height(
&styled_html,
width,
fonts,
&crate::htmlcss::NoImages,
)
.unwrap_or(0.0)
.max(MIN_SIZE)
};
(geo.schema_transform, width, height)
} else {
Expand Down
258 changes: 223 additions & 35 deletions crates/grida-canvas/src/htmlcss/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,22 @@ fn collect_element_with_counter(

let is_void_widget = detect_widget(&tag, node_data, dom, &mut el);

// Extract object-fit from Stylo for replaced elements (<img>)
if el.replaced.is_some() {
use style::properties::longhands::object_fit::computed_value::T as StyloObjectFit;
let of = style.get_position().clone_object_fit();
let object_fit = match of {
StyloObjectFit::Fill => types::ObjectFit::Fill,
StyloObjectFit::Contain => types::ObjectFit::Contain,
StyloObjectFit::Cover => types::ObjectFit::Cover,
StyloObjectFit::None => types::ObjectFit::None,
StyloObjectFit::ScaleDown => types::ObjectFit::ScaleDown,
};
if let Some(ref mut replaced) = el.replaced {
replaced.object_fit = object_fit;
}
}

// Collect children, merging consecutive inline content into InlineGroups
let mut pending_inline: Vec<InlineRunItem> = Vec::new();
let parent_text_align = el.font.text_align;
Expand Down Expand Up @@ -294,7 +310,7 @@ fn collect_element_with_counter(
// for sizing to work — don't flatten them into inline groups.
let is_inline = child.display == types::Display::Inline
|| child.display == types::Display::InlineBlock;
if is_inline && !child.widget.is_widget() {
if is_inline && !child.widget.is_widget() && child.replaced.is_none() {
collect_inline_items(&child, &mut pending_inline);
Comment on lines +313 to 314
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Inline <img>s still don't have an inline layout path.

By skipping child.replaced here, default inline images become standalone StyledNode::Elements. layout.rs only keeps text/inlines inside InlineGroup, so foo <img> bar will turn into separate box children instead of sharing one line. You'll need a dedicated inline-replaced item before taking images out of the flattener.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/grida-canvas/src/htmlcss/collect.rs` around lines 313 - 314, The
current check in collect_inline_items skips nodes with child.replaced, causing
inline <img> to remain as StyledNode::Element and break InlineGroup assembly;
update collect_inline_items (or the caller branch that currently does "if
is_inline && !child.widget.is_widget() && child.replaced.is_none()") to treat
inline replaced elements specially: detect replaced inline elements (e.g., <img>
/ nodes where child.replaced.is_some() and they are inline-level) and create a
dedicated inline-replaced item and push it into pending_inline instead of
skipping, so layout.rs can consume them as part of InlineGroup rather than as
separate box children. Ensure the logic still skips replaced block-level widgets
but handles inline replaced elements as inline items.

} else {
flush_inline_group(
Expand Down Expand Up @@ -326,12 +342,44 @@ const PLACEHOLDER_COLOR: CGColor = CGColor {
a: 255,
};

/// Detect form control elements and populate `StyledElement::widget`.
// ─── Replaced element (<img>) detection ────────────────────────────

/// Extract `<img>` attributes into a `ReplacedContent`.
///
/// Follows the HTML spec for replaced elements:
/// - `src` — image URL
/// - `alt` — alternative text (for placeholder display)
/// - `width`/`height` — intrinsic size hints
fn detect_img_element(node: &DemoNode) -> ReplacedContent {
let src = get_element_attr(node, "src").unwrap_or_default();
let alt = get_element_attr(node, "alt");
let attr_width = get_element_attr(node, "width").and_then(|s| s.parse::<u32>().ok());
let attr_height = get_element_attr(node, "height").and_then(|s| s.parse::<u32>().ok());

ReplacedContent {
src,
alt,
attr_width,
attr_height,
object_fit: types::ObjectFit::Fill, // HTML spec default for <img>
}
}

// ─── Widget (form control) detection ────────────────────────────────

/// Returns `true` for void elements (like `<input>`) whose DOM children
/// should be skipped.
fn detect_widget(tag: &str, node_data: &DemoNode, dom: &DemoDom, el: &mut StyledElement) -> bool {
match tag {
"img" => {
el.replaced = Some(detect_img_element(node_data));
// <img> is a replaced inline element — force inline-block so it
// gets its own Taffy node (not merged into InlineGroup).
if el.display == types::Display::Inline {
el.display = types::Display::InlineBlock;
}
true // <img> is a void element
}
"input" => {
detect_input_widget(node_data, el);
true // <input> is a void element
Expand Down Expand Up @@ -835,6 +883,16 @@ fn extract_style(tag: &str, style: &ComputedValues) -> StyledElement {
};
}

// Box sizing (in Stylo, box-sizing is in the "position" property group)
{
let pos = style.get_position();
use style::properties::longhands::box_sizing::computed_value::T as StyloBoxSizing;
el.box_sizing = match pos.clone_box_sizing() {
StyloBoxSizing::ContentBox => types::BoxSizing::ContentBox,
StyloBoxSizing::BorderBox => types::BoxSizing::BorderBox,
};
}

// Margin (may be auto or %)
el.margin = extract_css_margin(style);

Expand All @@ -844,6 +902,9 @@ fn extract_style(tag: &str, style: &ComputedValues) -> StyledElement {
// Border
el.border = extract_border(style);

// Border image (9-slice)
el.border_image = extract_border_image(style);

// Border radius
el.border_radius = extract_border_radius(style);

Expand Down Expand Up @@ -1069,6 +1130,109 @@ fn extract_border(style: &ComputedValues) -> BorderBox {
}
}

/// Extract CSS `border-image` properties (Chromium: NinePieceImage).
///
/// Returns `Some(BorderImage)` when `border-image-source` is set to a
/// non-none value. The source image URL is extracted the same way as
/// `background-image: url()` — via `GenericImage::Url` / `ComputedUrl`.
fn extract_border_image(style: &ComputedValues) -> Option<BorderImage> {
let b = style.get_border();

let source = convert_image(&b.border_image_source)?;

// border-image-slice: BorderImageSlice { offsets: Rect<NonNegative<NumberOrPercentage>>, fill }
let slice_computed = &b.border_image_slice;
let s = &slice_computed.offsets;
let resolve_nop = |v: &style::values::computed::NonNegativeNumberOrPercentage| -> f32 {
use style::values::computed::NumberOrPercentage;
match &v.0 {
NumberOrPercentage::Number(n) => *n,
NumberOrPercentage::Percentage(p) => p.0 * 100.0,
}
};
let slice = EdgeInsets {
top: resolve_nop(&s.0),
right: resolve_nop(&s.1),
bottom: resolve_nop(&s.2),
left: resolve_nop(&s.3),
};

// border-image-outset: Rect<NonNegativeLengthOrNumber>
let o = &b.border_image_outset;
let resolve_lon = |v: &style::values::computed::NonNegativeLengthOrNumber| -> f32 {
use style::values::generics::length::GenericLengthOrNumber;
match v {
GenericLengthOrNumber::Number(n) => n.0,
GenericLengthOrNumber::Length(lp) => lp.0.px(),
}
};
let outset = EdgeInsets {
top: resolve_lon(&o.0),
right: resolve_lon(&o.1),
bottom: resolve_lon(&o.2),
left: resolve_lon(&o.3),
};

// border-image-repeat: (keyword_x, keyword_y)
let repeat = &b.border_image_repeat;
let map_repeat =
|kw: &style::values::specified::border::BorderImageRepeatKeyword| -> types::BorderImageRepeat {
use style::values::specified::border::BorderImageRepeatKeyword as BIR;
match kw {
BIR::Stretch => types::BorderImageRepeat::Stretch,
BIR::Repeat => types::BorderImageRepeat::Repeat,
BIR::Round => types::BorderImageRepeat::Round,
BIR::Space => types::BorderImageRepeat::Space,
}
};

// border-image-width: Rect<BorderImageSideWidth>
// Number(n) is a multiplier of the corresponding border-width.
// LengthPercentage is an absolute value. Auto = use slice value.
let biw = &b.border_image_width;
let border_widths = [
b.border_top_width.to_f32_px(),
b.border_right_width.to_f32_px(),
b.border_bottom_width.to_f32_px(),
b.border_left_width.to_f32_px(),
];
let resolve_bisw =
|v: &style::values::computed::BorderImageSideWidth, border_w: f32| -> Option<f32> {
use style::values::generics::border::BorderImageSideWidth as BISW;
match v {
BISW::Number(n) => Some(n.0 * border_w),
BISW::LengthPercentage(lp) => Some(lp.0.to_length().map(|l| l.px()).unwrap_or(0.0)),
BISW::Auto => None,
}
};
let width = {
let t = resolve_bisw(&biw.0, border_widths[0]);
let r = resolve_bisw(&biw.1, border_widths[1]);
let bv = resolve_bisw(&biw.2, border_widths[2]);
let l = resolve_bisw(&biw.3, border_widths[3]);
if t.is_some() || r.is_some() || bv.is_some() || l.is_some() {
Some(EdgeInsets {
top: t.unwrap_or(0.0),
right: r.unwrap_or(0.0),
bottom: bv.unwrap_or(0.0),
left: l.unwrap_or(0.0),
})
} else {
None
}
};
Comment on lines +1208 to +1223
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

border-image-width: auto fallback may produce incorrect values.

When border-image-width has auto for some sides, the code sets those to 0.0. Per CSS spec, auto should fall back to the corresponding border-image-slice value. Currently, if only some sides are explicit, the auto sides become 0, which would produce no border rendering on those edges.

Consider either:

  1. Returning None for the whole width when any side is auto (let paint-time handle fallback)
  2. Substituting the slice value for auto sides here
🔧 Suggested fix (Option 1: keep None semantics consistent)
     let width = {
         let t = resolve_bisw(&biw.0, border_widths[0]);
         let r = resolve_bisw(&biw.1, border_widths[1]);
         let bv = resolve_bisw(&biw.2, border_widths[2]);
         let l = resolve_bisw(&biw.3, border_widths[3]);
-        if t.is_some() || r.is_some() || bv.is_some() || l.is_some() {
+        // Only produce explicit widths if ALL sides are specified (no auto)
+        if t.is_some() && r.is_some() && bv.is_some() && l.is_some() {
             Some(EdgeInsets {
-                top: t.unwrap_or(0.0),
-                right: r.unwrap_or(0.0),
-                bottom: bv.unwrap_or(0.0),
-                left: l.unwrap_or(0.0),
+                top: t.unwrap(),
+                right: r.unwrap(),
+                bottom: bv.unwrap(),
+                left: l.unwrap(),
             })
         } else {
             None
         }
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/grida-canvas/src/htmlcss/collect.rs` around lines 1208 - 1223, The
current code treats any unresolved (auto) side from resolve_bisw as 0.0 via
unwrap_or(0.0), but per the review we should not default auto to 0. Implement
Option 1: only construct and return Some(EdgeInsets) when all four sides are
Some; if any of t, r, bv, or l is None return None so paint-time can apply the
border-image-slice fallback. Modify the block that uses resolve_bisw on biw.0..3
(and border_widths) to check t.is_some() && r.is_some() && bv.is_some() &&
l.is_some() before calling unwrap on each to build EdgeInsets; otherwise set
width to None.


Some(BorderImage {
source,
slice,
fill: slice_computed.fill,
width,
outset,
repeat_x: map_repeat(&repeat.0),
repeat_y: map_repeat(&repeat.1),
})
}

/// Extract CSS `outline` properties.
///
/// Chromium: `ComputedStyle::OutlineWidth()`, `OutlineColor()`,
Expand Down Expand Up @@ -1108,6 +1272,60 @@ fn extract_outline(style: &ComputedValues) -> Outline {
}
}

/// Convert a Stylo `GenericImage` to our `StyleImage`.
///
/// Shared by `extract_background` and `extract_border_image` — both need
/// the same URL/gradient conversion from Stylo's computed image type.
fn convert_image(image: &style::values::computed::Image) -> Option<StyleImage> {
use style::values::computed::url::ComputedUrl;
use style::values::generics::image::{GenericGradient, GenericImage};

match image {
GenericImage::None => None,
GenericImage::Url(computed_url) => {
let url_str = match computed_url {
ComputedUrl::Valid(url) => url.as_str().to_string(),
ComputedUrl::Invalid(s) => s.to_string(),
};
if url_str.is_empty() {
None
} else {
Some(StyleImage::Url(url_str))
}
}
GenericImage::Gradient(gradient) => match gradient.as_ref() {
GenericGradient::Linear {
direction, items, ..
} => {
let stops = gradient_items_to_stops(items);
if stops.is_empty() {
return None;
}
let angle_deg = extract_gradient_angle(direction);
Some(StyleImage::LinearGradient(LinearGradient {
angle_deg,
stops,
}))
}
GenericGradient::Radial { items, .. } => {
let stops = gradient_items_to_stops(items);
if stops.is_empty() {
return None;
}
Some(StyleImage::RadialGradient(RadialGradient { stops }))
}
GenericGradient::Conic { items, .. } => {
let stops = conic_gradient_items_to_stops(items);
if stops.is_empty() {
return None;
}
Some(StyleImage::ConicGradient(ConicGradient { stops }))
}
},
_ => None,
}
}

fn extract_border_radius(style: &ComputedValues) -> CornerRadii {
let b = style.get_border();
let lp = |lp: &style::values::computed::NonNegativeLengthPercentage| -> f32 {
Expand Down Expand Up @@ -1168,8 +1386,6 @@ fn extract_inset(_style: &ComputedValues) -> CssEdgeInsets {
}

fn extract_background(style: &ComputedValues) -> Vec<BackgroundLayer> {
use style::values::generics::image::{GenericGradient, GenericImage};

let bg = style.get_background();
let mut layers: Vec<BackgroundLayer> = Vec::new();

Expand All @@ -1181,38 +1397,10 @@ fn extract_background(style: &ComputedValues) -> Vec<BackgroundLayer> {
}
}

// 2. Background image layers (gradients on top)
// 2. Background image layers (gradients and URL images on top)
for image in bg.background_image.0.iter() {
if let GenericImage::Gradient(gradient) = image {
match gradient.as_ref() {
GenericGradient::Linear {
direction, items, ..
} => {
let stops = gradient_items_to_stops(items);
if stops.is_empty() {
continue;
}
let angle_deg = extract_gradient_angle(direction);
layers.push(BackgroundLayer::LinearGradient(LinearGradient {
angle_deg,
stops,
}));
}
GenericGradient::Radial { items, .. } => {
let stops = gradient_items_to_stops(items);
if stops.is_empty() {
continue;
}
layers.push(BackgroundLayer::RadialGradient(RadialGradient { stops }));
}
GenericGradient::Conic { items, .. } => {
let stops = conic_gradient_items_to_stops(items);
if stops.is_empty() {
continue;
}
layers.push(BackgroundLayer::ConicGradient(ConicGradient { stops }));
}
}
if let Some(style_image) = convert_image(image) {
layers.push(BackgroundLayer::Image(style_image));
}
}

Expand Down
Loading
Loading