🐛 Fix nested fills for shapes with svg attrs

frontend/src/app/render_wasm/api.cljs

@@ -777,8 +777,19 @@
         rotation     (get shape :rotation)
         transform    (get shape :transform)
 
-        ;; Groups from imported SVG's can have their own fills
+        ;; If the shape comes from an imported SVG (we know this because
-        fills        (get shape :fills)
+        ;; it has the :svg-attrs attribute) and it does not have its
+        ;; own fill, we set a default black fill. This fill will be
+        ;; inherited by child nodes and emulates the behavior of
+        ;; standard SVG, where a node without an explicit fill
+        ;; defaults to black.
+        fills        (let [base-fills (get shape :fills)]
+                       (if (and ^boolean (contains? shape :svg-attrs)
+                                ^boolean (or ^boolean (= :svg-raw type)
+                                             ^boolean (= :group type))
+                                ^boolean (empty? base-fills))
+                         [{:fill-color "#000000" :fill-opacity 1}]
+                         base-fills))
 
         strokes      (if (= type :group)
                        [] (get shape :strokes))
@@ -815,7 +826,7 @@
     (when (and (some? content)
                (or (= type :path)
                    (= type :bool)))
-      (when (seq svg-attrs)
+      (when (some? svg-attrs)
         (set-shape-path-attrs svg-attrs))
       (set-shape-path-content content))
     (when (and (some? content) (= type :svg-raw))

render-wasm/src/render.rs

@@ -248,6 +248,7 @@ pub(crate) struct RenderState {
     // without their own fill definitions. This is necessary because in SVG, a group's `fill`
     // can affect its child elements if they don't specify one themselves. If the planned
     // migration to remove group-level fills is completed, this code should be removed.
+    // Frames contained in groups must reset this nested_fills stack pushing a new empty vector.
     pub nested_fills: Vec<Vec<Fill>>,
     pub nested_blurs: Vec<Option<Blur>>, // FIXME: why is this an option?
     pub show_grid: Option<Uuid>,
@@ -766,7 +767,11 @@ impl RenderState {
 
                 if shape.fills.is_empty()
                     && !matches!(shape.shape_type, Type::Group(_))
-                    && !shape.svg_attrs.fill_none
+                    && !matches!(shape.shape_type, Type::Frame(_))
+                    && !shape
+                        .svg_attrs
+                        .as_ref()
+                        .is_some_and(|attrs| attrs.fill_none)
                 {
                     if let Some(fills_to_render) = self.nested_fills.last() {
                         let fills_to_render = fills_to_render.clone();
@@ -994,6 +999,10 @@ impl RenderState {
             }
         }
 
+        if let Type::Frame(_) = element.shape_type {
+            self.nested_fills.push(Vec::new());
+        }
+
         let mut paint = skia::Paint::default();
         paint.set_blend_mode(element.blend_mode().into());
         paint.set_alpha_f(element.opacity());
@@ -1065,12 +1074,10 @@ impl RenderState {
                 }
             }
         }
-        if let Type::Group(_) = element.shape_type {
-            self.nested_fills.pop();
-        }
 
         match element.shape_type {
             Type::Frame(_) | Type::Group(_) => {
+                self.nested_fills.pop();
                 self.nested_blurs.pop();
             }
             _ => {}

render-wasm/src/render/strokes.rs

@@ -17,7 +17,7 @@ fn draw_stroke_on_rect(
     rect: &Rect,
     selrect: &Rect,
     corners: &Option<Corners>,
-    svg_attrs: &SvgAttrs,
+    svg_attrs: Option<&SvgAttrs>,
     scale: f32,
     shadow: Option<&ImageFilter>,
     blur: Option<&ImageFilter>,
@@ -53,7 +53,7 @@ fn draw_stroke_on_circle(
     stroke: &Stroke,
     rect: &Rect,
     selrect: &Rect,
-    svg_attrs: &SvgAttrs,
+    svg_attrs: Option<&SvgAttrs>,
     scale: f32,
     shadow: Option<&ImageFilter>,
     blur: Option<&ImageFilter>,
@@ -130,7 +130,7 @@ pub fn draw_stroke_on_path(
     path: &Path,
     selrect: &Rect,
     path_transform: Option<&Matrix>,
-    svg_attrs: &SvgAttrs,
+    svg_attrs: Option<&SvgAttrs>,
     scale: f32,
     shadow: Option<&ImageFilter>,
     blur: Option<&ImageFilter>,
@@ -217,7 +217,7 @@ fn handle_stroke_caps(
     selrect: &Rect,
     canvas: &skia::Canvas,
     is_open: bool,
-    svg_attrs: &SvgAttrs,
+    svg_attrs: Option<&SvgAttrs>,
     scale: f32,
     blur: Option<&ImageFilter>,
     antialias: bool,
@@ -389,7 +389,7 @@ fn draw_image_stroke_in_container(
     let canvas = render_state.surfaces.canvas(SurfaceId::Strokes);
     let container = &shape.selrect;
     let path_transform = shape.to_path_transform();
-    let svg_attrs = &shape.svg_attrs;
+    let svg_attrs = shape.svg_attrs.as_ref();
 
     // Save canvas and layer state
     let mut pb = skia::Paint::default();
@@ -529,7 +529,7 @@ pub fn render(
         .canvas(surface_id.unwrap_or(surface_id.unwrap_or(SurfaceId::Strokes)));
     let selrect = shape.selrect;
     let path_transform = shape.to_path_transform();
-    let svg_attrs = &shape.svg_attrs;
+    let svg_attrs = shape.svg_attrs.as_ref();
 
     if !matches!(shape.shape_type, Type::Text(_))
         && shadow.is_none()
@@ -603,7 +603,7 @@ pub fn render_text_paths(
         .surfaces
         .canvas(surface_id.unwrap_or(SurfaceId::Strokes));
     let selrect = &shape.selrect;
-    let svg_attrs = &shape.svg_attrs;
+    let svg_attrs = shape.svg_attrs.as_ref();
     let mut paint: skia_safe::Handle<_> =
         stroke.to_text_stroked_paint(false, selrect, svg_attrs, scale, antialias);
 

render-wasm/src/shapes.rs

@@ -176,7 +176,7 @@ pub struct Shape {
     pub opacity: f32,
     pub hidden: bool,
     pub svg: Option<skia::svg::Dom>,
-    pub svg_attrs: SvgAttrs,
+    pub svg_attrs: Option<SvgAttrs>,
     pub shadows: Vec<Shadow>,
     pub layout_item: Option<LayoutItem>,
     pub extrect: OnceCell<math::Rect>,
@@ -203,7 +203,7 @@ impl Shape {
             hidden: false,
             blur: None,
             svg: None,
-            svg_attrs: SvgAttrs::default(),
+            svg_attrs: None,
             shadows: Vec::with_capacity(1),
             layout_item: None,
             extrect: OnceCell::new(),
@@ -1093,8 +1093,10 @@ impl Shape {
             if let Some(path_transform) = self.to_path_transform() {
                 skia_path.transform(&path_transform);
             }
-            if self.svg_attrs.fill_rule == FillRule::Evenodd {
+            if let Some(svg_attrs) = &self.svg_attrs {
-                skia_path.set_fill_type(skia::PathFillType::EvenOdd);
+                if svg_attrs.fill_rule == FillRule::Evenodd {
+                    skia_path.set_fill_type(skia::PathFillType::EvenOdd);
+                }
             }
             Some(skia_path)
         } else {

render-wasm/src/shapes/strokes.rs

@@ -161,7 +161,7 @@ impl Stroke {
     pub fn to_paint(
         &self,
         rect: &Rect,
-        svg_attrs: &SvgAttrs,
+        svg_attrs: Option<&SvgAttrs>,
         scale: f32,
         antialias: bool,
     ) -> skia::Paint {
@@ -177,12 +177,14 @@ impl Stroke {
         paint.set_stroke_width(width);
         paint.set_anti_alias(antialias);
 
-        if svg_attrs.stroke_linecap == StrokeLineCap::Round {
+        if let Some(svg_attrs) = svg_attrs {
-            paint.set_stroke_cap(skia::paint::Cap::Round);
+            if svg_attrs.stroke_linecap == StrokeLineCap::Round {
-        }
+                paint.set_stroke_cap(skia::paint::Cap::Round);
+            }
 
-        if svg_attrs.stroke_linejoin == StrokeLineJoin::Round {
+            if svg_attrs.stroke_linejoin == StrokeLineJoin::Round {
-            paint.set_stroke_join(skia::paint::Join::Round);
+                paint.set_stroke_join(skia::paint::Join::Round);
+            }
         }
 
         if self.style != StrokeStyle::Solid {
@@ -227,7 +229,7 @@ impl Stroke {
         &self,
         is_open: bool,
         rect: &Rect,
-        svg_attrs: &SvgAttrs,
+        svg_attrs: Option<&SvgAttrs>,
         scale: f32,
         antialias: bool,
     ) -> skia::Paint {
@@ -251,7 +253,7 @@ impl Stroke {
         &self,
         is_open: bool,
         rect: &Rect,
-        svg_attrs: &SvgAttrs,
+        svg_attrs: Option<&SvgAttrs>,
         scale: f32,
         antialias: bool,
     ) -> skia::Paint {

render-wasm/src/shapes/svg_attrs.rs

@@ -1,3 +1,5 @@
+use crate::wasm::svg_attrs::{RawFillRule, RawStrokeLineCap, RawStrokeLineJoin};
+
 #[derive(Debug, Clone, PartialEq, Copy, Default)]
 pub enum FillRule {
     #[default]
@@ -47,3 +49,19 @@ impl Default for SvgAttrs {
         }
     }
 }
+
+impl SvgAttrs {
+    pub fn from_raw(
+        fill_rule: u8,
+        stroke_linecap: u8,
+        stroke_linejoin: u8,
+        fill_none: bool,
+    ) -> Self {
+        Self {
+            fill_rule: RawFillRule::from(fill_rule).into(),
+            stroke_linecap: RawStrokeLineCap::from(stroke_linecap).into(),
+            stroke_linejoin: RawStrokeLineJoin::from(stroke_linejoin).into(),
+            fill_none,
+        }
+    }
+}

render-wasm/src/wasm/svg_attrs.rs

@@ -1,6 +1,6 @@
 use macros::ToJs;
 
-use crate::shapes::{FillRule, StrokeLineCap, StrokeLineJoin};
+use crate::shapes::{FillRule, StrokeLineCap, StrokeLineJoin, SvgAttrs};
 use crate::{with_current_shape_mut, STATE};
 
 #[derive(PartialEq, ToJs)]
@@ -84,12 +84,11 @@ pub extern "C" fn set_shape_svg_attrs(
     fill_none: bool,
 ) {
     with_current_shape_mut!(state, |shape: &mut Shape| {
-        let fill_rule = RawFillRule::from(fill_rule);
+        shape.svg_attrs = Some(SvgAttrs::from_raw(
-        shape.svg_attrs.fill_rule = fill_rule.into();
+            fill_rule,
-        let stroke_linecap = RawStrokeLineCap::from(stroke_linecap);
+            stroke_linecap,
-        shape.svg_attrs.stroke_linecap = stroke_linecap.into();
+            stroke_linejoin,
-        let stroke_linejoin = RawStrokeLineJoin::from(stroke_linejoin);
+            fill_none,
-        shape.svg_attrs.stroke_linejoin = stroke_linejoin.into();
+        ));
-        shape.svg_attrs.fill_none = fill_none;
     });
 }