Skip to content

Commit

Permalink
RenderBox::applyVisualEffectOverflow() and RenderBox::outlineBoundsFo…
Browse files Browse the repository at this point in the history
…rRepaint() use different ways of computing outline/shadow extent

https://bugs.webkit.org/show_bug.cgi?id=264625
rdar://118582062

Reviewed by Alan Baradlay.

The logic in `ShadowData::adjustRectForShadow()` to compute the impact of box-shadow and outline
on a rect was wrong; the outline is not shadowed, so outline and shadow have independent effects
on the repaint rect. The correct implementation is to just compute their effects separately, and
take the union of the result, which is what `RenderBox::applyVisualEffectOverflow()` already
does.

So remove the outline argument from `ShadowData::adjustRectForShadow()` and delete
`RenderObject::adjustRectForOutlineAndShadow()`. `RenderBox::outlineBoundsForRepaint()` now just
calls `applyVisualEffectOverflow()`, and LegacyRenderSVGModelObject gets its own copy of the old
code because it's not clear how shadows and outlines interact in SVG.

There was also code duplication between `RenderStyle::shadowExtent()` and code in ShadowData, so
remove the RenderStyle code and have it call into ShadowData.

* LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt:
* LayoutTests/fast/box-shadow/negative-shadow-box-shrink-expected.txt:
* LayoutTests/fast/repaint/outline-and-shadow-repaint-expected.txt: Added.
* LayoutTests/fast/repaint/outline-and-shadow-repaint.html: Added.
* LayoutTests/platform/gtk/fast/repaint/4776765-expected.txt:
* LayoutTests/platform/mac-monterey-wk2/fast/repaint/4776765-expected.txt:
* LayoutTests/platform/mac-ventura-wk2/fast/repaint/4776765-expected.txt:
* LayoutTests/platform/mac-wk1/fast/repaint/4776765-expected.txt:
* LayoutTests/platform/mac/fast/repaint/4776765-expected.txt:
* Source/WebCore/display/css/DisplayBoxModelBox.cpp:
(WebCore::Display::BoxModelBox::absolutePaintingExtent const):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::outlineBoundsForRepaint const):
* Source/WebCore/rendering/RenderObject.cpp:
(WebCore::RenderObject::adjustRectForOutlineAndShadow const): Deleted.
* Source/WebCore/rendering/RenderObject.h:
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::shadowExtent): Deleted.
(WebCore::RenderStyle::shadowInsetExtent): Deleted.
* Source/WebCore/rendering/style/RenderStyle.h:
* Source/WebCore/rendering/style/RenderStyleInlines.h:
(WebCore::RenderStyle::boxShadowExtent const):
(WebCore::RenderStyle::boxShadowInsetExtent const):
(WebCore::RenderStyle::textShadowExtent const):
* Source/WebCore/rendering/style/ShadowData.cpp:
(WebCore::ShadowData::shadowOutsetExtent const):
(WebCore::ShadowData::shadowInsetExtent const):
(WebCore::ShadowData::adjustRectForShadow const):
(WebCore::calculateShadowExtent): Deleted.
* Source/WebCore/rendering/style/ShadowData.h:
(WebCore::ShadowData::shadowOutsetExtent):
(WebCore::ShadowData::shadowInsetExtent):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGModelObject.cpp:
(WebCore::adjustRectForOutlineAndShadow):
(WebCore::LegacyRenderSVGModelObject::outlineBoundsForRepaint const):

Canonical link: https://commits.webkit.org/271254@main
  • Loading branch information
smfr committed Nov 29, 2023
1 parent 47696ba commit 6aa1f67
Show file tree
Hide file tree
Showing 19 changed files with 147 additions and 104 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(repaint rects
(rect 100 200 600 100)
(rect 100 200 640 100)
(rect 100 160 640 40)
(rect 100 160 600 40)
(rect 100 200 640 100)
(rect 0 300 800 100)
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(repaint rects
(rect 100 200 600 100)
(rect 100 200 640 100)
(rect 100 160 640 40)
(rect 100 160 600 40)
(rect 100 200 640 100)
(rect 0 300 800 100)
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(repaint rects
(rect 88 80 388 390)
(rect 8 370 784 100)
(rect 8 304 784 66)
(rect 0 404 800 100)
)

31 changes: 31 additions & 0 deletions LayoutTests/fast/repaint/outline-and-shadow-repaint.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<!DOCTYPE html>
<html>
<head>
<style>
.box {
width: 300px;
height: 300px;
margin: 100px;
border: 2px solid black;
outline: 20px solid rgba(0, 128, 0, 0.5);
box-shadow: 30px 32px 10px 20px black;
}

body.changed .box {
height: 200px;
}
</style>
<script src="resources/text-based-repaint.js"></script>
<script>
function repaintTest()
{
document.body.classList.add('changed');
}

window.addEventListener('load', runRepaintTest, false);
</script>
</head>
<body>
<div class="box"></div>
</body>
</html>
2 changes: 1 addition & 1 deletion LayoutTests/platform/gtk/fast/repaint/4776765-expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
(rect 1 37 798 32)
(rect 8 44 784 18)
(rect 1 51 798 18)
(rect 1 44 798 7)
(rect 8 37 784 14)
(rect 1 19 15 31)
(rect 8 26 1 17)
(rect 1 37 15 31)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
(rect 1 37 798 32)
(rect 8 44 784 18)
(rect 1 51 798 18)
(rect 1 44 798 7)
(rect 8 37 784 14)
(rect 1 19 15 32)
(rect 8 26 1 18)
(rect 1 37 15 32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
(rect 1 37 798 32)
(rect 8 44 784 18)
(rect 1 51 798 18)
(rect 1 44 798 7)
(rect 8 37 784 14)
(rect 1 19 15 32)
(rect 8 26 1 18)
(rect 1 37 15 32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
(rect 1 37 798 32)
(rect 8 44 784 18)
(rect 1 51 798 18)
(rect 1 44 798 7)
(rect 8 37 784 14)
)

2 changes: 1 addition & 1 deletion LayoutTests/platform/mac/fast/repaint/4776765-expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
(rect 1 37 798 32)
(rect 8 44 784 18)
(rect 1 51 798 18)
(rect 1 44 798 7)
(rect 8 37 784 14)
(rect 1 19 16 32)
(rect 8 26 2 18)
(rect 1 37 16 32)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/display/css/DisplayBoxModelBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ UnadjustedAbsoluteFloatRect BoxModelBox::absolutePaintingExtent() const
auto paintingExtent = absoluteBorderBoxRect();

if (auto* shadow = style().boxShadow())
shadow->adjustRectForShadow(paintingExtent, 0);
shadow->adjustRectForShadow(paintingExtent);

return paintingExtent;
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/RenderBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -851,8 +851,8 @@ FloatQuad RenderBox::absoluteContentQuad() const

LayoutRect RenderBox::outlineBoundsForRepaint(const RenderLayerModelObject* repaintContainer, const RenderGeometryMap* geometryMap) const
{
LayoutRect box = borderBoundingBox();
adjustRectForOutlineAndShadow(box);
auto box = borderBoundingBox();
applyVisualEffectOverflow(box);

if (repaintContainer != this) {
FloatQuad containerRelativeQuad;
Expand Down
10 changes: 0 additions & 10 deletions Source/WebCore/rendering/RenderObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1902,16 +1902,6 @@ int RenderObject::nextOffset(int current) const
return current + 1;
}

void RenderObject::adjustRectForOutlineAndShadow(LayoutRect& rect) const
{
LayoutUnit outlineSize { outlineStyleForRepaint().outlineSize() };
if (const ShadowData* boxShadow = style().boxShadow()) {
boxShadow->adjustRectForShadow(rect, outlineSize);
return;
}
rect.inflate(outlineSize);
}

void RenderObject::imageChanged(CachedImage* image, const IntRect* rect)
{
imageChanged(static_cast<WrappedImagePtr>(image), rect);
Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/rendering/RenderObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,6 @@ class RenderObject : public CachedImageClient, public CanMakeCheckedPtr {
//////////////////////////////////////////
Node& nodeForNonAnonymous() const { ASSERT(!isAnonymous()); return m_node.get(); }

void adjustRectForOutlineAndShadow(LayoutRect&) const;

virtual void willBeDestroyed();

void setNeedsPositionedMovementLayoutBit(bool b) { m_bitfields.setNeedsPositionedMovementLayout(b); }
Expand Down
42 changes: 0 additions & 42 deletions Source/WebCore/rendering/style/RenderStyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2951,48 +2951,6 @@ void RenderStyle::setFontPalette(FontPalette value)
fontCascade().update(selector);
}

LayoutBoxExtent RenderStyle::shadowExtent(const ShadowData* shadow)
{
LayoutUnit top;
LayoutUnit right;
LayoutUnit bottom;
LayoutUnit left;

for ( ; shadow; shadow = shadow->next()) {
if (shadow->style() == ShadowStyle::Inset)
continue;

auto extentAndSpread = shadow->paintingExtent() + LayoutUnit(shadow->spread().value());
top = std::min<LayoutUnit>(top, LayoutUnit(shadow->y().value()) - extentAndSpread);
right = std::max<LayoutUnit>(right, LayoutUnit(shadow->x().value()) + extentAndSpread);
bottom = std::max<LayoutUnit>(bottom, LayoutUnit(shadow->y().value()) + extentAndSpread);
left = std::min<LayoutUnit>(left, LayoutUnit(shadow->x().value()) - extentAndSpread);
}

return { top, right, bottom, left };
}

LayoutBoxExtent RenderStyle::shadowInsetExtent(const ShadowData* shadow)
{
LayoutUnit top;
LayoutUnit right;
LayoutUnit bottom;
LayoutUnit left;

for ( ; shadow; shadow = shadow->next()) {
if (shadow->style() == ShadowStyle::Normal)
continue;

auto extentAndSpread = shadow->paintingExtent() + LayoutUnit(shadow->spread().value());
top = std::max<LayoutUnit>(top, LayoutUnit(shadow->y().value()) + extentAndSpread);
right = std::min<LayoutUnit>(right, LayoutUnit(shadow->x().value()) - extentAndSpread);
bottom = std::min<LayoutUnit>(bottom, LayoutUnit(shadow->y().value()) - extentAndSpread);
left = std::max<LayoutUnit>(left, LayoutUnit(shadow->x().value()) + extentAndSpread);
}

return { top, right, bottom, left };
}

void RenderStyle::getShadowHorizontalExtent(const ShadowData* shadow, LayoutUnit &left, LayoutUnit &right)
{
left = 0;
Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/rendering/style/RenderStyle.h
Original file line number Diff line number Diff line change
Expand Up @@ -2250,8 +2250,6 @@ class RenderStyle {
static constexpr bool isDisplayListItemType(DisplayType);
static constexpr bool isDisplayTableOrTablePart(DisplayType);

static LayoutBoxExtent shadowExtent(const ShadowData*);
static LayoutBoxExtent shadowInsetExtent(const ShadowData*);
static void getShadowHorizontalExtent(const ShadowData*, LayoutUnit& left, LayoutUnit& right);
static void getShadowVerticalExtent(const ShadowData*, LayoutUnit& top, LayoutUnit& bottom);

Expand Down
7 changes: 4 additions & 3 deletions Source/WebCore/rendering/style/RenderStyleInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "RenderStyle.h"
#include "ScrollTypes.h"
#include "ScrollbarColor.h"
#include "ShadowData.h"
#include "StyleAppearance.h"
#include "StyleBackgroundData.h"
#include "StyleBoxData.h"
Expand Down Expand Up @@ -139,8 +140,8 @@ inline BoxOrient RenderStyle::boxOrient() const { return static_cast<BoxOrient>(
inline BoxPack RenderStyle::boxPack() const { return static_cast<BoxPack>(m_nonInheritedData->miscData->deprecatedFlexibleBox->pack); }
inline StyleReflection* RenderStyle::boxReflect() const { return m_nonInheritedData->rareData->boxReflect.get(); }
inline const ShadowData* RenderStyle::boxShadow() const { return m_nonInheritedData->miscData->boxShadow.get(); }
inline LayoutBoxExtent RenderStyle::boxShadowExtent() const { return shadowExtent(boxShadow()); }
inline LayoutBoxExtent RenderStyle::boxShadowInsetExtent() const { return shadowInsetExtent(boxShadow()); }
inline LayoutBoxExtent RenderStyle::boxShadowExtent() const { return ShadowData::shadowOutsetExtent(boxShadow()); }
inline LayoutBoxExtent RenderStyle::boxShadowInsetExtent() const { return ShadowData::shadowInsetExtent(boxShadow()); }
inline BoxSizing RenderStyle::boxSizing() const { return m_nonInheritedData->boxData->boxSizing(); }
inline BoxSizing RenderStyle::boxSizingForAspectRatio() const { return aspectRatioType() == AspectRatioType::AutoAndRatio ? BoxSizing::ContentBox : boxSizing(); }
inline BreakBetween RenderStyle::breakAfter() const { return static_cast<BreakBetween>(m_nonInheritedData->rareData->breakAfter); }
Expand Down Expand Up @@ -684,7 +685,7 @@ inline TextOrientation RenderStyle::textOrientation() const { return static_cast
inline TextOverflow RenderStyle::textOverflow() const { return static_cast<TextOverflow>(m_nonInheritedData->miscData->textOverflow); }
inline TextSecurity RenderStyle::textSecurity() const { return static_cast<TextSecurity>(m_rareInheritedData->textSecurity); }
inline const ShadowData* RenderStyle::textShadow() const { return m_rareInheritedData->textShadow.get(); }
inline LayoutBoxExtent RenderStyle::textShadowExtent() const { return shadowExtent(textShadow()); }
inline LayoutBoxExtent RenderStyle::textShadowExtent() const { return ShadowData::shadowOutsetExtent(textShadow()); }
inline const StyleColor& RenderStyle::textStrokeColor() const { return m_rareInheritedData->textStrokeColor; }
inline float RenderStyle::textStrokeWidth() const { return m_rareInheritedData->textStrokeWidth; }
inline OptionSet<TextTransform> RenderStyle::textTransform() const { return OptionSet<TextTransform>::fromRaw(m_inheritedFlags.textTransform); }
Expand Down
83 changes: 51 additions & 32 deletions Source/WebCore/rendering/style/ShadowData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,45 +78,64 @@ bool ShadowData::operator==(const ShadowData& o) const
return true;
}

static inline void calculateShadowExtent(const ShadowData* shadow, LayoutUnit additionalOutlineSize, LayoutUnit& shadowLeft, LayoutUnit& shadowRight, LayoutUnit& shadowTop, LayoutUnit& shadowBottom)
LayoutBoxExtent ShadowData::shadowOutsetExtent() const
{
do {
LayoutUnit extentAndSpread = shadow->paintingExtent() + LayoutUnit(shadow->spread().value()) + additionalOutlineSize;
if (shadow->style() == ShadowStyle::Normal) {
shadowLeft = std::min(LayoutUnit(shadow->x().value()) - extentAndSpread, shadowLeft);
shadowRight = std::max(LayoutUnit(shadow->x().value()) + extentAndSpread, shadowRight);
shadowTop = std::min(LayoutUnit(shadow->y().value()) - extentAndSpread, shadowTop);
shadowBottom = std::max(LayoutUnit(shadow->y().value()) + extentAndSpread, shadowBottom);
}

shadow = shadow->next();
} while (shadow);
LayoutUnit top;
LayoutUnit right;
LayoutUnit bottom;
LayoutUnit left;

for (auto* shadow = this; shadow; shadow = shadow->next()) {
auto extentAndSpread = shadow->paintingExtent() + LayoutUnit(shadow->spread().value());
if (shadow->style() == ShadowStyle::Inset)
continue;

left = std::min(LayoutUnit(shadow->x().value()) - extentAndSpread, left);
right = std::max(LayoutUnit(shadow->x().value()) + extentAndSpread, right);
top = std::min(LayoutUnit(shadow->y().value()) - extentAndSpread, top);
bottom = std::max(LayoutUnit(shadow->y().value()) + extentAndSpread, bottom);
}

return { top, right, bottom, left };
}

void ShadowData::adjustRectForShadow(LayoutRect& rect, int additionalOutlineSize) const
LayoutBoxExtent ShadowData::shadowInsetExtent() const
{
LayoutUnit shadowLeft;
LayoutUnit shadowRight;
LayoutUnit shadowTop;
LayoutUnit shadowBottom;
calculateShadowExtent(this, additionalOutlineSize, shadowLeft, shadowRight, shadowTop, shadowBottom);

rect.move(shadowLeft, shadowTop);
rect.setWidth(rect.width() - shadowLeft + shadowRight);
rect.setHeight(rect.height() - shadowTop + shadowBottom);
LayoutUnit top;
LayoutUnit right;
LayoutUnit bottom;
LayoutUnit left;

for (auto* shadow = this; shadow; shadow = shadow->next()) {
if (shadow->style() == ShadowStyle::Normal)
continue;

auto extentAndSpread = shadow->paintingExtent() + LayoutUnit(shadow->spread().value());
top = std::max<LayoutUnit>(top, LayoutUnit(shadow->y().value()) + extentAndSpread);
right = std::min<LayoutUnit>(right, LayoutUnit(shadow->x().value()) - extentAndSpread);
bottom = std::min<LayoutUnit>(bottom, LayoutUnit(shadow->y().value()) - extentAndSpread);
left = std::max<LayoutUnit>(left, LayoutUnit(shadow->x().value()) + extentAndSpread);
}

return { top, right, bottom, left };
}

void ShadowData::adjustRectForShadow(FloatRect& rect, int additionalOutlineSize) const
void ShadowData::adjustRectForShadow(LayoutRect& rect) const
{
LayoutUnit shadowLeft = 0;
LayoutUnit shadowRight = 0;
LayoutUnit shadowTop = 0;
LayoutUnit shadowBottom = 0;
calculateShadowExtent(this, additionalOutlineSize, shadowLeft, shadowRight, shadowTop, shadowBottom);

rect.move(shadowLeft, shadowTop);
rect.setWidth(rect.width() - shadowLeft + shadowRight);
rect.setHeight(rect.height() - shadowTop + shadowBottom);
auto shadowExtent = shadowOutsetExtent();

rect.move(shadowExtent.left(), shadowExtent.top());
rect.setWidth(rect.width() - shadowExtent.left() + shadowExtent.right());
rect.setHeight(rect.height() - shadowExtent.top() + shadowExtent.bottom());
}

void ShadowData::adjustRectForShadow(FloatRect& rect) const
{
auto shadowExtent = shadowOutsetExtent();

rect.move(shadowExtent.left(), shadowExtent.top());
rect.setWidth(rect.width() - shadowExtent.left() + shadowExtent.right());
rect.setHeight(rect.height() - shadowExtent.top() + shadowExtent.bottom());
}

TextStream& operator<<(TextStream& ts, const ShadowData& data)
Expand Down
31 changes: 29 additions & 2 deletions Source/WebCore/rendering/style/ShadowData.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "FloatRect.h"
#include "LayoutRect.h"
#include "Length.h"
#include "LengthBox.h"
#include "LengthPoint.h"
#include "StyleColor.h"

Expand Down Expand Up @@ -64,6 +65,7 @@ class ShadowData {
const Length& y() const { return m_location.y(); }
const LengthPoint& location() const { return m_location; }
const Length& radius() const { return m_radius; }

LayoutUnit paintingExtent() const
{
// Blurring uses a Gaussian function whose std. deviation is m_radius/2, and which in theory
Expand All @@ -72,7 +74,9 @@ class ShadowData {
const float radiusExtentMultiplier = 1.4;
return LayoutUnit(ceilf(m_radius.value() * radiusExtentMultiplier));
}

const Length& spread() const { return m_spread; }

ShadowStyle style() const { return m_style; }

void setColor(const StyleColor& color) { m_color = color; }
Expand All @@ -83,8 +87,14 @@ class ShadowData {
const ShadowData* next() const { return m_next.get(); }
void setNext(std::unique_ptr<ShadowData>&& shadow) { m_next = WTFMove(shadow); }

void adjustRectForShadow(LayoutRect&, int additionalOutlineSize = 0) const;
void adjustRectForShadow(FloatRect&, int additionalOutlineSize = 0) const;
void adjustRectForShadow(LayoutRect&) const;
void adjustRectForShadow(FloatRect&) const;

LayoutBoxExtent shadowOutsetExtent() const;
LayoutBoxExtent shadowInsetExtent() const;

static LayoutBoxExtent shadowOutsetExtent(const ShadowData*);
static LayoutBoxExtent shadowInsetExtent(const ShadowData*);

private:
void deleteNextLinkedListWithoutRecursion();
Expand All @@ -104,6 +114,23 @@ inline ShadowData::~ShadowData()
deleteNextLinkedListWithoutRecursion();
}


inline LayoutBoxExtent ShadowData::shadowOutsetExtent(const ShadowData* shadow)
{
if (!shadow)
return { };

return shadow->shadowOutsetExtent();
}

inline LayoutBoxExtent ShadowData::shadowInsetExtent(const ShadowData* shadow)
{
if (!shadow)
return { };

return shadow->shadowInsetExtent();
}

WTF::TextStream& operator<<(WTF::TextStream&, const ShadowData&);

} // namespace WebCore
Loading

0 comments on commit 6aa1f67

Please sign in to comment.