[librsvg: 32/45] PaintSource::resolve() - Return a PaintServerError, not a RenderingError

[Federico Mena Quintero <federico src gnome org>]

commit c356f4fd34bcc7a3fc2a6430a86e134c4c733b29
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Sep 30 10:27:13 2019 -0500

    PaintSource::resolve() - Return a PaintServerError, not a RenderingError
    
    PaintServerError is a new error type, just for the step of resolving
    paint servers.  This should make it easier to implement the semantics
    where a paint server like
    
      fill="url(#not_found) rgb(0, 0, 0)"
    
    or one like
    
      fill="url(#link_to_somehing_not_a_paint_server) rgb(0, 0, 0)"
    
    means that we should use that fallback color, since the paint server
    could not be resolved.

 rsvg_internals/src/error.rs        | 33 +++++++++++++++++++++++++++++++++
 rsvg_internals/src/gradient.rs     | 20 +++++++++++---------
 rsvg_internals/src/paint_server.rs | 29 ++++++++++++++++++++++-------
 rsvg_internals/src/pattern.rs      |  9 +++++----
 4 files changed, 71 insertions(+), 20 deletions(-)
---
diff --git a/rsvg_internals/src/error.rs b/rsvg_internals/src/error.rs
index d8ad81e5..57d1afb0 100644
--- a/rsvg_internals/src/error.rs
+++ b/rsvg_internals/src/error.rs
@@ -10,6 +10,7 @@ use glib_sys;
 use libc;
 use markup5ever::LocalName;
 
+use crate::allowed_url::Fragment;
 use crate::parsers::ParseError;
 
 /// A simple error which refers to an attribute's value
@@ -122,6 +123,38 @@ impl From<cairo::Status> for RenderingError {
     }
 }
 
+#[derive(Debug)]
+pub enum PaintServerError {
+    LinkNotFound(Fragment),
+    InvalidLinkType(Fragment),
+    CircularReference(Fragment),
+}
+
+impl error::Error for PaintServerError {
+    fn description(&self) -> &str {
+        match *self {
+            PaintServerError::LinkNotFound(_) => "link to paint server not found",
+            PaintServerError::InvalidLinkType(_) => "link is to object of invalid type",
+            PaintServerError::CircularReference(_) => "circular reference in link"
+        }
+    }
+}
+
+impl fmt::Display for PaintServerError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match *self {
+            PaintServerError::LinkNotFound(ref frag) =>
+                write!(f, "link to paint server not found: {}", frag),
+
+            PaintServerError::InvalidLinkType(ref frag) =>
+                write!(f, "link {} is to object of invalid type", frag),
+
+            PaintServerError::CircularReference(ref frag) =>
+                write!(f, "circular reference in link {}", frag),
+        }
+    }
+}
+
 /// Helper for converting `Result<O, E>` into `Result<O, NodeError>`
 ///
 /// A `NodeError` requires a `LocalName` that corresponds to the attribute to which the
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index 23bdabb1..d66974cb 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -583,18 +583,18 @@ impl PaintSource for NodeGradient {
         &self,
         node: &RsvgNode,
         draw_ctx: &mut DrawingCtx,
-    ) -> Result<Option<Self::Resolved>, RenderingError> {
+    ) -> Result<Option<Self::Resolved>, PaintServerError> {
         let Unresolved { mut gradient, mut fallback } = self.get_unresolved(node);
 
         let mut stack = NodeStack::new();
 
         while !gradient.is_resolved() {
-            if let Some(acquired) = acquire_gradient(draw_ctx, fallback.as_ref()) {
+            if let Some(fragment) = fallback {
+                let acquired = acquire_gradient(draw_ctx, &fragment)?;
                 let acquired_node = acquired.get();
 
                 if stack.contains(acquired_node) {
-                    rsvg_log!("circular reference in gradient {}", node);
-                    return Err(RenderingError::CircularReference);
+                    return Err(PaintServerError::CircularReference(fragment.clone()));
                 }
 
                 let borrowed_node = acquired_node.borrow();
@@ -608,6 +608,7 @@ impl PaintSource for NodeGradient {
                 stack.push(acquired_node);
             } else {
                 gradient = gradient.resolve_from_defaults();
+                break;
             }
         }
 
@@ -703,16 +704,17 @@ impl Gradient {
 
 fn acquire_gradient<'a>(
     draw_ctx: &'a mut DrawingCtx,
-    name: Option<&Fragment>,
-) -> Option<AcquiredNode> {
-    name.and_then(move |fragment| draw_ctx.acquired_nodes().get_node(fragment))
+    fragment: &Fragment,
+) -> Result<AcquiredNode, PaintServerError> {
+    draw_ctx.acquired_nodes().get_node(fragment)
+        .ok_or(PaintServerError::LinkNotFound(fragment.clone()))
         .and_then(|acquired| {
             let node_type = acquired.get().borrow().get_type();
 
             if node_type == NodeType::Gradient {
-                Some(acquired)
+                Ok(acquired)
             } else {
-                None
+                Err(PaintServerError::InvalidLinkType(fragment.clone()))
             }
         })
 }
diff --git a/rsvg_internals/src/paint_server.rs b/rsvg_internals/src/paint_server.rs
index 4aad78a6..04295fb5 100644
--- a/rsvg_internals/src/paint_server.rs
+++ b/rsvg_internals/src/paint_server.rs
@@ -61,7 +61,7 @@ pub trait PaintSource {
         &self,
         node: &RsvgNode,
         draw_ctx: &mut DrawingCtx,
-    ) -> Result<Option<Self::Resolved>, RenderingError>;
+    ) -> Result<Option<Self::Resolved>, PaintServerError>;
 
     fn resolve_fallbacks_and_set_pattern(
         &self,
@@ -70,12 +70,27 @@ pub trait PaintSource {
         opacity: &UnitInterval,
         bbox: &BoundingBox,
     ) -> Result<bool, RenderingError> {
-        if let Some(resolved) = self.resolve(&node, draw_ctx)? {
-            let cascaded = CascadedValues::new_from_node(node);
-            let values = cascaded.get();
-            resolved.set_pattern_on_draw_context(values, draw_ctx, opacity, bbox)
-        } else {
-            Ok(false)
+        match self.resolve(&node, draw_ctx) {
+            Ok(Some(resolved)) => {
+                let cascaded = CascadedValues::new_from_node(node);
+                let values = cascaded.get();
+                resolved.set_pattern_on_draw_context(values, draw_ctx, opacity, bbox)
+            }
+
+            Ok(None) => Ok(false),
+
+            Err(PaintServerError::CircularReference(_)) => {
+                // FIXME: add a fragment or node id to this:
+                rsvg_log!("circular reference in paint server {}", node);
+                Err(RenderingError::CircularReference)
+            }
+
+            Err(e) => {
+                rsvg_log!("not using paint server {}: {}", node, e);
+
+                // "could not resolve" means caller needs to fall back to color
+                Ok(false)
+            }
         }
     }
 }
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 5306e6bc..83c1cb5d 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -8,7 +8,7 @@ use crate::aspect_ratio::*;
 use crate::bbox::*;
 use crate::coord_units::CoordUnits;
 use crate::drawing_ctx::{DrawingCtx, NodeStack};
-use crate::error::{AttributeResultExt, RenderingError};
+use crate::error::{AttributeResultExt, PaintServerError, RenderingError};
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::length::*;
 use crate::node::*;
@@ -144,7 +144,7 @@ impl PaintSource for NodePattern {
         &self,
         node: &RsvgNode,
         draw_ctx: &mut DrawingCtx,
-    ) -> Result<Option<Self::Resolved>, RenderingError> {
+    ) -> Result<Option<Self::Resolved>, PaintServerError> {
         *self.node.borrow_mut() = Some(node.downgrade());
 
         let mut result = node.borrow().get_impl::<NodePattern>().clone();
@@ -158,8 +158,9 @@ impl PaintSource for NodePattern {
                 let a_node = acquired.get();
 
                 if stack.contains(a_node) {
-                    rsvg_log!("circular reference in pattern {}", node);
-                    return Err(RenderingError::CircularReference);
+                    return Err(PaintServerError::CircularReference(
+                        result.fallback.as_ref().unwrap().clone(),
+                    ));
                 }
 
                 let node_data = a_node.borrow();