Attachment 412947 Details for Bug 217596 – Patch for review

[patch] Patch for review

NativeImageCaching.patch (text/plain), 17.58 KB, created by Said Abou-Hallawa on 2020-11-02 11:02:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2020-11-02 11:02:19 PST

Size: 17.58 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a259a42fa27f..5cdcda47544c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,14 @@
>+2020-11-02  Said Abou-Hallawa  <said@apple.com>
>+
>+        [GPU Process]: Cache the NativeImage in GPU Process and allow referencing it with its RemoteResourceIdentifier
>+        https://bugs.webkit.org/show_bug.cgi?id=217596
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/graphics/GraphicsContext.cpp:
>+        (WebCore::GraphicsContext::drawImage):
>+        (WebCore::GraphicsContext::drawTiledImage):
>+
> 2020-11-01  Said Abou-Hallawa  <said@apple.com>
> 
>         [GPU Process] Control the life cycle of the platform image by a new class named NativeImage
>diff --git a/Source/WebCore/platform/graphics/GraphicsContext.cpp b/Source/WebCore/platform/graphics/GraphicsContext.cpp
>index ee76981dceb0..c48ed8b767bc 100644
>--- a/Source/WebCore/platform/graphics/GraphicsContext.cpp
>+++ b/Source/WebCore/platform/graphics/GraphicsContext.cpp
>@@ -758,7 +758,7 @@ ImageDrawResult GraphicsContext::drawImage(Image& image, const FloatRect& destin
>     if (paintingDisabled())
>         return ImageDrawResult::DidNothing;
> 
>-    if (m_impl)
>+    if (m_impl && image.isPDFDocumentImage())
>         return m_impl->drawImage(image, destination, source, options);
> 
>     InterpolationQualityMaintainer interpolationQualityForThisScope(*this, options.interpolationQuality());
>@@ -770,7 +770,7 @@ ImageDrawResult GraphicsContext::drawTiledImage(Image& image, const FloatRect& d
>     if (paintingDisabled())
>         return ImageDrawResult::DidNothing;
> 
>-    if (m_impl)
>+    if (m_impl && image.isPDFDocumentImage())
>         return m_impl->drawTiledImage(image, destination, source, tileSize, spacing, options);
> 
>     InterpolationQualityMaintainer interpolationQualityForThisScope(*this, options.interpolationQuality());
>@@ -783,7 +783,7 @@ ImageDrawResult GraphicsContext::drawTiledImage(Image& image, const FloatRect& d
>     if (paintingDisabled())
>         return ImageDrawResult::DidNothing;
> 
>-    if (m_impl)
>+    if (m_impl && image.isPDFDocumentImage())
>         return m_impl->drawTiledImage(image, destination, source, tileScaleFactor, hRule, vRule, options);
> 
>     if (hRule == Image::StretchTile && vRule == Image::StretchTile) {
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 0afaa90709d8..5aed91bf7cd0 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,30 @@
>+2020-11-02  Said Abou-Hallawa  <said@apple.com>
>+
>+        [GPU Process]: Cache the NativeImage in GPU Process and allow referencing it with its RemoteResourceIdentifier
>+        https://bugs.webkit.org/show_bug.cgi?id=217596
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
>+        (WebKit::RemoteRenderingBackend::applyResourceItem):
>+        (WebKit::RemoteRenderingBackend::cachePlatformImage):
>+        * GPUProcess/graphics/RemoteRenderingBackend.h:
>+        * GPUProcess/graphics/RemoteRenderingBackend.messages.in:
>+        * GPUProcess/graphics/RemoteResourceCache.cpp:
>+        (WebKit::RemoteResourceCache::cachePlatformImage):
>+        (WebKit::RemoteResourceCache::cachedPlatformImage):
>+        (WebKit::RemoteResourceCache::releaseRemoteResource):
>+        * GPUProcess/graphics/RemoteResourceCache.h:
>+        * WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
>+        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
>+        (WebKit::RemoteRenderingBackendProxy::createImageBuffer):
>+        (WebKit::RemoteRenderingBackendProxy::cachePlatformImage):
>+        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
>+        * WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:
>+        (WebKit::RemoteResourceCacheProxy::nativeImageWillBeDeleted):
>+        (WebKit::RemoteResourceCacheProxy::lockRemoteNativeImageForRemoteClient):
>+        * WebProcess/GPU/graphics/RemoteResourceCacheProxy.h:
>+
> 2020-11-01  Said Abou-Hallawa  <said@apple.com>
> 
>         [GPU Process] Control the life cycle of the platform image by a new class named NativeImage
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp
>index d5bb0f2d03e6..1389c5430844 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp
>@@ -84,6 +84,19 @@ bool RemoteRenderingBackend::applyResourceItem(const DisplayList::Item& item, Gr
>         return true;
>     }
> 
>+    if (item.type() == DisplayList::ItemType::DrawNativeImage) {
>+        auto& drawItem = static_cast<const DisplayList::DrawNativeImage&>(item);
>+        if (!drawItem.renderingResourceIdentifier())
>+            return false;
>+
>+        auto image = m_remoteResourceCache.cachedPlatformImage(drawItem.renderingResourceIdentifier());
>+        if (!image)
>+            return false;
>+
>+        context.drawPlatformImage(image, drawItem.imageSize(), drawItem.destinationRect(), drawItem.source(), drawItem.options());
>+        return true;
>+    }
>+
>     return false;
> }
> 
>@@ -163,6 +176,11 @@ void RemoteRenderingBackend::getImageData(WebCore::AlphaPremultiplication output
>     completionHandler(IPC::ImageDataReference(WTFMove(imageData)));
> }
> 
>+void RemoteRenderingBackend::cachePlatformImage(const PlatformImagePtr& image, RenderingResourceIdentifier renderingResourceIdentifier)
>+{
>+    m_remoteResourceCache.cachePlatformImage(image, renderingResourceIdentifier);
>+}
>+
> void RemoteRenderingBackend::releaseRemoteResource(RenderingResourceIdentifier renderingResourceIdentifier)
> {
>     m_remoteResourceCache.releaseRemoteResource(renderingResourceIdentifier);
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h
>index 285e2f74b4d4..17a4504007d7 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h
>@@ -86,6 +86,7 @@ private:
>     void flushDisplayList(const WebCore::DisplayList::DisplayList&, WebCore::RenderingResourceIdentifier);
>     void flushDisplayListAndCommit(const WebCore::DisplayList::DisplayList&, DisplayListFlushIdentifier, WebCore::RenderingResourceIdentifier);
>     void getImageData(WebCore::AlphaPremultiplication outputFormat, WebCore::IntRect srcRect, WebCore::RenderingResourceIdentifier, CompletionHandler<void(IPC::ImageDataReference&&)>&&);
>+    void cachePlatformImage(const WebCore::PlatformImagePtr&, WebCore::RenderingResourceIdentifier);
>     void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
> 
>     RemoteResourceCache m_remoteResourceCache;
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.messages.in b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.messages.in
>index a88e0fb62624..408eba52c3a6 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.messages.in
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.messages.in
>@@ -27,6 +27,7 @@ messages -> RemoteRenderingBackend NotRefCounted {
>     void FlushDisplayList(WebCore::DisplayList::DisplayList displayList, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
>     void FlushDisplayListAndCommit(WebCore::DisplayList::DisplayList displayList, WebKit::DisplayListFlushIdentifier flushIdentifier, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
>     GetImageData(enum:uint8_t WebCore::AlphaPremultiplication outputFormat, WebCore::IntRect srcRect, WebCore::RenderingResourceIdentifier renderingResourceIdentifier) -> (IPC::ImageDataReference imageData) Synchronous
>+    void CachePlatformImage(WebCore::PlatformImagePtr image, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
>     void ReleaseRemoteResource(WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
> }
> 
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.cpp b/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.cpp
>index 2282068664f7..640e1e26c834 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.cpp
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.cpp
>@@ -42,10 +42,23 @@ ImageBuffer* RemoteResourceCache::cachedImageBuffer(RenderingResourceIdentifier
>     return m_imageBuffers.get(renderingResourceIdentifier);
> }
> 
>+void RemoteResourceCache::cachePlatformImage(const PlatformImagePtr& image, RenderingResourceIdentifier renderingResourceIdentifier)
>+{
>+    auto addResult = m_platformImages.add(renderingResourceIdentifier, image);
>+    ASSERT_UNUSED(addResult, addResult.isNewEntry);
>+}
>+
>+PlatformImagePtr RemoteResourceCache::cachedPlatformImage(RenderingResourceIdentifier renderingResourceIdentifier)
>+{
>+    return m_platformImages.get(renderingResourceIdentifier);
>+}
>+
> void RemoteResourceCache::releaseRemoteResource(RenderingResourceIdentifier renderingResourceIdentifier)
> {
>     if (m_imageBuffers.remove(renderingResourceIdentifier))
>         return;
>+    if (m_platformImages.remove(renderingResourceIdentifier))
>+        return;
>     // Caching the remote resource should have happened before releasing it.
>     ASSERT_NOT_REACHED();
> }
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.h b/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.h
>index 0546ad07c286..1ad0f598dc02 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.h
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteResourceCache.h
>@@ -44,12 +44,16 @@ public:
> 
>     void cacheImageBuffer(WebCore::RenderingResourceIdentifier, std::unique_ptr<WebCore::ImageBuffer>&&);
>     WebCore::ImageBuffer* cachedImageBuffer(WebCore::RenderingResourceIdentifier);
>+    void cachePlatformImage(const WebCore::PlatformImagePtr&, WebCore::RenderingResourceIdentifier);
>+    WebCore::PlatformImagePtr cachedPlatformImage(WebCore::RenderingResourceIdentifier);
>     void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
> 
> private:
>     using RemoteImageBufferHashMap = HashMap<WebCore::RenderingResourceIdentifier, std::unique_ptr<WebCore::ImageBuffer>>;
>+    using RemotePlatformImageHashMap = HashMap<WebCore::RenderingResourceIdentifier, WebCore::PlatformImagePtr>;
> 
>     RemoteImageBufferHashMap m_imageBuffers;
>+    RemotePlatformImageHashMap m_platformImages;
> };
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h b/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h
>index 2264ebcb6664..174a066ee1e8 100644
>--- a/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h
>+++ b/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h
>@@ -170,6 +170,13 @@ protected:
>         return true;
>     }
> 
>+    bool lockRemoteNativeImage(WebCore::NativeImage& image) override
>+    {
>+        if (!m_remoteRenderingBackendProxy)
>+            return false;
>+        return m_remoteRenderingBackendProxy->remoteResourceCacheProxy().lockRemoteNativeImageForRemoteClient(image, m_renderingResourceIdentifier);
>+    }
>+
>     void willAppendItem(const WebCore::DisplayList::Item&) override
>     {
>         constexpr size_t DisplayListBatchSize = 512;
>diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp b/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp
>index 7a01930ab66c..95d4018a3043 100644
>--- a/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp
>+++ b/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp
>@@ -89,7 +89,7 @@ bool RemoteRenderingBackendProxy::waitForFlushDisplayListWasCommitted()
> 
> std::unique_ptr<ImageBuffer> RemoteRenderingBackendProxy::createImageBuffer(const FloatSize& size, RenderingMode renderingMode, float resolutionScale, ColorSpace colorSpace)
> {
>-    std::unique_ptr<WebCore::ImageBuffer> imageBuffer;
>+    std::unique_ptr<ImageBuffer> imageBuffer;
> 
>     if (renderingMode == RenderingMode::Accelerated)
>         imageBuffer = AcceleratedRemoteImageBufferProxy::create(size, resolutionScale, *this);
>@@ -126,6 +126,11 @@ DisplayListFlushIdentifier RemoteRenderingBackendProxy::flushDisplayListAndCommi
>     return sentFlushIdentifier;
> }
> 
>+void RemoteRenderingBackendProxy::cachePlatformImage(const PlatformImagePtr& image, RenderingResourceIdentifier renderingResourceIdentifier)
>+{
>+    send(Messages::RemoteRenderingBackend::CachePlatformImage(image, renderingResourceIdentifier), m_renderingBackendIdentifier);
>+}
>+
> void RemoteRenderingBackendProxy::releaseRemoteResource(RenderingResourceIdentifier renderingResourceIdentifier)
> {
>     send(Messages::RemoteRenderingBackend::ReleaseRemoteResource(renderingResourceIdentifier), m_renderingBackendIdentifier);
>diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h b/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h
>index 99565b6f166b..3aa2032d8e82 100644
>--- a/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h
>+++ b/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h
>@@ -73,6 +73,7 @@ public:
>     RefPtr<WebCore::ImageData> getImageData(WebCore::AlphaPremultiplication outputFormat, const WebCore::IntRect& srcRect, WebCore::RenderingResourceIdentifier);
>     void flushDisplayList(const WebCore::DisplayList::DisplayList&, WebCore::RenderingResourceIdentifier);
>     DisplayListFlushIdentifier flushDisplayListAndCommit(const WebCore::DisplayList::DisplayList&, WebCore::RenderingResourceIdentifier);
>+    void cachePlatformImage(const WebCore::PlatformImagePtr&, WebCore::RenderingResourceIdentifier);
>     void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
> 
>     bool waitForImageBufferBackendWasCreated();
>diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp b/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp
>index 3f5a73bec3bf..728cf2e94dbd 100644
>--- a/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp
>+++ b/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp
>@@ -58,6 +58,15 @@ void RemoteResourceCacheProxy::releaseImageBuffer(RenderingResourceIdentifier re
>     releaseRemoteResource(renderingResourceIdentifier);
> }
> 
>+void RemoteResourceCacheProxy::nativeImageWillBeDeleted(RenderingResourceIdentifier renderingResourceIdentifier)
>+{
>+    if (!m_nativeImages.remove(renderingResourceIdentifier))
>+        return;
>+
>+    // Tell the GPU process to remove this resource unless it's pending replaying back.
>+    releaseRemoteResource(renderingResourceIdentifier);
>+}
>+
> bool RemoteResourceCacheProxy::lockRemoteImageBufferForRemoteClient(ImageBuffer& imageBuffer, RenderingResourceIdentifier remoteClientIdentifier)
> {
>     if (!imageBuffer.renderingResourceIdentifier())
>@@ -72,6 +81,22 @@ bool RemoteResourceCacheProxy::lockRemoteImageBufferForRemoteClient(ImageBuffer&
>     return true;
> }
> 
>+bool RemoteResourceCacheProxy::lockRemoteNativeImageForRemoteClient(NativeImage& image, RenderingResourceIdentifier remoteClientIdentifier)
>+{
>+    auto renderingResourceIdentifier = image.renderingResourceIdentifier();
>+    if (!renderingResourceIdentifier)
>+        return false;
>+    
>+    if (!m_nativeImages.contains(image.renderingResourceIdentifier())) {
>+        m_nativeImages.add(image.renderingResourceIdentifier());
>+        image.setObserver(this);
>+        m_remoteRenderingBackendProxy.cachePlatformImage(image.platformImage(), image.renderingResourceIdentifier());
>+    }
>+    
>+    lockRemoteResourceForRemoteClient(renderingResourceIdentifier, remoteClientIdentifier);
>+    return true;
>+}
>+
> void RemoteResourceCacheProxy::lockRemoteResourceForRemoteClient(RenderingResourceIdentifier renderingResourceIdentifier, RenderingResourceIdentifier remoteClientIdentifier)
> {
>     // Add the "resource -> client" dependency in m_lockedRemoteResourceForRemoteClients.
>diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h b/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h
>index e86be7deb092..44b9a1e8f1d1 100644
>--- a/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h
>+++ b/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h
>@@ -38,7 +38,7 @@ namespace WebKit {
> 
> class RemoteRenderingBackendProxy;
> 
>-class RemoteResourceCacheProxy {
>+class RemoteResourceCacheProxy : public WebCore::NativeImage::Observer {
> public:
>     RemoteResourceCacheProxy(RemoteRenderingBackendProxy&);
> 
>@@ -47,17 +47,22 @@ public:
>     void releaseImageBuffer(WebCore::RenderingResourceIdentifier);
> 
>     bool lockRemoteImageBufferForRemoteClient(WebCore::ImageBuffer&, WebCore::RenderingResourceIdentifier remoteClientIdentifier);
>+    bool lockRemoteNativeImageForRemoteClient(WebCore::NativeImage&, WebCore::RenderingResourceIdentifier remoteClientIdentifier);
>     void unlockRemoteResourcesForRemoteClient(WebCore::RenderingResourceIdentifier remoteClientIdentifier);
> 
> private:
>     using RemoteClientsHashSet = HashSet<WebCore::RenderingResourceIdentifier>;
>     using RemoteResourceClientsHashMap = HashMap<WebCore::RenderingResourceIdentifier, RemoteClientsHashSet>;
>     using RemoteImageBufferProxyHashMap = HashMap<WebCore::RenderingResourceIdentifier, WebCore::ImageBuffer*>;
>+    using RemoteNativeImageHashSet = HashSet<WebCore::RenderingResourceIdentifier>;
> 
>+    void nativeImageWillBeDeleted(WebCore::RenderingResourceIdentifier) override;
>+    
>     void lockRemoteResourceForRemoteClient(WebCore::RenderingResourceIdentifier, WebCore::RenderingResourceIdentifier remoteClientIdentifier);
>     void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
> 
>     RemoteImageBufferProxyHashMap m_imageBuffers;
>+    RemoteNativeImageHashSet m_nativeImages;
>     RemoteResourceClientsHashMap m_lockedRemoteResourceForRemoteClients;
>     RemoteResourceClientsHashMap m_pendingUnlockRemoteResourceForRemoteClients;
>     RemoteRenderingBackendProxy& m_remoteRenderingBackendProxy;

Actions: View | Formatted Diff | Diff

Attachments on bug 217596: 411086 | 411096 | 411098 | 411125 | 412064 | 412070 | 412586 | 412945 | 412947 | 413740 | 413742 | 413754 | 413761 | 413786 | 413842 | 413847 | 413875