Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with relative units in symbols #832

Merged
merged 6 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/resvg/tests/integration/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,7 @@ use crate::render;
#[test] fn structure_symbol_unused_symbol() { assert_eq!(render("tests/structure/symbol/unused-symbol"), 0); }
#[test] fn structure_symbol_with_custom_use_size() { assert_eq!(render("tests/structure/symbol/with-custom-use-size"), 0); }
#[test] fn structure_symbol_with_overflow_visible() { assert_eq!(render("tests/structure/symbol/with-overflow-visible"), 0); }
#[test] fn structure_symbol_with_size_on_use_and_relative_units() { assert_eq!(render("tests/structure/symbol/with-size-on-use-and-relative-units"), 0); }
#[test] fn structure_symbol_with_transform_on_use_no_size() { assert_eq!(render("tests/structure/symbol/with-transform-on-use-no-size"), 0); }
#[test] fn structure_symbol_with_transform_on_use() { assert_eq!(render("tests/structure/symbol/with-transform-on-use"), 0); }
#[test] fn structure_symbol_with_transform() { assert_eq!(render("tests/structure/symbol/with-transform"), 0); }
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 5 additions & 4 deletions crates/usvg/src/parser/units.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ pub(crate) fn convert_length(
if object_units == Units::ObjectBoundingBox {
n / 100.0
} else {
let view_box = state.view_box;
let width = state.use_size.0.unwrap_or(state.view_box.width());
LaurenzV marked this conversation as resolved.
Show resolved Hide resolved
let height = state.use_size.1.unwrap_or(state.view_box.height());

match aid {
AId::Cx
Expand All @@ -43,7 +44,7 @@ pub(crate) fn convert_length(
| AId::Width
| AId::X
| AId::X1
| AId::X2 => convert_percent(length, view_box.width()),
| AId::X2 => convert_percent(length, width),
AId::Cy
| AId::Dy
| AId::Fy
Expand All @@ -53,9 +54,9 @@ pub(crate) fn convert_length(
| AId::Ry
| AId::Y
| AId::Y1
| AId::Y2 => convert_percent(length, view_box.height()),
| AId::Y2 => convert_percent(length, height),
_ => {
let mut vb_len = view_box.width().powi(2) + view_box.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
50 changes: 28 additions & 22 deletions crates/usvg/src/parser/use_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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 @@ -53,7 +54,33 @@ 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 {
set_use_size(&mut use_state);
RazrFalcon marked this conversation as resolved.
Show resolved Hide resolved

if let Some(ts) = viewbox_transform(node, child, &use_state) {
new_ts = new_ts.pre_concat(ts);
}
Expand Down Expand Up @@ -105,28 +132,7 @@ 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.

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));
}
set_use_size(&mut use_state);

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