Don’t assume replaced elements have an intrinsic size

components/layout_2020/dom_traversal.rs

@@ -17,7 +17,6 @@ use std::sync::Arc;
 use style::dom::TNode;
 use style::properties::ComputedValues;
 use style::selector_parser::PseudoElement;
-use style::values::computed::Length;
 
 #[derive(Clone, Copy)]
 pub enum WhichPseudoElement {
@@ -299,7 +298,10 @@ impl Drop for BoxSlot<'_> {
 pub(crate) trait NodeExt<'dom>: 'dom + Copy + LayoutNode + Send + Sync {
     fn is_element(self) -> bool;
     fn as_text(self) -> Option<String>;
-    fn as_image(self) -> Option<(Option<Arc<NetImage>>, Vec2<Length>)>;
+
+    /// Returns the image if it’s loaded, and its size in image pixels
+    /// adjusted for `image_density`.
+    fn as_image(self) -> Option<(Option<Arc<NetImage>>, Vec2<f64>)>;
     fn first_child(self) -> Option<Self>;
     fn next_sibling(self) -> Option<Self>;
     fn parent_node(self) -> Option<Self>;
@@ -328,7 +330,7 @@ where
         }
     }
 
-    fn as_image(self) -> Option<(Option<Arc<NetImage>>, Vec2<Length>)> {
+    fn as_image(self) -> Option<(Option<Arc<NetImage>>, Vec2<f64>)> {
         let node = self.to_threadsafe();
         let (resource, metadata) = node.image_data()?;
         let (width, height) = resource
@@ -336,14 +338,14 @@ where
             .map(|image| (image.width, image.height))
             .or_else(|| metadata.map(|metadata| (metadata.width, metadata.height)))
             .unwrap_or((0, 0));
-        let (mut width, mut height) = (width as f32, height as f32);
+        let (mut width, mut height) = (width as f64, height as f64);
         if let Some(density) = node.image_density().filter(|density| *density != 1.) {
-            width = (width as f64 / density) as f32;
+            width = width / density;
-            height = (height as f64 / density) as f32;
+            height = height / density;
         }
         let size = Vec2 {
-            x: Length::new(width),
+            x: width,
-            y: Length::new(height),
+            y: height,
         };
         Some((resource, size))
     }

components/layout_2020/replaced.rs

@@ -4,7 +4,8 @@
 
 use crate::dom_traversal::NodeExt;
 use crate::fragments::{Fragment, ImageFragment};
-use crate::geom::{flow_relative, physical};
+use crate::geom::flow_relative::{Rect, Vec2};
+use crate::geom::physical;
 use crate::style_ext::ComputedValuesExt;
 use crate::ContainingBlock;
 use net_traits::image::base::Image;
@@ -12,12 +13,27 @@ use servo_arc::Arc as ServoArc;
 use std::sync::Arc;
 use style::properties::ComputedValues;
 use style::values::computed::{Length, LengthOrAuto};
+use style::values::CSSFloat;
 use style::Zero;
 
 #[derive(Debug)]
 pub(crate) struct ReplacedContent {
     pub kind: ReplacedContentKind,
-    pub intrinsic_size: physical::Vec2<Length>,
+
+    /// * Raster images always have an instrinsic width and height, with 1 image pixel = 1px.
+    ///   The intrinsic ratio should be based on dividing those.
+    ///   See https://github.com/w3c/csswg-drafts/issues/4572 for the case where either is zero.
+    ///   PNG specifically disallows this but I (SimonSapin) am not sure about other formats.
+    ///
+    /// * Form controls have both intrinsic width and height **but no intrinsic ratio**.
+    ///   See https://github.com/w3c/csswg-drafts/issues/1044 and
+    ///   https://drafts.csswg.org/css-images/#intrinsic-dimensions “In general, […]”
+    ///
+    /// * For SVG, see https://svgwg.org/svg2-draft/coords.html#SizingSVGInCSS
+    ///   and again https://github.com/w3c/csswg-drafts/issues/4572.
+    intrinsic_width: Option<Length>,
+    intrinsic_height: Option<Length>,
+    intrinsic_ratio: Option<CSSFloat>,
 }
 
 #[derive(Debug)]
@@ -27,10 +43,21 @@ pub(crate) enum ReplacedContentKind {
 
 impl ReplacedContent {
     pub fn for_element<'dom>(element: impl NodeExt<'dom>) -> Option<Self> {
-        if let Some((image, intrinsic_size)) = element.as_image() {
+        if let Some((image, intrinsic_size_in_dots)) = element.as_image() {
+            // FIXME: should 'image-resolution' (when implemented) be used *instead* of
+            // `script::dom::htmlimageelement::ImageRequest::current_pixel_density`?
+
+            // https://drafts.csswg.org/css-images-4/#the-image-resolution
+            let dppx = 1.0;
+
+            let width = (intrinsic_size_in_dots.x as CSSFloat) / dppx;
+            let height = (intrinsic_size_in_dots.y as CSSFloat) / dppx;
             return Some(Self {
                 kind: ReplacedContentKind::Image(image),
-                intrinsic_size,
+                intrinsic_width: Some(Length::new(width)),
+                intrinsic_height: Some(Length::new(height)),
+                // FIXME https://github.com/w3c/csswg-drafts/issues/4572
+                intrinsic_ratio: Some(width / height),
             });
         }
         None
@@ -39,7 +66,7 @@ impl ReplacedContent {
     pub fn make_fragments<'a>(
         &'a self,
         style: &ServoArc<ComputedValues>,
-        size: flow_relative::Vec2<Length>,
+        size: Vec2<Length>,
     ) -> Vec<Fragment> {
         match &self.kind {
             ReplacedContentKind::Image(image) => image
@@ -48,8 +75,8 @@ impl ReplacedContent {
                 .map(|image_key| {
                     Fragment::Image(ImageFragment {
                         style: style.clone(),
-                        rect: flow_relative::Rect {
+                        rect: Rect {
-                            start_corner: flow_relative::Vec2::zero(),
+                            start_corner: Vec2::zero(),
                             size,
                         },
                         image_key,
@@ -66,12 +93,21 @@ impl ReplacedContent {
         &self,
         containing_block: &ContainingBlock,
         style: &ComputedValues,
-    ) -> flow_relative::Vec2<Length> {
+    ) -> Vec2<Length> {
         let mode = style.writing_mode;
-        // FIXME(nox): We shouldn't pretend we always have a fully known intrinsic size.
+        let intrinsic_size = physical::Vec2 {
-        let intrinsic_size = self.intrinsic_size.size_to_flow_relative(mode);
+            x: self.intrinsic_width,
-        // FIXME(nox): This can divide by zero.
+            y: self.intrinsic_height,
-        let intrinsic_ratio = intrinsic_size.inline.px() / intrinsic_size.block.px();
+        };
+        let intrinsic_size = intrinsic_size.size_to_flow_relative(mode);
+        let intrinsic_ratio = self.intrinsic_ratio.map(|width_over_height| {
+            // inline-size over block-size
+            if style.writing_mode.is_vertical() {
+                1. / width_over_height
+            } else {
+                width_over_height
+            }
+        });
 
         let box_size = style.box_size().percentages_relative_to(containing_block);
         let min_box_size = style
@@ -82,25 +118,94 @@ impl ReplacedContent {
             .max_box_size()
             .percentages_relative_to(containing_block);
 
-        let clamp = |inline_size: Length, block_size: Length| {
+        let default_object_size = || {
-            (
+            // FIXME:
-                inline_size.clamp_between_extremums(min_box_size.inline, max_box_size.inline),
+            // “If 300px is too wide to fit the device, UAs should use the width of
-                block_size.clamp_between_extremums(min_box_size.block, max_box_size.block),
+            //  the largest rectangle that has a 2:1 ratio and fits the device instead.”
-            )
+            // “height of the largest rectangle that has a 2:1 ratio, has a height not greater
+            //  than 150px, and has a width not greater than the device width.”
+            physical::Vec2 {
+                x: Length::new(300.),
+                y: Length::new(150.),
+            }
+            .size_to_flow_relative(mode)
+        };
+        let clamp = |inline_size: Length, block_size: Length| Vec2 {
+            inline: inline_size.clamp_between_extremums(min_box_size.inline, max_box_size.inline),
+            block: block_size.clamp_between_extremums(min_box_size.block, max_box_size.block),
         };
         // https://drafts.csswg.org/css2/visudet.html#min-max-widths
         // https://drafts.csswg.org/css2/visudet.html#min-max-heights
-        let (inline_size, block_size) = match (box_size.inline, box_size.block) {
+        match (box_size.inline, box_size.block) {
             (LengthOrAuto::LengthPercentage(inline), LengthOrAuto::LengthPercentage(block)) => {
                 clamp(inline, block)
             },
             (LengthOrAuto::LengthPercentage(inline), LengthOrAuto::Auto) => {
-                clamp(inline, inline / intrinsic_ratio)
+                let block = if let Some(i_over_b) = intrinsic_ratio {
+                    inline / i_over_b
+                } else if let Some(block) = intrinsic_size.block {
+                    block
+                } else {
+                    default_object_size().block
+                };
+                clamp(inline, block)
             },
             (LengthOrAuto::Auto, LengthOrAuto::LengthPercentage(block)) => {
-                clamp(block * intrinsic_ratio, block)
+                let inline = if let Some(i_over_b) = intrinsic_ratio {
+                    block * i_over_b
+                } else if let Some(inline) = intrinsic_size.inline {
+                    inline
+                } else {
+                    default_object_size().inline
+                };
+                clamp(inline, block)
             },
             (LengthOrAuto::Auto, LengthOrAuto::Auto) => {
+                let inline_size =
+                    match (intrinsic_size.inline, intrinsic_size.block, intrinsic_ratio) {
+                        (Some(inline), _, _) => inline,
+                        (None, Some(block), Some(i_over_b)) => {
+                            // “used height” in CSS 2 is always gonna be the intrinsic one,
+                            // since it is available.
+                            block * i_over_b
+                        },
+                        // FIXME
+                        //
+                        // “If 'height' and 'width' both have computed values of 'auto'
+                        // and the element has an intrinsic ratio but no intrinsic height or width,
+                        // […]”
+                        //
+                        // In this `match` expression this would be an additional arm here:
+                        //
+                        // ```
+                        // (Vec2 { inline: None, block: None }, Some(_)) => {…}
+                        // ```
+                        //
+                        // “[…] then the used value of 'width' is undefined in CSS 2.
+                        // However, it is suggested that, if the containing block's width
+                        // does not itself depend on the replaced element's width,
+                        // then the used value of 'width' is calculated from the constraint
+                        // equation used for block-level, non-replaced elements in normal flow.”
+                        _ => default_object_size().inline,
+                    };
+                let block_size = if let Some(block) = intrinsic_size.block {
+                    block
+                } else if let Some(i_over_b) = intrinsic_ratio {
+                    // “used width” in CSS 2 is what we just computed above
+                    inline_size / i_over_b
+                } else {
+                    default_object_size().block
+                };
+
+                let i_over_b = if let Some(i_over_b) = intrinsic_ratio {
+                    i_over_b
+                } else {
+                    return clamp(inline_size, block_size);
+                };
+
+                // https://drafts.csswg.org/css2/visudet.html#min-max-widths
+                // “However, for replaced elements with an intrinsic ratio and both
+                //  'width' and 'height' specified as 'auto', the algorithm is as follows”
                 enum Violation {
                     None,
                     Below(Length),
@@ -119,87 +224,84 @@ impl ReplacedContent {
                     }
                 };
                 match (
-                    violation(
+                    violation(inline_size, min_box_size.inline, max_box_size.inline),
-                        intrinsic_size.inline,
+                    violation(block_size, min_box_size.block, max_box_size.block),
-                        min_box_size.inline,
-                        max_box_size.inline,
-                    ),
-                    violation(intrinsic_size.block, min_box_size.block, max_box_size.block),
                 ) {
                     // Row 1.
-                    (Violation::None, Violation::None) => {
+                    (Violation::None, Violation::None) => Vec2 {
-                        (intrinsic_size.inline, intrinsic_size.block)
+                        inline: inline_size,
+                        block: block_size,
                     },
                     // Row 2.
-                    (Violation::Above(max_inline_size), Violation::None) => {
+                    (Violation::Above(max_inline_size), Violation::None) => Vec2 {
-                        let block_size =
+                        inline: max_inline_size,
-                            (max_inline_size / intrinsic_ratio).max(min_box_size.block);
+                        block: (max_inline_size / i_over_b).max(min_box_size.block),
-                        (max_inline_size, block_size)
                     },
                     // Row 3.
-                    (Violation::Below(min_inline_size), Violation::None) => {
+                    (Violation::Below(min_inline_size), Violation::None) => Vec2 {
-                        let block_size =
+                        inline: min_inline_size,
-                            (min_inline_size / intrinsic_ratio).clamp_below_max(max_box_size.block);
+                        block: (min_inline_size / i_over_b).clamp_below_max(max_box_size.block),
-                        (min_inline_size, block_size)
                     },
                     // Row 4.
-                    (Violation::None, Violation::Above(max_block_size)) => {
+                    (Violation::None, Violation::Above(max_block_size)) => Vec2 {
-                        let inline_size =
+                        inline: (max_block_size * i_over_b).max(min_box_size.inline),
-                            (max_block_size * intrinsic_ratio).max(min_box_size.inline);
+                        block: max_block_size,
-                        (inline_size, max_block_size)
                     },
                     // Row 5.
-                    (Violation::None, Violation::Below(min_block_size)) => {
+                    (Violation::None, Violation::Below(min_block_size)) => Vec2 {
-                        let inline_size =
+                        inline: (min_block_size * i_over_b).clamp_below_max(max_box_size.inline),
-                            (min_block_size * intrinsic_ratio).clamp_below_max(max_box_size.inline);
+                        block: min_block_size,
-                        (inline_size, min_block_size)
                     },
                     // Rows 6-7.
                     (Violation::Above(max_inline_size), Violation::Above(max_block_size)) => {
-                        if max_inline_size.px() / intrinsic_size.inline.px() <=
+                        if max_inline_size.px() / inline_size.px() <=
-                            max_block_size.px() / intrinsic_size.block.px()
+                            max_block_size.px() / block_size.px()
                         {
                             // Row 6.
-                            let block_size =
+                            Vec2 {
-                                (max_inline_size / intrinsic_ratio).max(min_box_size.block);
+                                inline: max_inline_size,
-                            (max_inline_size, block_size)
+                                block: (max_inline_size / i_over_b).max(min_box_size.block),
+                            }
                         } else {
                             // Row 7.
-                            let inline_size =
+                            Vec2 {
-                                (max_block_size * intrinsic_ratio).max(min_box_size.inline);
+                                inline: (max_block_size * i_over_b).max(min_box_size.inline),
-                            (inline_size, max_block_size)
+                                block: max_block_size,
+                            }
                         }
                     },
                     // Rows 8-9.
                     (Violation::Below(min_inline_size), Violation::Below(min_block_size)) => {
-                        if min_inline_size.px() / intrinsic_size.inline.px() <=
+                        if min_inline_size.px() / inline_size.px() <=
-                            min_block_size.px() / intrinsic_size.block.px()
+                            min_block_size.px() / block_size.px()
                         {
                             // Row 8.
-                            let inline_size = (min_block_size * intrinsic_ratio)
+                            Vec2 {
-                                .clamp_below_max(max_box_size.inline);
+                                inline: (min_block_size * i_over_b)
-                            (inline_size, min_block_size)
+                                    .clamp_below_max(max_box_size.inline),
+                                block: min_block_size,
+                            }
                         } else {
                             // Row 9.
-                            let block_size = (min_inline_size / intrinsic_ratio)
+                            Vec2 {
-                                .clamp_below_max(max_box_size.block);
+                                inline: min_inline_size,
-                            (min_inline_size, block_size)
+                                block: (min_inline_size / i_over_b)
+                                    .clamp_below_max(max_box_size.block),
+                            }
                         }
                     },
                     // Row 10.
-                    (Violation::Below(min_inline_size), Violation::Above(max_block_size)) => {
+                    (Violation::Below(min_inline_size), Violation::Above(max_block_size)) => Vec2 {
-                        (min_inline_size, max_block_size)
+                        inline: min_inline_size,
+                        block: max_block_size,
                     },
                     // Row 11.
-                    (Violation::Above(max_inline_size), Violation::Below(min_block_size)) => {
+                    (Violation::Above(max_inline_size), Violation::Below(min_block_size)) => Vec2 {
-                        (max_inline_size, min_block_size)
+                        inline: max_inline_size,
+                        block: min_block_size,
                     },
                 }
             },
-        };
-        flow_relative::Vec2 {
-            inline: inline_size,
-            block: block_size,
         }
     }
 }