Title: [280245] trunk/Source/WebKit
Revision
280245
Author
mmaxfi...@apple.com
Date
2021-07-23 09:51:28 -0700 (Fri, 23 Jul 2021)

Log Message

[GPU Process] Migrate the DisplayList::Replayer::Delegate from RemoteImageBuffer to RemoteRenderingBackend
https://bugs.webkit.org/show_bug.cgi?id=228219

Reviewed by Wenson Hsieh.

This is part 1 of https://bugs.webkit.org/show_bug.cgi?id=228216.

For this task, we need to count resource uses:
1. As the recorder in the web process records DisplayList items which reference resources, those uses need to increment a counter.
2. As the replayer in the GPU process replays DisplayList items which reference resources, those uses need to increment a parallel counter.

The most natural place for the counters to live is in RemoteResourceCacheProxy and RemoteResourceCache, respectively. These two classes
are accessible from the RemoteRenderingBackendProxy and RemoteRenderingBackend, respectively. The most natural interface between the
DisplayList classes like DisplayList::Recorder and DisplayList::Replayer and the higher level WebKit classes is to use their delegate
interfaces.

For 1 above, RemoteImageBufferProxy has access to the RemoteRenderingBackendProxy and is already a DisplayList::Recorder::Delegate, so
there's no problem there. However, for 2, messages are delivered first to the RemoteRenderingBackend, which is the place that has access to
the RemoteResourceCache. Making the RemoteRenderingBackend have the DisplayList::Replayer::Delegate would be a natural place to either
A) call the necessary function in the RemoteImageBuffer, or B) interact with the RemoteResourceCache to increment the necessary counter.

Indeed, this makes a lot of sense because, for 2 of the 3 delegate methods, RemoteImageBuffer just immediately turns around and forwards
the call to the RemoteRenderingBackend anyway. So, migrating this interface to RemoteRenderingBackend actually ends up making it a bit
simpler.

No new tests because there is no behavior change.

* GPUProcess/graphics/RemoteImageBuffer.h:
(WebKit::RemoteImageBuffer::apply):
(): Deleted.
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::ReplayerDelegate::ReplayerDelegate):
(WebKit::RemoteRenderingBackend::ReplayerDelegate::apply):
(WebKit::RemoteRenderingBackend::ReplayerDelegate::didCreateMaskImageBuffer):
(WebKit::RemoteRenderingBackend::ReplayerDelegate::didResetMaskImageBuffer):
(WebKit::RemoteRenderingBackend::submit):
* GPUProcess/graphics/RemoteRenderingBackend.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (280244 => 280245)


--- trunk/Source/WebKit/ChangeLog	2021-07-23 16:07:58 UTC (rev 280244)
+++ trunk/Source/WebKit/ChangeLog	2021-07-23 16:51:28 UTC (rev 280245)
@@ -1,3 +1,43 @@
+2021-07-23  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [GPU Process] Migrate the DisplayList::Replayer::Delegate from RemoteImageBuffer to RemoteRenderingBackend
+        https://bugs.webkit.org/show_bug.cgi?id=228219
+
+        Reviewed by Wenson Hsieh.
+
+        This is part 1 of https://bugs.webkit.org/show_bug.cgi?id=228216.
+
+        For this task, we need to count resource uses:
+        1. As the recorder in the web process records DisplayList items which reference resources, those uses need to increment a counter.
+        2. As the replayer in the GPU process replays DisplayList items which reference resources, those uses need to increment a parallel counter.
+
+        The most natural place for the counters to live is in RemoteResourceCacheProxy and RemoteResourceCache, respectively. These two classes
+        are accessible from the RemoteRenderingBackendProxy and RemoteRenderingBackend, respectively. The most natural interface between the
+        DisplayList classes like DisplayList::Recorder and DisplayList::Replayer and the higher level WebKit classes is to use their delegate
+        interfaces.
+
+        For 1 above, RemoteImageBufferProxy has access to the RemoteRenderingBackendProxy and is already a DisplayList::Recorder::Delegate, so
+        there's no problem there. However, for 2, messages are delivered first to the RemoteRenderingBackend, which is the place that has access to
+        the RemoteResourceCache. Making the RemoteRenderingBackend have the DisplayList::Replayer::Delegate would be a natural place to either
+        A) call the necessary function in the RemoteImageBuffer, or B) interact with the RemoteResourceCache to increment the necessary counter.
+
+        Indeed, this makes a lot of sense because, for 2 of the 3 delegate methods, RemoteImageBuffer just immediately turns around and forwards
+        the call to the RemoteRenderingBackend anyway. So, migrating this interface to RemoteRenderingBackend actually ends up making it a bit
+        simpler.
+
+        No new tests because there is no behavior change.
+
+        * GPUProcess/graphics/RemoteImageBuffer.h:
+        (WebKit::RemoteImageBuffer::apply):
+        (): Deleted.
+        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+        (WebKit::RemoteRenderingBackend::ReplayerDelegate::ReplayerDelegate):
+        (WebKit::RemoteRenderingBackend::ReplayerDelegate::apply):
+        (WebKit::RemoteRenderingBackend::ReplayerDelegate::didCreateMaskImageBuffer):
+        (WebKit::RemoteRenderingBackend::ReplayerDelegate::didResetMaskImageBuffer):
+        (WebKit::RemoteRenderingBackend::submit):
+        * GPUProcess/graphics/RemoteRenderingBackend.h:
+
 2021-07-22  Devin Rousso  <drou...@apple.com>
 
         [Live Text] [iOS] Analysis should also search for any App Clip codes

Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteImageBuffer.h (280244 => 280245)


--- trunk/Source/WebKit/GPUProcess/graphics/RemoteImageBuffer.h	2021-07-23 16:07:58 UTC (rev 280244)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteImageBuffer.h	2021-07-23 16:51:28 UTC (rev 280245)
@@ -38,7 +38,7 @@
 namespace WebKit {
 
 template<typename BackendType>
-class RemoteImageBuffer : public WebCore::ConcreteImageBuffer<BackendType>, public WebCore::DisplayList::Replayer::Delegate {
+class RemoteImageBuffer : public WebCore::ConcreteImageBuffer<BackendType> {
     using BaseConcreteImageBuffer = WebCore::ConcreteImageBuffer<BackendType>;
     using BaseConcreteImageBuffer::context;
     using BaseConcreteImageBuffer::m_backend;
@@ -75,7 +75,9 @@
 #endif
 
 private:
-    bool apply(WebCore::DisplayList::ItemHandle item, WebCore::GraphicsContext& context) override
+    friend class RemoteRenderingBackend;
+
+    bool apply(WebCore::DisplayList::ItemHandle item, WebCore::GraphicsContext& context)
     {
         if (item.is<WebCore::DisplayList::GetPixelBuffer>()) {
             auto& getPixelBufferItem = item.get<WebCore::DisplayList::GetPixelBuffer>();
@@ -108,16 +110,6 @@
         return m_remoteRenderingBackend.applyMediaItem(item, context);
     }
 
-    void didCreateMaskImageBuffer(WebCore::ImageBuffer& imageBuffer) final
-    {
-        m_remoteRenderingBackend.didCreateMaskImageBuffer(imageBuffer);
-    }
-
-    void didResetMaskImageBuffer() final
-    {
-        m_remoteRenderingBackend.didResetMaskImageBuffer();
-    }
-
     RemoteRenderingBackend& m_remoteRenderingBackend;
     WebCore::RenderingResourceIdentifier m_renderingResourceIdentifier;
 };

Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (280244 => 280245)


--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp	2021-07-23 16:07:58 UTC (rev 280244)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp	2021-07-23 16:51:28 UTC (rev 280245)
@@ -184,16 +184,42 @@
         wakeUpAndApplyDisplayList(std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments);
 }
 
+RemoteRenderingBackend::ReplayerDelegate::ReplayerDelegate(WebCore::ImageBuffer& destination, RemoteRenderingBackend& remoteRenderingBackend, GPUConnectionToWebProcess& gpuConnectionToWebProcess)
+    : m_destination(destination)
+    , m_remoteRenderingBackend(remoteRenderingBackend)
+    , m_gpuConnectionToWebProcess(gpuConnectionToWebProcess)
+{
+}
+
+bool RemoteRenderingBackend::ReplayerDelegate::apply(WebCore::DisplayList::ItemHandle item, WebCore::GraphicsContext& graphicsContext)
+{
+    // FIXME: Inspect the item and record resource use here.
+
+    auto apply = [&](auto&& destination) {
+        return destination.apply(item, graphicsContext);
+    };
+
+    if (m_destination.renderingMode() == RenderingMode::Accelerated)
+        return apply(static_cast<AcceleratedRemoteImageBuffer&>(m_destination));
+    return apply(static_cast<UnacceleratedRemoteImageBuffer&>(m_destination));
+}
+
+void RemoteRenderingBackend::ReplayerDelegate::didCreateMaskImageBuffer(WebCore::ImageBuffer& imageBuffer)
+{
+    m_remoteRenderingBackend.didCreateMaskImageBuffer(imageBuffer);
+}
+
+void RemoteRenderingBackend::ReplayerDelegate::didResetMaskImageBuffer()
+{
+    m_remoteRenderingBackend.didResetMaskImageBuffer();
+}
+
 DisplayList::ReplayResult RemoteRenderingBackend::submit(const DisplayList::DisplayList& displayList, ImageBuffer& destination)
 {
     if (displayList.isEmpty())
         return { };
 
-    DisplayList::Replayer::Delegate* replayerDelegate = nullptr;
-    if (destination.renderingMode() == RenderingMode::Accelerated)
-        replayerDelegate = static_cast<AcceleratedRemoteImageBuffer*>(&destination);
-    else
-        replayerDelegate = static_cast<UnacceleratedRemoteImageBuffer*>(&destination);
+    ReplayerDelegate replayerDelegate(destination, *this, m_gpuConnectionToWebProcess);
 
     return WebCore::DisplayList::Replayer {
         destination.context(),
@@ -202,7 +228,7 @@
         &remoteResourceCache().nativeImages(),
         &remoteResourceCache().fonts(),
         m_currentMaskImageBuffer.get(),
-        replayerDelegate
+        &replayerDelegate
     }.replay();
 }
 

Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h (280244 => 280245)


--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h	2021-07-23 16:07:58 UTC (rev 280244)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h	2021-07-23 16:51:28 UTC (rev 280245)
@@ -147,6 +147,20 @@
     void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
     void didCreateSharedDisplayListHandle(WebCore::DisplayList::ItemBufferIdentifier, const SharedMemory::IPCHandle&, WebCore::RenderingResourceIdentifier destinationBufferIdentifier);
 
+    class ReplayerDelegate : public WebCore::DisplayList::Replayer::Delegate {
+    public:
+        ReplayerDelegate(WebCore::ImageBuffer&, RemoteRenderingBackend&, GPUConnectionToWebProcess&);
+
+    private:
+        bool apply(WebCore::DisplayList::ItemHandle, WebCore::GraphicsContext&) final;
+        void didCreateMaskImageBuffer(WebCore::ImageBuffer&) final;
+        void didResetMaskImageBuffer() final;
+
+        WebCore::ImageBuffer& m_destination;
+        RemoteRenderingBackend& m_remoteRenderingBackend;
+        Ref<GPUConnectionToWebProcess> m_gpuConnectionToWebProcess;
+    };
+
     struct PendingWakeupInformation {
         GPUProcessWakeupMessageArguments arguments;
         std::optional<WebCore::RenderingResourceIdentifier> missingCachedResourceIdentifier;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to