Bug 235073

Summary: [GPU Process] [SVG Resources] Make SVG resources create remote ImageBuffers for their drawing
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, darin, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, schenney, sergio, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 234165    
Attachments:
Description Flags
Patch
darin: review+
Patch none

Description Said Abou-Hallawa 2022-01-11 09:13:50 PST
This will force the SVG resources (i.e. clippers, masks, patterns and gradients) to apply their drawing in the GPU Process. The filters are tracked by bug 231253. We still need to optimize the usage of these ImageBuffers, like not getting a NativeImage from the remote ImageBuffer in WebProcess and then draw the NativeImage to another remote ImageBuffer.
Comment 1 Radar WebKit Bug Importer 2022-01-11 09:15:35 PST
<rdar://problem/87402419>
Comment 2 Said Abou-Hallawa 2022-01-11 09:20:42 PST
Created attachment 448852 [details]
Patch
Comment 3 Darin Adler 2022-01-11 09:47:08 PST
Comment on attachment 448852 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [GPU Process] Make SVG resources create remote ImageBuffers for their drawing

Is there a way to test that this has the desired result?

> Source/WebKit/GPUProcess/graphics/QualifiedResourceHeap.h:164
> +        if (!std::holds_alternative<Ref<T>>(iterator->value))
> +            return nullptr;
>          return std::get<Ref<T>>(iterator->value).ptr();

Would be nice to use get_if instead of calling holds_alternative and then get. Would reduce reference count churn, and code would be less repetitive:

    auto value = std::get_if<Ref<T>>(&iterator->value);
    return value ? value->ptr() : nullptr;
Comment 4 Said Abou-Hallawa 2022-01-11 21:49:00 PST
Created attachment 448906 [details]
Patch
Comment 5 Said Abou-Hallawa 2022-01-11 22:12:47 PST
Comment on attachment 448852 [details]
Patch

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

>> Source/WebCore/ChangeLog:3
>> +        [GPU Process] Make SVG resources create remote ImageBuffers for their drawing
> 
> Is there a way to test that this has the desired result?

This change fixed some of the layout tests when running them with "--use-gpu-process". I un-skipped them from LayoutTests/gpu-process/TestExpectations

>> Source/WebKit/GPUProcess/graphics/QualifiedResourceHeap.h:164
>>          return std::get<Ref<T>>(iterator->value).ptr();
> 
> Would be nice to use get_if instead of calling holds_alternative and then get. Would reduce reference count churn, and code would be less repetitive:
> 
>     auto value = std::get_if<Ref<T>>(&iterator->value);
>     return value ? value->ptr() : nullptr;

Fixed.
Comment 6 EWS 2022-01-11 23:50:33 PST
Committed r287911 (245944@main): <https://commits.webkit.org/245944@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448906 [details].