8000 layout: Improve sizing for inline SVG · servo/servo@1276c5b · GitHub
[go: up one dir, main page]
Skip to content

Commit 1276c5b

Browse files
LoirooriolLoirooriol
committed
layout: Improve sizing for inline SVG
The metadata provided by usvg has unreliable sizes. Ignore it, and rely on the `width`, `height` and `viewBox` attributes instead. Note that inline SVG with a natural aspect ratio but no natural sizes should stretch to the containing block. This is left for a follow-up. Signed-off-by: Oriol Brufau <obrufau@igalia.com>
1 parent 82ca2b9 commit 1276c5b

File tree

82 files changed

+169
-269
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+169
-269
lines changed

‎Cargo.lock‎

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,28 +124,28 @@ rustls = { version = "0.23", default-features = false, features = ["logging", "s
124124
rustls-pemfile = "2.0"
125125
rustls-pki-types = "1.12"
126126
script_traits = { path = "components/shared/script" }
127-
selectors = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
127+
selectors = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
128128
serde = "1.0.219"
129129
serde_bytes = "0.11"
130130
serde_json = "1.0"
131131
servo-media = { git = "https://github.com/servo/media" }
132132
servo-media-dummy = { git = "https://github.com/servo/media" }
133133
servo-media-gstreamer = { git = "https://github.com/servo/media" }
134134
servo-tracing = { path = "components/servo_tracing" }
135-
servo_arc = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
135+
servo_arc = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
136136
smallbitvec = "2.6.0"
137137
smallvec = { version = "1.15", features = ["serde", "union"] }
138138
static_assertions = "1.1"
139139
string_cache = "0.8"
140140
string_cache_codegen = "0.5"
141141
strum = "0.26"
142142
strum_macros = "0.26"
143-
stylo = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
144-
stylo_atoms = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
145-
stylo_config = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
146-
stylo_dom = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
147-
stylo_malloc_size_of = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
148-
stylo_traits = { git = "https://github.com/servo/stylo", branch = "2025-08-01" }
143+
stylo = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
144+
stylo_atoms = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
145+
stylo_config = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
146+
stylo_dom = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
147+
stylo_malloc_size_of = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
148+
stylo_traits = { git = "https://github.com/servo/stylo", rev = "refs/pull/229/head" }
149149
surfman = { git = "https://github.com/servo/surfman", rev = "f7688b4585f9e0b5d4bf8ee8e4a91e82349610b1", features = ["chains"] }
150150
syn = { version = "2", default-features = false, features = ["clone-impls", "derive", "parsing"] }
151151
synstructure = "0.13"

‎components/layout/replaced.rs‎

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
44

5-
use app_units::Au;
5+
use app_units::{Au, MAX_AU};
66
use base::id::{BrowsingContextId, PipelineId};
77
use data_url::DataUrl;
88
use embedder_traits::ViewportDetails;
@@ -86,6 +86,16 @@ impl NaturalSizes {
8686
}
8787
}
8888

89+
pub(crate) fn from_natural_size_in_dots(natural_size_in_dots: PhysicalSize<f64>) -> Self {
90+
// FIXME: should 'image-resolution' (when implemented) be used *instead* of
91+
// `script::dom::htmlimageelement::ImageRequest::current_pixel_density`?
92+
// https://drafts.csswg.org/css-images-4/#the-image-resolution
93+
let dppx = 1.0;
94+
let width = natural_size_in_dots.width as f32 / dppx;
95+
let height = natural_size_in_dots.height as f32 / dppx;
96+
Self::from_width_and_height(width, height)
97+
}
98+
8999
pub(crate) fn empty() -> Self {
90100
Self {
91101
width: None,
@@ -117,7 +127,7 @@ pub(crate) enum ReplacedContentKind {
117127
IFrame(IFrameInfo),
118128
Canvas(CanvasInfo),
119129
Video(Option<VideoInfo>),
120-
SVGElement(VectorImage),
130+
SVGElement(Option<VectorImage>),
121131
}
122132

123133
impl ReplacedContents {
@@ -132,29 +142,30 @@ impl ReplacedContents {
132142
}
133143
}
134144

135-
let (kind, natural_size_in_dots) = {
145+
let (kind, natural_size) = {
136146
if let Some((image, natural_size_in_dots)) = element.as_image() {
137147
(
138148
ReplacedContentKind::Image(image),
139-
Some(natural_size_in_dots),
149+
NaturalSizes::from_natural_size_in_dots(natural_size_in_dots),
140150
)
141151
} else if let Some((canvas_info, natural_size_in_dots)) = element.as_canvas() {
142152
(
143153
ReplacedContentKind::Canvas(canvas_info),
144-
Some(natural_size_in_dots),
154+
NaturalSizes::from_natural_size_in_dots(natural_size_in_dots),
145155
)
146156
} else if let Some((pipeline_id, browsing_context_id)) = element.as_iframe() {
147157
(
148158
ReplacedContentKind::IFrame(IFrameInfo {
149159
pipeline_id,
150160
browsing_context_id,
151161
}),
152-
None,
162+
NaturalSizes::empty(),
153163
)
154164
} else if let Some((image_key, natural_size_in_dots)) = element.as_video() {
155165
(
156166
ReplacedContentKind::Video(image_key.map(|key| VideoInfo { image_key: key })),
157-
natural_size_in_dots,
167+
natural_size_in_dots
168+
.map_or_else(NaturalSizes::empty, NaturalSizes::from_natural_size_in_dots),
158169
)
159170
} else if let Some(svg_data) = element.as_svg() {
160171
let svg_source = match svg_data.source {
@@ -176,19 +187,18 @@ impl ReplacedContents {
176187
let result = context
177188
.image_resolver
178189
.get_cached_image_for_url(element.opaque(), svg_source, UsePlaceholder::No)
179-
.ok()?;
180-
181-
let Image::Vector(vector_image) = result else {
182-
unreachable!("SVG element can't contain a raster image.")
190+
.ok();
191+
192+
let vector_image = result.map(|result| match result {
193+
Image::Vector(vector_image) => vector_image,
194+
_ => unreachable!("SVG element can't contain a raster image."),
195+
});
196+
let natural_size = NaturalSizes {
197+
width: svg_data.width.map(Au::from_px),
198+
height: svg_data.height.map(Au::from_px),
199+
ratio: svg_data.ratio,
183200
};
184-
let physical_size = PhysicalSize::new(
185-
vector_image.metadata.width as f64,
186-
vector_image.metadata.height as f64,
187-
);
188-
(
189-
ReplacedContentKind::SVGElement(vector_image),
190-
Some(physical_size),
191-
)
201+
(ReplacedContentKind::SVGElement(vector_image), natural_size)
192202
} else {
193203
return None;
194204
}
@@ -200,18 +210,6 @@ impl ReplacedContents {
200210
.handle_animated_image(element.opaque(), image.clone());
201211
}
202212

203-
let natural_size = if let Some(naturalc_size_in_dots) = natural_size_in_dots {
204-
// FIXME: should 'image-resolution' (when implemented) be used *instead* of
205-
// `script::dom::htmlimageelement::ImageRequest::current_pixel_density`?
206-
// https://drafts.csswg.org/css-images-4/#the-image-resolution
207-
let dppx = 1.0;
208-
let width = (naturalc_size_in_dots.width as CSSFloat) / dppx;
209-
let height = (naturalc_size_in_dots.height as CSSFloat) / dppx;
210-
NaturalSizes::from_width_and_height(width, height)
211-
} else {
212-
NaturalSizes::empty()
213-
};
214-
215213
let base_fragment_info = BaseFragmentInfo::new_for_node(element.opaque());
216214
Some(Self {
217215
kind,
@@ -427,14 +425,33 @@ impl ReplacedContents {
427425
}))]
428426
},
429427
ReplacedContentKind::SVGElement(vector_image) => {
428+
let Some(vector_image) = vector_image else {
429+
return vec![];
430+
};
430431
let scale = layout_context.style_context.device_pixel_ratio();
431-
let width = object_fit_size.width.scale_by(scale.0).to_px();
432-
let height = object_fit_size.height.scale_by(scale.0).to_px();
433-
let size = Size2D::new(width, height);
432+
// TODO: This is incorrect if the SVG has a viewBox.
433+
let size = PhysicalSize::new(
434+
vector_image
435+
.metadata
436+
.width
437+
.try_into()
438+
.map_or(MAX_AU, Au::from_px),
439+
vector_image
440+
.metadata
441+
.height
442+
.try_into()
443+
.map_or(MAX_AU, Au::from_px),
444+
);
445+
let rect = PhysicalRect::from_size(size);
446+
let clip = rect.clone();
447+
let raster_size = Size2D::new(
448+
size.width.scale_by(scale.0).to_px(),
449+
size.height.scale_by(scale.0).to_px(),
450+
);
434451
let tag = self.base_fragment_info.tag.unwrap();
435452
layout_context
436453
.image_resolver
437-
.rasterize_vector_image(vector_image.id, size, tag.node)
454+
.rasterize_vector_image(vector_image.id, raster_size, tag.node)
438455
.and_then(|image| image.id)
439456
.map(|image_key| {
440457
Fragment::Image(ArcRefCell::new(ImageFragment {

‎components/script/dom/svgsvgelement.rs‎

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,20 @@ use std::cell::RefCell;
66

77
use base64::Engine as _;
88
use dom_struct::dom_struct;
9-
use html5ever::{LocalName, Prefix};
9+
use html5ever::{LocalName, Prefix, local_name, ns};
1010
use js::rust::HandleObject;
1111
use layout_api::SVGElementData;
1212
use servo_url::ServoUrl;
13+
use style::attr::{AttrValue, parse_unsigned_integer};
1314
use xml5ever::serialize::TraversalScope;
1415

1516
use crate::dom::attr::Attr;
1617
use crate::dom::bindings::inheritance::Castable;
1718
use crate::dom::bindings::root::{DomRoot, LayoutDom};
19+
use crate::dom::bindings::str::DOMString;
1820
use crate::dom::document::Document;
19-
use crate::dom::element::AttributeMutation;
20-
use crate::dom::node::Node;
21+
use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers};
22+
use crate::dom::node::{Node, NodeDamage};
2123
use crate::dom::svggraphicselement::SVGGraphicsElement;
2224
use crate::dom::virtualmethods::VirtualMethods;
2325
use crate::script_runtime::CanGc;
@@ -87,14 +89,48 @@ pub(crate) trait LayoutSVGSVGElementHelpers {
8789
fn data(self) -> SVGElementData;
8890
}
8991

92+
fn ratio_from_view_box(view_box: &AttrValue) -> Option<f32> {
93+
let mut iter = view_box.chars().into_iter();
94+
let _min_x = parse_unsigned_integer(&mut iter).ok()?;
95+
let _min_y = parse_unsigned_integer(&mut iter).ok()?;
96+
let width = parse_unsigned_integer(&mut iter).ok()?;
97+
let height = parse_unsigned_integer(&mut iter).ok()?;
98+
// Try to consume another one, just for discarding whitespace.
99+
let trailing = parse_unsigned_integer(&mut iter);
100+
if trailing.is_ok() || iter.next().is_some() {
101+
return None;
102+
}
103+
Some(width as f32 / height as f32)
104+
}
105+
90106
impl LayoutSVGSVGElementHelpers for LayoutDom<'_, SVGSVGElement> {
91107
fn data(self) -> SVGElementData {
108+
let element = self.upcast::<Element>();
109+
let get_size = |attr| {
110+
element
111+
.get_attr_for_layout(&ns!(), &attr)
112+
.map(|val| val.as_int())
113+
.filter(|val| *val >= 0)
114+
};
115+
let width = get_size(local_name!("width"));
116+
let height = get_size(local_name!("height"));
117+
let ratio = match (width, height) {
118+
(Some(width), Some(height)) if width != 0 && height != 0 => {
119+
Some(width as f32 / height as f32)
120+
},
121+
_ => element
122+
.get_attr_for_layout(&ns!(), &local_name!("viewBox"))
123+
.and_then(ratio_from_view_box),
124+
};
92125
SVGElementData {
93126
source: self
94127
.unsafe_get()
95128
.cached_serialized_data_url
96129
.borrow()
97130
.clone(),
131+
width,
132+
height,
133+
ratio,
98134
}
99135
}
100136
}
@@ -110,6 +146,24 @@ impl VirtualMethods for SVGSVGElement {
110146
.attribute_mutated(attr, mutation, can_gc);
111147

112148
self.invalidate_cached_serialized_subtree();
149+
150+
match attr.local_name() {
151+
&local_name!("width") | &local_name!("height") | &local_name!("viewBox") => {
152+
self.upcast::<Node>().dirty(NodeDamage::Other);
153+
},
154+
_ => {},
155+
};
156+
}
157+
158+
fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
159+
match *name {
160+
// TODO: This should accept lengths in arbitrary units instead of assuming px.
161+
local_name!("width") | local_name!("height") => AttrValue::from_i32(value.into(), -1),
162+
_ => self
163+
.super_type()
164+
.unwrap()
165+
.parse_plain_attribute(name, value),
166+
}
113167
}
114168

115169
fn children_changed(&self, mutation: &super::node::ChildrenMutation) {

0 commit comments

Comments
 (0)
0