Skip to content

Commit

Permalink
Apply code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@LaurenzV
LaurenzV committed Oct 14, 2024
1 parent 7d52057 commit 6efa16b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 34 deletions.
11 changes: 5 additions & 6 deletions crates/usvg/src/parser/units.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ pub(crate) fn convert_length(
if object_units == Units::ObjectBoundingBox {
n / 100.0
} else {
// Prefer the dimensions of a `use` node, if existing.
let viewbox_width = state.use_size.0.unwrap_or(state.view_box.width());
let viewbox_height = state.use_size.1.unwrap_or(state.view_box.height());
let width = state.use_size.0.unwrap_or(state.view_box.width());
let height = state.use_size.1.unwrap_or(state.view_box.height());

match aid {
AId::Cx
Expand All @@ -45,7 +44,7 @@ pub(crate) fn convert_length(
| AId::Width
| AId::X
| AId::X1
| AId::X2 => convert_percent(length, viewbox_width),
| AId::X2 => convert_percent(length, width),
AId::Cy
| AId::Dy
| AId::Fy
Expand All @@ -55,9 +54,9 @@ pub(crate) fn convert_length(
| AId::Ry
| AId::Y
| AId::Y1
| AId::Y2 => convert_percent(length, viewbox_height),
| AId::Y2 => convert_percent(length, height),
_ => {
let mut vb_len = viewbox_width.powi(2) + viewbox_height.powi(2);
let mut vb_len = width.powi(2) + height.powi(2);
vb_len = (vb_len / 2.0).sqrt();
convert_percent(length, vb_len)
}
Expand Down
71 changes: 43 additions & 28 deletions crates/usvg/src/parser/use_node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use svgtypes::{Length, LengthUnit};

use super::svgtree::{AId, EId, SvgNode};
use super::{converter, style};
use crate::parser::converter::State;
use crate::tree::ContextElement;
use crate::{Group, IsValidLength, Node, NonZeroRect, Path, Size, Transform, ViewBox};

Expand Down Expand Up @@ -54,36 +53,32 @@ pub(crate) fn convert(

let linked_to_symbol = child.tag_name() == Some(EId::Symbol);

let set_use_size = |use_state: &mut State| {
let def = Length::new(100.0, LengthUnit::Percent);
// As per usual, the SVG spec doesn't clarify this edge case,
// but it seems like `use` size has to be reset by each `use`.
// Meaning if we have two nested `use` elements, where one had set `width` and
// other set `height`, we have to ignore the first `width`.
//
// Example:
// <use id="use1" xlink:href="#use2" width="100"/>
// <use id="use2" xlink:href="#svg2" height="100"/>
// <svg id="svg2" x="40" y="40" width="80" height="80" xmlns="http://www.w3.org/2000/svg"/>
//
// In this case `svg2` size is 80x100 and not 100x100.
use_state.use_size = (None, None);

// Width and height can be set independently.
if node.has_attribute(AId::Width) {
use_state.use_size.0 = Some(node.convert_user_length(AId::Width, &use_state, def));
}
if node.has_attribute(AId::Height) {
use_state.use_size.1 = Some(node.convert_user_length(AId::Height, &use_state, def));
}
};

if linked_to_symbol {
// If a `use` element has a width/height attribute and references a symbol
// then relative units (like percentages) should be resolved relative
// to the width/height of the `use` element, and not the original SVG.
// This is why we need to set the `use_size` here.
set_use_size(&mut use_state);
// This is why we need to (potentially) adapt the view box here.
use_state.view_box = {
let def = Length::new(100.0, LengthUnit::Percent);
let x = use_state.view_box.x();
let y = use_state.view_box.y();

let width = if node.has_attribute(AId::Width) {
node.convert_user_length(AId::Width, &use_state, def)
} else {
use_state.view_box.width()
};

let height = if node.has_attribute(AId::Height) {
node.convert_user_length(AId::Height, &use_state, def)
} else {
use_state.view_box.height()
};

NonZeroRect::from_xywh(x, y, width, height)
// Fail silently if the rect is not valid.
.unwrap_or(use_state.view_box)
};

if let Some(ts) = viewbox_transform(node, child, &use_state) {
new_ts = new_ts.pre_concat(ts);
Expand Down Expand Up @@ -136,7 +131,27 @@ pub(crate) fn convert(
// When a `use` element references a `svg` element,
// we have to remember `use` element size and use it
// instead of `svg` element size.
set_use_size(&mut use_state);
let def = Length::new(100.0, LengthUnit::Percent);
// As per usual, the SVG spec doesn't clarify this edge case,
// but it seems like `use` size has to be reset by each `use`.
// Meaning if we have two nested `use` elements, where one had set `width` and
// other set `height`, we have to ignore the first `width`.
//
// Example:
// <use id="use1" xlink:href="#use2" width="100"/>
// <use id="use2" xlink:href="#svg2" height="100"/>
// <svg id="svg2" x="40" y="40" width="80" height="80" xmlns="http://www.w3.org/2000/svg"/>
//
// In this case `svg2` size is 80x100 and not 100x100.
use_state.use_size = (None, None);

// Width and height can be set independently.
if node.has_attribute(AId::Width) {
use_state.use_size.0 = Some(node.convert_user_length(AId::Width, &use_state, def));
}
if node.has_attribute(AId::Height) {
use_state.use_size.1 = Some(node.convert_user_length(AId::Height, &use_state, def));
}

convert_children(node, orig_ts, &use_state, cache, true, parent);
} else {
Expand Down

0 comments on commit 6efa16b

Please sign in to comment.