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

Commit cd2bc12

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` and `height` attributes instead. The `viewBox` attribute is left for a follow-up. Signed-off-by: Oriol Brufau <obrufau@igalia.com>
1 parent 82ca2b9 commit cd2bc12

30 files changed

+132
-112
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: 50 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-im 6D38 ages-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,31 @@ 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+
let size = PhysicalSize::new(
433+
vector_image
434+
.metadata
435+
.width
436+
.try_into()
437+
.map_or(MAX_AU, Au::from_px),
438+
vector_image
439+
.metadata
440+
.height
441+
.try_into()
442+
.map_or(MAX_AU, Au::from_px),
443+
);
444+
let rect = PhysicalRect::from_size(size);
445+
let raster_size = Size2D::new(
446+
size.width.scale_by(scale.0).to_px(),
447+
size.height.scale_by(scale.0).to_px(),
448+
);
434449
let tag = self.base_fragment_info.tag.unwrap();
435450
layout_context
436451
.image_resolver
437-
.rasterize_vector_image(vector_image.id, size, tag.node)
452+
.rasterize_vector_image(vector_image.id, raster_size, tag.node)
438453
.and_then(|image| image.id)
439454
.map(|image_key| {
440455
Fragment::Image(ArcRefCell::new(ImageFragment {

‎components/script/dom/svgsvgelement.rs‎

Lines changed: 42 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;
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;
@@ -89,12 +91,31 @@ pub(crate) trait LayoutSVGSVGElementHelpers {
8991

9092
impl LayoutSVGSVGElementHelpers for LayoutDom<'_, SVGSVGElement> {
9193
fn data(self) -> SVGElementData {
94+
let get_size = |attr| {
95+
self.upcast::<Element>()
96+
.get_attr_for_layout(&ns!(), &attr)
97+
.map(|val| val.as_int())
98+
.filter(|val| *val >= 0)
99+
};
100+
let width = get_size(local_name!("width"));
101+
let height = get_size(local_name!("height"));
102+
let ratio = match (width, height) {
103+
(Some(width), Some(height)) if width != 0 && height != 0 => {
104+
Some(width as f32 / height as f32)
105+
},
106+
// TODO: It should be possible to have a natural aspect ratio even if one or both
107+
// natural sizes are missing, via the `viewBox` attribute.
108+
_ => None,
109+
};
92110
SVGElementData {
93111
source: self
94112
.unsafe_get()
95113
.cached_serialized_data_url
96114
.borrow()
97115
.clone(),
116+
width,
117+
height,
118+
ratio,
98119
}
99120
}
100121
}
@@ -110,6 +131,24 @@ impl VirtualMethods for SVGSVGElement {
110131
.attribute_mutated(attr, mutation, can_gc);
111132

112133
self.invalidate_cached_serialized_subtree();
134+
135+
match attr.local_name() {
136+
&local_name!("width") | &local_name!("height") => {
137+
self.upcast::<Node>().dirty(NodeDamage::Other);
138+
},
139+
_ => {},
140+
};
141+
}
142+
143+
fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
144+
match *name {
145+
// TODO: This should accept lengths in arbitrary units instead of assuming px.
146+
local_name!("width") | local_name!("height") => AttrValue::from_i32(value.into(), -1),
147+
_ => self
148+
.super_type()
149+
.unwrap()
150+
.parse_plain_attribute(name, value),
151+
}
113152
}
114153

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

‎components/shared/layout/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ pub struct HTMLCanvasData {
135135
pub struct SVGElementData {
136136
/// The SVG's XML source represented as a base64 encoded `data:` url.
137137
pub source: Option<Result<ServoUrl, ()>>,
138+
pub width: Option<i32>,
139+
pub height: Option<i32>,
140+
pub ratio: Option<f32>,
138141
}
139142

140143
/// The address of a node known to be valid. These are sent from script to layout.

‎tests/wpt/meta/css/CSS2/normal-flow/inline-block-replaced-height-009.xht.ini‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎tests/wpt/meta/css/CSS2/normal-flow/inline-block-replaced-width-007.xht.ini‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎tests/wpt/meta/css/CSS2/normal-flow/inline-block-replaced-width-008.xht.ini‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎tests/wpt/meta/css/CSS2/normal-flow/inline-replaced-height-009.xht.ini‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

0 commit comments

Comments
 (0)
0