[librsvg: 5/10] node: remove NodeData::get_type()

[Federico Mena Quintero <federico src gnome org>]

commit 0a779259035d8b2ae615f58dcbe2936bdf53bb78
Author: Paolo Borelli <pborelli gnome org>
Date:   Mon Mar 16 08:28:04 2020 +0100

    node: remove NodeData::get_type()
    
    Always call get_type on the element.

 rsvg_internals/src/css.rs                        | 10 +++---
 rsvg_internals/src/document.rs                   | 18 +++++++----
 rsvg_internals/src/drawing_ctx.rs                | 41 +++++++++++++-----------
 rsvg_internals/src/filters/component_transfer.rs |  2 +-
 rsvg_internals/src/filters/light/lighting.rs     | 35 ++++++++------------
 rsvg_internals/src/filters/merge.rs              |  4 +--
 rsvg_internals/src/filters/mod.rs                | 12 +++----
 rsvg_internals/src/gradient.rs                   |  2 +-
 rsvg_internals/src/node.rs                       | 16 +++------
 rsvg_internals/src/xml.rs                        |  2 +-
 10 files changed, 67 insertions(+), 75 deletions(-)
---
diff --git a/rsvg_internals/src/css.rs b/rsvg_internals/src/css.rs
index 25738692..2fe174ce 100644
--- a/rsvg_internals/src/css.rs
+++ b/rsvg_internals/src/css.rs
@@ -498,7 +498,7 @@ impl selectors::Element for RsvgElement {
     /// Whether this element is a `link`.
     fn is_link(&self) -> bool {
         // FIXME: is this correct for SVG <a>, not HTML <a>?
-        self.0.borrow().get_type() == NodeType::Link
+        self.0.is_element() && self.0.borrow_element().get_type() == NodeType::Link
     }
 
     /// Returns whether the element is an HTML <slot> element.
@@ -815,10 +815,10 @@ mod tests {
 
         // Node types
 
-        assert!(a.borrow().get_type() == NodeType::Svg);
-        assert!(b.borrow().get_type() == NodeType::Rect);
-        assert!(c.borrow().get_type() == NodeType::Circle);
-        assert!(d.borrow().get_type() == NodeType::Rect);
+        assert!(a.borrow_element().get_type() == NodeType::Svg);
+        assert!(b.borrow_element().get_type() == NodeType::Rect);
+        assert!(c.borrow_element().get_type() == NodeType::Circle);
+        assert!(d.borrow_element().get_type() == NodeType::Rect);
 
         let a = RsvgElement(a);
         let b = RsvgElement(b);
diff --git a/rsvg_internals/src/document.rs b/rsvg_internals/src/document.rs
index 91a63ca6..77ef7aa8 100644
--- a/rsvg_internals/src/document.rs
+++ b/rsvg_internals/src/document.rs
@@ -293,13 +293,15 @@ impl<'i> AcquiredNodes<'i> {
 
         if node_types.is_empty() {
             Ok(node)
-        } else {
-            let node_type = node.borrow().get_type();
+        } else if node.is_element() {
+            let node_type = node.borrow_element().get_type();
             if node_types.iter().find(|&&t| t == node_type).is_some() {
                 Ok(node)
             } else {
                 Err(AcquireError::InvalidLinkType(fragment.clone()))
             }
+        } else {
+            Err(AcquireError::InvalidLinkType(fragment.clone()))
         }
     }
 
@@ -355,7 +357,11 @@ impl<'i> AcquiredNodes<'i> {
 fn node_is_accessed_by_reference(node: &RsvgNode) -> bool {
     use NodeType::*;
 
-    match node.borrow().get_type() {
+    if !node.is_element() {
+        return false;
+    }
+
+    match node.borrow_element().get_type() {
         ClipPath | Filter | LinearGradient | Marker | Mask | Pattern | RadialGradient => true,
 
         _ => false,
@@ -501,9 +507,8 @@ impl DocumentBuilder {
         } = self;
 
         match tree {
-            None => Err(LoadingError::SvgHasNoElements),
-            Some(root) => {
-                if root.borrow().get_type() == NodeType::Svg {
+            Some(root) if root.is_element() => {
+                if root.borrow_element().get_type() == NodeType::Svg {
                     let mut document = Document {
                         tree: root,
                         ids,
@@ -520,6 +525,7 @@ impl DocumentBuilder {
                     Err(LoadingError::RootElementIsNotSvg)
                 }
             }
+            _ => Err(LoadingError::SvgHasNoElements),
         }
     }
 }
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 7a5c60e2..0b38dd81 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -475,11 +475,12 @@ impl DrawingCtx {
                 let mask = values.mask.0.get();
 
                 // The `filter` property does not apply to masks.
-                let filter = if node.borrow().get_type() == NodeType::Mask {
-                    None
-                } else {
-                    values.filter.0.get()
-                };
+                let filter =
+                    if node.is_element() && node.borrow_element().get_type() == NodeType::Mask {
+                        None
+                    } else {
+                        values.filter.0.get()
+                    };
 
                 let UnitInterval(opacity) = values.opacity.0;
 
@@ -814,7 +815,9 @@ impl DrawingCtx {
                     Ok(acquired) => {
                         let node = acquired.get();
 
-                        had_paint_server = match node.borrow().get_type() {
+                        assert!(node.is_element());
+
+                        had_paint_server = match node.borrow_element().get_type() {
                             NodeType::LinearGradient => node
                                 .borrow_element()
                                 .get_impl::<LinearGradient>()
@@ -1168,19 +1171,7 @@ impl DrawingCtx {
 
         let child = acquired.get();
 
-        if child.borrow().get_type() != NodeType::Symbol {
-            let cr = self.get_cairo_context();
-            cr.translate(use_rect.x0, use_rect.y0);
-
-            self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-                dc.draw_node_from_stack(
-                    &child,
-                    an,
-                    &CascadedValues::new_from_values(&child, values),
-                    clipping,
-                )
-            })
-        } else {
+        if child.is_element() && child.borrow_element().get_type() == NodeType::Symbol {
             let elt = child.borrow_element();
             let symbol = elt.get_impl::<Symbol>();
 
@@ -1207,6 +1198,18 @@ impl DrawingCtx {
                     clipping,
                 )
             })
+        } else {
+            let cr = self.get_cairo_context();
+            cr.translate(use_rect.x0, use_rect.y0);
+
+            self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
+                dc.draw_node_from_stack(
+                    &child,
+                    an,
+                    &CascadedValues::new_from_values(&child, values),
+                    clipping,
+                )
+            })
         }
     }
 }
diff --git a/rsvg_internals/src/filters/component_transfer.rs 
b/rsvg_internals/src/filters/component_transfer.rs
index 89907746..03378731 100644
--- a/rsvg_internals/src/filters/component_transfer.rs
+++ b/rsvg_internals/src/filters/component_transfer.rs
@@ -294,7 +294,7 @@ impl FilterEffect for FeComponentTransfer {
         {
             node.children()
                 .rev()
-                .filter(|c| c.borrow().get_type() == node_type)
+                .filter(|c| c.is_element() && c.borrow_element().get_type() == node_type)
                 .find(|c| c.borrow_element().get_impl::<F>().channel() == channel)
         };
 
diff --git a/rsvg_internals/src/filters/light/lighting.rs b/rsvg_internals/src/filters/light/lighting.rs
index 01b41a3e..b8dd67b5 100644
--- a/rsvg_internals/src/filters/light/lighting.rs
+++ b/rsvg_internals/src/filters/light/lighting.rs
@@ -490,13 +490,13 @@ impl_lighting_filter!(FeDiffuseLighting, diffuse_alpha);
 impl_lighting_filter!(FeSpecularLighting, specular_alpha);
 
 fn find_light_source(node: &RsvgNode, ctx: &FilterContext) -> Result<LightSource, FilterError> {
-    let mut light_sources = node
-        .children()
-        .rev()
-        .filter(|c| match c.borrow().get_type() {
-            NodeType::FeDistantLight | NodeType::FePointLight | NodeType::FeSpotLight => true,
-            _ => false,
-        });
+    let mut light_sources = node.children().rev().filter(|c| {
+        c.is_element()
+            && match c.borrow_element().get_type() {
+                NodeType::FeDistantLight | NodeType::FePointLight | NodeType::FeSpotLight => true,
+                _ => false,
+            }
+    });
 
     let node = light_sources.next();
     if node.is_none() || light_sources.next().is_some() {
@@ -504,23 +504,16 @@ fn find_light_source(node: &RsvgNode, ctx: &FilterContext) -> Result<LightSource
     }
 
     let node = node.unwrap();
-    if node.borrow_element().is_in_error() {
+    let elt = node.borrow_element();
+
+    if elt.is_in_error() {
         return Err(FilterError::ChildNodeInError);
     }
 
-    let light_source = match node.borrow().get_type() {
-        NodeType::FeDistantLight => node
-            .borrow_element()
-            .get_impl::<FeDistantLight>()
-            .transform(ctx),
-        NodeType::FePointLight => node
-            .borrow_element()
-            .get_impl::<FePointLight>()
-            .transform(ctx),
-        NodeType::FeSpotLight => node
-            .borrow_element()
-            .get_impl::<FeSpotLight>()
-            .transform(ctx),
+    let light_source = match elt.get_type() {
+        NodeType::FeDistantLight => elt.get_impl::<FeDistantLight>().transform(ctx),
+        NodeType::FePointLight => elt.get_impl::<FePointLight>().transform(ctx),
+        NodeType::FeSpotLight => elt.get_impl::<FeSpotLight>().transform(ctx),
         _ => unreachable!(),
     };
 
diff --git a/rsvg_internals/src/filters/merge.rs b/rsvg_internals/src/filters/merge.rs
index ddaac9d1..0cd26ee6 100644
--- a/rsvg_internals/src/filters/merge.rs
+++ b/rsvg_internals/src/filters/merge.rs
@@ -90,7 +90,7 @@ impl FilterEffect for FeMerge {
         let mut bounds = self.base.get_bounds(ctx);
         for child in node
             .children()
-            .filter(|c| c.borrow().get_type() == NodeType::FeMergeNode)
+            .filter(|c| c.is_element() && c.borrow_element().get_type() == NodeType::FeMergeNode)
         {
             let elt = child.borrow_element();
 
@@ -111,7 +111,7 @@ impl FilterEffect for FeMerge {
         let mut output_surface = None;
         for child in node
             .children()
-            .filter(|c| c.borrow().get_type() == NodeType::FeMergeNode)
+            .filter(|c| c.is_element() && c.borrow_element().get_type() == NodeType::FeMergeNode)
         {
             output_surface = Some(child.borrow_element().get_impl::<FeMergeNode>().render(
                 ctx,
diff --git a/rsvg_internals/src/filters/mod.rs b/rsvg_internals/src/filters/mod.rs
index d3aee6d7..cd2d975c 100644
--- a/rsvg_internals/src/filters/mod.rs
+++ b/rsvg_internals/src/filters/mod.rs
@@ -117,13 +117,11 @@ impl NodeTrait for Primitive {
         // With ObjectBoundingBox, only fractions and percents are allowed.
         let primitiveunits = parent
             .and_then(|parent| {
-                if parent.borrow().get_type() == NodeType::Filter {
-                    Some(
-                        parent
-                            .borrow_element()
-                            .get_impl::<Filter>()
-                            .get_primitive_units(),
-                    )
+                assert!(parent.is_element());
+                let parent_elt = parent.borrow_element();
+
+                if parent_elt.get_type() == NodeType::Filter {
+                    Some(parent_elt.get_impl::<Filter>().get_primitive_units())
                 } else {
                     None
                 }
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index fe46f4fb..cba6a955 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -444,7 +444,7 @@ impl UnresolvedGradient {
     /// Looks for <stop> children inside a linearGradient or radialGradient node,
     /// and adds their info to the UnresolvedGradient &self.
     fn add_color_stops_from_node(&mut self, node: &RsvgNode) {
-        let node_type = node.borrow().get_type();
+        let node_type = node.borrow_element().get_type();
 
         assert!(node_type == NodeType::LinearGradient || node_type == NodeType::RadialGradient);
 
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index dfae73c4..2d9a80a2 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -125,13 +125,6 @@ impl NodeData {
     pub fn new_chars() -> NodeData {
         NodeData::Text(NodeChars::new())
     }
-
-    pub fn get_type(&self) -> NodeType {
-        match *self {
-            NodeData::Element(ref e) => e.node_type,
-            NodeData::Text(_) => NodeType::Chars,
-        }
-    }
 }
 
 impl Element {
@@ -314,14 +307,13 @@ impl Element {
 
 impl fmt::Display for NodeData {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self.get_type())?;
-
         match *self {
             NodeData::Element(ref e) => {
-                write!(f, " id={}", e.get_id().unwrap_or("None"))?;
+                write!(f, "{:?} id={}", e.get_type(), e.get_id().unwrap_or("None"))?;
+            }
+            NodeData::Text(_) => {
+                write!(f, "Chars")?;
             }
-
-            _ => (),
         }
 
         Ok(())
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index f83f8017..fdc60053 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -358,7 +358,7 @@ impl XmlState {
         let mut inner = self.inner.borrow_mut();
         let current_node = inner.current_node.as_ref().unwrap();
 
-        assert!(current_node.borrow().get_type() == NodeType::Style);
+        assert!(current_node.borrow_element().get_type() == NodeType::Style);
 
         let style_type = current_node
             .borrow_element()