Read remote images back into local images before sending them to othe…

…r threads. https://bugs.webkit.org/show_bug.cgi?id=248416 Reviewed by Kimmo Kinnunen. We currently can't use RemoteRenderingBackendProxy from other threads, so sending RemoteImageBufferProxys to other threads leaves us in an invalid state. This reads them back into local images before transferring so that we can access the contents. * Source/WebCore/html/ImageBitmap.cpp: (WebCore::ImageBitmap::detachBitmaps): (WebCore::ImageBitmap::~ImageBitmap): * Source/WebCore/html/ImageBitmapBacking.cpp: (WebCore::ImageBitmapBacking::takeImageBufferForDifferentThread): * Source/WebCore/html/ImageBitmapBacking.h: * Source/WebCore/html/OffscreenCanvas.cpp: (WebCore::OffscreenCanvas::detach): (WebCore::OffscreenCanvas::commitToPlaceholderCanvas): (WebCore::OffscreenCanvas::takeImageBufferForDifferentThread const): (WebCore::OffscreenCanvas::takeImageBuffer const): Deleted. * Source/WebCore/html/OffscreenCanvas.h: * Source/WebCore/platform/graphics/ImageBuffer.cpp: (WebCore::ImageBuffer::cloneForDifferentThread): (WebCore::ImageBuffer::sinkIntoBufferForDifferentThread): * Source/WebCore/platform/graphics/ImageBuffer.h: * Source/WebCore/platform/graphics/NativeImage.h: * Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.cpp: (WebKit::RemoteImageBufferProxy::drawConsuming): (WebKit::RemoteImageBufferProxy::sinkIntoNativeImage): (WebKit::RemoteImageBufferProxy::sinkIntoBufferForDifferentThread): (WebKit::RemoteImageBufferProxy::cloneForDifferentThread): * Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h: Canonical link: https://commits.webkit.org/257103@main
WebKit · Nov 29, 2022 · ab926d6 · ab926d6
1 parent 9b75120
commit ab926d6
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 13 deletions.
diff --git a/Source/WebCore/html/ImageBitmap.cpp b/Source/WebCore/html/ImageBitmap.cpp
@@ -123,8 +123,13 @@ RefPtr<ImageBuffer> ImageBitmap::createImageBuffer(ScriptExecutionContext& scrip
 
 Vector<std::optional<ImageBitmapBacking>> ImageBitmap::detachBitmaps(Vector<RefPtr<ImageBitmap>>&& bitmaps)
 {
-    return WTF::map(WTFMove(bitmaps), [](auto&& bitmap) {
-        return bitmap->takeImageBitmapBacking();
+    return WTF::map(WTFMove(bitmaps), [](auto&& bitmap) -> std::optional<ImageBitmapBacking> {
+        std::optional<ImageBitmapBacking> backing = bitmap->takeImageBitmapBacking();
+        if (!backing)
+            return std::nullopt;
+        if (auto copyBuffer = backing->takeImageBufferForDifferentThread())
+            return ImageBitmapBacking(WTFMove(copyBuffer), backing->serializationState());
+        return std::nullopt;
     });
 }
 
@@ -905,10 +910,6 @@ ImageBitmap::ImageBitmap(std::optional<ImageBitmapBacking>&& backingStore)
 
 ImageBitmap::~ImageBitmap()
 {
-    if (isMainThread())
-        return;
-    if (auto imageBuffer = takeImageBuffer())
-        callOnMainThread([imageBuffer = WTFMove(imageBuffer)] { });
 }
 
 std::optional<ImageBitmapBacking> ImageBitmap::takeImageBitmapBacking()

diff --git a/Source/WebCore/html/ImageBitmapBacking.cpp b/Source/WebCore/html/ImageBitmapBacking.cpp
@@ -45,6 +45,11 @@ RefPtr<ImageBuffer> ImageBitmapBacking::takeImageBuffer()
     return WTFMove(m_bitmapData);
 }
 
+RefPtr<ImageBuffer> ImageBitmapBacking::takeImageBufferForDifferentThread()
+{
+    return ImageBuffer::sinkIntoBufferForDifferentThread(WTFMove(m_bitmapData));
+}
+
 unsigned ImageBitmapBacking::width() const
 {
     // FIXME: Is this the right width?

diff --git a/Source/WebCore/html/ImageBitmapBacking.h b/Source/WebCore/html/ImageBitmapBacking.h
@@ -42,6 +42,7 @@ class ImageBitmapBacking {
 
     ImageBuffer* buffer() const;
     RefPtr<ImageBuffer> takeImageBuffer();
+    RefPtr<ImageBuffer> takeImageBufferForDifferentThread();
 
     unsigned width() const;
     unsigned height() const;

diff --git a/Source/WebCore/html/OffscreenCanvas.cpp b/Source/WebCore/html/OffscreenCanvas.cpp
@@ -446,7 +446,7 @@ std::unique_ptr<DetachedOffscreenCanvas> OffscreenCanvas::detach()
 
     m_detached = true;
 
-    auto detached = makeUnique<DetachedOffscreenCanvas>(takeImageBuffer(), size(), originClean());
+    auto detached = makeUnique<DetachedOffscreenCanvas>(takeImageBufferForDifferentThread(), size(), originClean());
     detached->m_placeholderCanvas = std::exchange(m_placeholderData->canvas, nullptr);
 
     return detached;
@@ -492,7 +492,7 @@ void OffscreenCanvas::commitToPlaceholderCanvas()
 
     Locker locker { m_placeholderData->bufferLock };
     bool shouldPushBuffer = !m_placeholderData->pendingCommitBuffer;
-    m_placeholderData->pendingCommitBuffer = imageBuffer->clone();
+    m_placeholderData->pendingCommitBuffer = imageBuffer->cloneForDifferentThread();
     if (m_placeholderData->pendingCommitBuffer && shouldPushBuffer)
         pushBufferToPlaceholder();
 }
@@ -538,15 +538,15 @@ void OffscreenCanvas::setImageBufferAndMarkDirty(RefPtr<ImageBuffer>&& buffer)
     didDraw(FloatRect(FloatPoint(), size()));
 }
 
-RefPtr<ImageBuffer> OffscreenCanvas::takeImageBuffer() const
+RefPtr<ImageBuffer> OffscreenCanvas::takeImageBufferForDifferentThread() const
 {
     ASSERT(m_detached);
 
     if (size().isEmpty())
         return nullptr;
 
     clearCopiedImage();
-    return setImageBuffer(nullptr);
+    return ImageBuffer::sinkIntoBufferForDifferentThread(setImageBuffer(nullptr));
 }
 
 void OffscreenCanvas::reset()

diff --git a/Source/WebCore/html/OffscreenCanvas.h b/Source/WebCore/html/OffscreenCanvas.h
@@ -178,7 +178,7 @@ class OffscreenCanvas final : public RefCounted<OffscreenCanvas>, public CanvasB
 #endif
 
     void createImageBuffer() const final;
-    RefPtr<ImageBuffer> takeImageBuffer() const;
+    RefPtr<ImageBuffer> takeImageBufferForDifferentThread() const;
 
     void reset();
 

diff --git a/Source/WebCore/platform/graphics/ImageBuffer.cpp b/Source/WebCore/platform/graphics/ImageBuffer.cpp
@@ -185,6 +185,11 @@ RefPtr<ImageBuffer> ImageBuffer::clone() const
     return copyImageBuffer(const_cast<ImageBuffer&>(*this), PreserveResolution::Yes);
 }
 
+RefPtr<ImageBuffer> ImageBuffer::cloneForDifferentThread()
+{
+    return clone();
+}
+
 GraphicsContext& ImageBuffer::context() const
 {
     ASSERT(m_backend);
@@ -234,6 +239,20 @@ RefPtr<NativeImage> ImageBuffer::sinkIntoNativeImage()
     return nullptr;
 }
 
+RefPtr<ImageBuffer> ImageBuffer::sinkIntoBufferForDifferentThread(RefPtr<ImageBuffer> buffer)
+{
+    if (!buffer)
+        return nullptr;
+    ASSERT(buffer->hasOneRef());
+    return buffer->sinkIntoBufferForDifferentThread();
+}
+
+RefPtr<ImageBuffer> ImageBuffer::sinkIntoBufferForDifferentThread()
+{
+    ASSERT(hasOneRef());
+    return this;
+}
+
 RefPtr<Image> ImageBuffer::filteredImage(Filter& filter)
 {
     auto* backend = ensureBackendCreated();

diff --git a/Source/WebCore/platform/graphics/ImageBuffer.h b/Source/WebCore/platform/graphics/ImageBuffer.h
@@ -48,7 +48,7 @@ enum class ImageBufferOptions : uint8_t {
     UseDisplayList  = 1 << 1
 };
 
-class ImageBuffer : public ThreadSafeRefCounted<ImageBuffer, WTF::DestructionThread::Main>, public CanMakeWeakPtr<ImageBuffer> {
+class ImageBuffer : public ThreadSafeRefCounted<ImageBuffer>, public CanMakeWeakPtr<ImageBuffer> {
 public:
     struct CreationContext {
         // clang 13.1.6 throws errors if we use default initializers here.
@@ -135,6 +135,7 @@ class ImageBuffer : public ThreadSafeRefCounted<ImageBuffer, WTF::DestructionThr
     static FloatRect clampedRect(const FloatRect&);
 
     RefPtr<ImageBuffer> clone() const;
+    WEBCORE_EXPORT virtual RefPtr<ImageBuffer> cloneForDifferentThread();
 
     WEBCORE_EXPORT virtual GraphicsContext& context() const;
     WEBCORE_EXPORT virtual void flushContext();
@@ -177,6 +178,8 @@ class ImageBuffer : public ThreadSafeRefCounted<ImageBuffer, WTF::DestructionThr
     static RefPtr<NativeImage> sinkIntoNativeImage(RefPtr<ImageBuffer>);
     WEBCORE_EXPORT static RefPtr<Image> sinkIntoImage(RefPtr<ImageBuffer>, PreserveResolution = PreserveResolution::No);
 
+    static RefPtr<ImageBuffer> sinkIntoBufferForDifferentThread(RefPtr<ImageBuffer>);
+
     WEBCORE_EXPORT virtual void draw(GraphicsContext& destContext, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions&);
     WEBCORE_EXPORT virtual void drawPattern(GraphicsContext& destContext, const FloatRect& destRect, const FloatRect& srcRect, const AffineTransform& patternTransform, const FloatPoint& phase, const FloatSize& spacing, const ImagePaintingOptions&);
 
@@ -215,6 +218,7 @@ class ImageBuffer : public ThreadSafeRefCounted<ImageBuffer, WTF::DestructionThr
 protected:
     WEBCORE_EXPORT ImageBuffer(const ImageBufferBackend::Parameters&, const ImageBufferBackend::Info&, std::unique_ptr<ImageBufferBackend>&& = nullptr, RenderingResourceIdentifier = RenderingResourceIdentifier::generate());
 
+    WEBCORE_EXPORT virtual RefPtr<ImageBuffer> sinkIntoBufferForDifferentThread();
     ImageBufferBackend::Parameters m_parameters;
     ImageBufferBackend::Info m_backendInfo;
     std::unique_ptr<ImageBufferBackend> m_backend;

diff --git a/Source/WebCore/platform/graphics/NativeImage.h b/Source/WebCore/platform/graphics/NativeImage.h
@@ -38,7 +38,7 @@
 
 namespace WebCore {
 
-class NativeImage : public ThreadSafeRefCounted<NativeImage, WTF::DestructionThread::Main>, public CanMakeWeakPtr<NativeImage> {
+class NativeImage : public ThreadSafeRefCounted<NativeImage>, public CanMakeWeakPtr<NativeImage> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     class Observer {

diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.cpp b/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.cpp
@@ -194,6 +194,23 @@ RefPtr<NativeImage> RemoteImageBufferProxy::sinkIntoNativeImage()
     return copyNativeImage();
 }
 
+RefPtr<ImageBuffer> RemoteImageBufferProxy::sinkIntoBufferForDifferentThread()
+{
+    ASSERT(hasOneRef());
+    // We can't use these on a different thread, so make a local clone instead.
+    return cloneForDifferentThread();
+}
+
+RefPtr<ImageBuffer> RemoteImageBufferProxy::cloneForDifferentThread()
+{
+    auto copyBuffer = ImageBuffer::create(logicalSize(), renderingPurpose(), resolutionScale(), colorSpace(), pixelFormat());
+    if (!copyBuffer)
+        return nullptr;
+
+    copyBuffer->context().drawImageBuffer(*this, FloatPoint { }, CompositeOperator::Copy);
+    return copyBuffer;
+}
+
 RefPtr<Image> RemoteImageBufferProxy::filteredImage(Filter& filter)
 {
     if (UNLIKELY(!m_remoteRenderingBackendProxy))

diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h b/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h
@@ -74,6 +74,9 @@ class RemoteImageBufferProxy : public WebCore::ImageBuffer {
     RefPtr<WebCore::NativeImage> copyNativeImageForDrawing(WebCore::BackingStoreCopy = WebCore::CopyBackingStore) const final;
     RefPtr<WebCore::NativeImage> sinkIntoNativeImage() final;
 
+    RefPtr<ImageBuffer> sinkIntoBufferForDifferentThread() final;
+    RefPtr<ImageBuffer> cloneForDifferentThread() final;
+
     RefPtr<WebCore::Image> filteredImage(WebCore::Filter&) final;
 
     void drawConsuming(WebCore::GraphicsContext& destContext, const WebCore::FloatRect& destRect, const WebCore::FloatRect& srcRect, const WebCore::ImagePaintingOptions&) final;