diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index 5e3e0e20cd..8d1300ebae 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -87,22 +87,23 @@ struct PathRenderSignature<'a> { opacity: u32, // Converted to fixed-point for comparison (multiply by 1000) blend_mode: crate::shapes::BlendMode, transform: &'a Matrix, + svg_attrs: Option<&'a crate::shapes::SvgAttrs>, } impl<'a> PathRenderSignature<'a> { fn from_shape(shape: &'a Shape, clip_bounds: &'a Option) -> Option { use crate::shapes::Type; - + // Only paths can have this signature if !matches!(shape.shape_type, Type::Path(_) | Type::Bool(_)) { return None; } - + // Cannot combine paths with shadows or blur if !shape.shadows.is_empty() || shape.blur.is_some() { return None; } - + Some(Self { clip_bounds, fills: &shape.fills, @@ -110,36 +111,44 @@ impl<'a> PathRenderSignature<'a> { opacity: (shape.opacity * 1000.0) as u32, // Convert to fixed-point blend_mode: shape.blend_mode(), transform: &shape.transform, + svg_attrs: shape.svg_attrs.as_ref(), }) } - + // Manual comparison since Matrix doesn't implement Eq fn equals(&self, other: &Self) -> bool { // Compare clip_bounds if self.clip_bounds != other.clip_bounds { return false; } - + // Compare fills (using PartialEq) if self.fills != other.fills { return false; } - + // Compare strokes (using PartialEq) if self.strokes != other.strokes { return false; } - + // Compare opacity if self.opacity != other.opacity { return false; } - + // Compare blend_mode if self.blend_mode != other.blend_mode { return false; } - + + // Compare svg_attrs (especially fill_rule, which affects rendering when paths overlap) + // Different fill_rules (EvenOdd vs Nonzero) produce different visual results + // when paths are combined, so we must ensure they match + if self.svg_attrs != other.svg_attrs { + return false; + } + // Compare transform (Matrix comparison - check if they're approximately equal) // For now, use exact equality, but we could add epsilon comparison if needed self.transform == other.transform @@ -148,14 +157,41 @@ impl<'a> PathRenderSignature<'a> { // SIMPLE OPTIMIZATION: Helper to check if two shapes can be rendered together // Uses render signatures for efficient comparison -fn can_render_paths_together(shape1: &Shape, shape2: &Shape, clip_bounds1: &Option, clip_bounds2: &Option) -> bool { +fn can_render_paths_together( + shape1: &Shape, + shape2: &Shape, + clip_bounds1: &Option, + clip_bounds2: &Option, +) -> bool { let sig1 = PathRenderSignature::from_shape(shape1, clip_bounds1); let sig2 = PathRenderSignature::from_shape(shape2, clip_bounds2); - - match (sig1, sig2) { + + // First check if signatures match (same rendering properties) + let signatures_match = match (sig1, sig2) { (Some(sig1), Some(sig2)) => sig1.equals(&sig2), - _ => false, + _ => return false, + }; + + if !signatures_match { + return false; } + + // Even if signatures match, we cannot combine paths that overlap + // because combining them changes how fill_rule (especially EvenOdd) is applied + // When paths overlap: + // - Rendering separately: each path evaluates its fill_rule independently + // - Combining: the combined path evaluates fill_rule as a single path, which can give different results + let selrect1 = shape1.selrect(); + let selrect2 = shape2.selrect(); + + // Check if the selection rectangles overlap (approximation of path overlap) + if selrect1.intersects(selrect2) { + // Paths overlap, so we cannot safely combine them + // This is especially important for EvenOdd fill_rule, but applies to all fill_rules + return false; + } + + true } impl NodeRenderState { @@ -1852,20 +1888,26 @@ impl RenderState { // SIMPLE OPTIMIZATION: If this is a path, check if consecutive siblings // are also paths with the same properties and combine them all // We check BEFORE rendering to avoid rendering the first path twice - let shape_to_render = if matches!(element.shape_type, Type::Path(_) | Type::Bool(_)) { + let shape_to_render = if matches!(element.shape_type, Type::Path(_) | Type::Bool(_)) + { // Collect all consecutive paths with same properties let mut path_ids = vec![element.id]; let mut path_selrects = vec![element.selrect()]; - + // Keep checking and collecting consecutive compatible paths while let Some(next_node) = self.pending_nodes.last() { if next_node.visited_children || next_node.is_root() { break; } - + if let Some(next_element) = tree.get(&next_node.id) { // Check if next element is also a path with same properties - if can_render_paths_together(element, next_element, &clip_bounds, &next_node.clip_bounds) { + if can_render_paths_together( + element, + next_element, + &clip_bounds, + &next_node.clip_bounds, + ) { // Remove it from pending_nodes and add to our collection let _next_node = self.pending_nodes.pop().unwrap(); path_ids.push(next_element.id); @@ -1877,29 +1919,34 @@ impl RenderState { break; } } - + // If we collected more than one path, combine them all if path_ids.len() > 1 { // Combine paths by joining their segments (simpler than boolean union) // This avoids the complexity and potential errors of boolean operations - use crate::shapes::{ToPath, Path as ShapePath}; - + // NOTE: The can_render_paths_together function ensures: + // 1. svg_attrs (including fill_rule) match + // 2. paths don't overlap (checked via selrect intersection) + // When paths overlap, combining them changes how fill_rule is applied + // (especially EvenOdd), producing different visual results than rendering separately. + use crate::shapes::{Path as ShapePath, ToPath}; + let mut combined_path = element.to_path(tree); for path_id in path_ids.iter().skip(1) { if let Some(path_shape) = tree.get(path_id) { let other_path = path_shape.to_path(tree); // Simple join: concatenate segments (like join_paths does) let mut segments = combined_path.segments().clone(); - segments.extend(other_path.segments().iter().cloned()); + segments.extend(other_path.segments().iter()); combined_path = ShapePath::new(segments); } } - + // Create a temporary shape with the combined path // Use properties from the first shape (they're the same anyway) let mut combined_shape = element.clone(); combined_shape.shape_type = crate::shapes::Type::Path(combined_path); - + // Update selrect to encompass all paths let mut left = path_selrects[0].left(); let mut top = path_selrects[0].top(); @@ -1912,7 +1959,7 @@ impl RenderState { bottom = bottom.max(selrect.bottom()); } combined_shape.set_selrect(left, top, right, bottom); - + Some(combined_shape) } else { None @@ -1920,7 +1967,7 @@ impl RenderState { } else { None }; - + // Render the shape (either combined or original) let final_shape = shape_to_render.as_ref().unwrap_or(element); self.render_shape( @@ -1934,7 +1981,7 @@ impl RenderState { None, None, ); - + // If we combined paths, skip normal processing of the remaining nodes if shape_to_render.is_some() { continue; @@ -2095,8 +2142,13 @@ impl RenderState { }; // Check if cache is valid (same root_ids) - let cache_valid = self.cached_root_ids.as_ref() - .map(|cached| cached.len() == root_ids.len() && cached.iter().zip(root_ids.iter()).all(|(a, b)| a == b)) + let cache_valid = self + .cached_root_ids + .as_ref() + .map(|cached| { + cached.len() == root_ids.len() + && cached.iter().zip(root_ids.iter()).all(|(a, b)| a == b) + }) .unwrap_or(false); if cache_valid { @@ -2109,10 +2161,10 @@ impl RenderState { .enumerate() .map(|(i, id)| (*id, i)) .collect(); - + self.cached_root_ids = Some(root_ids.clone()); self.cached_root_ids_map = Some(root_ids_map.clone()); - + (root_ids, root_ids_map) } }; diff --git a/render-wasm/src/render/surfaces.rs b/render-wasm/src/render/surfaces.rs index 4f36e69838..80c0e61853 100644 --- a/render-wasm/src/render/surfaces.rs +++ b/render-wasm/src/render/surfaces.rs @@ -246,7 +246,7 @@ impl Surfaces { pub fn update_render_context(&mut self, render_area: skia::Rect, scale: f32) { let translation = self.get_render_context_translation(render_area, scale); - + // When context changes (zoom/pan/tile), clear all render surfaces first // to remove any residual content from previous tiles, then mark as dirty // so they get redrawn with new transformations @@ -254,18 +254,18 @@ impl Surfaces { | SurfaceId::Strokes as u32 | SurfaceId::InnerShadows as u32 | SurfaceId::TextDropShadows as u32; - + // Clear surfaces before updating transformations to remove residual content self.apply_mut(surface_ids, |s| { s.canvas().clear(skia::Color::TRANSPARENT); }); - + // Mark all render surfaces as dirty so they get redrawn self.mark_dirty(SurfaceId::Fills); self.mark_dirty(SurfaceId::Strokes); self.mark_dirty(SurfaceId::InnerShadows); self.mark_dirty(SurfaceId::TextDropShadows); - + // Update transformations self.apply_mut(surface_ids, |s| { let canvas = s.canvas();