Skip to content

Commit f27a530

Browse files
committed
[GPU Process] (REGRESSION r285597): Set the filterRegion of the CSSFilter after it is created
https://bugs.webkit.org/show_bug.cgi?id=233849 Reviewed by Cameron McCormack. Source/WebCore: In this patch: 1. sourceImageRect is no longer passed as an argument to SVGFilter. It should be passed only when SVGFilter::apply() is called. 2. CSSFilter::create() will create and build the FilterFunctions from the FilterOperations. So we have to pass the targetBoundingBox since the FilterEffects will need it when calculating the outsets. 3. The steps in RenderLayerFilters::beginFilterEffect() are: a) Recreate the CSSFilter if the targetBoundingBox changes. b) Calculate the filterRegion = targetBoundingBox + outsets c) Clamp the filterRegion if needed. This will change the filterScale. d) Set sourceImageRect = filterRegion. e) Recreate the sourceImage if needed. f) setup the context for drawing the target renderer. 4. Managing the targetBoundingBox and the filterRegion is moved from CSSFilter to RenderLayerFilters. * css/CSSFilterImageValue.cpp: (WebCore::CSSFilterImageValue::image): * platform/graphics/filters/Filter.cpp: (WebCore::Filter::Filter): * platform/graphics/filters/Filter.h: (WebCore::Filter::Filter): * rendering/CSSFilter.cpp: (WebCore::CSSFilter::create): (WebCore::CSSFilter::CSSFilter): (WebCore::createSVGFilter): (WebCore::CSSFilter::buildFilterFunctions): (WebCore::CSSFilter::apply): (WebCore::CSSFilter::setFilterRegion): (WebCore::m_hasFilterThatShouldBeRestrictedBySecurityOrigin): Deleted. (WebCore::CSSFilter::updateBackingStoreRect): Deleted. (WebCore::CSSFilter::computeSourceImageRectForDirtyRect): Deleted. (WebCore::CSSFilter::setSourceImageRect): Deleted. * rendering/CSSFilter.h: * rendering/RenderLayerFilters.cpp: (WebCore::RenderLayerFilters::buildFilter): (WebCore::RenderLayerFilters::allocateBackingStoreIfNeeded): (WebCore::RenderLayerFilters::beginFilterEffect): (WebCore::RenderLayerFilters::applyFilterEffect): (WebCore::RenderLayerFilters::allocateBackingStore): Deleted. * rendering/RenderLayerFilters.h: * rendering/svg/RenderSVGResourceFilter.cpp: (WebCore::RenderSVGResourceFilter::applyResource): * rendering/svg/SVGRenderTreeAsText.cpp: (WebCore::writeSVGResourceContainer): * svg/graphics/filters/SVGFilter.cpp: (WebCore::SVGFilter::create): (WebCore::SVGFilter::SVGFilter): * svg/graphics/filters/SVGFilter.h: LayoutTests: Unskip layout tests which were skipped in r285597. * TestExpectations: Canonical link: https://commits.webkit.org/244878@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286546 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent cc3580d commit f27a530

File tree

14 files changed

+162
-118
lines changed

14 files changed

+162
-118
lines changed

LayoutTests/ChangeLog

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2021-12-06 Said Abou-Hallawa <said@apple.com>
2+
3+
[GPU Process] (REGRESSION r285597): Set the filterRegion of the CSSFilter after it is created
4+
https://bugs.webkit.org/show_bug.cgi?id=233849
5+
6+
Reviewed by Cameron McCormack.
7+
8+
Unskip layout tests which were skipped in r285597.
9+
10+
* TestExpectations:
11+
112
2021-12-04 Antoine Quint <graouts@webkit.org>
213

314
CSS animation sorting may crash due to AnimationList copy upon CSS Animation removal

LayoutTests/TestExpectations

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5163,19 +5163,3 @@ imported/blink/http/tests/plugins [ Skip ]
51635163
imported/blink/plugins [ Skip ]
51645164
js/dom/reflect-set-onto-dom.html [ Skip ]
51655165
userscripts/user-script-plugin-document.html [ Skip ]
5166-
5167-
# These filter related failures should be fixed once webkit.org/b/232705 is resolved
5168-
webkit.org/b/232705 compositing/filters/sw-nested-shadow-overlaps-hw-nested-shadow.html [ Skip ]
5169-
webkit.org/b/232705 compositing/filters/sw-shadow-overlaps-hw-layer.html [ Skip ]
5170-
webkit.org/b/232705 compositing/filters/sw-shadow-overlaps-hw-shadow.html [ Skip ]
5171-
webkit.org/b/232705 css3/filters/effect-reference-delete.html [ Skip ]
5172-
webkit.org/b/232705 css3/filters/svg-blur-filter-clipped.html [ Skip ]
5173-
webkit.org/b/232705 fast/filter-image/background-filter-image.html [ Skip ]
5174-
webkit.org/b/232705 fast/filter-image/filter-image-blur.html [ Skip ]
5175-
webkit.org/b/232705 fast/filter-image/filter-image-svg.html [ Skip ]
5176-
webkit.org/b/232705 fast/filter-image/filter-image.html [ Skip ]
5177-
webkit.org/b/232705 imported/mozilla/svg/dynamic-filter-contents-01a.svg [ Skip ]
5178-
webkit.org/b/232705 imported/mozilla/svg/filters/feComposite-2.svg [ Skip ]
5179-
webkit.org/b/232705 imported/mozilla/svg/filters/feSpecularLighting-1.svg [ Skip ]
5180-
webkit.org/b/232705 svg/custom/resources-css-scaled.html [ Skip ]
5181-
webkit.org/b/232705 svg/filters/feLighting-clipped.svg [ Skip ]

Source/WebCore/ChangeLog

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,63 @@
1+
2021-12-06 Said Abou-Hallawa <said@apple.com>
2+
[GPU Process] (REGRESSION r285597): Set the filterRegion of the CSSFilter after it is created
3+
https://bugs.webkit.org/show_bug.cgi?id=233849
4+
5+
Reviewed by Cameron McCormack.
6+
7+
In this patch:
8+
9+
1. sourceImageRect is no longer passed as an argument to SVGFilter. It
10+
should be passed only when SVGFilter::apply() is called.
11+
12+
2. CSSFilter::create() will create and build the FilterFunctions from the
13+
FilterOperations. So we have to pass the targetBoundingBox since the
14+
FilterEffects will need it when calculating the outsets.
15+
16+
3. The steps in RenderLayerFilters::beginFilterEffect() are:
17+
a) Recreate the CSSFilter if the targetBoundingBox changes.
18+
b) Calculate the filterRegion = targetBoundingBox + outsets
19+
c) Clamp the filterRegion if needed. This will change the filterScale.
20+
d) Set sourceImageRect = filterRegion.
21+
e) Recreate the sourceImage if needed.
22+
f) setup the context for drawing the target renderer.
23+
24+
4. Managing the targetBoundingBox and the filterRegion is moved from
25+
CSSFilter to RenderLayerFilters.
26+
27+
* css/CSSFilterImageValue.cpp:
28+
(WebCore::CSSFilterImageValue::image):
29+
* platform/graphics/filters/Filter.cpp:
30+
(WebCore::Filter::Filter):
31+
* platform/graphics/filters/Filter.h:
32+
(WebCore::Filter::Filter):
33+
* rendering/CSSFilter.cpp:
34+
(WebCore::CSSFilter::create):
35+
(WebCore::CSSFilter::CSSFilter):
36+
(WebCore::createSVGFilter):
37+
(WebCore::CSSFilter::buildFilterFunctions):
38+
(WebCore::CSSFilter::apply):
39+
(WebCore::CSSFilter::setFilterRegion):
40+
(WebCore::m_hasFilterThatShouldBeRestrictedBySecurityOrigin): Deleted.
41+
(WebCore::CSSFilter::updateBackingStoreRect): Deleted.
42+
(WebCore::CSSFilter::computeSourceImageRectForDirtyRect): Deleted.
43+
(WebCore::CSSFilter::setSourceImageRect): Deleted.
44+
* rendering/CSSFilter.h:
45+
* rendering/RenderLayerFilters.cpp:
46+
(WebCore::RenderLayerFilters::buildFilter):
47+
(WebCore::RenderLayerFilters::allocateBackingStoreIfNeeded):
48+
(WebCore::RenderLayerFilters::beginFilterEffect):
49+
(WebCore::RenderLayerFilters::applyFilterEffect):
50+
(WebCore::RenderLayerFilters::allocateBackingStore): Deleted.
51+
* rendering/RenderLayerFilters.h:
52+
* rendering/svg/RenderSVGResourceFilter.cpp:
53+
(WebCore::RenderSVGResourceFilter::applyResource):
54+
* rendering/svg/SVGRenderTreeAsText.cpp:
55+
(WebCore::writeSVGResourceContainer):
56+
* svg/graphics/filters/SVGFilter.cpp:
57+
(WebCore::SVGFilter::create):
58+
(WebCore::SVGFilter::SVGFilter):
59+
* svg/graphics/filters/SVGFilter.h:
60+
161
2021-12-06 Alex Christensen <achristensen@webkit.org>
262

363
WKWebpagePreferences._activeContentRuleListActionPatterns should be an NSDictionary of identifier to allowed patterns

Source/WebCore/css/CSSFilterImageValue.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,12 @@ RefPtr<Image> CSSFilterImageValue::image(RenderElement& renderer, const FloatSiz
124124
auto sourceImageRect = FloatRect { { }, size };
125125
sourceImage->context().drawImage(*image, sourceImageRect);
126126

127-
auto cssFilter = CSSFilter::create(m_filterOperations, renderingMode);
128-
129-
cssFilter->setSourceImageRect(sourceImageRect);
130-
if (!cssFilter->buildFilterFunctions(renderer, m_filterOperations))
127+
auto cssFilter = CSSFilter::create(renderer, m_filterOperations, renderingMode, FloatSize { 1, 1 }, Filter::ClipOperation::Intersect, sourceImageRect);
128+
if (!cssFilter)
131129
return &Image::nullImage();
132130

131+
cssFilter->setFilterRegion(sourceImageRect);
132+
133133
if (auto image = sourceImage->filteredImage(*cssFilter))
134134
return image;
135135

Source/WebCore/platform/graphics/filters/Filter.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,12 @@
3232

3333
namespace WebCore {
3434

35-
Filter::Filter(Filter::Type filterType, RenderingMode renderingMode, const FloatSize& filterScale, ClipOperation clipOperation)
35+
Filter::Filter(Filter::Type filterType, RenderingMode renderingMode, const FloatSize& filterScale, ClipOperation clipOperation, const FloatRect& filterRegion)
3636
: FilterFunction(filterType)
3737
, m_renderingMode(renderingMode)
3838
, m_filterScale(filterScale)
3939
, m_clipOperation(clipOperation)
40-
{
41-
}
42-
43-
Filter::Filter(Filter::Type filterType, RenderingMode renderingMode, const FloatSize& filterScale, const FloatRect& sourceImageRect, const FloatRect& filterRegion, ClipOperation clipOperation)
44-
: FilterFunction(filterType)
45-
, m_renderingMode(renderingMode)
46-
, m_filterScale(filterScale)
47-
, m_sourceImageRect(sourceImageRect)
4840
, m_filterRegion(filterRegion)
49-
, m_clipOperation(clipOperation)
5041
{
5142
}
5243

Source/WebCore/platform/graphics/filters/Filter.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,17 @@ class Filter : public FilterFunction {
7777

7878
protected:
7979
using FilterFunction::FilterFunction;
80-
Filter(Filter::Type, RenderingMode, const FloatSize& filterScale, ClipOperation = ClipOperation::Intersect);
81-
Filter(Filter::Type, RenderingMode, const FloatSize& filterScale, const FloatRect& sourceImageRect, const FloatRect& filterRegion, ClipOperation = ClipOperation::Intersect);
80+
Filter(Filter::Type, RenderingMode, const FloatSize& filterScale, ClipOperation, const FloatRect& filterRegion = { });
8281

8382
private:
8483
RenderingMode m_renderingMode;
85-
8684
FloatSize m_filterScale;
87-
FloatRect m_sourceImageRect;
85+
ClipOperation m_clipOperation;
8886
FloatRect m_filterRegion;
8987

88+
// FIXME: these should not be members of Filter. They should be passed to Filter::apply().
89+
FloatRect m_sourceImageRect;
9090
RefPtr<ImageBuffer> m_sourceImage;
91-
92-
ClipOperation m_clipOperation;
9391
};
9492

9593
} // namespace WebCore

Source/WebCore/rendering/CSSFilter.cpp

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,26 @@
4444

4545
namespace WebCore {
4646

47-
RefPtr<CSSFilter> CSSFilter::create(const FilterOperations& operations, RenderingMode renderingMode, float scaleFactor, ClipOperation clipOperation)
47+
RefPtr<CSSFilter> CSSFilter::create(RenderElement& renderer, const FilterOperations& operations, RenderingMode renderingMode, const FloatSize& filterScale, ClipOperation clipOperation, const FloatRect& targetBoundingBox)
4848
{
4949
bool hasFilterThatMovesPixels = operations.hasFilterThatMovesPixels();
5050
bool hasFilterThatShouldBeRestrictedBySecurityOrigin = operations.hasFilterThatShouldBeRestrictedBySecurityOrigin();
5151

52-
return adoptRef(*new CSSFilter(renderingMode, scaleFactor, clipOperation, hasFilterThatMovesPixels, hasFilterThatShouldBeRestrictedBySecurityOrigin));
52+
auto filter = adoptRef(*new CSSFilter(renderingMode, filterScale, clipOperation, hasFilterThatMovesPixels, hasFilterThatShouldBeRestrictedBySecurityOrigin));
53+
54+
if (!filter->buildFilterFunctions(renderer, operations, targetBoundingBox))
55+
return nullptr;
56+
57+
return filter;
5358
}
5459

5560
RefPtr<CSSFilter> CSSFilter::create(Vector<Ref<FilterFunction>>&& functions)
5661
{
5762
return adoptRef(new CSSFilter(WTFMove(functions)));
5863
}
5964

60-
CSSFilter::CSSFilter(RenderingMode renderingMode, float scaleFactor, ClipOperation clipOperation, bool hasFilterThatMovesPixels, bool hasFilterThatShouldBeRestrictedBySecurityOrigin)
61-
: Filter(Filter::Type::CSSFilter, renderingMode, FloatSize { scaleFactor, scaleFactor }, clipOperation)
65+
CSSFilter::CSSFilter(RenderingMode renderingMode, const FloatSize& filterScale, ClipOperation clipOperation, bool hasFilterThatMovesPixels, bool hasFilterThatShouldBeRestrictedBySecurityOrigin)
66+
: Filter(Filter::Type::CSSFilter, renderingMode, filterScale, clipOperation)
6267
, m_hasFilterThatMovesPixels(hasFilterThatMovesPixels)
6368
, m_hasFilterThatShouldBeRestrictedBySecurityOrigin(hasFilterThatShouldBeRestrictedBySecurityOrigin)
6469
{
@@ -213,7 +218,7 @@ static RefPtr<FilterEffect> createSepiaEffect(const BasicColorMatrixFilterOperat
213218
return FEColorMatrix::create(FECOLORMATRIX_TYPE_MATRIX, WTFMove(inputParameters));
214219
}
215220

216-
static RefPtr<SVGFilter> createSVGFilter(CSSFilter& filter, const ReferenceFilterOperation& filterOperation, RenderElement& renderer, FilterEffect& previousEffect)
221+
static RefPtr<SVGFilter> createSVGFilter(CSSFilter& filter, const ReferenceFilterOperation& filterOperation, RenderElement& renderer, const FloatRect& targetBoundingBox, FilterEffect& previousEffect)
217222
{
218223
auto& referencedSVGResources = renderer.ensureReferencedSVGResources();
219224
auto* filterElement = referencedSVGResources.referencedFilterElement(renderer.document(), filterOperation);
@@ -227,10 +232,10 @@ static RefPtr<SVGFilter> createSVGFilter(CSSFilter& filter, const ReferenceFilte
227232
}
228233

229234
SVGFilterBuilder builder;
230-
return SVGFilter::create(*filterElement, builder, filter.renderingMode(), filter.filterScale(), filter.sourceImageRect(), filter.filterRegion(), filter.clipOperation(), previousEffect);
235+
return SVGFilter::create(*filterElement, builder, filter.renderingMode(), filter.filterScale(), filter.clipOperation(), targetBoundingBox, previousEffect);
231236
}
232237

233-
bool CSSFilter::buildFilterFunctions(RenderElement& renderer, const FilterOperations& operations)
238+
bool CSSFilter::buildFilterFunctions(RenderElement& renderer, const FilterOperations& operations, const FloatRect& targetBoundingBox)
234239
{
235240
m_functions.clear();
236241
m_outsets = { };
@@ -287,7 +292,7 @@ bool CSSFilter::buildFilterFunctions(RenderElement& renderer, const FilterOperat
287292
break;
288293

289294
case FilterOperation::REFERENCE:
290-
filter = createSVGFilter(*this, downcast<ReferenceFilterOperation>(*operation), renderer, *previousEffect);
295+
filter = createSVGFilter(*this, downcast<ReferenceFilterOperation>(*operation), renderer, targetBoundingBox, *previousEffect);
291296
effect = nullptr;
292297
break;
293298

@@ -330,18 +335,6 @@ bool CSSFilter::buildFilterFunctions(RenderElement& renderer, const FilterOperat
330335
return true;
331336
}
332337

333-
bool CSSFilter::updateBackingStoreRect(const FloatRect& filterRect)
334-
{
335-
if (filterRect.isEmpty() || ImageBuffer::sizeNeedsClamping(filterRect.size()))
336-
return false;
337-
338-
if (filterRect == sourceImageRect())
339-
return false;
340-
341-
setSourceImageRect(filterRect);
342-
return true;
343-
}
344-
345338
RefPtr<FilterEffect> CSSFilter::lastEffect() const
346339
{
347340
if (m_functions.isEmpty())
@@ -378,33 +371,24 @@ void CSSFilter::clearIntermediateResults()
378371
RefPtr<FilterImage> CSSFilter::apply()
379372
{
380373
for (auto& function : m_functions) {
374+
if (function->isSVGFilter())
375+
downcast<SVGFilter>(function.ptr())->setSourceImageRect(sourceImageRect());
381376
if (!function->apply(*this))
382377
return nullptr;
383378
}
384379
return lastEffect()->filterImage();
385380
}
386381

387-
LayoutRect CSSFilter::computeSourceImageRectForDirtyRect(const LayoutRect& filterBoxRect, const LayoutRect& dirtyRect)
388-
{
389-
// The result of this function is the area in the "filterBoxRect" that needs to be repainted, so that we fully cover the "dirtyRect".
390-
auto rectForRepaint = dirtyRect;
391-
if (hasFilterThatMovesPixels())
392-
rectForRepaint += outsets();
393-
rectForRepaint.intersect(filterBoxRect);
394-
return rectForRepaint;
395-
}
396-
397-
void CSSFilter::setSourceImageRect(const FloatRect& sourceImageRect)
382+
void CSSFilter::setFilterRegion(const FloatRect& filterRegion)
398383
{
399-
Filter::setFilterRegion(sourceImageRect);
400-
Filter::setSourceImageRect(sourceImageRect);
384+
Filter::setFilterRegion(filterRegion);
401385

402386
for (auto& function : m_functions) {
403-
if (function->isSVGFilter()) {
404-
downcast<SVGFilter>(function.ptr())->setFilterRegion(sourceImageRect);
405-
downcast<SVGFilter>(function.ptr())->setSourceImageRect(sourceImageRect);
406-
}
387+
if (function->isSVGFilter())
388+
downcast<SVGFilter>(function.ptr())->setFilterRegion(filterRegion);
407389
}
390+
391+
clampFilterRegionIfNeeded();
408392
}
409393

410394
IntOutsets CSSFilter::outsets() const

Source/WebCore/rendering/CSSFilter.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,12 @@ class SourceGraphic;
4242
class CSSFilter final : public Filter {
4343
WTF_MAKE_FAST_ALLOCATED;
4444
public:
45-
static RefPtr<CSSFilter> create(const FilterOperations&, RenderingMode, float scaleFactor = 1, ClipOperation = ClipOperation::Intersect);
45+
static RefPtr<CSSFilter> create(RenderElement&, const FilterOperations&, RenderingMode, const FloatSize& filterScale, ClipOperation, const FloatRect& targetBoundingBox);
4646
WEBCORE_EXPORT static RefPtr<CSSFilter> create(Vector<Ref<FilterFunction>>&&);
4747

4848
const Vector<Ref<FilterFunction>>& functions() const { return m_functions; }
4949

50-
void setSourceImageRect(const FloatRect&);
51-
bool buildFilterFunctions(RenderElement&, const FilterOperations&);
50+
void setFilterRegion(const FloatRect&);
5251

5352
bool hasFilterThatMovesPixels() const { return m_hasFilterThatMovesPixels; }
5453
bool hasFilterThatShouldBeRestrictedBySecurityOrigin() const { return m_hasFilterThatShouldBeRestrictedBySecurityOrigin; }
@@ -59,13 +58,11 @@ class CSSFilter final : public Filter {
5958
void clearIntermediateResults();
6059
RefPtr<FilterImage> apply() final;
6160

62-
bool updateBackingStoreRect(const FloatRect& filterRect);
63-
64-
LayoutRect computeSourceImageRectForDirtyRect(const LayoutRect& filterBoxRect, const LayoutRect& dirtyRect);
65-
6661
private:
67-
CSSFilter(RenderingMode, float scaleFactor, ClipOperation, bool hasFilterThatMovesPixels, bool hasFilterThatShouldBeRestrictedBySecurityOrigin);
62+
CSSFilter(RenderingMode, const FloatSize& filterScale, ClipOperation, bool hasFilterThatMovesPixels, bool hasFilterThatShouldBeRestrictedBySecurityOrigin);
6863
CSSFilter(Vector<Ref<FilterFunction>>&&);
64+
65+
bool buildFilterFunctions(RenderElement&, const FilterOperations&, const FloatRect& targetBoundingBox);
6966

7067
#if USE(CORE_IMAGE)
7168
bool supportsCoreImageRendering() const final;

0 commit comments

Comments
 (0)