Bug 233849 - [GPU Process] (REGRESSION r285597): Set the filterRegion of the CSSFilter after it is created
Summary: [GPU Process] (REGRESSION r285597): Set the filterRegion of the CSSFilter aft...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
Keywords: InRadar
Depends on:
Blocks: 231253
  Show dependency treegraph
Reported: 2021-12-03 19:09 PST by Said Abou-Hallawa
Modified: 2021-12-06 10:21 PST (History)
19 users (show)

See Also:

Combined Patch (this bug + bug 233843) (46.96 KB, patch)
2021-12-03 19:31 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch for review (34.84 KB, patch)
2021-12-03 19:57 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (35.69 KB, patch)
2021-12-03 22:51 PST, Said Abou-Hallawa
heycam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (31.71 KB, patch)
2021-12-06 02:17 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2021-12-03 19:09:44 PST
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().
Comment 1 Said Abou-Hallawa 2021-12-03 19:31:29 PST
Created attachment 445938 [details]
Combined Patch (this bug + bug 233843)
Comment 2 Said Abou-Hallawa 2021-12-03 19:57:19 PST
Created attachment 445944 [details]
Patch for review
Comment 3 Said Abou-Hallawa 2021-12-03 22:51:37 PST
Created attachment 445961 [details]
Comment 4 Cameron McCormack (:heycam) 2021-12-04 16:54:13 PST
Comment on attachment 445961 [details]

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();

> 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,".
Comment 5 Said Abou-Hallawa 2021-12-06 02:17:29 PST
Created attachment 446015 [details]
Comment 6 EWS 2021-12-06 10:20:52 PST
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].
Comment 7 Radar WebKit Bug Importer 2021-12-06 10:21:34 PST