Skip to content

Commit 25bd452

Browse files
smfrsmfr
committed
Tiling gaps with feDisplacementMap in a CSS reference filter
https://bugs.webkit.org/show_bug.cgi?id=279290 rdar://135448018 Reviewed by Said Abou-Hallawa. webkit.org/b/266295 describes a number of tiling issues with SVG reference filters. In many cases, the culprit was `<feDisplacementMap>`. This filter moves pixels, so `SVGFEDisplacementMapElement` needs to implement `outsets()`; the max displacement is based on half of the scale value. `FEDisplacementMap::calculateOutsets` converts this into possible outsets on each side. To fix rendering issues with tiling, we address the FIXME in `RenderLayerFilters::beginFilterEffect()` by ensuring that the dirty rect does not affect `referenceBox`. This code is also simplified to no longer store `m_filterRegion`, since we can read it from `m_filter`, and the two blocks of code related to computing `filterRegion` are unified. There are two rects computed here, with different roles: `dirtyFilterRegion` designates the area of the input that needs to be redrawn; it's the dirty rect inflated by filter outsets (to deal with filters that move pixels), clipped to `filterBoxRect`. It's passed to `GraphicsContextSwitcher` and becomes the bounds of the filter source image. `filterRegion` is `dirtyFilterRegion` expanded by outsets to denote the bounds of the filter result. Filter geometry is still incorrect with LBSE, but using `objectBoundingBox()` as input to the GraphicsContextSwitcher seems to improve things. RenderLayerFilters now takes the scale at constructor time. The test case exercises both issues. Test: css3/filters/fedisplacement-ref-filter-with-tiling.html * LayoutTests/css3/filters/blur-clipped-with-overflow.html: * LayoutTests/css3/filters/fedisplacement-ref-filter-with-tiling-expected.html: Added. * LayoutTests/css3/filters/fedisplacement-ref-filter-with-tiling.html: Added. * Source/WebCore/platform/graphics/filters/FEDisplacementMap.cpp: (WebCore::FEDisplacementMap::calculateOutsets): * Source/WebCore/platform/graphics/filters/FEDisplacementMap.h: * Source/WebCore/platform/graphics/filters/FilterEffect.cpp: (WebCore::FilterEffect::apply): (WebCore::FilterEffect::externalRepresentation const): * Source/WebCore/rendering/RenderLayer.cpp: (WebCore::RenderLayer::calculateClipRects const): * Source/WebCore/rendering/RenderLayerFilters.cpp: (WebCore::RenderLayerFilters::create): (WebCore::RenderLayerFilters::RenderLayerFilters): (WebCore::RenderLayerFilters::beginFilterEffect): * Source/WebCore/rendering/RenderLayerFilters.h: * Source/WebCore/svg/SVGFEDisplacementMapElement.cpp: (WebCore::SVGFEDisplacementMapElement::outsets const): * Source/WebCore/svg/SVGFEDisplacementMapElement.h: Canonical link: https://commits.webkit.org/304830@main
1 parent fdb4331 commit 25bd452

11 files changed

+157
-59
lines changed

‎LayoutTests/css3/filters/blur-clipped-with-overflow.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<!DOCTYPE html>
2-
32
<html>
43
<head>
4+
<meta name="fuzzy" content="maxDifference=0-1; totalPixels=0-27300">
55
<style>
66
.filtered {
77
overflow: hidden;

‎LayoutTests/css3/filters/fedisplacement-ref-filter-with-tiling-expected.html‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<style>
5+
body {
6+
overflow: hidden;
7+
}
8+
.test {
9+
height: 300px;
10+
width: 600px;
11+
}
12+
13+
.filtered {
14+
background-color: green;
15+
border: 10px solid black;
16+
filter: url(#filter);
17+
}
18+
19+
.composited {
20+
margin: 100px;
21+
transform: translateZ(0);
22+
width: 1024px;
23+
}
24+
25+
svg {
26+
position: absolute;
27+
width: 0;
28+
height: 0;
29+
}
30+
</style>
31+
</head>
32+
<body>
33+
<svg>
34+
<filter id="filter" x=0 y=0 width="120%" height=100%>
35+
<feFlood flood-color="black" flood-opacity="1"/>
36+
<feDisplacementmap in="SourceGraphic" scale="100" xchannelselector="G"/>
37+
</filter>
38+
</svg>
39+
<div class="composited">
40+
<div class="test filtered"></div>
41+
</div>
42+
</body>
43+
</html>

‎LayoutTests/css3/filters/fedisplacement-ref-filter-with-tiling.html‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<style>
5+
body {
6+
overflow: hidden;
7+
}
8+
.test {
9+
height: 300px;
10+
width: 600px;
11+
}
12+
13+
.filtered {
14+
background-color: green;
15+
border: 10px solid black;
16+
filter: url(#filter);
17+
}
18+
19+
.composited {
20+
margin: 100px;
21+
transform: translateZ(0);
22+
width: 4097px; /* Make it tiled */
23+
}
24+
25+
svg {
26+
position: absolute;
27+
width: 0;
28+
height: 0;
29+
}
30+
</style>
31+
</head>
32+
<body>
33+
<svg>
34+
<filter id="filter" x="0" y="0" width="120%" height=100%>
35+
<feFlood flood-color="black" flood-opacity="1"/>
36+
<feDisplacementmap in="SourceGraphic" scale="100" xchannelselector="G"/>
37+
</filter>
38+
</svg>
39+
<div class="composited">
40+
<div class="test filtered"></div>
41+
</div>
42+
</body>
43+
</html>

‎Source/WebCore/platform/graphics/filters/FEDisplacementMap.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ FloatRect FEDisplacementMap::calculateImageRect(const Filter& filter, std::span<
8181
return filter.maxEffectRect(primitiveSubregion);
8282
}
8383

84+
IntOutsets FEDisplacementMap::calculateOutsets(const FloatSize& maxDisplacement)
85+
{
86+
auto intDisplacement = expandedIntSize(maxDisplacement);
87+
return { intDisplacement.height(), intDisplacement.width(), intDisplacement.height(), intDisplacement.width() };
88+
}
89+
8490
const DestinationColorSpace& FEDisplacementMap::resultColorSpace(std::span<const Ref<FilterImage>> inputs) const
8591
{
8692
// Spec: The 'color-interpolation-filters' property only applies to the 'in2' source image

‎Source/WebCore/platform/graphics/filters/FEDisplacementMap.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class FEDisplacementMap final : public FilterEffect {
5252
float scale() const { return m_scale; }
5353
bool setScale(float);
5454

55+
static IntOutsets calculateOutsets(const FloatSize& maxDisplacement);
56+
5557
private:
5658
FEDisplacementMap(ChannelSelectorType xChannelSelector, ChannelSelectorType yChannelSelector, float, DestinationColorSpace);
5759

‎Source/WebCore/platform/graphics/filters/FilterEffect.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ RefPtr<FilterImage> FilterEffect::apply(const Filter& filter, std::span<const Re
183183

184184
LOG_WITH_STREAM(Filters, stream
185185
<< "FilterEffect " << filterName() << " " << this << " apply(): " << *this
186-
<< "\n filterPrimitiveSubregion " << primitiveSubregion
186+
<< " filterPrimitiveSubregion " << primitiveSubregion
187187
<< "\n absolutePaintRect " << absoluteImageRect
188188
<< "\n maxEffectRect " << filter.maxEffectRect(primitiveSubregion)
189189
<< "\n filter scale " << filter.filterScale());
@@ -223,7 +223,6 @@ TextStream& FilterEffect::externalRepresentation(TextStream& ts, FilterRepresent
223223
if (representation == FilterRepresentation::Debugging) {
224224
TextStream::IndentScope indentScope(ts);
225225
ts.dumpProperty("operating colorspace"_s, operatingColorSpace());
226-
ts << '\n' << indent;
227226
}
228227
return ts;
229228
}

‎Source/WebCore/rendering/RenderLayer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6367,8 +6367,8 @@ RenderLayerFilters& RenderLayer::ensureLayerFilters()
63676367
if (m_filters)
63686368
return *m_filters;
63696369

6370-
m_filters = RenderLayerFilters::create(*this);
6371-
m_filters->setFilterScale({ page().deviceScaleFactor(), page().deviceScaleFactor() });
6370+
auto scale = page().deviceScaleFactor();
6371+
m_filters = RenderLayerFilters::create(*this, { scale, scale });
63726372
return *m_filters;
63736373
}
63746374

‎Source/WebCore/rendering/RenderLayerFilters.cpp‎

Lines changed: 45 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,14 @@ namespace WebCore {
4949

5050
WTF_MAKE_TZONE_ALLOCATED_IMPL(RenderLayerFilters);
5151

52-
Ref<RenderLayerFilters> RenderLayerFilters::create(RenderLayer& layer)
52+
Ref<RenderLayerFilters> RenderLayerFilters::create(RenderLayer& layer, FloatSize scale)
5353
{
54-
return adoptRef(*new RenderLayerFilters(layer));
54+
return adoptRef(*new RenderLayerFilters(layer, scale));
5555
}
5656

57-
RenderLayerFilters::RenderLayerFilters(RenderLayer& layer)
57+
RenderLayerFilters::RenderLayerFilters(RenderLayer& layer, FloatSize scale)
5858
: m_layer(&layer)
59+
, m_filterScale(scale)
5960
{
6061
}
6162

@@ -158,62 +159,59 @@ IntOutsets RenderLayerFilters::calculateOutsets(RenderElement& renderer, const F
158159

159160
GraphicsContext* RenderLayerFilters::beginFilterEffect(RenderElement& renderer, GraphicsContext& context, const LayoutRect& filterBoxRect, const LayoutRect& dirtyRect, const LayoutRect& layerRepaintRect, const LayoutRect& clipRect)
160161
{
161-
auto expandedDirtyRect = dirtyRect;
162-
auto targetBoundingBox = intersection(filterBoxRect, dirtyRect);
163-
164162
auto preferredFilterRenderingModes = renderer.page().preferredFilterRenderingModes(context);
163+
auto outsets = calculateOutsets(renderer, filterBoxRect);
164+
165+
auto dirtyFilterRegion = dirtyRect;
166+
auto filterRegion = dirtyRect;
167+
168+
if (auto* shape = dynamicDowncast<RenderSVGShape>(renderer)) {
169+
// In LBSE, the filter region will be recomputed in createReferenceFilter().
170+
// FIXME: The LBSE filter geometry is not correct.
171+
filterRegion = dirtyFilterRegion = enclosingLayoutRect(shape->objectBoundingBox());
172+
} else {
173+
if (!outsets.isZero()) {
174+
// FIXME: This flipping was added for drop-shadow, but it's not obvious that it's correct.
175+
LayoutBoxExtent flippedOutsets { outsets.bottom(), outsets.left(), outsets.top(), outsets.right() };
176+
dirtyFilterRegion.expand(flippedOutsets);
177+
}
165178

166-
auto outsets = calculateOutsets(renderer, targetBoundingBox);
167-
if (!outsets.isZero()) {
168-
LayoutBoxExtent flippedOutsets { outsets.bottom(), outsets.left(), outsets.top(), outsets.right() };
169-
expandedDirtyRect.expand(flippedOutsets);
170-
}
179+
dirtyFilterRegion = intersection(filterBoxRect, dirtyFilterRegion);
180+
filterRegion = dirtyFilterRegion;
171181

172-
if (is<RenderSVGShape>(renderer))
173-
targetBoundingBox = enclosingLayoutRect(renderer.objectBoundingBox());
174-
else {
175-
// Calculate targetBoundingBox since it will be used if the filter is created.
176-
targetBoundingBox = intersection(filterBoxRect, expandedDirtyRect);
182+
if (!outsets.isZero())
183+
filterRegion.expand(toLayoutBoxExtent(outsets));
177184
}
178185

179-
if (targetBoundingBox.isEmpty())
186+
if (filterRegion.isEmpty())
180187
return nullptr;
181188

182-
if (!m_filter || m_targetBoundingBox != targetBoundingBox || m_preferredFilterRenderingModes != preferredFilterRenderingModes) {
183-
m_targetBoundingBox = targetBoundingBox;
184-
// FIXME: This rebuilds the entire effects chain even if the filter style didn't change.
185-
m_filter = CSSFilterRenderer::create(renderer, renderer.style().filter(), {
186-
.referenceBox = m_targetBoundingBox, // FIXME: It's wrong for the dirty rect to feed into the reference box: webkit.org/b/279290.
187-
.filterRegion = m_targetBoundingBox,
188-
.scale = m_filterScale,
189-
}, preferredFilterRenderingModes, context);
190-
}
189+
auto geometryReferenceGeometryChanged = [](auto& existingGeometry, auto& newGeometry) {
190+
return existingGeometry.referenceBox != newGeometry.referenceBox || existingGeometry.scale != newGeometry.scale;
191+
};
191192

192-
if (!m_filter)
193-
return nullptr;
194-
195-
Ref filter = *m_filter;
196-
auto filterRegion = m_targetBoundingBox;
193+
auto geometry = FilterGeometry {
194+
.referenceBox = filterBoxRect,
195+
.filterRegion = filterRegion,
196+
.scale = m_filterScale,
197+
};
197198

198-
if (filter->hasFilterThatMovesPixels()) {
199-
// For CSSFilterRenderer, filterRegion = targetBoundingBox + filter->outsets()
200-
filterRegion.expand(toLayoutBoxExtent(outsets));
201-
} else if (auto* shape = dynamicDowncast<RenderSVGShape>(renderer))
202-
filterRegion = shape->currentSVGLayoutRect();
203-
204-
if (filterRegion.isEmpty())
205-
return nullptr;
206-
207-
// For CSSFilterRenderer, sourceImageRect = filterRegion.
208199
bool hasUpdatedBackingStore = false;
209-
if (m_filterRegion != filterRegion || m_preferredFilterRenderingModes != preferredFilterRenderingModes) {
210-
m_filterRegion = filterRegion;
211-
m_preferredFilterRenderingModes = preferredFilterRenderingModes;
200+
if (!m_filter || geometryReferenceGeometryChanged(m_filter->geometry(), geometry) || m_preferredFilterRenderingModes != preferredFilterRenderingModes) {
201+
// FIXME: This rebuilds the entire effects chain even if the filter style didn't change.
202+
m_filter = CSSFilterRenderer::create(renderer, renderer.style().filter(), geometry, preferredFilterRenderingModes, context);
203+
hasUpdatedBackingStore = true;
204+
} else if (filterRegion != m_filter->filterRegion()) {
205+
m_filter->setFilterRegion(filterRegion);
212206
hasUpdatedBackingStore = true;
213207
}
214208

215-
filter->setFilterRegion(m_filterRegion);
209+
m_preferredFilterRenderingModes = preferredFilterRenderingModes;
216210

211+
if (!m_filter)
212+
return nullptr;
213+
214+
Ref filter = *m_filter;
217215
if (!filter->hasFilterThatMovesPixels())
218216
m_repaintRect = dirtyRect;
219217
else if (hasUpdatedBackingStore || !hasSourceImage())
@@ -229,9 +227,9 @@ GraphicsContext* RenderLayerFilters::beginFilterEffect(RenderElement& renderer,
229227
if (!m_targetSwitcher || hasUpdatedBackingStore) {
230228
FloatRect sourceImageRect;
231229
if (is<RenderSVGShape>(renderer))
232-
sourceImageRect = renderer.strokeBoundingBox();
230+
sourceImageRect = renderer.objectBoundingBox();
233231
else
234-
sourceImageRect = m_targetBoundingBox;
232+
sourceImageRect = dirtyFilterRegion;
235233
m_targetSwitcher = GraphicsContextSwitcher::create(context, sourceImageRect, DestinationColorSpace::SRGB(), { WTF::move(filter) });
236234
}
237235

‎Source/WebCore/rendering/RenderLayerFilters.h‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class GraphicsContextSwitcher;
4848
class RenderLayerFilters final : public RefCounted<RenderLayerFilters>, private CachedSVGDocumentClient {
4949
WTF_MAKE_TZONE_ALLOCATED(RenderLayerFilters);
5050
public:
51-
static Ref<RenderLayerFilters> create(RenderLayer&);
51+
static Ref<RenderLayerFilters> create(RenderLayer&, FloatSize scale);
5252
virtual ~RenderLayerFilters();
5353

5454
void detachFromLayer() { m_layer = nullptr; }
@@ -70,8 +70,6 @@ class RenderLayerFilters final : public RefCounted<RenderLayerFilters>, private
7070
void updateReferenceFilterClients(const Style::Filter&);
7171
void removeReferenceFilterClients();
7272

73-
void setFilterScale(const FloatSize& filterScale) { m_filterScale = filterScale; }
74-
7573
static bool isIdentity(RenderElement&);
7674
static IntOutsets calculateOutsets(RenderElement&, const FloatRect& targetBoundingBox);
7775

@@ -82,7 +80,7 @@ class RenderLayerFilters final : public RefCounted<RenderLayerFilters>, private
8280
void applyFilterEffect(GraphicsContext& destinationContext);
8381

8482
private:
85-
explicit RenderLayerFilters(RenderLayer&);
83+
explicit RenderLayerFilters(RenderLayer&, FloatSize scale);
8684

8785
void notifyFinished(CachedResource&, const NetworkLoadMetrics&, LoadWillContinueInAnotherProcess) final;
8886
void resetDirtySourceRect() { m_dirtySourceRect = LayoutRect(); }
@@ -91,13 +89,12 @@ class RenderLayerFilters final : public RefCounted<RenderLayerFilters>, private
9189
Vector<RefPtr<Element>> m_internalSVGReferences;
9290
Vector<CachedResourceHandle<CachedSVGDocument>> m_externalSVGReferences;
9391

94-
LayoutRect m_targetBoundingBox;
9592
LayoutRect m_dirtySourceRect;
9693
LayoutRect m_repaintRect;
9794

98-
OptionSet<FilterRenderingMode> m_preferredFilterRenderingModes { FilterRenderingMode::Software };
9995
FloatSize m_filterScale { 1, 1 };
100-
FloatRect m_filterRegion;
96+
97+
OptionSet<FilterRenderingMode> m_preferredFilterRenderingModes { FilterRenderingMode::Software };
10198

10299
RefPtr<CSSFilterRenderer> m_filter;
103100
std::unique_ptr<GraphicsContextSwitcher> m_targetSwitcher;

‎Source/WebCore/svg/SVGFEDisplacementMapElement.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#include "FEDisplacementMap.h"
2525
#include "NodeName.h"
26+
#include "SVGFilterRenderer.h"
2627
#include "SVGNames.h"
2728
#include "SVGPropertyOwnerRegistry.h"
2829
#include <wtf/TZoneMallocInlines.h>
@@ -122,6 +123,14 @@ void SVGFEDisplacementMapElement::svgAttributeChanged(const QualifiedName& attrN
122123
}
123124
}
124125

126+
IntOutsets SVGFEDisplacementMapElement::outsets(const FloatRect& targetBoundingBox, SVGUnitTypes::SVGUnitType primitiveUnits) const
127+
{
128+
auto halfScale = std::abs(scale() / 2);
129+
auto maxDisplacement = FloatSize { halfScale, halfScale };
130+
auto adjustedDisplacement = SVGFilterRenderer::calculateResolvedSize(maxDisplacement, targetBoundingBox, primitiveUnits);
131+
return FEDisplacementMap::calculateOutsets(adjustedDisplacement);
132+
}
133+
125134
RefPtr<FilterEffect> SVGFEDisplacementMapElement::createFilterEffect(const FilterEffectVector&, const GraphicsContext&) const
126135
{
127136
return FEDisplacementMap::create(xChannelSelector(), yChannelSelector(), scale());

0 commit comments

Comments
 (0)