Bug 219423

Summary:

[GPU Process] Allow CSSFilters and SVGFilters to use RemoteImageBuffers if GPU DOM rendering is enabled

Product:

WebKit

Reporter:

Said Abou-Hallawa <sabouhallawa>

Component:

Layout and Rendering

Assignee:

Said Abou-Hallawa <sabouhallawa>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

annulen, bfulgham, cdumez, changseok, dino, eric.carlson, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, graouts, gyuyoung.kim, hi, japhet, jer.noble, joepeck, kangil.han, kondapallykalyan, macpherson, menard, pdr, philipj, ryuan.choi, schenney, sergio, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	ews-feeder: commit-queue-

Description Said Abou-Hallawa 2020-12-02 00:58:59 PST

The filters are drawing commands and filter effects commands. The first step is to move the drawing commands to the GPU Process.

Comment 1 Said Abou-Hallawa 2020-12-02 01:29:03 PST

Created attachment 415198 [details]
Patch

Comment 2 Simon Fraser (smfr) 2020-12-02 08:31:16 PST

Comment on attachment 415198 [details]
Patch

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

> Source/WebCore/platform/graphics/filters/Filter.h:37
> -    Filter(const AffineTransform& absoluteTransform, float filterScale = 1)
> +    Filter(const AffineTransform& absoluteTransform, float filterScale = 1, HostWindow* hostWindow = nullptr)
>          : m_absoluteTransform(absoluteTransform)

I don't think this is the right approach. Filter is much too low-level a class to deal with HostWindow.

Comment 3 Said Abou-Hallawa 2020-12-02 10:26:38 PST

Comment on attachment 415198 [details]
Patch

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

>> Source/WebCore/platform/graphics/filters/Filter.h:37
>>          : m_absoluteTransform(absoluteTransform)
> 
> I don't think this is the right approach. Filter is much too low-level a class to deal with HostWindow.

ImageBuffer deals with HostWindow and ImageBuffer is even in a lower level than Filter because Filter owns and creates the ImageBuffer. So why is it okay to make imageBuffer deal with HostWindow but not for the Filter?

Comment 4 Simon Fraser (smfr) 2020-12-02 10:43:06 PST

(In reply to Said Abou-Hallawa from comment #3)
> Comment on attachment 415198 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=415198&action=review
> 
> >> Source/WebCore/platform/graphics/filters/Filter.h:37
> >>          : m_absoluteTransform(absoluteTransform)
> > 
> > I don't think this is the right approach. Filter is much too low-level a class to deal with HostWindow.
> 
> ImageBuffer deals with HostWindow and ImageBuffer is even in a lower level
> than Filter because Filter owns and creates the ImageBuffer. So why is it
> okay to make imageBuffer deal with HostWindow but not for the Filter?

Filter is a lightweight mostly-data class which is context-free (which is a good thing). If it has a HostWindow then it's encumbered with more state.

If we need a usage of HostWindow, it should be passed in as an argument to functions where Filters create ImageBuffers, but not stored as a member variable.

And this would look much nicer if what was passed in was an ImageBufferFactory, not a HostWindow.

Comment 5 Said Abou-Hallawa 2020-12-03 02:23:19 PST

Created attachment 415286 [details]
Patch

Comment 6 Simon Fraser (smfr) 2020-12-03 09:51:56 PST

Comment on attachment 415286 [details]
Patch

Let's make adding ImageBufferFactory its own patch.

Comment 7 Said Abou-Hallawa 2020-12-03 12:47:48 PST

Created attachment 415327 [details]
Patch

Comment 8 Radar WebKit Bug Importer 2020-12-09 00:59:13 PST

<rdar://problem/72128302>

Comment 9 Said Abou-Hallawa 2022-01-13 10:59:44 PST

The approach of this patch was incorrect. This correct approach was taken by bug 231253.

*** This bug has been marked as a duplicate of bug 231253 ***