[webkit-reviews] review granted: [Bug 218928] Initial implementation of DOM rendering via the GPU process : [Attachment 414106] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 218928: Initial implementation of DOM rendering via the GPU process
https://bugs.webkit.org/show_bug.cgi?id=218928

Attachment 414106: Patch

https://bugs.webkit.org/attachment.cgi?id=414106&action=review




--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 414106
  --> https://bugs.webkit.org/attachment.cgi?id=414106
Patch

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

> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:183
> +	   m_frontBuffer.imageBuffer =
m_layer->context()->ensureRemoteRenderingBackendProxy().createImageBuffer(backi
ngStoreSize(), m_acceleratesDrawing ? WebCore::RenderingMode::Accelerated :
WebCore::RenderingMode::Unaccelerated, 1, WebCore::ColorSpace::SRGB,
pixelFormat());

This feels a bit weird. Maybe RemoteLayerBackingStore needs a delegate.

> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:321
> +    // FIXME: This method has a weird name. This is "submit work".
> +    m_frontBuffer.imageBuffer->flushDrawingContextAndCommit();

Yes

> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:362
> +    auto bitmap =
ShareableBitmap::create(WTF::get<ShareableBitmap::Handle>(*m_bufferHandle));
> +    layer.contents = (__bridge id)bitmap->makeCGImageCopy().get();

Hmm, could someone write into the sharable bitmap while the layer owns it? Or
is the "copy" really a copy?