Skip to content

Commit

Permalink
Fix defs id generation.
Browse files Browse the repository at this point in the history
  • Loading branch information
RazrFalcon committed Feb 11, 2024
1 parent 96eec97 commit 1961564
Show file tree
Hide file tree
Showing 12 changed files with 146 additions and 22 deletions.
14 changes: 9 additions & 5 deletions crates/usvg/src/parser/clippath.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ pub(crate) fn convert(
// Only `userSpaceOnUse` clipPaths can be shared,
// because `objectBoundingBox` one will be converted into user one
// and will become node-specific.
if units == Units::UserSpaceOnUse {
let cacheable = units == Units::UserSpaceOnUse;
if cacheable {
if let Some(clip) = cache.clip_paths.get(node.element_id()) {
return Some(clip.clone());
}
Expand Down Expand Up @@ -62,7 +63,12 @@ pub(crate) fn convert(
}
}

let id = NonEmptyString::new(node.element_id().to_string())?;
let mut id = NonEmptyString::new(node.element_id().to_string())?;
// Generate ID only when we're parsing `objectBoundingBox` clip for the second time.
if !cacheable && cache.clip_paths.contains_key(id.get()) {
id = cache.gen_clip_path_id();
}
let id_copy = id.get().to_string();

let mut clip = ClipPath {
id,
Expand All @@ -78,9 +84,7 @@ pub(crate) fn convert(
if clip.root.has_children() {
clip.root.calculate_bounding_boxes();
let clip = Arc::new(clip);
cache
.clip_paths
.insert(node.element_id().to_string(), clip.clone());
cache.clip_paths.insert(id_copy, clip.clone());
Some(clip)
} else {
// A clip path without children is invalid.
Expand Down
22 changes: 21 additions & 1 deletion crates/usvg/src/parser/converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub struct Cache {
#[cfg(feature = "text")]
pattern_index: usize,
clip_path_index: usize,
mask_index: usize,
filter_index: usize,
}

Expand Down Expand Up @@ -98,6 +99,17 @@ impl Cache {
}
}

pub(crate) fn gen_mask_id(&mut self) -> NonEmptyString {
loop {
self.mask_index += 1;
let new_id = format!("mask{}", self.mask_index);
let new_hash = string_hash(&new_id);
if !self.all_ids.contains(&new_hash) {
return NonEmptyString::new(new_id).unwrap();
}
}
}

pub(crate) fn gen_filter_id(&mut self) -> NonEmptyString {
loop {
self.filter_index += 1;
Expand Down Expand Up @@ -275,7 +287,15 @@ pub(crate) fn convert_doc(

for node in svg_doc.descendants() {
if let Some(tag) = node.tag_name() {
if matches!(tag, EId::Filter | EId::ClipPath) {
if matches!(
tag,
EId::ClipPath
| EId::Filter
| EId::LinearGradient
| EId::Mask
| EId::Pattern
| EId::RadialGradient
) {
if !node.element_id().is_empty() {
cache.all_ids.insert(string_hash(node.element_id()));
}
Expand Down
14 changes: 9 additions & 5 deletions crates/usvg/src/parser/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ fn convert_url(
// Only `userSpaceOnUse` clipPaths can be shared,
// because `objectBoundingBox` one will be converted into user one
// and will become node-specific.
if units == Units::UserSpaceOnUse && primitive_units == Units::UserSpaceOnUse {
let cacheable = units == Units::UserSpaceOnUse && primitive_units == Units::UserSpaceOnUse;
if cacheable {
if let Some(filter) = cache.filters.get(node.element_id()) {
return Ok(Some(filter.clone()));
}
Expand Down Expand Up @@ -247,17 +248,20 @@ fn convert_url(
return Err(());
}

let id = NonEmptyString::new(node.element_id().to_string()).ok_or(())?;
let mut id = NonEmptyString::new(node.element_id().to_string()).ok_or(())?;
// Generate ID only when we're parsing `objectBoundingBox` filter for the second time.
if !cacheable && cache.filters.contains_key(id.get()) {
id = cache.gen_filter_id();
}
let id_copy = id.get().to_string();

let filter = Arc::new(Filter {
id,
rect,
primitives,
});

cache
.filters
.insert(node.element_id().to_string(), filter.clone());
cache.filters.insert(id_copy, filter.clone());

Ok(Some(filter))
}
Expand Down
7 changes: 5 additions & 2 deletions crates/usvg/src/parser/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,14 @@ pub(crate) fn convert(node: SvgNode, state: &converter::State, parent: &mut Grou
Length::new_number(actual_size.height() as f64),
);

match (node.attribute::<Length>(AId::Width), node.attribute::<Length>(AId::Height)) {
match (
node.attribute::<Length>(AId::Width),
node.attribute::<Length>(AId::Height),
) {
(Some(_), None) => {
// Only width was defined, so we need to scale height accordingly.
height = actual_size.height() * (width / actual_size.width());
},
}
(None, Some(_)) => {
// Only height was defined, so we need to scale width accordingly.
width = actual_size.width() * (height / actual_size.height());
Expand Down
23 changes: 14 additions & 9 deletions crates/usvg/src/parser/mask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@ pub(crate) fn convert(
// 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 {
let cacheable = units == Units::UserSpaceOnUse && content_units == Units::UserSpaceOnUse;
if cacheable {
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)),
Expand All @@ -63,15 +62,23 @@ pub(crate) fn convert(
}
}

let mut id = NonEmptyString::new(node.element_id().to_string())?;
// Generate ID only when we're parsing `objectBoundingBox` mask for the second time.
if !cacheable && cache.masks.contains_key(id.get()) {
id = cache.gen_mask_id();
}
let id_copy = id.get().to_string();

if mask_all {
let mask = Mask {
let mask = Arc::new(Mask {
id,
rect,
kind: MaskType::Luminance,
mask: None,
root: Group::empty(),
};
return Some(Arc::new(mask));
});
cache.masks.insert(id_copy, mask.clone());
return Some(mask);
}

// Resolve linked mask.
Expand Down Expand Up @@ -139,8 +146,6 @@ pub(crate) fn convert(
mask.root.calculate_bounding_boxes();

let mask = Arc::new(mask);
cache
.masks
.insert(node.element_id().to_string(), mask.clone());
cache.masks.insert(id_copy, mask.clone());
Some(mask)
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions crates/usvg/tests/files/mask-with-object-units-multi-use.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions crates/usvg/tests/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ fn filter_id_with_prefix() {
resave_with_prefix("filter-id-with-prefix", "prefix-");
}

#[test]
fn filter_with_object_units_multi_use() {
resave("filter-with-object-units-multi-use");
}

#[test]
fn preserve_id_clip_path_v1() {
resave("preserve-id-clip-path-v1");
Expand Down Expand Up @@ -125,6 +130,16 @@ fn clip_path_with_complex_text() {
resave("clip-path-with-complex-text");
}

#[test]
fn clip_path_with_object_units_multi_use() {
resave("clip-path-with-object-units-multi-use");
}

#[test]
fn mask_with_object_units_multi_use() {
resave("mask-with-object-units-multi-use");
}

#[test]
fn text_with_generated_gradients() {
resave("text-with-generated-gradients");
Expand Down

0 comments on commit 1961564

Please sign in to comment.