(#930): Validate all clipPath and mask transforms

Previously, an invalid "transform" property specified for clipPath
would bubble up as a non-recoverable "invalid matrix" error from
Cairo, stopping rendering at that point.  Instead, we want to validate
all transforms and just not render the offending element.

This commit has many changes:

* Introduce a ValidTransform newtype, created with "impl
  TryFrom<Transform> for Transform".  This sees if the transform is
  invertible before wrapping it.  The idea is to do all transform
  operations first, then see if the result is valid by trying to convert
  to a ValidTransform.

* There is no "impl From<Transform> for cairo::Matrix" anymore, but
  there is From<ValidTransform>.  That way, we guarantee that an
  unvalidated transform cannot be easily set on a Cairo context.

* DrawingCtx::get_transform() returns a ValidTransform, since a) it
  comes from the cr, and Cairo already validated that transform for
  us.  Otherwise, the cr would have been in an error state.

* There is a new variant RenderingError::InvalidTransform, which lets
  us use "?" everywhere as normal.  This error case is caught in
  Node::draw() and the node is just not drawn.  Eventually we can move
  to a scheme where we distinguish between fatal
  errors (e.g. RenderingError::LimitExceeded) and recoverable ones.

Fixes https://gitlab.gnome.org/GNOME/librsvg/-/issues/930

Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/800>

12 files changed, 191 insertions, 101 deletions

diff --git a/src/aspect_ratio.rs b/src/aspect_ratio.rs
index 8003f9f2..42d6c76f 100644
--- a/src/aspect_ratio.rs
+++ b/src/aspect_ratio.rs
@@ -15,13 +15,12 @@
 //! [spec]: https://www.w3.org/TR/SVG/coords.html#PreserveAspectRatioAttribute
 
 use cssparser::{BasicParseError, Parser};
-use std::fmt;
 use std::ops::Deref;
 
 use crate::error::*;
 use crate::parsers::Parse;
 use crate::rect::Rect;
-use crate::transform::Transform;
+use crate::transform::{Transform, ValidTransform};
 use crate::viewbox::ViewBox;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -79,17 +78,6 @@ struct Align {
     fit: FitMode,
 }
 
-#[derive(Debug, PartialEq, Eq)]
-pub struct NonInvertibleTransform;
-
-impl fmt::Display for NonInvertibleTransform {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "Not invertible")
-    }
-}
-
-impl std::error::Error for NonInvertibleTransform {}
-
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct AspectRatio {
     defer: bool,
@@ -159,7 +147,7 @@ impl AspectRatio {
         &self,
         vbox: Option<ViewBox>,
         viewport: &Rect,
-    ) -> Result<Option<Transform>, NonInvertibleTransform> {
+    ) -> Result<Option<ValidTransform>, InvalidTransform> {
         // width or height set to 0 disables rendering of the element
         // https://www.w3.org/TR/SVG/struct.html#SVGElementWidthAttribute
         // https://www.w3.org/TR/SVG/struct.html#UseElementWidthAttribute
@@ -187,11 +175,7 @@ impl AspectRatio {
             Transform::new_translate(viewport.x0, viewport.y0)
         };
 
-        if transform.is_invertible() {
-            Ok(Some(transform))
-        } else {
-            Err(NonInvertibleTransform)
-        }
+        ValidTransform::try_from(transform).map(Some)
     }
 }
 
@@ -412,41 +396,50 @@ mod tests {
     #[test]
     fn empty_viewport() {
         let a = AspectRatio::default();
-        let t = a.viewport_to_viewbox_transform(
-            Some(ViewBox::parse_str("10 10 40 40").unwrap()),
-            &Rect::from_size(0.0, 0.0),
-        );
-
-        assert_eq!(t, Ok(None));
+        let t = a
+            .viewport_to_viewbox_transform(
+                Some(ViewBox::parse_str("10 10 40 40").unwrap()),
+                &Rect::from_size(0.0, 0.0),
+            )
+            .unwrap();
+
+        assert_eq!(t, None);
     }
 
     #[test]
     fn empty_viewbox() {
         let a = AspectRatio::default();
-        let t = a.viewport_to_viewbox_transform(
-            Some(ViewBox::parse_str("10 10 0 0").unwrap()),
-            &Rect::from_size(10.0, 10.0),
-        );
-
-        assert_eq!(t, Ok(None));
+        let t = a
+            .viewport_to_viewbox_transform(
+                Some(ViewBox::parse_str("10 10 0 0").unwrap()),
+                &Rect::from_size(10.0, 10.0),
+            )
+            .unwrap();
+
+        assert_eq!(t, None);
     }
 
     #[test]
     fn valid_viewport_and_viewbox() {
         let a = AspectRatio::default();
-        let t = a.viewport_to_viewbox_transform(
-            Some(ViewBox::parse_str("10 10 40 40").unwrap()),
-            &Rect::new(1.0, 1.0, 2.0, 2.0),
-        );
+        let t = a
+            .viewport_to_viewbox_transform(
+                Some(ViewBox::parse_str("10 10 40 40").unwrap()),
+                &Rect::new(1.0, 1.0, 2.0, 2.0),
+            )
+            .unwrap();
 
         assert_eq!(
             t,
-            Ok(Some(
-                Transform::identity()
-                    .pre_translate(1.0, 1.0)
-                    .pre_scale(0.025, 0.025)
-                    .pre_translate(-10.0, -10.0)
-            ))
+            Some(
+                ValidTransform::try_from(
+                    Transform::identity()
+                        .pre_translate(1.0, 1.0)
+                        .pre_scale(0.025, 0.025)
+                        .pre_translate(-10.0, -10.0)
+                )
+                .unwrap()
+            )
         );
     }
 
@@ -458,6 +451,6 @@ mod tests {
             &Rect::new(1.0, 1.0, 2.0, 2.0),
         );
 
-        assert_eq!(t, Err(NonInvertibleTransform));
+        assert_eq!(t, Err(InvalidTransform));
     }
 }
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index 444a7582..a50ab36f 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -43,7 +43,7 @@ use crate::surface_utils::{
     shared_surface::ExclusiveImageSurface, shared_surface::SharedImageSurface,
     shared_surface::SurfaceType,
 };
-use crate::transform::Transform;
+use crate::transform::{Transform, ValidTransform};
 use crate::unit_interval::UnitInterval;
 use crate::viewbox::ViewBox;
 
@@ -117,7 +117,7 @@ pub enum ClipMode {
 /// so that it isn't recalculated every so often.
 struct PathHelper<'a> {
     cr: &'a cairo::Context,
-    transform: Transform,
+    transform: ValidTransform,
     path: &'a Path,
     is_square_linecap: bool,
     has_path: Option<bool>,
@@ -126,7 +126,7 @@ struct PathHelper<'a> {
 impl<'a> PathHelper<'a> {
     pub fn new(
         cr: &'a cairo::Context,
-        transform: Transform,
+        transform: ValidTransform,
         path: &'a Path,
         linecap: StrokeLinecap,
     ) -> Self {
@@ -238,7 +238,12 @@ pub fn draw_tree(
 
     // Translate so (0, 0) is at the viewport's upper-left corner.
     let transform = user_transform.pre_translate(viewport.x0, viewport.y0);
-    cr.set_matrix(transform.into());
+
+    // Here we exit immediately if the transform is not valid, since we are in the
+    // toplevel drawing function.  Downstream cases would simply not render the current
+    // element and ignore the error.
+    let valid_transform = ValidTransform::try_from(transform)?;
+    cr.set_matrix(valid_transform.into());
 
     // Per the spec, so the viewport has (0, 0) as upper-left.
     let viewport = viewport.translate((-viewport.x0, -viewport.y0));
@@ -357,12 +362,14 @@ impl DrawingCtx {
         self.measuring
     }
 
-    fn get_transform(&self) -> Transform {
-        Transform::from(self.cr.matrix())
+    fn get_transform(&self) -> ValidTransform {
+        let t = Transform::from(self.cr.matrix());
+        ValidTransform::try_from(t)
+            .expect("Cairo should already have checked that its current transform is valid")
     }
 
     pub fn empty_bbox(&self) -> BoundingBox {
-        BoundingBox::new().with_transform(self.get_transform())
+        BoundingBox::new().with_transform(*self.get_transform())
     }
 
     fn size_for_temporary_surface(&self) -> (i32, i32) {
@@ -543,9 +550,10 @@ impl DrawingCtx {
             let values = cascaded.get();
 
             let node_transform = values.transform().post_transform(&transform);
+            let transform_for_clip = ValidTransform::try_from(node_transform)?;
 
             let orig_transform = self.get_transform();
-            self.cr.transform(node_transform.into());
+            self.cr.transform(transform_for_clip.into());
 
             for child in node.children().filter(|c| {
                 c.is_element() && element_can_be_used_inside_clip_path(&c.borrow_element())
@@ -607,6 +615,7 @@ impl DrawingCtx {
         let mask_element = mask_node.borrow_element();
 
         let mask_transform = values.transform().post_transform(&transform);
+        let transform_for_mask = ValidTransform::try_from(mask_transform)?;
 
         let mask_content_surface = self.create_surface_for_toplevel_viewport()?;
 
@@ -614,7 +623,7 @@ impl DrawingCtx {
         // reference to the surface before we access the pixels
         {
             let mask_cr = cairo::Context::new(&mask_content_surface)?;
-            mask_cr.set_matrix(mask_transform.into());
+            mask_cr.set_matrix(transform_for_mask.into());
 
             let bbtransform = Transform::new_unchecked(
                 bbox_rect.width(),
@@ -637,8 +646,7 @@ impl DrawingCtx {
                 if bbox_rect.is_empty() {
                     return Ok(None);
                 }
-                assert!(bbtransform.is_invertible());
-                mask_cr.transform(bbtransform.into());
+                mask_cr.transform(ValidTransform::try_from(bbtransform)?.into());
             }
 
             // TODO: this is the last place where push_coord_units() is called.  The call to
@@ -691,20 +699,13 @@ impl DrawingCtx {
         draw_fn: &mut dyn FnMut(
             &mut AcquiredNodes<'_>,
             &mut DrawingCtx,
-            &Transform,
+            &ValidTransform,
         ) -> Result<BoundingBox, RenderingError>,
     ) -> Result<BoundingBox, RenderingError> {
-        if !stacking_ctx.transform.is_invertible() {
-            // https://www.w3.org/TR/css-transforms-1/#transform-function-lists
-            //
-            // "If a transform function causes the current transformation matrix of an
-            // object to be non-invertible, the object and its content do not get
-            // displayed."
-            return Ok(self.empty_bbox());
-        }
+        let stacking_ctx_transform = ValidTransform::try_from(stacking_ctx.transform)?;
 
         let orig_transform = self.get_transform();
-        self.cr.transform(stacking_ctx.transform.into());
+        self.cr.transform(stacking_ctx_transform.into());
 
         let res = if clipping {
             let current_transform = self.get_transform();
@@ -746,7 +747,7 @@ impl DrawingCtx {
                     // Compute our assortment of affines
 
                     let affines = CompositingAffines::new(
-                        affine_at_start,
+                        *affine_at_start,
                         self.initial_viewport.transform,
                         self.cr_stack.borrow().len(),
                     );
@@ -763,7 +764,7 @@ impl DrawingCtx {
                         }
                     };
 
-                    cr.set_matrix(affines.for_temporary_surface.into());
+                    cr.set_matrix(ValidTransform::try_from(affines.for_temporary_surface)?.into());
 
                     let (source_surface, mut res, bbox) = {
                         let mut temporary_draw_ctx = self.nested(cr);
@@ -859,12 +860,15 @@ impl DrawingCtx {
 
                     // Set temporary surface as source
 
-                    self.cr.set_matrix(affines.compositing.into());
+                    self.cr
+                        .set_matrix(ValidTransform::try_from(affines.compositing)?.into());
                     self.cr.set_source_surface(&source_surface, 0.0, 0.0)?;
 
                     // Clip
 
-                    self.cr.set_matrix(affines.outside_temporary_surface.into());
+                    self.cr.set_matrix(
+                        ValidTransform::try_from(affines.outside_temporary_surface)?.into(),
+                    );
                     self.clip_to_node(&stacking_ctx.clip_in_object_space, acquired_nodes, &bbox)?;
 
                     // Mask
@@ -881,7 +885,9 @@ impl DrawingCtx {
                                 if let Some(surf) = mask_surf {
                                     self.cr.push_group();
 
-                                    self.cr.set_matrix(affines.compositing.into());
+                                    self.cr.set_matrix(
+                                        ValidTransform::try_from(affines.compositing)?.into(),
+                                    );
                                     self.cr.mask_surface(&surf, 0.0, 0.0)?;
 
                                     Ok(self.cr.pop_group_to_source()?)
@@ -896,7 +902,8 @@ impl DrawingCtx {
                     {
                         // Composite the temporary surface
 
-                        self.cr.set_matrix(affines.compositing.into());
+                        self.cr
+                            .set_matrix(ValidTransform::try_from(affines.compositing)?.into());
                         self.cr.set_operator(stacking_ctx.mix_blend_mode.into());
 
                         if opacity < 1.0 {
@@ -1003,7 +1010,7 @@ impl DrawingCtx {
                         surface,
                         acquired_nodes,
                         self,
-                        self.get_transform(),
+                        *self.get_transform(),
                         node_bbox,
                     )
                 })
@@ -1022,7 +1029,7 @@ impl DrawingCtx {
         }
     }
 
-    fn set_gradient(&mut self, gradient: &UserSpaceGradient) -> Result<(), cairo::Error> {
+    fn set_gradient(&mut self, gradient: &UserSpaceGradient) -> Result<(), RenderingError> {
         let g = match gradient.variant {
             GradientVariant::Linear { x1, y1, x2, y2 } => {
                 cairo::Gradient::clone(&cairo::LinearGradient::new(x1, y1, x2, y2))
@@ -1038,7 +1045,7 @@ impl DrawingCtx {
             } => cairo::Gradient::clone(&cairo::RadialGradient::new(fx, fy, fr, cx, cy, r)),
         };
 
-        g.set_matrix(gradient.transform.into());
+        g.set_matrix(ValidTransform::try_from(gradient.transform)?.into());
         g.set_extend(cairo::Extend::from(gradient.spread));
 
         for stop in &gradient.stops {
@@ -1053,7 +1060,7 @@ impl DrawingCtx {
             );
         }
 
-        self.cr.set_source(&g)
+        Ok(self.cr.set_source(&g)?)
     }
 
     fn set_pattern(
@@ -1116,7 +1123,7 @@ impl DrawingCtx {
         let cr_pattern = cairo::Context::new(&surface)?;
 
         // Set up transformations to be determined by the contents units
-        cr_pattern.set_matrix(caffine.into());
+        cr_pattern.set_matrix(ValidTransform::try_from(caffine)?.into());
 
         // Draw everything
 
@@ -1156,11 +1163,11 @@ impl DrawingCtx {
         let pattern = cairo::SurfacePattern::create(&surface);
 
         if let Some(m) = affine.invert() {
-            pattern.set_matrix(m.into())
+            pattern.set_matrix(ValidTransform::try_from(m)?.into());
+            pattern.set_extend(cairo::Extend::Repeat);
+            pattern.set_filter(cairo::Filter::Best);
+            self.cr.set_source(&pattern)?;
         }
-        pattern.set_extend(cairo::Extend::Repeat);
-        pattern.set_filter(cairo::Filter::Best);
-        self.cr.set_source(&pattern)?;
 
         Ok(true)
     }
@@ -1212,7 +1219,7 @@ impl DrawingCtx {
     ) -> Result<SharedImageSurface, RenderingError> {
         let mut surface = ExclusiveImageSurface::new(width, height, SurfaceType::SRgb)?;
 
-        surface.draw::<RenderingError>(&mut |cr| {
+        surface.draw(&mut |cr| {
             let mut temporary_draw_ctx = self.nested(cr);
 
             // FIXME: we are ignoring any error
@@ -1330,7 +1337,10 @@ impl DrawingCtx {
                                 path_helper.set()?;
                                 let backup_matrix = if shape.stroke.non_scaling {
                                     let matrix = cr.matrix();
-                                    cr.set_matrix(dc.initial_viewport.transform.into());
+                                    cr.set_matrix(
+                                        ValidTransform::try_from(dc.initial_viewport.transform)?
+                                            .into(),
+                                    );
                                     Some(matrix)
                                 } else {
                                     None
@@ -1552,7 +1562,7 @@ impl DrawingCtx {
         &self,
         width: i32,
         height: i32,
-    ) -> Result<SharedImageSurface, cairo::Error> {
+    ) -> Result<SharedImageSurface, RenderingError> {
         // TODO: as far as I can tell this should not render elements past the last (topmost) one
         // with enable-background: new (because technically we shouldn't have been caching them).
         // Right now there are no enable-background checks whatsoever.
@@ -1570,7 +1580,7 @@ impl DrawingCtx {
         //   https://www.w3.org/TR/compositing-1/#isolation
         let mut surface = ExclusiveImageSurface::new(width, height, SurfaceType::SRgb)?;
 
-        surface.draw::<cairo::Error>(&mut |cr| {
+        surface.draw(&mut |cr| {
             // TODO: apparently DrawingCtx.cr_stack is just a way to store pairs of
             // (surface, transform).  Can we turn it into a DrawingCtx.surface_stack
             // instead?  See what CSS isolation would like to call that; are the pairs just
@@ -1582,7 +1592,7 @@ impl DrawingCtx {
                     depth,
                 );
 
-                cr.set_matrix(affines.for_snapshot.into());
+                cr.set_matrix(ValidTransform::try_from(affines.for_snapshot)?.into());
                 cr.set_source_surface(&draw.target(), 0.0, 0.0)?;
                 cr.paint()?;
             }
@@ -1590,7 +1600,7 @@ impl DrawingCtx {
             Ok(())
         })?;
 
-        surface.share()
+        Ok(surface.share()?)
     }
 
     pub fn draw_node_to_surface(
@@ -1609,7 +1619,7 @@ impl DrawingCtx {
 
         {
             let cr = cairo::Context::new(&surface)?;
-            cr.set_matrix(affine.into());
+            cr.set_matrix(ValidTransform::try_from(affine)?.into());
 
             self.cr = cr;
             self.initial_viewport = Viewport {
@@ -1724,7 +1734,8 @@ impl DrawingCtx {
 
         let orig_transform = self.get_transform();
 
-        self.cr.transform(values.transform().into());
+        self.cr
+            .transform(ValidTransform::try_from(values.transform())?.into());
 
         let use_element = node.borrow_element();
 
@@ -1821,7 +1832,7 @@ impl DrawingCtx {
         self.cr.set_matrix(orig_transform.into());
 
         if let Ok(bbox) = res {
-            let mut res_bbox = BoundingBox::new().with_transform(orig_transform);
+            let mut res_bbox = BoundingBox::new().with_transform(*orig_transform);
             res_bbox.insert(&bbox);
             Ok(res_bbox)
         } else {
@@ -2053,7 +2064,7 @@ fn compute_stroke_and_fill_extents(
     {
         let backup_matrix = if stroke.non_scaling {
             let matrix = cr.matrix();
-            cr.set_matrix(initial_viewport.transform.into());
+            cr.set_matrix(ValidTransform::try_from(initial_viewport.transform)?.into());
             Some(matrix)
         } else {
             None
@@ -2245,10 +2256,10 @@ impl From<cairo::Matrix> for Transform {
     }
 }
 
-impl From<Transform> for cairo::Matrix {
+impl From<ValidTransform> for cairo::Matrix {
     #[inline]
-    fn from(t: Transform) -> Self {
-        Self::new(t.xx, t.yx, t.xy, t.yy, t.x0, t.y0)
+    fn from(t: ValidTransform) -> cairo::Matrix {
+        cairo::Matrix::new(t.xx, t.yx, t.xy, t.yy, t.x0, t.y0)
     }
 }
 
diff --git a/src/error.rs b/src/error.rs
index 62c8e757..5d5cabc3 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -128,6 +128,12 @@ pub enum RenderingError {
     /// A particular implementation-defined limit was exceeded.
     LimitExceeded(ImplementationLimit),
 
+    /// A non-invertible transform was generated.
+    ///
+    /// This should not be a fatal error; we should catch it and just not render
+    /// the problematic element.
+    InvalidTransform,
+
     /// Tried to reference an SVG element that does not exist.
     IdNotFound,
 
@@ -147,6 +153,12 @@ impl From<DefsLookupErrorKind> for RenderingError {
     }
 }
 
+impl From<InvalidTransform> for RenderingError {
+    fn from(_: InvalidTransform) -> RenderingError {
+        RenderingError::InvalidTransform
+    }
+}
+
 impl error::Error for RenderingError {}
 
 impl fmt::Display for RenderingError {
@@ -154,6 +166,7 @@ impl fmt::Display for RenderingError {
         match *self {
             RenderingError::Rendering(ref s) => write!(f, "rendering error: {s}"),
             RenderingError::LimitExceeded(ref l) => write!(f, "{l}"),
+            RenderingError::InvalidTransform => write!(f, "invalid transform"),
             RenderingError::IdNotFound => write!(f, "element id not found"),
             RenderingError::InvalidId(ref s) => write!(f, "invalid id: {s:?}"),
             RenderingError::OutOfMemory(ref s) => write!(f, "out of memory: {s}"),
@@ -167,6 +180,13 @@ impl From<cairo::Error> for RenderingError {
     }
 }
 
+/// Indicates that a transform is not invertible.
+///
+/// This generally represents an error from [`ValidTransform::try_from`], which is what we use
+/// to check affine transforms for validity.
+#[derive(Debug, PartialEq)]
+pub struct InvalidTransform;
+
 /// Errors from [`crate::document::AcquiredNodes`].
 pub enum AcquireError {
     /// An element with the specified id was not found.
diff --git a/src/filters/context.rs b/src/filters/context.rs
index 96babf78..a3b28d69 100644
--- a/src/filters/context.rs
+++ b/src/filters/context.rs
@@ -189,7 +189,7 @@ impl FilterContext {
         let res = self.background_surface.get_or_init(|| {
             draw_ctx
                 .get_snapshot(self.source_surface.width(), self.source_surface.height())
-                .map_err(FilterError::CairoError)
+                .map_err(FilterError::Rendering)
         });
 
         res.as_ref().map(|s| s.clone()).map_err(|e| e.clone())
diff --git a/src/filters/displacement_map.rs b/src/filters/displacement_map.rs
index e3396e5a..773a3135 100644
--- a/src/filters/displacement_map.rs
+++ b/src/filters/displacement_map.rs
@@ -124,7 +124,7 @@ impl DisplacementMap {
             input_1.surface().surface_type(),
         )?;
 
-        surface.draw::<cairo::Error>(&mut |cr| {
+        surface.draw(&mut |cr| {
             for (x, y, displacement_pixel) in Pixels::within(&displacement_surface, bounds) {
                 let get_value = |channel| match channel {
                     ColorChannel::R => displacement_pixel.r,
diff --git a/src/node.rs b/src/node.rs
index c83dcfd8..f1d4191f 100644
--- a/src/node.rs
+++ b/src/node.rs
@@ -315,7 +315,21 @@ impl NodeDraw for Node {
         clipping: bool,
     ) -> Result<BoundingBox, RenderingError> {
         match *self.borrow() {
-            NodeData::Element(ref e) => e.draw(self, acquired_nodes, cascaded, draw_ctx, clipping),
+            NodeData::Element(ref e) => {
+                match e.draw(self, acquired_nodes, cascaded, draw_ctx, clipping) {
+                    Ok(bbox) => Ok(bbox),
+
+                    // https://www.w3.org/TR/css-transforms-1/#transform-function-lists
+                    //
+                    // "If a transform function causes the current transformation matrix of an
+                    // object to be non-invertible, the object and its content do not get
+                    // displayed."
+                    Err(RenderingError::InvalidTransform) => Ok(draw_ctx.empty_bbox()),
+
+                    Err(e) => Err(e),
+                }
+            }
+
             _ => Ok(draw_ctx.empty_bbox()),
         }
     }
diff --git a/src/surface_utils/shared_surface.rs b/src/surface_utils/shared_surface.rs
index 4fdd66c2..b94f2ebb 100644
--- a/src/surface_utils/shared_surface.rs
+++ b/src/surface_utils/shared_surface.rs
@@ -8,6 +8,7 @@ use gdk_pixbuf::{Colorspace, Pixbuf};
 use nalgebra::{storage::Storage, Dim, Matrix};
 use rgb::FromSlice;
 
+use crate::error::*;
 use crate::rect::{IRect, Rect};
 use crate::surface_utils::srgb;
 use crate::util::clamp;
@@ -1375,10 +1376,10 @@ impl ImageSurface<Exclusive> {
 
     /// Draw on the surface using cairo
     #[inline]
-    pub fn draw<E: From<cairo::Error>>(
+    pub fn draw(
         &mut self,
-        draw_fn: &mut dyn FnMut(cairo::Context) -> Result<(), E>,
-    ) -> Result<(), E> {
+        draw_fn: &mut dyn FnMut(cairo::Context) -> Result<(), RenderingError>,
+    ) -> Result<(), RenderingError> {
         let cr = cairo::Context::new(&self.surface)?;
         draw_fn(cr)
     }
diff --git a/src/text.rs b/src/text.rs
index e209d8b2..29d1ccd1 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -24,7 +24,7 @@ use crate::properties::{
 use crate::rect::Rect;
 use crate::session::Session;
 use crate::space::{xml_space_normalize, NormalizeDefault, XmlSpaceNormalize};
-use crate::transform::Transform;
+use crate::transform::{Transform, ValidTransform};
 use crate::xml::Attributes;
 
 /// The state of a text layout operation.
@@ -33,7 +33,7 @@ struct LayoutContext {
     writing_mode: WritingMode,
 
     /// Current transform in the DrawingCtx.
-    transform: Transform,
+    transform: ValidTransform,
 
     /// Font options from the DrawingCtx.
     font_options: FontOptions,
@@ -464,7 +464,7 @@ impl PositionedSpan {
 
         let gravity = layout.context().gravity();
 
-        let bbox = compute_text_box(&layout, x, y, layout_context.transform, gravity);
+        let bbox = compute_text_box(&layout, x, y, *layout_context.transform, gravity);
 
         let stroke_paint = self.values.stroke().0.resolve(
             acquired_nodes,
@@ -831,7 +831,7 @@ impl Draw for Text {
                     }
                 }
 
-                let empty_bbox = BoundingBox::new().with_transform(*transform);
+                let empty_bbox = BoundingBox::new().with_transform(**transform);
 
                 let text_bbox = layout_spans.iter().fold(empty_bbox, |mut bbox, span| {
                     if let Some(ref span_bbox) = span.bbox {
diff --git a/src/transform.rs b/src/transform.rs
index f99f8a05..a042b778 100644
--- a/src/transform.rs
+++ b/src/transform.rs
@@ -13,6 +13,7 @@
 //! [attr]: https://www.w3.org/TR/SVG11/coords.html#TransformAttribute
 
 use cssparser::{Parser, Token};
+use std::ops::Deref;
 
 use crate::angle::Angle;
 use crate::error::*;
@@ -22,6 +23,37 @@ use crate::properties::ComputedValues;
 use crate::property_macros::Property;
 use crate::rect::Rect;
 
+/// A transform that has been checked to be invertible.
+///
+/// We need to validate user-supplied transforms before setting them on Cairo,
+/// so we use this type for that.
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub struct ValidTransform(Transform);
+
+impl TryFrom<Transform> for ValidTransform {
+    type Error = InvalidTransform;
+
+    /// Validates a [`Transform`] before converting it to a [`ValidTransform`].
+    ///
+    /// A transform is valid if it is invertible.  For example, a
+    /// matrix with all-zeros is not invertible, and it is invalid.
+    fn try_from(t: Transform) -> Result<ValidTransform, InvalidTransform> {
+        if t.is_invertible() {
+            Ok(ValidTransform(t))
+        } else {
+            Err(InvalidTransform)
+        }
+    }
+}
+
+impl Deref for ValidTransform {
+    type Target = Transform;
+
+    fn deref(&self) -> &Transform {
+        &self.0
+    }
+}
+
 /// A 2D transformation matrix.
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct Transform {
diff --git a/tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform-ref.svg b/tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform-ref.svg
new file mode 100644
index 00000000..cae82675
--- /dev/null
+++ b/tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform-ref.svg
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200">
+  <!-- Nothing; the test file should render nothing, so this is empty -->
+</svg>
diff --git a/tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform.svg b/tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform.svg
new file mode 100644
index 00000000..2c834fd5
--- /dev/null
+++ b/tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform.svg
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200">
+  <!-- This clipPath has an invalid transform. We should just not render the circle below,
+       but not exit with an error. -->
+  <clipPath id="clip" transform="scale(0)">
+    <rect x="100" y="0" width="100" height="100"/>
+  </clipPath>
+  <circle cx="100" cy="100" r="50" fill="green" clip-path="url(#clip)"/>
+</svg>
diff --git a/tests/src/reference.rs b/tests/src/reference.rs
index 90d5d0ab..837bf1d8 100644
--- a/tests/src/reference.rs
+++ b/tests/src/reference.rs
@@ -1145,3 +1145,9 @@ test_svg_reference!(
     "tests/fixtures/reftests/bugs-reftests/bug885-vector-effect-non-scaling-stroke.svg",
     "tests/fixtures/reftests/bugs-reftests/bug885-vector-effect-non-scaling-stroke-ref.svg"
 );
+
+test_svg_reference!(
+    bug_930_invalid_clip_path_transform,
+    "tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform.svg",
+    "tests/fixtures/reftests/bugs-reftests/bug930-invalid-clip-path-transform-ref.svg"
+);