Bug 219423 - [GPU Process] Allow CSSFilters and SVGFilters to use RemoteImageBuffers if GPU DOM rendering is enabled
Summary: [GPU Process] Allow CSSFilters and SVGFilters to use RemoteImageBuffers if GP...
Status: RESOLVED DUPLICATE of bug 231253
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
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-02 00:58 PST by Said Abou-Hallawa
Modified: 2022-01-13 10:59 PST (History)
29 users (show)

See Also:


Attachments
Patch (27.40 KB, patch)
2020-12-02 01:29 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (179.52 KB, patch)
2020-12-03 02:23 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (88.36 KB, patch)
2020-12-03 12:47 PST, Said Abou-Hallawa
ews-feeder: commit-queue-
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 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 ***