Bug 218928 – Initial implementation of DOM rendering via the GPU process

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Initial implementation of DOM rendering via the GPU process

[Tim Horton]

Created attachment 414106 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 414106 [details]
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(backingStoreSize(), 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?

[Tim Horton]

Created attachment 414155 [details]
Patch

[Tim Horton]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 414106 [details]
> 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(backingStoreSize(), m_acceleratesDrawing ? WebCore::RenderingMode::Accelerated : WebCore::RenderingMode::Unaccelerated, 1, WebCore::ColorSpace::SRGB, pixelFormat());
> 
> This feels a bit weird. Maybe RemoteLayerBackingStore needs a delegate.

A little weird, but RemoteLayerTreeContext is kind of RLBS's "delegate", in a sense. (Other things that it uses RemoteLayerTreeContext for include "hey I was created", and "I'm about to paint", both of which are veryyyy delegatey.

> > 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?

It's not a real copy (I have no idea why it's called that). "Someone" could, for sure, but we won't. The same is true of the IOSurfaces, though??

[EWS]

Committed r269824: <https://trac.webkit.org/changeset/269824>

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

[Radar WebKit Bug Importer]

<rdar://problem/71410310>

[Sam Weinig]

Comment on attachment 414155 [details]
Patch

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

> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h:43
> +class ThreadSafeRemoteImageBufferFlusher : public WebCore::ThreadSafeImageBufferFlusher {

Can this be final?