[librsvg: 3/12] Shape now has its markers

[Federico Mena Quintero <federico src gnome org>]

commit 208910824e41be6bfd564f0dccbcaa9bf99f127f
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Jun 3 12:06:57 2021 -0500

    Shape now has its markers
    
    Markers are resolved early, when we construct a Shape.  This lets us
    avoid passing ComputedValues into render_markers_for_shape().

 src/drawing_ctx.rs | 10 +++-------
 src/marker.rs      | 37 ++++---------------------------------
 src/shapes.rs      | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 52 insertions(+), 45 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index 3c746676..e0badae2 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -35,7 +35,7 @@ use crate::property_defs::{
     StrokeLinecap, StrokeLinejoin, TextRendering,
 };
 use crate::rect::Rect;
-use crate::shapes::{Markers, Shape};
+use crate::shapes::Shape;
 use crate::surface_utils::{
     shared_surface::ExclusiveImageSurface, shared_surface::SharedImageSurface,
     shared_surface::SurfaceType,
@@ -1260,12 +1260,8 @@ impl DrawingCtx {
                             }
 
                             PaintTarget::Markers => {
-                                if shape.markers == Markers::Yes {
-                                    path_helper.unset();
-                                    marker::render_markers_for_shape(
-                                        shape, dc, an, values, clipping,
-                                    )?;
-                                }
+                                path_helper.unset();
+                                marker::render_markers_for_shape(shape, dc, an, clipping)?;
                             }
                         }
                     }
diff --git a/src/marker.rs b/src/marker.rs
index a4ed8247..5abcc090 100644
--- a/src/marker.rs
+++ b/src/marker.rs
@@ -14,13 +14,11 @@ use crate::drawing_ctx::DrawingCtx;
 use crate::element::{Draw, ElementResult, SetAttributes};
 use crate::error::*;
 use crate::float_eq_cairo::ApproxEqCairo;
-use crate::iri::Iri;
 use crate::layout::StackingContext;
 use crate::length::*;
 use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw};
 use crate::parsers::{Parse, ParseValue};
 use crate::path_builder::{arc_segment, ArcParameterization, CubicBezierCurve, Path, PathCommand};
-use crate::properties::ComputedValues;
 use crate::rect::Rect;
 use crate::shapes::Shape;
 use crate::transform::Transform;
@@ -615,18 +613,13 @@ pub fn render_markers_for_shape(
     shape: &Shape,
     draw_ctx: &mut DrawingCtx,
     acquired_nodes: &mut AcquiredNodes<'_>,
-    values: &ComputedValues,
     clipping: bool,
 ) -> Result<BoundingBox, RenderingError> {
     if shape.stroke.width.approx_eq_cairo(0.0) {
         return Ok(draw_ctx.empty_bbox());
     }
 
-    let marker_start = acquire_marker(acquired_nodes, &values.marker_start().0);
-    let marker_mid = acquire_marker(acquired_nodes, &values.marker_mid().0);
-    let marker_end = acquire_marker(acquired_nodes, &values.marker_end().0);
-
-    if marker_start.is_none() && marker_mid.is_none() && marker_end.is_none() {
+    if shape.marker_start.is_none() && shape.marker_mid.is_none() && shape.marker_end.is_none() {
         return Ok(draw_ctx.empty_bbox());
     }
 
@@ -635,9 +628,9 @@ pub fn render_markers_for_shape(
         draw_ctx.empty_bbox(),
         &mut |marker_type: MarkerType, x: f64, y: f64, computed_angle: Angle| {
             let marker_node = match marker_type {
-                MarkerType::Start => &marker_start,
-                MarkerType::Middle => &marker_mid,
-                MarkerType::End => &marker_end,
+                MarkerType::Start => &shape.marker_start,
+                MarkerType::Middle => &shape.marker_mid,
+                MarkerType::End => &shape.marker_end,
             };
 
             if let Some(ref marker) = *marker_node {
@@ -658,28 +651,6 @@ pub fn render_markers_for_shape(
     )
 }
 
-fn acquire_marker(acquired_nodes: &mut AcquiredNodes<'_>, iri: &Iri) -> Option<Node> {
-    iri.get().and_then(|id| {
-        acquired_nodes
-            .acquire(id)
-            .map_err(|e| {
-                rsvg_log!("cannot render marker: {}", e);
-                ()
-            })
-            .ok()
-            .and_then(|acquired| {
-                let node = acquired.get();
-
-                if is_element_of_type!(node, Marker) {
-                    Some(node.clone())
-                } else {
-                    rsvg_log!("{} is not a marker element", id);
-                    None
-                }
-            })
-    })
-}
-
 fn emit_markers_for_path<E>(
     path: &Path,
     empty_bbox: BoundingBox,
diff --git a/src/shapes.rs b/src/shapes.rs
index 12a3483c..576e2c3f 100644
--- a/src/shapes.rs
+++ b/src/shapes.rs
@@ -11,6 +11,7 @@ use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
 use crate::element::{Draw, ElementResult, SetAttributes};
 use crate::error::*;
+use crate::iri::Iri;
 use crate::layout::{StackingContext, Stroke};
 use crate::length::*;
 use crate::node::{CascadedValues, Node, NodeBorrow};
@@ -21,8 +22,8 @@ use crate::path_parser;
 use crate::property_defs::{ClipRule, FillRule, PaintOrder, ShapeRendering};
 use crate::xml::Attributes;
 
-#[derive(Copy, Clone, PartialEq)]
-pub enum Markers {
+#[derive(PartialEq)]
+enum Markers {
     No,
     Yes,
 }
@@ -35,7 +36,6 @@ struct ShapeDef {
 // TODO: move Shape to layout.rs?
 pub struct Shape {
     pub path: Rc<SvgPath>,
-    pub markers: Markers,
     pub is_visible: bool,
     pub paint_order: PaintOrder,
     pub stroke: Stroke,
@@ -44,7 +44,9 @@ pub struct Shape {
     pub fill_rule: FillRule,
     pub clip_rule: ClipRule,
     pub shape_rendering: ShapeRendering,
-    // TODO: resolve the markers here, to avoid passing ComputedValues to render_markers_for_shape()
+    pub marker_start: Option<Node>,
+    pub marker_mid: Option<Node>,
+    pub marker_end: Option<Node>,
 }
 
 impl ShapeDef {
@@ -94,9 +96,22 @@ macro_rules! impl_draw {
                 let clip_rule = values.clip_rule();
                 let shape_rendering = values.shape_rendering();
 
+                let marker_start;
+                let marker_mid;
+                let marker_end;
+
+                if shape_def.markers == Markers::Yes {
+                    marker_start = acquire_marker(acquired_nodes, &values.marker_start().0);
+                    marker_mid = acquire_marker(acquired_nodes, &values.marker_mid().0);
+                    marker_end = acquire_marker(acquired_nodes, &values.marker_end().0);
+                } else {
+                    marker_start = None;
+                    marker_mid = None;
+                    marker_end = None;
+                }
+
                 let shape = Shape {
                     path: shape_def.path,
-                    markers: shape_def.markers,
                     is_visible,
                     paint_order,
                     stroke,
@@ -105,6 +120,9 @@ macro_rules! impl_draw {
                     fill_rule,
                     clip_rule,
                     shape_rendering,
+                    marker_start,
+                    marker_mid,
+                    marker_end,
                 };
 
                 let elt = node.borrow_element();
@@ -124,6 +142,28 @@ macro_rules! impl_draw {
     };
 }
 
+fn acquire_marker(acquired_nodes: &mut AcquiredNodes<'_>, iri: &Iri) -> Option<Node> {
+    iri.get().and_then(|id| {
+        acquired_nodes
+            .acquire(id)
+            .map_err(|e| {
+                rsvg_log!("cannot render marker: {}", e);
+                ()
+            })
+            .ok()
+            .and_then(|acquired| {
+                let node = acquired.get();
+
+                if is_element_of_type!(node, Marker) {
+                    Some(node.clone())
+                } else {
+                    rsvg_log!("{} is not a marker element", id);
+                    None
+                }
+            })
+    })
+}
+
 fn make_ellipse(cx: f64, cy: f64, rx: f64, ry: f64) -> SvgPath {
     let mut builder = PathBuilder::default();