Bug 233849 – [GPU Process] (REGRESSION r285597): Set the filterRegion of the CSSFilter after it is created

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Said Abou-Hallawa]

The filterRegion() of the CSSFilter is the union of the targetBoundingBox and its outsets. The outsets of the CSSFilter is the union of the outsets of all its functions including the referenced SVGFilter. Notice that if the outsets of a FilterFunction are relative they are calculated relative on the targetBoundingBox. So we have to create the CSSFilter first giving the create() function the targetBoundingBox. Then we calculate the outsets of the CSSFilter and finally we calculate and set its filterRegion().

[Said Abou-Hallawa]

Created attachment 445938 [details]
Combined Patch (this bug + bug 233843)

[Said Abou-Hallawa]

Created attachment 445944 [details]
Patch for review

[Said Abou-Hallawa]

Created attachment 445961 [details]
Patch

[Cameron McCormack (:heycam)]

Comment on attachment 445961 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445961&action=review

> Source/WebCore/ChangeLog:27
> +           b) Recreate the sourceImage if needed.
> +           c) setup the context for drawing the target renderer.

"e)" and "f)"

> Source/WebCore/rendering/RenderLayerFilters.cpp:174
> +        filterRegion += filter.outsets();
> +        filterRegion = intersection(filterBoxRect, filterRegion);

What about:

filterRegion += filter.outsets();
filterRegion.intersect(filterBoxRect);

> Source/WebCore/rendering/RenderLayerFilters.cpp:185
> +    if (m_sourceImageRect != filterRegion) {
> +        m_sourceImageRect = filterRegion;
> +        hasUpdatedBackingStore = true;
>      }

After this point, m_sourceImageRect and filterRegion are the same, so I think we should consistently use one or the other in the remainder of the function.

> Source/WebCore/rendering/RenderLayerFilters.cpp:202
> +    filter.setFilterRegion(m_sourceImageRect);

So here I think we should use filterRegion.

> Source/WebCore/rendering/RenderLayerFilters.cpp:-189
> -    if (!sourceGraphicsContext || filter.filterRegion().isEmpty() || ImageBuffer::sizeNeedsClamping(filter.filterRegion().size()))

In this old code, how would it be that ImageBuffer::sizeNeedsClamping would return true, since earlier we call clampFilterRegionIfNeeded?

> Source/WebCore/svg/graphics/filters/SVGFilter.h:50
> +    SVGFilter(RenderingMode, const FloatSize& filterScale, ClipOperation, const FloatRect& filterRegion, const FloatRect& targetBoundingBox,  SVGUnitTypes::SVGUnitType primitiveUnits);

Nit: two spaces after "targetBoundingBox,".

[Said Abou-Hallawa]

Created attachment 446015 [details]
Patch

[EWS]

Committed r286546 (244878@main): <https://commits.webkit.org/244878@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446015 [details].

[Radar WebKit Bug Importer]

<rdar://problem/86110979>