Bug 217532

Summary: [GPU Process] Add additional support for painting video elements to 2D contexts
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: MediaAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, peng.liu6, philipj, sabouhallawa, sam, sergio, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Wenson Hsieh 2020-10-09 13:32:49 PDT 
In particular:
- CanvasRenderingContext2D.createPattern(HTMLVideoElement, …);
- createImageBitmap(HTMLVideoElement, …);
Comment 1 Wenson Hsieh 2020-10-09 14:26:48 PDT Comment hidden (obsolete)
Comment 2 Wenson Hsieh 2020-10-09 14:34:07 PDT 
Created attachment 410976 [details]
Patch
Comment 3 EWS 2020-10-09 16:39:39 PDT 
Committed r268299: <https://trac.webkit.org/changeset/268299>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410976 [details].
Comment 4 Radar WebKit Bug Importer 2020-10-09 16:40:17 PDT 
<rdar://problem/70158828>
Comment 5 Sam Weinig 2020-10-10 10:25:10 PDT 
Comment on attachment 410976 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:7208
> +    case RenderingPurpose::MediaPainting:
> +        return m_page->settings().useGPUProcessForMediaEnabled();

It's not ideal that WebCore::Settings is being used for this. WebCore really shouldn't know about the GPUProcess. Perhaps its ok, because Settings is just like "a bag of state", but we should try to avoid this. This could probably be avoided by just storing this state on the WebPage and/or keeping around the WebPreferencesStore that get's passed in preferencesDidChange.
Comment 6 Wenson Hsieh 2020-10-10 10:37:12 PDT 
Comment on attachment 410976 [details]
Patch

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

>> Source/WebKit/WebProcess/WebPage/WebPage.cpp:7208
>> +        return m_page->settings().useGPUProcessForMediaEnabled();
> 
> It's not ideal that WebCore::Settings is being used for this. WebCore really shouldn't know about the GPUProcess. Perhaps its ok, because Settings is just like "a bag of state", but we should try to avoid this. This could probably be avoided by just storing this state on the WebPage and/or keeping around the WebPreferencesStore that get's passed in preferencesDidChange.

Yeah — Said and I avoided adding a similar “render canvas in GPU process” flag in WebCore::Settings for this reason. I think ideally, the GPUP-for-media setting should be refactored to be more like the canvas one (but I didn’t try to fix that in this patch).