Merge branch 'check-clip-mask-transforms' into 'main'

(#930): Validate all clipPath and mask transforms

Closes #930

See merge request GNOME/librsvg!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"
+);