REGRESSION(r289580): Canvas: putImageData sometimes draws nothing

https://bugs.webkit.org/show_bug.cgi?id=240802 rdar://93801722 Reviewed by Simon Fraser. RemoteImageBufferProxy::putPixelBuffer() needs to setNeedsFlush(true) once the request to change the backend is sent to GPUProcess. If WebProcess has access to the ImageBufferBackend, flushDrawingContext() will be called from copyNativeImage(). This call has to wait all DisplayList items and PutPixelBuffer messages to be flushed to the backend before copyNativeImage() copies the pixels of the backend to a NativeImage. * Source/WebCore/platform/graphics/ImageBuffer.h: (WebCore::ImageBuffer::setNeedsFlush): * Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h: (WebKit::RemoteDisplayListRecorderProxy::send): (WebKit::RemoteDisplayListRecorderProxy::resetNeedsFlush): Deleted. (WebKit::RemoteDisplayListRecorderProxy::needsFlush const): Deleted. (): Deleted. * Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h: (WebKit::RemoteImageBufferProxy::~RemoteImageBufferProxy): Canonical link: https://commits.webkit.org/251039@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294928 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 27, 2022 · 749c20ef7e6693fc1e69be46db08eedabc4d8a1b · 749c20e
1 parent f0b425e
commit 749c20ef7e6693fc1e69be46db08eedabc4d8a1b
Showing 9 changed files with 89 additions and 31 deletions.
diff --git a/LayoutTests/fast/canvas/canvas-put-image-data-no-draw-expected.html b/LayoutTests/fast/canvas/canvas-put-image-data-no-draw-expected.html
@@ -0,0 +1,13 @@
+<body>
+    <canvas id="target" width="400" height="400"></canvas>
+    <script>
+        const targetCanvas = document.getElementById('target');
+        const target = targetCanvas.getContext('2d');
+
+        const canvasWidth = targetCanvas.width;
+        const canvasHeight = targetCanvas.height
+
+        target.fillStyle = 'green';
+        target.fillRect(0, 0, canvasWidth, canvasHeight);
+    </script>
+</body>
diff --git a/LayoutTests/fast/canvas/canvas-put-image-data-no-draw.html b/LayoutTests/fast/canvas/canvas-put-image-data-no-draw.html
@@ -0,0 +1,50 @@
+<body>
+    <canvas id="target" width="400" height="400"></canvas>
+    <script>
+        const targetCanvas = document.getElementById('target');
+        const target = targetCanvas.getContext('2d');
+
+        const canvasWidth = targetCanvas.width;
+        const canvasHeight = targetCanvas.height
+
+        var sourceCanvas = document.createElement('canvas');
+        sourceCanvas.width  = canvasWidth;
+        sourceCanvas.height = canvasHeight;
+
+        const source = sourceCanvas.getContext('2d');
+
+        let progressX = 0;
+        let progressY = 0;
+        const paintSize = 100;
+
+        source.fillStyle = 'green';
+        source.fillRect(0, 0, canvasWidth, canvasHeight);
+        const imagedata = source.getImageData(0, 0, canvasWidth, canvasHeight);
+
+        function drawLoop() {
+            target.putImageData(imagedata, 0, 0, progressX, progressY, paintSize, paintSize);
+
+            progressX += paintSize;
+
+            if (progressX + paintSize <= canvasWidth) {
+                requestAnimationFrame(drawLoop);
+                return;
+            }
+
+            if (progressY + paintSize > canvasHeight) {
+                if (window.testRunner)
+                    testRunner.notifyDone();
+                return;
+            }
+
+            progressX = 0;
+            progressY += paintSize;
+            requestAnimationFrame(drawLoop);
+        }
+
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        requestAnimationFrame(drawLoop);
+    </script>
+</body>
diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -1712,27 +1712,6 @@ webkit.org/b/240123 imported/w3c/web-platform-tests/webrtc/protocol/rtp-clockrat
 
 webkit.org/b/240670 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-move-into-script-disabled-iframe.html [ Pass Crash ]
 
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance/textures/image_bitmap_from_image_bitmap/tex-2d-rgb-rgb-unsigned_short_5_6_5.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance/textures/image_bitmap_from_image_bitmap/tex-2d-rgba-rgba-unsigned_byte.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance/textures/image_bitmap_from_image_bitmap/tex-2d-rgba-rgba-unsigned_short_4_4_4_4.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-r11f_g11f_b10f-rgb-unsigned_int_10f_11f_11f_rev.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-r16f-red-float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-r8-red-unsigned_byte.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rgb5_a1-rgba-unsigned_short_5_5_5_1.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-rgb16f-rgb-float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-rgb565-rgb-unsigned_short_5_6_5.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-rgb9_e5-rgb-half_float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-rgba32f-rgba-float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-rgba8ui-rgba_integer-unsigned_byte.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-r32f-red-float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rgb8-rgb-unsigned_byte.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rgb9_e5-rgb-float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rgba4-rgba-unsigned_byte.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-rgb16f-rgb-half_float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-srgb8-rgb-unsigned_byte.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-r11f_g11f_b10f-rgb-float.html [ Pass Failure ]
-webkit.org/b/240735 [ Release ] webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-r16f-red-half_float.html [ Pass Failure ]
-
 webkit.org/b/240814 webrtc/canvas-to-peer-connection.html [ Pass Failure ]
 
 webkit.org/b/240821 [ Monterey Release ] webgl/max-active-contexts-webglcontextlost-prevent-default.html [ Slow ]

diff --git a/Source/WebCore/platform/graphics/ImageBuffer.h b/Source/WebCore/platform/graphics/ImageBuffer.h
@@ -100,6 +100,7 @@ class ImageBuffer : public ThreadSafeRefCounted<ImageBuffer, WTF::DestructionThr
     virtual void flushDrawingContext() { }
     virtual bool flushDrawingContextAsync() { return false; }
     virtual void didFlush(GraphicsContextFlushIdentifier) { }
+    virtual void setNeedsFlush(bool) { }
 
     virtual FloatSize logicalSize() const = 0;
     virtual IntSize truncatedLogicalSize() const = 0; // This truncates the real size. You probably should be calling logicalSize() instead.

diff --git a/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp b/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp
@@ -95,6 +95,9 @@ void Recorder::appendStateChangeItemIfNecessary()
     // FIXME: This is currently invoked in an ad-hoc manner when recording drawing items. We should consider either
     // splitting GraphicsContext state changes into individual display list items, or refactoring the code such that
     // this method is automatically invoked when recording a drawing item.
+
+    setNeedsFlush(true);
+
     auto& state = currentState().state;
     if (!state.changes())
         return;

diff --git a/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h b/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h
@@ -135,6 +135,8 @@ class Recorder : public GraphicsContext {
     virtual bool recordResourceUse(const SourceImage&) = 0;
     virtual bool recordResourceUse(Font&) = 0;
 
+    virtual void setNeedsFlush(bool) { }
+
     struct ContextState {
         GraphicsContextState state;
         std::optional<GraphicsContextState> lastDrawingState;

diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.cpp b/Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.cpp
@@ -434,6 +434,13 @@ bool RemoteDisplayListRecorderProxy::recordResourceUse(Font& font)
     return true;
 }
 
+void RemoteDisplayListRecorderProxy::setNeedsFlush(bool needsFlush)
+{
+    if (UNLIKELY(!m_imageBuffer))
+        return;
+    m_imageBuffer->setNeedsFlush(needsFlush);
+}
+
 void RemoteDisplayListRecorderProxy::flushContext(GraphicsContextFlushIdentifier identifier)
 {
     send(Messages::RemoteDisplayListRecorder::FlushContext(identifier));

diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h b/Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h
@@ -42,9 +42,6 @@ class RemoteDisplayListRecorderProxy : public WebCore::DisplayList::Recorder {
     RemoteDisplayListRecorderProxy(WebCore::ImageBuffer&, RemoteRenderingBackendProxy&, const WebCore::FloatRect& initialClip, const WebCore::AffineTransform&);
     ~RemoteDisplayListRecorderProxy() = default;
 
-    void resetNeedsFlush() { m_needsFlush = false; }
-    bool needsFlush() const { return m_needsFlush; }
-
     void convertToLuminanceMask() final;
     void transformToColorSpace(const WebCore::DestinationColorSpace&) final;
     void flushContext(WebCore::GraphicsContextFlushIdentifier);
@@ -55,8 +52,6 @@ class RemoteDisplayListRecorderProxy : public WebCore::DisplayList::Recorder {
     {
         if (UNLIKELY(!m_renderingBackend))
             return;
-
-        m_needsFlush = true;
         m_renderingBackend->sendToStream(WTFMove(message), m_destinationBufferIdentifier);
     }
 
@@ -137,14 +132,15 @@ class RemoteDisplayListRecorderProxy : public WebCore::DisplayList::Recorder {
     bool recordResourceUse(const WebCore::SourceImage&) final;
     bool recordResourceUse(WebCore::Font&) final;
 
+    void setNeedsFlush(bool) final;
+
     RefPtr<WebCore::ImageBuffer> createImageBuffer(const WebCore::FloatSize&, float resolutionScale, const WebCore::DestinationColorSpace&, std::optional<WebCore::RenderingMode>, std::optional<WebCore::RenderingMethod>) const final;
     RefPtr<WebCore::ImageBuffer> createAlignedImageBuffer(const WebCore::FloatSize&, const WebCore::DestinationColorSpace&, std::optional<WebCore::RenderingMethod>) const final;
     RefPtr<WebCore::ImageBuffer> createAlignedImageBuffer(const WebCore::FloatRect&, const WebCore::DestinationColorSpace&, std::optional<WebCore::RenderingMethod>) const final;
 
     WebCore::RenderingResourceIdentifier m_destinationBufferIdentifier;
     WeakPtr<WebCore::ImageBuffer> m_imageBuffer;
     WeakPtr<RemoteRenderingBackendProxy> m_renderingBackend;
-    bool m_needsFlush { false };
 };
 
 } // namespace WebKit

diff --git a/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h b/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h
@@ -66,7 +66,7 @@ class RemoteImageBufferProxy : public WebCore::ConcreteImageBuffer<BackendType>
     ~RemoteImageBufferProxy()
     {
         if (!m_remoteRenderingBackendProxy || m_remoteRenderingBackendProxy->isGPUProcessConnectionClosed()) {
-            m_remoteDisplayList.resetNeedsFlush();
+            setNeedsFlush(false);
             return;
         }
 
@@ -116,6 +116,11 @@ class RemoteImageBufferProxy : public WebCore::ConcreteImageBuffer<BackendType>
         m_receivedFlushIdentifierChangedCondition.notifyAll();
     }
 
+    void setNeedsFlush(bool needsFlush) final
+    {
+        m_needsFlush = needsFlush;
+    }
+
     void waitForDidFlushWithTimeout()
     {
         if (!m_remoteRenderingBackendProxy)
@@ -246,7 +251,7 @@ class RemoteImageBufferProxy : public WebCore::ConcreteImageBuffer<BackendType>
 
     void clearBackend() final
     {
-        m_remoteDisplayList.resetNeedsFlush();
+        setNeedsFlush(false);
         didFlush(m_sentFlushIdentifier);
         BaseConcreteImageBuffer::clearBackend();
     }
@@ -271,6 +276,7 @@ class RemoteImageBufferProxy : public WebCore::ConcreteImageBuffer<BackendType>
         auto& mutableThis = const_cast<RemoteImageBufferProxy&>(*this);
         mutableThis.flushDrawingContextAsync();
         m_remoteRenderingBackendProxy->putPixelBufferForImageBuffer(m_renderingResourceIdentifier, pixelBuffer, srcRect, destPoint, destFormat);
+        setNeedsFlush(true);
     }
 
     void convertToLuminanceMask() final
@@ -309,13 +315,13 @@ class RemoteImageBufferProxy : public WebCore::ConcreteImageBuffer<BackendType>
         if (UNLIKELY(!m_remoteRenderingBackendProxy))
             return false;
 
-        if (!m_remoteDisplayList.needsFlush())
+        if (!m_needsFlush)
             return hasPendingFlush();
 
         m_sentFlushIdentifier = WebCore::GraphicsContextFlushIdentifier::generate();
         LOG_WITH_STREAM(SharedDisplayLists, stream << "RemoteImageBufferProxy " << m_renderingResourceIdentifier << " flushDrawingContextAsync - flush " << m_sentFlushIdentifier);
         m_remoteDisplayList.flushContext(m_sentFlushIdentifier);
-        m_remoteDisplayList.resetNeedsFlush();
+        setNeedsFlush(false);
         return true;
     }
 
@@ -348,6 +354,7 @@ class RemoteImageBufferProxy : public WebCore::ConcreteImageBuffer<BackendType>
     WebCore::GraphicsContextFlushIdentifier m_receivedFlushIdentifier WTF_GUARDED_BY_LOCK(m_receivedFlushIdentifierLock); // Only modified on the main thread but may get queried on a secondary thread.
     WeakPtr<RemoteRenderingBackendProxy> m_remoteRenderingBackendProxy;
     RemoteDisplayListRecorderProxy m_remoteDisplayList;
+    bool m_needsFlush { false };
 };
 
 template<typename BackendType>