From 38a48c74883d4bac2be618e74e8f376064cfd74c Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sun, 27 Sep 2026 00:29:25 +0200 Subject: [PATCH] fix(paint): wire style.clip-path into the paint pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CssStyle::clip_path` was declared, exported in the JSON Schema with seven variants and their documentation, and read by nothing. A scenario that set it validated, rendered, and looked exactly as though the property were absent. The only three `clip_path` call sites in the tree were Skia's own `Canvas::clip_path`, unrelated to the CSS property. Present since 676e49e. Five variants now build a Skia path and clip with it: `Inset` (with optional corner radius), `Circle`, `Ellipse`, `Polygon` and `Path`. `None` stays indistinguishable from the property being absent. Percentages resolve against the border box, per axis: `inset`'s verticals on the height and horizontals on the width, `ellipse`'s `rx` on the width and `ry` on the height, `polygon`'s `x`/`y` likewise. `circle`'s radius uses the CSS reference, `sqrt(w² + h²) / √2`, rather than a convenient `min(w, h)` — a 50% circle on a non-square box should match what a browser draws. `path`'s data is offset by the box's own origin, so `M0 0` is the element's corner and not the video's. The clip opens before the outset shadows and closes after the inset ones, so the element's whole rendering is masked — background, border and shadow included. That is CSS `clip-path`, and it is deliberately the opposite of the placement `overflow: hidden` uses here: that one opens after the decorations, sparing the node's own outset shadow, which an existing test pins. `NodePath { id }` is left unimplemented: reading another node's geometry needs an `id → resolved path` lookup the paint pass does not have, and #339 itself separates that variant from the other five for this reason. It is not left silent, though — a silent no-op is the defect being fixed here. It writes to stderr and says what to do instead. Tracked separately. Seven tests. Six fail against the unwired property, including the sample the issue reports: a square with `clip-path: inset{right: 120}` painted at x=330 just as it did at x=200. End to end on a rendered still, x=330 goes from rgb(139, 92, 246) to rgb(0, 0, 0) while x=200 stays violet. `rules/clip-path.md` documents the six shapes, the local-coordinates trap on `path`, the eight-point polygon for a chamfered frame, and that `node-path` is declared but inert. Before this the property's only mention in the skills was a line in `timeline-sequencing.md` saying it cannot be animated, while the exported schema advertised it globally — which is what made it read as working. Closes #339 --- .../rustmotion-core/src/engine/paint_pass.rs | 294 +++++++++++++++++- crates/rustmotion/skills/SKILL.md | 2 + crates/rustmotion/skills/rules/clip-path.md | 53 ++++ 3 files changed, 345 insertions(+), 4 deletions(-) create mode 100644 crates/rustmotion/skills/rules/clip-path.md diff --git a/crates/rustmotion-core/src/engine/paint_pass.rs b/crates/rustmotion-core/src/engine/paint_pass.rs index c6abfb1..1b82711 100644 --- a/crates/rustmotion-core/src/engine/paint_pass.rs +++ b/crates/rustmotion-core/src/engine/paint_pass.rs @@ -7,8 +7,8 @@ use skia_safe::{ }; use crate::css::style::{ - Background, BackgroundLayer, BorderEdges, BorderRadius, BorderStyle, BoxShadow, Color, - CssStyle, Edges, Overflow, TransformFn, TransformOrigin, + Background, BackgroundLayer, BorderEdges, BorderRadius, BorderStyle, BoxShadow, ClipPath, + Color, CssStyle, Edges, Overflow, TransformFn, TransformOrigin, }; use crate::css::units::{parse_origin_component, LengthContext, LengthPercentage, ParsedLength}; use crate::engine::box_tree::{BoxKind, BoxNode, NodeId}; @@ -308,6 +308,18 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u false }; + let opened_clip_path = match node.css.clip_path.as_ref() { + Some(clip) => match clip_path_to_skia(clip, box_layout, &length_ctx) { + Some(path) => { + canvas.save(); + canvas.clip_path(&path, ClipOp::Intersect, true); + true + } + None => false, + }, + None => false, + }; + if let Some(shadows) = node.css.box_shadow.as_ref() { for shadow in shadows { if shadow.inset.unwrap_or(false) { @@ -391,6 +403,9 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u if opened_shimmer_layer { canvas.restore(); } + if opened_clip_path { + canvas.restore(); + } if opened_opacity_layer { canvas.restore(); @@ -1247,6 +1262,113 @@ fn paint_gradient_border( canvas.draw_drrect(outer, inner, &paint); } +fn clip_path_to_skia( + clip: &ClipPath, + layout: &BoxLayout, + ctx: &LengthContext, +) -> Option { + let ctx_w = LengthContext { + parent_size: layout.width, + ..*ctx + }; + let ctx_h = LengthContext { + parent_size: layout.height, + ..*ctx + }; + + match clip { + ClipPath::None => None, + + ClipPath::Inset { + top, + right, + bottom, + left, + radius, + } => { + let t = top.resolve(&ctx_h); + let r = right.resolve(&ctx_w); + let b = bottom.resolve(&ctx_h); + let l = left.resolve(&ctx_w); + let width = (layout.width - l - r).max(0.0); + let height = (layout.height - t - b).max(0.0); + let rect = Rect::from_xywh(layout.x + l, layout.y + t, width, height); + let corners = radius + .as_ref() + .map(|br| resolve_border_radius(br, layout, ctx)) + .unwrap_or([0.0; 4]); + let mut builder = PathBuilder::new(); + builder.add_rrect(rrect_from_corners(rect, corners), None, None); + Some(builder.detach()) + } + + ClipPath::Circle { radius, origin } => { + let (cx, cy, _) = resolve_origin(origin.as_ref(), layout, ctx); + let reference = LengthContext { + parent_size: (layout.width.powi(2) + layout.height.powi(2)).sqrt() + / std::f32::consts::SQRT_2, + ..*ctx + }; + let r = radius.resolve(&reference); + if r <= 0.0 { + return Some(PathBuilder::new().detach()); + } + let mut builder = PathBuilder::new(); + builder.add_circle((cx, cy), r, None); + Some(builder.detach()) + } + + ClipPath::Ellipse { rx, ry, origin } => { + let (cx, cy, _) = resolve_origin(origin.as_ref(), layout, ctx); + let a = rx.resolve(&ctx_w); + let b = ry.resolve(&ctx_h); + if a <= 0.0 || b <= 0.0 { + return Some(PathBuilder::new().detach()); + } + let mut builder = PathBuilder::new(); + builder.add_oval( + Rect::from_xywh(cx - a, cy - b, a * 2.0, b * 2.0), + None, + None, + ); + Some(builder.detach()) + } + + ClipPath::Polygon { points } => { + if points.len() < 3 { + return Some(PathBuilder::new().detach()); + } + let mut builder = PathBuilder::new(); + for (i, (px, py)) in points.iter().enumerate() { + let x = layout.x + px.resolve(&ctx_w); + let y = layout.y + py.resolve(&ctx_h); + if i == 0 { + builder.move_to((x, y)); + } else { + builder.line_to((x, y)); + } + } + builder.close(); + Some(builder.detach()) + } + + ClipPath::Path { d } => { + let parsed = skia_safe::Path::from_svg(d)?; + Some(parsed.with_offset((layout.x, layout.y))) + } + + ClipPath::NodePath { id } => { + eprintln!( + "rustmotion: clip-path {{ kind: node-path, id: \"{id}\" }} is not implemented \ + yet — reading another node's geometry needs a resolved-path lookup the paint \ + pass does not have. Nothing is clipped. Use kind: path with the same data, or \ + follow the tracking issue." + ); + None + } + } +} + fn border_rrect(layout: &BoxLayout, radius: [f32; 4]) -> RRect { let rect = Rect::from_xywh(layout.x, layout.y, layout.width, layout.height); rrect_from_corners(rect, radius) @@ -2510,8 +2632,8 @@ mod paint_order_tests { use super::*; use crate::css::style::{ - Background, BoxShadow, Color as CssColor, CssStyle, Display, FilterFn, FlexDirection, - Overflow, Position, Size as CSize, + Background, BoxShadow, ClipPath, Color as CssColor, CssStyle, Display, FilterFn, + FlexDirection, Overflow, Position, Size as CSize, }; use crate::css::taffy_bridge::ConversionContext; use crate::css::units::{Length, LengthPercentage as CLP}; @@ -2585,6 +2707,170 @@ mod paint_order_tests { n } + fn clipped_square(clip: Option) -> BoxNode { + BoxNode { + id: 0, + kind: BoxKind::Container, + css: CssStyle { + position: Some(Position::Absolute), + left: Some(CLP::Px(0.0)), + top: Some(CLP::Px(0.0)), + width: Some(CSize::Length(CLP::Px(400.0))), + height: Some(CSize::Length(CLP::Px(400.0))), + background: Some(Background::Color(CssColor::String("#ff0000".into()))), + clip_path: clip, + ..Default::default() + }, + children: vec![], + intrinsic: None, + source_path: None, + window: None, + } + } + + fn is_red_at(clip: Option, x: u32, y: u32) -> bool { + let mut root = root_node(400.0, 400.0, "#000000", vec![clipped_square(clip)]); + let buf = render_pixels(&mut root, 400, 400); + let i = ((y * 400 + x) * 4) as usize; + buf[i] > 200 && buf[i + 1] < 50 && buf[i + 2] < 50 + } + + #[test] + fn clip_path_inset_removes_the_region_outside_it() { + assert!( + is_red_at(None, 350, 200), + "sanity: unclipped, x=350 is inside the square" + ); + let inset = ClipPath::Inset { + top: CLP::Px(0.0), + right: CLP::Px(120.0), + bottom: CLP::Px(0.0), + left: CLP::Px(0.0), + radius: None, + }; + assert!( + is_red_at(Some(inset.clone()), 200, 200), + "inside the inset box must still paint" + ); + assert!( + !is_red_at(Some(inset), 330, 200), + "inset right:120 must remove x=330 — this is the sample from the issue that \ + stayed painted while clip-path was read by nothing" + ); + } + + #[test] + fn clip_path_circle_keeps_the_centre_and_drops_the_corner() { + let circle = ClipPath::Circle { + radius: CLP::Px(100.0), + origin: None, + }; + assert!( + is_red_at(Some(circle.clone()), 200, 200), + "the centre is inside a centred r=100 circle" + ); + assert!( + !is_red_at(Some(circle), 20, 20), + "the top-left corner is outside a centred r=100 circle" + ); + } + + #[test] + fn clip_path_ellipse_is_wider_than_it_is_tall() { + let ellipse = ClipPath::Ellipse { + rx: CLP::Px(180.0), + ry: CLP::Px(40.0), + origin: None, + }; + assert!( + is_red_at(Some(ellipse.clone()), 360, 200), + "x=360 is within rx=180 of the centre" + ); + assert!( + !is_red_at(Some(ellipse), 200, 360), + "y=360 is outside ry=40 of the centre" + ); + } + + #[test] + fn clip_path_polygon_cuts_a_triangle() { + let triangle = ClipPath::Polygon { + points: vec![ + (CLP::Px(200.0), CLP::Px(0.0)), + (CLP::Px(400.0), CLP::Px(400.0)), + (CLP::Px(0.0), CLP::Px(400.0)), + ], + }; + assert!( + is_red_at(Some(triangle.clone()), 200, 300), + "low centre is inside the triangle" + ); + assert!( + !is_red_at(Some(triangle), 20, 20), + "the top-left corner is outside the triangle" + ); + } + + #[test] + fn clip_path_path_takes_svg_data_relative_to_the_box() { + let left_half = ClipPath::Path { + d: "M0 0 L200 0 L200 400 L0 400 Z".to_string(), + }; + assert!( + is_red_at(Some(left_half.clone()), 100, 200), + "the left half stays" + ); + assert!( + !is_red_at(Some(left_half), 300, 200), + "the right half is clipped away" + ); + } + + #[test] + fn clip_path_none_paints_exactly_as_no_clip_path_at_all() { + for (x, y) in [(20, 20), (200, 200), (380, 380)] { + assert_eq!( + is_red_at(Some(ClipPath::None), x, y), + is_red_at(None, x, y), + "clip-path: none must be indistinguishable from absent at ({x}, {y})" + ); + } + } + + #[test] + fn clip_path_clips_the_background_and_the_outset_shadow_too() { + let mut node = clipped_square(Some(ClipPath::Inset { + top: CLP::Px(0.0), + right: CLP::Px(200.0), + bottom: CLP::Px(0.0), + left: CLP::Px(0.0), + radius: None, + })); + node.css.width = Some(CSize::Length(CLP::Px(200.0))); + node.css.height = Some(CSize::Length(CLP::Px(200.0))); + node.css.left = Some(CLP::Px(100.0)); + node.css.top = Some(CLP::Px(100.0)); + node.css.background = Some(Background::Color(CssColor::String("#ffffff".into()))); + node.css.box_shadow = Some(vec![BoxShadow { + offset_x: Length::Px(0.0), + offset_y: Length::Px(0.0), + blur: None, + spread: Some(Length::Px(40.0)), + color: Some(CssColor::String("#ff0000".into())), + inset: None, + }]); + + let mut root = root_node(400.0, 400.0, "#000000", vec![node]); + let buf = render_pixels(&mut root, 400, 400); + + let right_of_the_cut = count_red_in(&buf, 400, 260, 100, 400, 300); + assert_eq!( + right_of_the_cut, 0, + "clip-path clips the element itself, shadow included — unlike overflow:hidden, \ + which clips only the content and deliberately spares the node's own outset shadow" + ); + } + fn card_with_shadow(overflow_hidden: bool) -> BoxNode { let mut css = CssStyle { position: Some(Position::Absolute), diff --git a/crates/rustmotion/skills/SKILL.md b/crates/rustmotion/skills/SKILL.md index a56d4ad..6f71b6a 100644 --- a/crates/rustmotion/skills/SKILL.md +++ b/crates/rustmotion/skills/SKILL.md @@ -234,6 +234,7 @@ Read individual rule files for detailed explanations, GOOD/BAD examples, and con - [rules/html-css-mental-model.md](rules/html-css-mental-model.md) - **CRITICAL:** Think HTML/CSS — flow layout first, absolute only for decorative/overlay elements - [rules/validate-json.md](rules/validate-json.md) - Always validate generated JSON with `rustmotion validate` before presenting - [rules/geometry-safety.md](rules/geometry-safety.md) - Keep all content inside the viewport: `white-space`, `auto_scroll`, `overflow` semantics + violation kinds +- [rules/clip-path.md](rules/clip-path.md) - Non-rectangular masking: the six `clip-path` shapes, how their percentages resolve, and why `node-path` is not one of them yet - [rules/even-dimensions.md](rules/even-dimensions.md) - Use even width/height for H.264 encoding - [rules/composition-recipes.md](rules/composition-recipes.md) - **Read this before reaching for a UI-widget component.** Composing KPI cards, pill rows, progress bars, and other former "frozen composition" shapes from primitives, `components`, and `for-each` - [rules/templates-and-iteration.md](rules/templates-and-iteration.md) - `for-each`/`components`/`use` mechanics: bindings, param defaults, ordering of passes, named errors @@ -935,6 +936,7 @@ All components are discriminated by `"type"`. Rendered in array order (first = b | `letter-spacing` | f32 | `null` | | `white-space` | enum | unset (wraps) — set `"nowrap"`/`"pre"` for single-line text. There is no `wrap` field. The validator emits `unwrappable_text_overflow` if the natural width exceeds the box. See [rules/geometry-safety.md](rules/geometry-safety.md). | | `overflow` | enum | `"visible"` — CSS-like: `"visible"` (default, children may bleed) or `"hidden"` (clip at the box). Validator only checks the **viewport**, never a `visible` parent. | +| `clip-path` | object | Non-rectangular mask on the element itself, background/border/outer-shadow included. `kind`: `inset`, `circle`, `ellipse`, `polygon`, `path`, `none`. A chamfered frame is an eight-point `polygon`. `kind: node-path` is declared but **not implemented** and warns on stderr. See [rules/clip-path.md](rules/clip-path.md). | **Per-character / per-word animation (char animation presets):** diff --git a/crates/rustmotion/skills/rules/clip-path.md b/crates/rustmotion/skills/rules/clip-path.md new file mode 100644 index 0000000..56a305a --- /dev/null +++ b/crates/rustmotion/skills/rules/clip-path.md @@ -0,0 +1,53 @@ +# `style.clip-path` — masquage non rectangulaire + +`overflow: hidden` découpe au rectangle du parent. `clip-path` découpe l'élément +**lui-même** à une forme arbitraire — et contrairement à `overflow`, il emporte le +fond, la bordure et l'ombre externe avec lui, comme en CSS. + +Six formes, toutes résolues contre la **border box** du nœud : + +| `kind` | Champs | Résolution des `%` | +|---|---|---| +| `none` | — | identique à l'absence de la propriété | +| `inset` | `top` `right` `bottom` `left`, `radius` optionnel | verticaux sur la hauteur, horizontaux sur la largeur | +| `circle` | `radius`, `origin` optionnel | `sqrt(w² + h²) / √2`, la référence CSS | +| `ellipse` | `rx` `ry`, `origin` optionnel | `rx` sur la largeur, `ry` sur la hauteur | +| `polygon` | `points`: `[[x, y], …]` | `x` sur la largeur, `y` sur la hauteur | +| `path` | `d`: données de chemin SVG | coordonnées relatives au coin haut-gauche de la boîte | + +`origin` prend la même forme que `transform-origin` et vaut le centre par défaut. + +```json +{ + "type": "div", + "style": { + "width": 400, "height": 400, "background": "#8B5CF6", + "clip-path": { "kind": "polygon", + "points": [[200, 0], [400, 400], [0, 400]] } + } +} +``` + +Un cadre chanfreiné — les quatre coins coupés en diagonale — s'écrit en `polygon` +à huit points. C'est l'usage pour lequel la propriété a été câblée. + +## Deux pièges + +**`path` est en coordonnées locales.** Les données sont décalées du coin +haut-gauche de la boîte, pas du coin de la vidéo : un `M0 0` commence à l'angle de +l'élément. Un chemin copié depuis un éditeur SVG dont le viewBox ne commence pas à +l'origine arrivera décalé. + +**`kind: node-path` n'est pas implémenté.** La variante existe au schéma — elle +désigne un autre nœud par `id` pour en reprendre la géométrie — mais rien ne la +résout encore : le pass de peinture n'a pas de table `id → chemin résolu`. Elle +n'échoue pas silencieusement, elle **écrit sur stderr** et ne découpe rien. Pour +un masque partagé entre deux nœuds, répète le même `kind: path` sur les deux en +attendant. + +## Ce que `clip-path` ne fait pas + +Il ne s'interpole pas dans un `timeline`. C'est une propriété de peinture non +supportée à l'animation — voir [timeline-sequencing.md](timeline-sequencing.md). +Pour une révélation progressive, animer un `transform` sous un parent +`overflow: hidden` reste la voie.