Bug 208375 - Add an internal setting to enable or disable canvas rendering in the GPU Process
Summary: Add an internal setting to enable or disable canvas rendering in the GPU Process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-28 07:33 PST by Wenson Hsieh
Modified: 2020-02-28 19:01 PST (History)
11 users (show)

See Also:


Attachments
Patch (3.64 KB, patch)
2020-02-28 07:36 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (20.64 KB, patch)
2020-02-28 16:17 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2020-02-28 07:33:19 PST
SSIA
Comment 1 Wenson Hsieh 2020-02-28 07:36:47 PST
Created attachment 391978 [details]
Patch
Comment 2 Sam Weinig 2020-02-28 08:52:58 PST
Comment on attachment 391978 [details]
Patch

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

> Source/WebCore/page/Settings.yaml:726
> +useRemoteDrawingForCanvas:
> +  initial: false

This doesn't seem like something WebCore should know or care about.
Comment 3 Said Abou-Hallawa 2020-02-28 09:05:21 PST
Comment on attachment 391978 [details]
Patch

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

> Source/WebCore/html/HTMLCanvasElement.cpp:899
> +    bool usesRemoteDrawing = document().settings().useRemoteDrawingForCanvas();

Please remove the FIXME comment.

>> Source/WebCore/page/Settings.yaml:726
>> +  initial: false
> 
> This doesn't seem like something WebCore should know or care about.

Can you please be more explict?

The plan is to enable remote rendering for Canvas drawing only. How can we know outside the HTMLCanvasElement::create() that we want to create an ImageBuffer for remote drawing?

Should we add a function like this one:

    HostWindow::createImageBufferForCanvas(...)

And call it from HTMLCanvasElement::createImageBuffer()?
Comment 4 Wenson Hsieh 2020-02-28 09:21:22 PST
Comment on attachment 391978 [details]
Patch

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

>> Source/WebCore/html/HTMLCanvasElement.cpp:899
>> +    bool usesRemoteDrawing = document().settings().useRemoteDrawingForCanvas();
> 
> Please remove the FIXME comment.

Oops — good catch!

>>> Source/WebCore/page/Settings.yaml:726
>>> +  initial: false
>> 
>> This doesn't seem like something WebCore should know or care about.
> 
> Can you please be more explict?
> 
> The plan is to enable remote rendering for Canvas drawing only. How can we know outside the HTMLCanvasElement::create() that we want to create an ImageBuffer for remote drawing?
> 
> Should we add a function like this one:
> 
>     HostWindow::createImageBufferForCanvas(...)
> 
> And call it from HTMLCanvasElement::createImageBuffer()?

Interesting...my understanding was that the GPU Process was not something WebCore should care about, but that the notion of "remote drawing" was okay (hence, RemoteAccelerated on RenderingMode).
Comment 5 Tim Horton 2020-02-28 13:45:05 PST
Comment on attachment 391978 [details]
Patch

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

>>>> Source/WebCore/page/Settings.yaml:726
>>>> +  initial: false
>>> 
>>> This doesn't seem like something WebCore should know or care about.
>> 
>> Can you please be more explict?
>> 
>> The plan is to enable remote rendering for Canvas drawing only. How can we know outside the HTMLCanvasElement::create() that we want to create an ImageBuffer for remote drawing?
>> 
>> Should we add a function like this one:
>> 
>>     HostWindow::createImageBufferForCanvas(...)
>> 
>> And call it from HTMLCanvasElement::createImageBuffer()?
> 
> Interesting...my understanding was that the GPU Process was not something WebCore should care about, but that the notion of "remote drawing" was okay (hence, RemoteAccelerated on RenderingMode).

I'm with Sam, doesn't seem like we need to infect WebCore with this. Just hide it behind ChromeClient like everything else (see e.g.:

    // Allows ports to customize the type of graphics layers created by this page.
    virtual GraphicsLayerFactory* graphicsLayerFactory() const { return nullptr; }

)
Comment 6 Wenson Hsieh 2020-02-28 14:32:06 PST
Comment on attachment 391978 [details]
Patch

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

>>>>> Source/WebCore/page/Settings.yaml:726
>>>>> +  initial: false
>>>> 
>>>> This doesn't seem like something WebCore should know or care about.
>>> 
>>> Can you please be more explict?
>>> 
>>> The plan is to enable remote rendering for Canvas drawing only. How can we know outside the HTMLCanvasElement::create() that we want to create an ImageBuffer for remote drawing?
>>> 
>>> Should we add a function like this one:
>>> 
>>>     HostWindow::createImageBufferForCanvas(...)
>>> 
>>> And call it from HTMLCanvasElement::createImageBuffer()?
>> 
>> Interesting...my understanding was that the GPU Process was not something WebCore should care about, but that the notion of "remote drawing" was okay (hence, RemoteAccelerated on RenderingMode).
> 
> I'm with Sam, doesn't seem like we need to infect WebCore with this. Just hide it behind ChromeClient like everything else (see e.g.:
> 
>     // Allows ports to customize the type of graphics layers created by this page.
>     virtual GraphicsLayerFactory* graphicsLayerFactory() const { return nullptr; }
> 
> )

Okay, I see — it seems we should (at the very least) be asking the client layer for a RenderingMode, then, so it never needs to have a notion of Remote{Acclerated|Unaccelerated} at all.
Comment 7 Wenson Hsieh 2020-02-28 16:17:27 PST
Created attachment 392030 [details]
Patch
Comment 8 WebKit Commit Bot 2020-02-28 19:00:27 PST
Comment on attachment 392030 [details]
Patch

Clearing flags on attachment: 392030

Committed r257677: <https://trac.webkit.org/changeset/257677>
Comment 9 WebKit Commit Bot 2020-02-28 19:00:28 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2020-02-28 19:01:17 PST
<rdar://problem/59911910>