usvg::Mask is always in userSpaceOnUse units now.

CHANGELOG.md

@@ -19,7 +19,8 @@ This changelog also contains important changes in dependencies.
 - All `usvg::Tree` parsing methods require the `fontdb` argument now.
 - All `defs` children like gradients, patterns, clipPaths, masks and filters are guarantee
   to have a unique, non-empty ID.
-- `usvg::ClipPath` is always in `userSpaceOnUse` units now.
+- `usvg::ClipPath` and `usvg::Mask` are always in `userSpaceOnUse` units now.
+- `usvg::Mask` is allowed to have no children now.
 - Text nodes will not be parsed when the `text` build feature isn't enabled.
 - `usvg::Tree::clip_paths`, `usvg::Tree::masks`, `usvg::Tree::filters` returns
   a pre-collected slice of unique nodes now.
@@ -38,7 +39,9 @@ This changelog also contains important changes in dependencies.
 
 ### Removed
 - `usvg::Tree::postprocess()` and `usvg::PostProcessingSteps`. No longer needed.
-- `usvg::ClipPath::units()`. It's always `userSpaceOnUse` now.
+- `usvg::ClipPath::units()`, `usvg::Mask::units()`, `usvg::Mask::content_units()`.
+  They are always `userSpaceOnUse` now.
+
 
 ### Fixed
 - Text bounding box is accounted during SVG size resolving.

crates/resvg/src/mask.rs

@@ -11,59 +11,25 @@ pub fn apply(
     transform: tiny_skia::Transform,
     pixmap: &mut tiny_skia::Pixmap,
 ) {
-    let mut content_transform = tiny_skia::Transform::default();
-    if mask.content_units() == usvg::Units::ObjectBoundingBox {
-        let object_bbox = match object_bbox.to_non_zero_rect() {
-            Some(v) => v,
-            None => {
-                log::warn!("Masking of zero-sized shapes is not allowed.");
-                return;
-            }
-        };
-
-        let ts = usvg::Transform::from_bbox(object_bbox);
-        content_transform = ts;
-    }
-
-    if mask.units() == usvg::Units::ObjectBoundingBox && object_bbox.to_non_zero_rect().is_none() {
-        // `objectBoundingBox` units and zero-sized bbox? Clear the canvas and return.
-        // Technically a UB, but this is what Chrome and Firefox do.
+    if mask.root().children().is_empty() {
         pixmap.fill(tiny_skia::Color::TRANSPARENT);
         return;
     }
 
-    let region = if mask.units() == usvg::Units::ObjectBoundingBox {
-        if let Some(bbox) = object_bbox.to_non_zero_rect() {
-            mask.rect().bbox_transform(bbox)
-        } else {
-            // The actual values does not matter. Will not be used anyway.
-            tiny_skia::NonZeroRect::from_xywh(0.0, 0.0, 1.0, 1.0).unwrap()
-        }
-    } else {
-        mask.rect()
-    };
-
     let mut mask_pixmap = tiny_skia::Pixmap::new(pixmap.width(), pixmap.height()).unwrap();
 
     {
         // TODO: only when needed
         // Mask has to be clipped by mask.region
         let mut alpha_mask = tiny_skia::Mask::new(pixmap.width(), pixmap.height()).unwrap();
         alpha_mask.fill_path(
-            &tiny_skia::PathBuilder::from_rect(region.to_rect()),
+            &tiny_skia::PathBuilder::from_rect(mask.rect().to_rect()),
             tiny_skia::FillRule::Winding,
             true,
             transform,
         );
 
-        let content_transform = transform.pre_concat(content_transform);
-        crate::render::render_nodes(
-            mask.root(),
-            ctx,
-            content_transform,
-            None,
-            &mut mask_pixmap.as_mut(),
-        );
+        crate::render::render_nodes(mask.root(), ctx, transform, None, &mut mask_pixmap.as_mut());
 
         mask_pixmap.apply_mask(&alpha_mask);
     }

crates/usvg/docs/spec.adoc

@@ -208,10 +208,7 @@ Doesn't have a `xlink:href` attribute because all attributes and children will b
 * `height` = <<positive-number-type,<positive-number> >>
 * `mask-type` = `alpha`? +
   Default: luminance
-* `maskUnits` = `userSpaceOnUse`? +
-  Default: objectBoundingBox
-* `maskContentUnits` = `objectBoundingBox`? +
-  Default: userSpaceOnUse
+* `maskUnits` = `userSpaceOnUse`
 
 [[filter-element]]
 

crates/usvg/src/parser/clippath.rs

@@ -7,12 +7,12 @@ use std::sync::Arc;
 
 use super::converter;
 use super::svgtree::{AId, EId, SvgNode};
-use crate::{ClipPath, Group, NonEmptyString, Rect, Transform, Units};
+use crate::{ClipPath, Group, NonEmptyString, NonZeroRect, Transform, Units};
 
 pub(crate) fn convert(
     node: SvgNode,
     state: &converter::State,
-    object_bbox: Option<Rect>,
+    object_bbox: Option<NonZeroRect>,
     cache: &mut converter::Cache,
 ) -> Option<Arc<ClipPath>> {
     // A `clip-path` attribute must reference a `clipPath` element.
@@ -39,7 +39,7 @@ pub(crate) fn convert(
     }
 
     if units == Units::ObjectBoundingBox {
-        let object_bbox = match object_bbox?.to_non_zero_rect() {
+        let object_bbox = match object_bbox {
             Some(v) => v,
             None => {
                 log::warn!("Clipping of zero-sized shapes is not allowed.");

crates/usvg/src/parser/converter.rs

@@ -626,7 +626,7 @@ pub(crate) fn convert_group(
     let mut mask = None;
     if state.parent_clip_path.is_none() {
         if let Some(link) = node.attribute::<SvgNode>(AId::Mask) {
-            mask = (super::mask::convert)(link, state, cache);
+            mask = super::mask::convert(link, state, object_bbox, cache);
             if mask.is_none() {
                 return None;
             }

crates/usvg/src/parser/mask.rs

@@ -8,25 +8,19 @@ use svgtypes::{Length, LengthUnit as Unit};
 
 use super::svgtree::{AId, EId, SvgNode};
 use super::{converter, OptionLog};
-use crate::{Group, Mask, MaskType, NonEmptyString, NonZeroRect, Units};
+use crate::{Group, Mask, MaskType, Node, NonEmptyString, NonZeroRect, Transform, Units};
 
 pub(crate) fn convert(
     node: SvgNode,
     state: &converter::State,
+    object_bbox: Option<NonZeroRect>,
     cache: &mut converter::Cache,
 ) -> Option<Arc<Mask>> {
     // A `mask` attribute must reference a `mask` element.
     if node.tag_name() != Some(EId::Mask) {
         return None;
     }
 
-    // Check if this element was already converted.
-    if let Some(mask) = cache.masks.get(node.element_id()) {
-        return Some(mask.clone());
-    }
-
-    let id = NonEmptyString::new(node.element_id().to_string())?;
-
     let units = node
         .attribute(AId::MaskUnits)
         .unwrap_or(Units::ObjectBoundingBox);
@@ -35,19 +29,55 @@ pub(crate) fn convert(
         .attribute(AId::MaskContentUnits)
         .unwrap_or(Units::UserSpaceOnUse);
 
+    // Check if this element was already converted.
+    //
+    // Only `userSpaceOnUse` masks can be shared,
+    // because `objectBoundingBox` one will be converted into user one
+    // and will become node-specific.
+    if units == Units::UserSpaceOnUse && content_units == Units::UserSpaceOnUse {
+        if let Some(mask) = cache.masks.get(node.element_id()) {
+            return Some(mask.clone());
+        }
+    }
+
+    let id = NonEmptyString::new(node.element_id().to_string())?;
+
     let rect = NonZeroRect::from_xywh(
         node.convert_length(AId::X, units, state, Length::new(-10.0, Unit::Percent)),
         node.convert_length(AId::Y, units, state, Length::new(-10.0, Unit::Percent)),
         node.convert_length(AId::Width, units, state, Length::new(120.0, Unit::Percent)),
         node.convert_length(AId::Height, units, state, Length::new(120.0, Unit::Percent)),
     );
-    let rect =
+    let mut rect =
         rect.log_none(|| log::warn!("Mask '{}' has an invalid size. Skipped.", node.element_id()))?;
 
+    let mut mask_all = false;
+    if units == Units::ObjectBoundingBox {
+        if let Some(bbox) = object_bbox {
+            rect = rect.bbox_transform(bbox)
+        } else {
+            // When mask units are `objectBoundingBox` and bbox is zero-sized - the whole
+            // element should be masked.
+            // Technically an UB, but this is what Chrome and Firefox do.
+            mask_all = true;
+        }
+    }
+
+    if mask_all {
+        let mask = Mask {
+            id,
+            rect,
+            kind: MaskType::Luminance,
+            mask: None,
+            root: Group::empty(),
+        };
+        return Some(Arc::new(mask));
+    }
+
     // Resolve linked mask.
     let mut mask = None;
     if let Some(link) = node.attribute::<SvgNode>(AId::Mask) {
-        mask = convert(link, state, cache);
+        mask = convert(link, state, object_bbox, cache);
 
         // Linked `mask` must be valid.
         if mask.is_none() {
@@ -63,25 +93,54 @@ pub(crate) fn convert(
 
     let mut mask = Mask {
         id,
-        units,
-        content_units,
         rect,
         kind,
         mask,
         root: Group::empty(),
     };
 
-    converter::convert_children(node, state, cache, &mut mask.root);
+    // To emulate content `objectBoundingBox` units we have to put
+    // mask children into a group with a transform.
+    let mut subroot = None;
+    if content_units == Units::ObjectBoundingBox {
+        let object_bbox = match object_bbox {
+            Some(v) => v,
+            None => {
+                log::warn!("Masking of zero-sized shapes is not allowed.");
+                return None;
+            }
+        };
 
-    if mask.root.has_children() {
-        mask.root.calculate_bounding_boxes();
-        let mask = Arc::new(mask);
-        cache
-            .masks
-            .insert(node.element_id().to_string(), mask.clone());
-        Some(mask)
-    } else {
-        // A mask without children is invalid.
-        None
+        let mut g = Group::empty();
+        g.transform = Transform::from_bbox(object_bbox);
+        // Make sure to set `abs_transform`, because it must propagate to all children.
+        g.abs_transform = g.transform;
+
+        subroot = Some(g);
     }
+
+    {
+        // Prefer `subroot` to `mask.root`.
+        let real_root = subroot.as_mut().unwrap_or(&mut mask.root);
+        converter::convert_children(node, state, cache, real_root);
+
+        // A mask without children at this point is invalid.
+        // Only masks with zero bbox and `objectBoundingBox` can be empty.
+        if !real_root.has_children() {
+            return None;
+        }
+    }
+
+    if let Some(mut subroot) = subroot {
+        subroot.calculate_bounding_boxes();
+        mask.root.children.push(Node::Group(Box::new(subroot)));
+    }
+
+    mask.root.calculate_bounding_boxes();
+
+    let mask = Arc::new(mask);
+    cache
+        .masks
+        .insert(node.element_id().to_string(), mask.clone());
+    Some(mask)
 }

crates/usvg/src/tree/mod.rs

@@ -826,8 +826,6 @@ impl Default for MaskType {
 #[derive(Debug)]
 pub struct Mask {
     pub(crate) id: NonEmptyString,
-    pub(crate) units: Units,
-    pub(crate) content_units: Units,
     pub(crate) rect: NonZeroRect,
     pub(crate) kind: MaskType,
     pub(crate) mask: Option<Arc<Mask>>,
@@ -843,20 +841,6 @@ impl Mask {
         self.id.get()
     }
 
-    /// Coordinate system units.
-    ///
-    /// `maskUnits` in SVG.
-    pub fn units(&self) -> Units {
-        self.units
-    }
-
-    /// Content coordinate system units.
-    ///
-    /// `maskContentUnits` in SVG.
-    pub fn content_units(&self) -> Units {
-        self.content_units
-    }
-
     /// Mask rectangle.
     ///
     /// `x`, `y`, `width` and `height` in SVG.
@@ -879,6 +863,8 @@ impl Mask {
     }
 
     /// Mask children.
+    ///
+    /// A mask can have no children, in which case the whole element should be masked out.
     pub fn root(&self) -> &Group {
         &self.root
     }
@@ -1796,7 +1782,7 @@ impl Group {
         }
     }
 
-    pub(crate) fn calculate_object_bbox(&mut self) -> Option<Rect> {
+    pub(crate) fn calculate_object_bbox(&mut self) -> Option<NonZeroRect> {
         let mut bbox = BBox::default();
         for child in &self.children {
             let mut c_bbox = child.bounding_box();
@@ -1809,7 +1795,7 @@ impl Group {
             bbox = bbox.expand(c_bbox);
         }
 
-        bbox.to_rect()
+        bbox.to_non_zero_rect()
     }
 
     pub(crate) fn calculate_bounding_boxes(&mut self) -> Option<()> {

crates/usvg/src/writer.rs

@@ -528,11 +528,10 @@ fn write_defs(tree: &Tree, opt: &XmlOptions, xml: &mut XmlWriter) {
         if mask.kind == MaskType::Alpha {
             xml.write_svg_attribute(AId::MaskType, "alpha");
         }
-        xml.write_units(AId::MaskUnits, mask.units, Units::ObjectBoundingBox);
         xml.write_units(
-            AId::MaskContentUnits,
-            mask.content_units,
+            AId::MaskUnits,
             Units::UserSpaceOnUse,
+            Units::ObjectBoundingBox,
         );
         xml.write_rect_attrs(mask.rect);