Bug 205200 – Implement encoding/decoding for DisplayList::DrawNativeImage

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Wenson Hsieh]

This is exercised by multiple failing layout tests in fast/canvas, when rendering canvas in the GPU process.

[Wenson Hsieh]

Created attachment 385579 [details]
Patch

[Wenson Hsieh]

Created attachment 385580 [details]
Fix GTK/WPE

[Simon Fraser (smfr)]

Comment on attachment 385580 [details]
Fix GTK/WPE

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

> Source/WebKit/ChangeLog:10
> +        Add helper functions to encode and decode NativeImagePtr (RetainPtr<CGImageRef> on Cocoa platforms). This
> +        mirrors the implementation of encoding and decoding for WebCore::Image, which create and draws into
> +        ShareableBitmap, and send a handle to the data over IPC.

I think we should try to remove drawNativeImage. Here you're forced to call into CG in the web process, and we're trying to avoid that in general.

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1384
> +    Optional<FloatRect> destination;

destinationRect

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:1050
> +    if (graphicsContext)
> +        graphicsContext->drawNativeImage(image, { }, FloatRect({ }, imageSize), FloatRect({ }, imageSize));

If we fail to make a graphicsContext shouldn't the encoding fail?

[Wenson Hsieh]

Comment on attachment 385580 [details]
Fix GTK/WPE

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

Thanks for the review!

>> Source/WebKit/ChangeLog:10
>> +        ShareableBitmap, and send a handle to the data over IPC.
> 
> I think we should try to remove drawNativeImage. Here you're forced to call into CG in the web process, and we're trying to avoid that in general.

Added a sentence to the ChangeLog describing this, and also filed webkit.org/b/205213.

>> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1384
>> +    Optional<FloatRect> destination;
> 
> destinationRect

👍🏻. Also, renamed `m_destination` to `m_destinationRect`.

>> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:1050
>> +        graphicsContext->drawNativeImage(image, { }, FloatRect({ }, imageSize), FloatRect({ }, imageSize));
> 
> If we fail to make a graphicsContext shouldn't the encoding fail?

Good call — I changed both this function and the existing encodeImage() function to encode `!!graphicsContext`, and then made decoding fail if we failed to make a graphicsContext.

[Wenson Hsieh]

Created attachment 385630 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 385630 [details]
Patch

Clearing flags on attachment: 385630

Committed r253495: <https://trac.webkit.org/changeset/253495>

[Radar WebKit Bug Importer]

<rdar://problem/57923065>