Repository: servo/servo
File: components/layout/replaced.rs (Rust)

Code before:
            } else {
                None
            }
        };
        let width = svg_data.width.and_then(attr_to_computed);
        let height = svg_data.height.and_then(attr_to_computed);

        let ratio = if let (Some(width), Some(height)) = (width, height) &&
            !width.is_zero() &&
            !height.is_zero()
        {
            Some(width.px() / height.px())
        } else {
            Self::svg_ratio_from_view_box(svg_data.view_box)
        };

        let natural_size = NaturalSizes {
            width: width.map(|w| Au::from_f32_px(w.px())),
            height: height.map(|h| Au::from_f32_px(h.px())),
            ratio,
        };
        Some((ReplacedContentKind::SVGElement(vector_image), natural_size))
    }

    fn svg_ratio_from_view_box(view_box: Option<&AttrValue>) -> Option<f32> {
        if let Some(view_box) = view_box {
            let mut iter = view_box.chars();
            let _min_x = parse_integer(&mut iter).ok()?;
            let _min_y = parse_integer(&mut iter).ok()?;

            let width = parse_unsigned_integer(&mut iter).ok()?;
            if width == 0 {
                return None;
            }

            let height = parse_unsigned_integer(&mut iter).ok()?;
            if height == 0 {
                return None;
            }

            let mut iter = iter.skip_while(|c| char_is_whitespace(*c));
            iter.next().is_none().then(|| width as f32 / height as f32)
        } else {
            None
        }
    }

    pub fn from_image_url(
        node: ServoThreadSafeLayoutNode<'_>,
        context: &LayoutContext,
        image_url: &ComputedUrl,

Review comment (suggestion):
`let view_box = view_box?;`

But the function is called `svg_ratio_from_view_box` so I think it should get a viewbox, not an option.

Change the caller to `svg_data.view_box.and_then(Self::svg_ratio_from_view_box)`.

Also, `svg_ratio_from_view_box` seems completely unrelated to `ReplacedContents`, so I would move it outside.

Code after:
            },
            Some(Err(_)) => {
                // Don't attempt to serialize if previous attempt had errored.
                return None;
            },
            Some(Ok(svg_source)) => svg_source,
        };

        let result = context
            .image_resolver
            .get_cached_image_for_url(
                node.opaque(),
                svg_source,
                LayoutImageDestination::BoxTreeConstruction,
            )
            .ok();

        let vector_image = result.map(|result| match result {
            Image::Vector(vector_image) => vector_image,
            _ => unreachable!("SVG element can't contain a raster image."),
        });

        Some((ReplacedContentKind::SVGElement(vector_image), natural_size))
    }

    pub fn from_image_url(
        node: ServoThreadSafeLayoutNode<'_>,
        context: &LayoutContext,
        image_url: &ComputedUrl,
    ) -> Option<Self> {
        if let ComputedUrl::Valid(image_url) = image_url {
            let (image, width, height) = match context.image_resolver.get_or_request_image_or_meta(
                node.opaque(),
                image_url.clone().into(),
                LayoutImageDestination::BoxTreeConstruction,
            ) {
                LayoutImageCacheResult::DataAvailable(img_or_meta) => match img_or_meta {
                    ImageOrMetadataAvailable::ImageAvailable { image, .. } => {
                        let metadata = image.metadata();
                        (
                            Some(image.clone()),
                            metadata.width as f32,
                            metadata.height as f32,
                        )
                    },
                    ImageOrMetadataAvailable::MetadataAvailable(metadata, _id) => {
                        (None, metadata.width as f32, metadata.height as f32)
                    },
                },
                LayoutImageCacheResult::Pending | LayoutImageCacheResult::LoadError => return None,
            };