Merge pull request #6651 from penpot/superalex-path-fixes

🐛 Path fixes

frontend/src/app/render_wasm/api.cljs

@@ -698,8 +698,10 @@
                        false)
         rotation     (dm/get-prop shape :rotation)
         transform    (dm/get-prop shape :transform)
-        fills        (if (= type :group)
-                       [] (dm/get-prop shape :fills))
+
+        ;; Groups from imported SVG's can have their own fills
+        fills        (dm/get-prop shape :fills)
+
         strokes      (if (= type :group)
                        [] (dm/get-prop shape :strokes))
         children     (dm/get-prop shape :shapes)

render-wasm/src/render.rs

@@ -19,7 +19,9 @@ use options::RenderOptions;
 use surfaces::{SurfaceId, Surfaces};
 
 use crate::performance;
-use crate::shapes::{modified_children_ids, Corners, Shape, StrokeKind, StructureEntry, Type};
+use crate::shapes::{
+    modified_children_ids, Corners, Fill, Shape, StrokeKind, StructureEntry, Type,
+};
 use crate::tiles::{self, PendingTiles, TileRect};
 use crate::uuid::Uuid;
 use crate::view::Viewbox;
@@ -97,6 +99,11 @@ pub(crate) struct RenderState {
     pub tile_viewbox: tiles::TileViewbox,
     pub tiles: tiles::TileHashMap,
     pub pending_tiles: PendingTiles,
+    // nested_fills maintains a stack of group  fills that apply to nested shapes
+    // 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.
+    pub nested_fills: Vec<Vec<Fill>>,
 }
 
 pub fn get_cache_size(viewbox: Viewbox, scale: f32) -> skia::ISize {
@@ -162,6 +169,7 @@ impl RenderState {
                 1.0,
             ),
             pending_tiles: PendingTiles::new_empty(),
+            nested_fills: vec![],
         }
     }
 
@@ -435,9 +443,18 @@ impl RenderState {
                     s.canvas().concat(&matrix);
                 });
 
+                if shape.fills.is_empty() && !matches!(shape.shape_type, Type::Group(_)) {
+                    if let Some(fills_to_render) = self.nested_fills.last() {
+                        let fills_to_render = fills_to_render.clone();
+                        for fill in fills_to_render.iter() {
+                            fills::render(self, &shape, fill, antialias);
+                        }
+                    }
+                } else {
                     for fill in shape.fills().rev() {
                         fills::render(self, &shape, fill, antialias);
                     }
+                }
 
                 for stroke in shape.strokes().rev() {
                     shadows::render_stroke_drop_shadows(self, &shape, stroke, antialias);
@@ -599,6 +616,8 @@ impl RenderState {
         // an extra save_layer to keep all the masked group separate from
         // other already drawn elements.
         if let Type::Group(group) = element.shape_type {
+            let fills = &element.fills;
+            self.nested_fills.push(fills.to_vec());
             if group.masked {
                 let paint = skia::Paint::default();
                 let layer_rec = skia::canvas::SaveLayerRec::default().paint(&paint);
@@ -645,6 +664,9 @@ impl RenderState {
                 }
             }
         }
+        if let Type::Group(_) = element.shape_type {
+            self.nested_fills.pop();
+        }
         self.surfaces.canvas(SurfaceId::Current).restore();
     }
 

render-wasm/src/render/fills.rs

@@ -135,6 +135,9 @@ pub fn render(render_state: &mut RenderState, shape: &Shape, fill: &Fill, antial
                 .surfaces
                 .draw_path_to(SurfaceId::Fills, shape, paint);
         }
+        (_, Type::Group(_)) => {
+            // Groups can have fills but they propagate them to their children
+        }
         (_, _) => {
             unreachable!("This shape should not have fills")
         }

render-wasm/src/render/strokes.rs

@@ -187,9 +187,11 @@ fn handle_stroke_caps(
     scale: f32,
     antialias: bool,
 ) {
-    let points_count = path.count_points();
-    let mut points = vec![Point::default(); points_count];
-    let c_points = path.get_points(&mut points);
+    let mut points = vec![Point::default(); path.count_points()];
+    path.get_points(&mut points);
+    // Curves can have duplicated points, so let's remove consecutive duplicated points
+    points.dedup();
+    let c_points = points.len();
 
     // Closed shapes don't have caps
     if c_points >= 2 && is_open {
@@ -213,7 +215,7 @@ fn handle_stroke_caps(
             stroke.width,
             &mut paint_stroke,
             last_point,
-            &points[points_count - 2],
+            &points[c_points - 2],
         );
     }
 }

render-wasm/src/shapes.rs

@@ -608,7 +608,7 @@ impl Shape {
 
     pub fn visually_insignificant(&self, scale: f32) -> bool {
         let extrect = self.extrect();
-        extrect.width() * scale < MIN_VISIBLE_SIZE || extrect.height() * scale < MIN_VISIBLE_SIZE
+        extrect.width() * scale < MIN_VISIBLE_SIZE && extrect.height() * scale < MIN_VISIBLE_SIZE
     }
 
     pub fn should_use_antialias(&self, scale: f32) -> bool {