Changeset 280963 in webkit

Timestamp:

Aug 12, 2021, 6:35:06 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html fails on Cocoa
​https://bugs.webkit.org/show_bug.cgi?id=228821
<rdar://problem/81562236>

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2021-08-12
Reviewed by Kenneth Russell.

When uploading the WebGL texture content from videos, use the video
visible data size instead of video element size. The video data can
be scaled with its filters, but we should upload only the original
pixels.

The GPU codepath already did this, but CPU codepath did not.
This change fixes the CPU codepath.

Fixes webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html
webgl/2.0.y/conformance/textures/misc/texture-corner-case-videos.html

• html/canvas/WebGLRenderingContextBase.cpp:

(WebCore::WebGLRenderingContextBase::texImageSourceHelper):
(WebCore::WebGLRenderingContextBase::videoFrameToImage):
(WebCore::WebGLRenderingContextBase::LRUImageBufferCache::imageBuffer):

• html/canvas/WebGLRenderingContextBase.h:

Location:

trunk

Files:

• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (modified) (4 diffs)
• Source/WebCore/html/canvas/WebGLRenderingContextBase.h (modified) (2 diffs)

trunk/LayoutTests/TestExpectations

@@ -3687 +3687 @@
 webgl/1.0.x/conformance/misc/invalid-passed-params.html [ Pass ]
 webgl/1.0.x/conformance/glsl/bugs/character-set.html [ Pass ]
+webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html [ Pass ]
 
 # WebGL conformance test suite 2.0.1 is skipped until 2.0.0 is retired.
 webgl/2.0.y [ Skip ]
 
-# Explicitly enable tests which we have fixed and do not have corresponding 2.0.y test functionality.
+# Explicitly enable tests which we have fixed and do not have corresponding 2.0.0 test functionality.
 webgl/2.0.y/conformance/canvas/to-data-url-test.html [ Pass ]
 webgl/2.0.y/conformance/misc/invalid-passed-params.html [ Pass ]
 webgl/2.0.y/conformance/glsl/bugs/character-set.html [ Pass ]
+webgl/2.0.y/conformance/textures/misc/texture-corner-case-videos.html [ Pass ]
 
 # WebGL 1.0.3 and 2.0.0 tests where behavior is obsolete and WebKit contains implementation

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-08-12  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html fails on Cocoa
+        https://bugs.webkit.org/show_bug.cgi?id=228821
+        <rdar://problem/81562236>
+
+        Reviewed by Kenneth Russell.
+
+        When uploading the WebGL texture content from videos, use the video
+        visible data size instead of video element size. The video data can
+        be scaled with its filters, but we should upload only the original
+        pixels.
+
+        The GPU codepath already did this, but CPU codepath did not.
+        This change fixes the CPU codepath.
+
+        Fixes webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html
+        webgl/2.0.y/conformance/textures/misc/texture-corner-case-videos.html
+
+        * html/canvas/WebGLRenderingContextBase.cpp:
+        (WebCore::WebGLRenderingContextBase::texImageSourceHelper):
+        (WebCore::WebGLRenderingContextBase::videoFrameToImage):
+        (WebCore::WebGLRenderingContextBase::LRUImageBufferCache::imageBuffer):
+        * html/canvas/WebGLRenderingContextBase.h:
+
 2021-08-12  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp

@@ -4935 +4935 @@
 
         // Fallback pure SW path.
-        RefPtr<Image> image = videoFrameToImage(video.get(), DontCopyBackingStore);
+        RefPtr<Image> image = videoFrameToImage(video.get(), DontCopyBackingStore, functionName);
         if (!image)
             return { };
@@ -5769 +5769 @@
 #if ENABLE(VIDEO)
 
-RefPtr<Image> WebGLRenderingContextBase::videoFrameToImage(HTMLVideoElement* video, BackingStoreCopy backingStoreCopy)
-{
-    IntSize size(video->videoWidth(), video->videoHeight());
-    ImageBuffer* buf = m_generatedImageCache.imageBuffer(size);
-    if (!buf) {
-        synthesizeGLError(GraphicsContextGL::OUT_OF_MEMORY, "texImage2D", "out of memory");
+RefPtr<Image> WebGLRenderingContextBase::videoFrameToImage(HTMLVideoElement* video, BackingStoreCopy backingStoreCopy, const char* functionName)
+{
+#if USE(AVFOUNDATION)
+    auto nativeImage = video->nativeImageForCurrentTime();
+    // Currently we might be missing an image due to MSE not being able to provide the first requested frame.
+    // https://bugs.webkit.org/show_bug.cgi?id=228997
+    if (!nativeImage)
         return nullptr;
-    }
-    FloatRect destRect(0, 0, size.width(), size.height());
-    // FIXME: Turn this into a GPU-GPU texture copy instead of CPU readback.
-    video->paintCurrentFrameInContext(buf->context(), destRect);
-    return buf->copyImage(backingStoreCopy);
+    IntSize imageSize = nativeImage->size();
+    if (imageSize.isEmpty()) {
+        synthesizeGLError(GraphicsContextGL::INVALID_VALUE, functionName, "video visible size is empty");
+        return nullptr;
+    }
+    FloatRect imageRect { { }, imageSize };
+    ImageBuffer* imageBuffer = m_generatedImageCache.imageBuffer(imageSize, CompositeOperator::Copy);
+    if (!imageBuffer) {
+        synthesizeGLError(GraphicsContextGL::OUT_OF_MEMORY, functionName, "out of memory");
+        return nullptr;
+    }
+    imageBuffer->context().drawNativeImage(*nativeImage, imageRect.size(), imageRect, imageRect, CompositeOperator::Copy);
+#else
+    // This is a legacy code path that produces incompatible texture size when the
+    // video visible size is different to the natural size. This should be removed
+    // once all platforms implement nativeImageForCurrentTime().
+    IntSize videoSize { static_cast<int>(video->videoWidth()), static_cast<int>(video->videoHeight()) };
+    ImageBuffer* imageBuffer = m_generatedImageCache.imageBuffer(videoSize);
+    if (!imageBuffer) {
+        synthesizeGLError(GraphicsContextGL::OUT_OF_MEMORY, functionName, "out of memory");
+        return nullptr;
+    }
+    video->paintCurrentFrameInContext(imageBuffer->context(), { { }, videoSize });
+#endif
+    RefPtr<Image> image = imageBuffer->copyImage(backingStoreCopy);
+    if (!image) {
+        synthesizeGLError(GraphicsContextGL::OUT_OF_MEMORY, functionName, "out of memory");
+        return nullptr;
+    }
+    return image;
 }
 
@@ -7622 +7648 @@
 }
 
-ImageBuffer* WebGLRenderingContextBase::LRUImageBufferCache::imageBuffer(const IntSize& size)
+ImageBuffer* WebGLRenderingContextBase::LRUImageBufferCache::imageBuffer(const IntSize& size, CompositeOperator fillOperator)
 {
     size_t i;
@@ -7632 +7658 @@
             continue;
         bubbleToFront(i);
-        buf->context().clearRect(FloatRect({ }, FloatSize(size)));
+        if (fillOperator != CompositeOperator::Copy && fillOperator != CompositeOperator::Clear)
+            buf->context().clearRect({ { }, size });
         return buf;
     }

trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h

@@ -525 +525 @@
 
 #if ENABLE(VIDEO)
-    RefPtr<Image> videoFrameToImage(HTMLVideoElement*, BackingStoreCopy);
+    RefPtr<Image> videoFrameToImage(HTMLVideoElement*, BackingStoreCopy, const char* functionName);
 #endif
 
@@ -615 +615 @@
     public:
         LRUImageBufferCache(int capacity);
-        // The pointer returned is owned by the image buffer map.
-        ImageBuffer* imageBuffer(const IntSize& size);
+        // Returns pointer to a cleared image buffer that is owned by the cache. The pointer is valid until next call.
+        // Using fillOperator == CompositeOperator::Copy can be used to omit the clear of the buffer.
+        ImageBuffer* imageBuffer(const IntSize&, CompositeOperator fillOperator = CompositeOperator::SourceOver);
     private:
         void bubbleToFront(size_t idx);