Bug 208560 – [GPU Process] Implement CanvasRenderingContext2D.getImageData()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[GPU Process] Implement CanvasRenderingContext2D.getImageData()

[Myles C. Maxfield]

Created attachment 392373 [details]
WIP

[Myles C. Maxfield]

Created attachment 392374 [details]
WIP

[Myles C. Maxfield]

Created attachment 392488 [details]
WIP

[Myles C. Maxfield]

Created attachment 392494 [details]
WIP

[Radar WebKit Bug Importer]

<rdar://problem/60060618>

[Myles C. Maxfield]

Created attachment 392498 [details]
Patch

[Myles C. Maxfield]

Created attachment 392511 [details]
Patch

[Jon Lee]

Comment on attachment 392511 [details]
Patch

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

> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBuffer.h:89
> +    bool asyncronouslyFlushDrawingContext()

sp: asynchronously

[Myles C. Maxfield]

Created attachment 392659 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 392659 [details]
Patch

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

> Source/WebKit/GPUProcess/graphics/RemoteImageBufferMessageHandlerProxy.h:51
> +    virtual RefPtr<WebCore::ImageData> performGetImageData(WebCore::AlphaPremultiplication outputFormat, WebCore::IntRect srcRect) = 0;

const WebCore::IntRect&

> Source/WebKit/GPUProcess/graphics/RemoteImageBufferProxy.h:60
> +    RefPtr<WebCore::ImageData> performGetImageData(WebCore::AlphaPremultiplication outputFormat, WebCore::IntRect srcRect) override
> +    {
> +        return BaseConcreteImageBuffer::getImageData(outputFormat, srcRect);
> +    }

You could have the same name getImageData() instead of performGetImageData(). Just define RemoteImageBufferMessageHandlerProxy::getImageData() exactly the same as ImageBuffer::getImageData() is defined. Change the above one to be 

    RefPtr<WebCore::ImageData> getImageData(WebCore::AlphaPremultiplication outputFormat, const WebCore::IntRect& srcRect) const override
    {
        return BaseConcreteImageBuffer::getImageData(outputFormat, srcRect);
    }

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackendProxy.cpp:108
> +            completionHandler(WebCore::IntSize(), IPC::SharedBufferDataReference());

Should not we do a better job here? Can't we return an ImageData with the required size with zero bytes?

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackendProxy.messages.in:29
> +    CreateImageBuffer(WebCore::FloatSize logicalSize, WebCore::RenderingMode renderingMode, float resolutionScale, WebCore::ColorSpace colorSpace, WebKit::ImageBufferIdentifier imageBufferIdentifier)
> +    ReleaseImageBuffer(WebKit::ImageBufferIdentifier imageBufferIdentifier)
> +    FlushImageBufferDrawingContext(WebCore::DisplayList::DisplayList displayList, WebKit::ImageBufferFlushIdentifier flushIdentifier, WebKit::ImageBufferIdentifier imageBufferIdentifier)
> +    GetImageData(enum:uint8_t WebCore::AlphaPremultiplication outputFormat, WebCore::IntRect srcRect, WebKit::ImageBufferIdentifier imageBufferIdentifier) -> (WebCore::IntSize size, IPC::SharedBufferDataReference data) Synchronous

Is there a reason for removing the void? I am asking because Tim added asked me to add them verbally and I did not ask why.

> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBuffer.h:89
> +    bool asynchronouslyFlushDrawingContext()

asynchronously is a little bit confusing since the message is asynchronous message anyway. Can't we keep this function flushDrawingContext() and name the other flushDrawingContextAndWait()?

> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferMessageHandler.cpp:77
> +    return ImageData::create(size, Uint8ClampedArray::create(reinterpret_cast<const uint8_t*>(data.data()), data.size()));

The ImageData should be encoded and decoded as one unit. right?

[Myles C. Maxfield]

Created attachment 392842 [details]
WIP

[Myles C. Maxfield]

Created attachment 392855 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 392855 [details]
Patch for committing

Clearing flags on attachment: 392855

Committed r258069: <https://trac.webkit.org/changeset/258069>

[Jer Noble]

Follow up unified-build fix: Committed r258072: <https://trac.webkit.org/changeset/258072>