Skip to content

Commit

Permalink
Set intrinsic size for inline SVG earlier
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=257614
rdar://problem/110480382

Reviewed by Simon Fraser.

This patch aligns WebKit with Gecko / Firefox and Blink / Chromium.

Merge: https://chromium.googlesource.com/chromium/src.git/+/545b2183e1b4ee3eb433537f9a6386b5337d6588

RenderReplaced has a m_intrinsicSize that's updated when computing
logical widths and heights (and only if needed; specified style makes
it not being set at all).

But m_intrinsicSize can be used earlier that that, when computing
preferred widths for the container, see
RenderReplaced::computeIntrinsicLogicalWidths called from
RenderReplaced::computePreferredLogicalWidths().

This patch computes the intrinsic size in the constructor to avoid
returning the stale default size.

* Source/WebCore/rendering/svg/LegacyRenderSVGRoot.cpp:
(1) Add constants for 'defaultWidth' and 'defaultHeight'
(2) Add 'intrinsicSize' logic
(3) Introduce new function 'calculateIntrinsicSize'
(LegacyRenderSVGRoot::computeIntrinsicRatioInformation): Call above function and remove unneeded comments
* Source/WebCore/rendering/svg/LegacyRenderSVGRoot.h: Definition of 'calculateIntrinsicSize'
* Source/WebCore/rendering/svg/RenderSVGRoot.cpp: Same changes as 'Legacy' for LBSE
* Source/WebCore/rendering/svg/RenderSVGRoot.h: Ditto
* LayoutTests/svg/in-html/inside-inline-block.html: Add Test Case
* LayoutTests/svg/in-html/inside-inline-block-expected.html: Add Test Case Expectation

Canonical link: https://commits.webkit.org/266314@main
  • Loading branch information
@Ahmad-S792
Ahmad-S792 authored and Ahmad Saleem committed Jul 26, 2023
1 parent c556dfa commit dbdb89f
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 34 deletions.
24 changes: 24 additions & 0 deletions LayoutTests/svg/in-html/inside-inline-block-expected.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!DOCTYPE html>
<title>SVG inside inline block</title>
<style>
body { margin: 0 }
div {
width: 100px;
background: green;
position: absolute;
top: 0;
left: 0
}
.cover1 {
top: 0;
left: 0;
height: 150px
}
.cover2 {
top: 50px;
left: 100px;
height: 100px
}
</style>
<div class="cover1"></div>
<div class="cover2"></div>
38 changes: 38 additions & 0 deletions LayoutTests/svg/in-html/inside-inline-block.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!DOCTYPE html>
<title>SVG inside inline block</title>
<style>
body { margin: 0 }
span { display: inline-block }
div {
width: 100px;
background: green;
position: absolute;
top: 0;
left: 0
}
.cover1 {
top: 0;
left: 0;
height: 150px
}
.cover2 {
top: 50px;
left: 100px;
height: 100px
}
svg {
width: 100%;
height: 100%
}
</style>
<span>
<svg width="100">
<rect width="100%" height="100%" fill="red"/>
</svg>
</span><span>
<svg width="100" height="100" style="width: 100%; height: 100%;">
<rect width="100%" height="100%" fill="red"/>
</svg>
</span>
<div class="cover1"></div>
<div class="cover2"></div>
32 changes: 16 additions & 16 deletions Source/WebCore/rendering/svg/LegacyRenderSVGRoot.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,21 @@ namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(LegacyRenderSVGRoot);

const int defaultWidth = 300;
const int defaultHeight = 150;

LegacyRenderSVGRoot::LegacyRenderSVGRoot(SVGSVGElement& element, RenderStyle&& style)
: RenderReplaced(element, WTFMove(style))
, m_isLayoutSizeChanged(false)
, m_needsBoundariesOrTransformUpdate(true)
, m_hasBoxDecorations(false)
{
LayoutSize intrinsicSize(calculateIntrinsicSize());
if (!intrinsicSize.width())
intrinsicSize.setWidth(defaultWidth);
if (!intrinsicSize.height())
intrinsicSize.setHeight(defaultHeight);
setIntrinsicSize(intrinsicSize);
}

LegacyRenderSVGRoot::~LegacyRenderSVGRoot() = default;
Expand All @@ -76,22 +85,17 @@ bool LegacyRenderSVGRoot::hasIntrinsicAspectRatio() const
return computeIntrinsicAspectRatio();
}

FloatSize LegacyRenderSVGRoot::calculateIntrinsicSize() const
{
return FloatSize(floatValueForLength(svgSVGElement().intrinsicWidth(), 0), floatValueForLength(svgSVGElement().intrinsicHeight(), 0));
}

void LegacyRenderSVGRoot::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, FloatSize& intrinsicRatio) const
{
ASSERT(!shouldApplySizeContainment());

// Spec: http://www.w3.org/TR/SVG/coords.html#IntrinsicSizing
// SVG needs to specify how to calculate some intrinsic sizing properties to enable inclusion within other languages.

// The intrinsic aspect ratio of the viewport of SVG content is necessary for example, when including SVG from an ‘object’
// element in HTML styled with CSS. It is possible (indeed, common) for an SVG graphic to have an intrinsic aspect ratio but
// not to have an intrinsic width or height. The intrinsic aspect ratio must be calculated based upon the following rules:
// - The aspect ratio is calculated by dividing a width by a height.
// - If the ‘width’ and ‘height’ of the rootmost ‘svg’ element are both specified with unit identifiers (in, mm, cm, pt, pc,
// px, em, ex) or in user units, then the aspect ratio is calculated from the ‘width’ and ‘height’ attributes after
// resolving both values to user units.
intrinsicSize.setWidth(floatValueForLength(svgSVGElement().intrinsicWidth(), 0));
intrinsicSize.setHeight(floatValueForLength(svgSVGElement().intrinsicHeight(), 0));
// https://www.w3.org/TR/SVG/coords.html#IntrinsicSizing
intrinsicSize = calculateIntrinsicSize();

if (style().aspectRatioType() == AspectRatioType::Ratio) {
intrinsicRatio = FloatSize::narrowPrecision(style().aspectRatioLogicalWidth(), style().aspectRatioLogicalHeight());
Expand All @@ -102,10 +106,6 @@ void LegacyRenderSVGRoot::computeIntrinsicRatioInformation(FloatSize& intrinsicS
if (!intrinsicSize.isEmpty())
intrinsicRatio = { intrinsicSize.width(), intrinsicSize.height() };
else {
// - If either/both of the ‘width’ and ‘height’ of the rootmost ‘svg’ element are in percentage units (or omitted), the
// aspect ratio is calculated from the width and height values of the ‘viewBox’ specified for the current SVG document
// fragment. If the ‘viewBox’ is not correctly specified, or set to 'none', the intrinsic aspect ratio cannot be
// calculated and is considered unspecified.
FloatSize viewBoxSize = svgSVGElement().viewBox().size();
if (!viewBoxSize.isEmpty()) {
// The viewBox can only yield an intrinsic ratio, not an intrinsic size.
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/rendering/svg/LegacyRenderSVGRoot.h
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2004, 2005, 2007 Nikolas Zimmermann <zimmermann@kde.org>
* Copyright (C) 2004, 2005, 2007 Rob Buis <buis@kde.org>
* Copyright (C) 2009 Google, Inc. All rights reserved.
* Copyright (C) 2009-2016 Google, Inc. All rights reserved.
* Copyright (C) 2009 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
Expand Down Expand Up @@ -106,6 +106,8 @@ class LegacyRenderSVGRoot final : public RenderReplaced {
void updateCachedBoundaries();
void buildLocalToBorderBoxTransform();

FloatSize calculateIntrinsicSize() const;

IntSize m_containerSize;
FloatRect m_objectBoundingBox;
bool m_objectBoundingBoxValid { false };
Expand Down
32 changes: 16 additions & 16 deletions Source/WebCore/rendering/svg/RenderSVGRoot.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,18 @@ namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(RenderSVGRoot);

const int defaultWidth = 300;
const int defaultHeight = 150;

RenderSVGRoot::RenderSVGRoot(SVGSVGElement& element, RenderStyle&& style)
: RenderReplaced(element, WTFMove(style))
{
LayoutSize intrinsicSize(calculateIntrinsicSize());
if (!intrinsicSize.width())
intrinsicSize.setWidth(defaultWidth);
if (!intrinsicSize.height())
intrinsicSize.setHeight(defaultHeight);
setIntrinsicSize(intrinsicSize);
}

RenderSVGRoot::~RenderSVGRoot() = default;
Expand All @@ -89,22 +98,17 @@ bool RenderSVGRoot::hasIntrinsicAspectRatio() const
return computeIntrinsicAspectRatio();
}

FloatSize RenderSVGRoot::calculateIntrinsicSize() const
{
return FloatSize(floatValueForLength(svgSVGElement().intrinsicWidth(), 0), floatValueForLength(svgSVGElement().intrinsicHeight(), 0));
}

void RenderSVGRoot::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, FloatSize& intrinsicRatio) const
{
ASSERT(!shouldApplySizeContainment());

// Spec: http://www.w3.org/TR/SVG/coords.html#IntrinsicSizing
// SVG needs to specify how to calculate some intrinsic sizing properties to enable inclusion within other languages.

// The intrinsic aspect ratio of the viewport of SVG content is necessary for example, when including SVG from an ‘object’
// element in HTML styled with CSS. It is possible (indeed, common) for an SVG graphic to have an intrinsic aspect ratio but
// not to have an intrinsic width or height. The intrinsic aspect ratio must be calculated based upon the following rules:
// - The aspect ratio is calculated by dividing a width by a height.
// - If the ‘width’ and ‘height’ of the rootmost ‘svg’ element are both specified with unit identifiers (in, mm, cm, pt, pc,
// px, em, ex) or in user units, then the aspect ratio is calculated from the ‘width’ and ‘height’ attributes after
// resolving both values to user units.
intrinsicSize.setWidth(floatValueForLength(svgSVGElement().intrinsicWidth(), 0));
intrinsicSize.setHeight(floatValueForLength(svgSVGElement().intrinsicHeight(), 0));
// https://www.w3.org/TR/SVG/coords.html#IntrinsicSizing
intrinsicSize = calculateIntrinsicSize();

if (style().aspectRatioType() == AspectRatioType::Ratio) {
intrinsicRatio = FloatSize::narrowPrecision(style().aspectRatioLogicalWidth(), style().aspectRatioLogicalHeight());
Expand All @@ -115,10 +119,6 @@ void RenderSVGRoot::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, F
if (!intrinsicSize.isEmpty())
intrinsicRatioValue = { intrinsicSize.width(), intrinsicSize.height() };
else {
// - If either/both of the ‘width’ and ‘height’ of the rootmost ‘svg’ element are in percentage units (or omitted), the
// aspect ratio is calculated from the width and height values of the ‘viewBox’ specified for the current SVG document
// fragment. If the ‘viewBox’ is not correctly specified, or set to 'none', the intrinsic aspect ratio cannot be
// calculated and is considered unspecified.
FloatSize viewBoxSize = svgSVGElement().viewBox().size();
if (!viewBoxSize.isEmpty()) {
// The viewBox can only yield an intrinsic ratio, not an intrinsic size.
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/rendering/svg/RenderSVGRoot.h
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2004, 2005, 2007 Nikolas Zimmermann <zimmermann@kde.org>
* Copyright (C) 2004, 2005, 2007 Rob Buis <buis@kde.org>
* Copyright (C) 2009 Google, Inc. All rights reserved.
* Copyright (C) 2009-2016 Google, Inc. All rights reserved.
* Copyright (C) 2009 Apple Inc. All rights reserved.
* Copyright (C) 2020, 2021, 2022 Igalia S.L.
*
Expand Down Expand Up @@ -104,6 +104,8 @@ class RenderSVGRoot final : public RenderReplaced {
bool needsHasSVGTransformFlags() const final;
void updateLayerTransform() final;

FloatSize calculateIntrinsicSize() const;

bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) final;

LayoutRect overflowClipRect(const LayoutPoint& location, RenderFragmentContainer* = nullptr, OverlayScrollbarSizeRelevancy = IgnoreOverlayScrollbarSize, PaintPhase = PaintPhase::BlockBackground) const final;
Expand Down

0 comments on commit dbdb89f

Please sign in to comment.