Title: [222198] trunk
Revision
222198
Author
[email protected]
Date
2017-09-18 19:55:33 -0700 (Mon, 18 Sep 2017)

Log Message

[WebGL] VideoTextureCopierCV doesn't correctly restore vertex attribute state
https://bugs.webkit.org/show_bug.cgi?id=176771
<rdar://problem/34386621>

Reviewed by Antoine Quint.

Source/WebCore:

The OpenGL context in VideoTextureCopierCV wasn't being restored to
the state it had before rendering a video to a texture. Specifically
the vertex attribute values were never recorded by the state saver.

Update the existing test of VideoTextureCopierCV so that it is
explicitly doing something different from the WebCore code, which
means that state will have to be correctly restored for the test
to pass.

* platform/graphics/cv/VideoTextureCopierCV.cpp:
(WebCore::VideoTextureCopierCV::copyVideoTextureToPlatformTexture): Make sure
to record the vertex attribute state once we know the location of the position attribute.
(WebCore::VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver):
(WebCore::VideoTextureCopierCV::GC3DStateSaver::~GC3DStateSaver):
(WebCore::VideoTextureCopierCV::GC3DStateSaver::saveVertexAttribState): Save all the
applicable vertex attribute state information.
* platform/graphics/cv/VideoTextureCopierCV.h: GC3DStateSaver can use a reference
to the GC3D rather than a pointer.

LayoutTests:

Tweak this test to make sure it does something different from
the code in WebCore to render a video into a texture, thus
ensuring that we're testing state is correctly restored.

* fast/canvas/webgl/texImage2D-video-flipY-true.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (222197 => 222198)


--- trunk/LayoutTests/ChangeLog	2017-09-19 02:52:38 UTC (rev 222197)
+++ trunk/LayoutTests/ChangeLog	2017-09-19 02:55:33 UTC (rev 222198)
@@ -1,5 +1,19 @@
 2017-09-19  Dean Jackson  <[email protected]>
 
+        [WebGL] VideoTextureCopierCV doesn't correctly restore vertex attribute state
+        https://bugs.webkit.org/show_bug.cgi?id=176771
+        <rdar://problem/34386621>
+
+        Reviewed by Antoine Quint.
+
+        Tweak this test to make sure it does something different from
+        the code in WebCore to render a video into a texture, thus
+        ensuring that we're testing state is correctly restored.
+
+        * fast/canvas/webgl/texImage2D-video-flipY-true.html:
+
+2017-09-19  Dean Jackson  <[email protected]>
+
         [WebGL] accelerated texImage2D for video doesn't respect flipY
         https://bugs.webkit.org/show_bug.cgi?id=176491
         <rdar://problem/33833511>

Modified: trunk/LayoutTests/fast/canvas/webgl/texImage2D-video-flipY-true.html (222197 => 222198)


--- trunk/LayoutTests/fast/canvas/webgl/texImage2D-video-flipY-true.html	2017-09-19 02:52:38 UTC (rev 222197)
+++ trunk/LayoutTests/fast/canvas/webgl/texImage2D-video-flipY-true.html	2017-09-19 02:55:33 UTC (rev 222198)
@@ -123,18 +123,16 @@
     let positionAttribute = gl.getAttribLocation(program, "a_position");
     gl.enableVertexAttribArray(positionAttribute);
 
-    let vertices = new Float32Array([
-       -1, -1,
-       1, -1,
-       1, 1,
-       1, 1,
-       -1, 1,
-       -1, -1
-    ]);
+    // Unlike the corresponding texImage2D-video-flipY-false test, draw a quad using
+    // four points in a triangle strip, with 3 floats per point. This way we're doing
+    // something different from the WebCore code that renders the video into a texture,
+    // ensuring that state restoration is correctly restored.
+    let vertices = new Float32Array([-1.0, -1.0, 0.0, 1.0, -1.0, 0.0, -1.0, 1.0, 0.0, 1.0, 1.0, 0.0]);
 
     let buffer = gl.createBuffer();
     gl.bindBuffer(gl.ARRAY_BUFFER, buffer);
     gl.bufferData(gl.ARRAY_BUFFER, vertices, gl.STATIC_DRAW);
+    gl.vertexAttribPointer(positionAttribute, 3, gl.FLOAT, false, 0, 0);
 
     let updateTexture = function (texture, data) {
         gl.bindTexture(gl.TEXTURE_2D, texture);
@@ -188,9 +186,8 @@
         gl.uniform1i(textureUniform, 0);
 
         gl.bindBuffer(gl.ARRAY_BUFFER, buffer);
-        gl.vertexAttribPointer(positionAttribute, 2, gl.FLOAT, false, 0, 0);
 
-        gl.drawArrays(gl.TRIANGLES, 0, 6);
+        gl.drawArrays(gl.TRIANGLE_STRIP, 0, 4);
 
         animationFrame = requestAnimationFrame(drawFunction);
     };

Modified: trunk/Source/WebCore/ChangeLog (222197 => 222198)


--- trunk/Source/WebCore/ChangeLog	2017-09-19 02:52:38 UTC (rev 222197)
+++ trunk/Source/WebCore/ChangeLog	2017-09-19 02:55:33 UTC (rev 222198)
@@ -1,5 +1,32 @@
 2017-09-19  Dean Jackson  <[email protected]>
 
+        [WebGL] VideoTextureCopierCV doesn't correctly restore vertex attribute state
+        https://bugs.webkit.org/show_bug.cgi?id=176771
+        <rdar://problem/34386621>
+
+        Reviewed by Antoine Quint.
+
+        The OpenGL context in VideoTextureCopierCV wasn't being restored to
+        the state it had before rendering a video to a texture. Specifically
+        the vertex attribute values were never recorded by the state saver.
+
+        Update the existing test of VideoTextureCopierCV so that it is
+        explicitly doing something different from the WebCore code, which
+        means that state will have to be correctly restored for the test
+        to pass.
+
+        * platform/graphics/cv/VideoTextureCopierCV.cpp:
+        (WebCore::VideoTextureCopierCV::copyVideoTextureToPlatformTexture): Make sure
+        to record the vertex attribute state once we know the location of the position attribute.
+        (WebCore::VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver):
+        (WebCore::VideoTextureCopierCV::GC3DStateSaver::~GC3DStateSaver):
+        (WebCore::VideoTextureCopierCV::GC3DStateSaver::saveVertexAttribState): Save all the
+        applicable vertex attribute state information.
+        * platform/graphics/cv/VideoTextureCopierCV.h: GC3DStateSaver can use a reference
+        to the GC3D rather than a pointer.
+
+2017-09-19  Dean Jackson  <[email protected]>
+
         [WebGL] accelerated texImage2D for video doesn't respect flipY
         https://bugs.webkit.org/show_bug.cgi?id=176491
         <rdar://problem/33833511>

Modified: trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp (222197 => 222198)


--- trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp	2017-09-19 02:52:38 UTC (rev 222197)
+++ trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp	2017-09-19 02:55:33 UTC (rev 222198)
@@ -263,7 +263,7 @@
     if (!inputVideoTexture)
         return false;
 
-    GC3DStateSaver stateSaver(&m_context.get());
+    GC3DStateSaver stateSaver(m_context.get());
 
     if (!m_program) {
         if (!initializeContextObjects()) {
@@ -272,6 +272,8 @@
         }
     }
 
+    stateSaver.saveVertexAttribState(m_positionAttributeLocation);
+
     GLfloat lowerLeft[2] = { 0, 0 };
     GLfloat lowerRight[2] = { 0, 0 };
     GLfloat upperRight[2] = { 0, 0 };
@@ -343,25 +345,42 @@
     return true;
 }
 
-VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver(GraphicsContext3D* context)
+VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver(GraphicsContext3D& context)
     : m_context(context)
 {
-    ASSERT(context);
-    m_context->getIntegerv(GraphicsContext3D::TEXTURE_BINDING_2D, &m_texture);
-    m_context->getIntegerv(GraphicsContext3D::FRAMEBUFFER_BINDING, &m_framebuffer);
-    m_context->getIntegerv(GraphicsContext3D::CURRENT_PROGRAM, &m_program);
-    m_context->getIntegerv(GraphicsContext3D::ARRAY_BUFFER_BINDING, &m_arrayBuffer);
-    m_context->getIntegerv(GraphicsContext3D::VIEWPORT, m_viewport);
+    m_context.getIntegerv(GraphicsContext3D::TEXTURE_BINDING_2D, &m_texture);
+    m_context.getIntegerv(GraphicsContext3D::FRAMEBUFFER_BINDING, &m_framebuffer);
+    m_context.getIntegerv(GraphicsContext3D::CURRENT_PROGRAM, &m_program);
+    m_context.getIntegerv(GraphicsContext3D::ARRAY_BUFFER_BINDING, &m_arrayBuffer);
+    m_context.getIntegerv(GraphicsContext3D::VIEWPORT, m_viewport);
+
 }
 
 VideoTextureCopierCV::GC3DStateSaver::~GC3DStateSaver()
 {
-    m_context->bindTexture(GraphicsContext3D::TEXTURE_BINDING_2D, m_texture);
-    m_context->bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_framebuffer);
-    m_context->useProgram(m_program);
-    m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_arrayBuffer);
-    m_context->viewport(m_viewport[0], m_viewport[1], m_viewport[2], m_viewport[3]);
+    if (m_vertexAttribEnabled)
+        m_context.enableVertexAttribArray(m_vertexAttribIndex);
+    else
+        m_context.disableVertexAttribArray(m_vertexAttribIndex);
+
+    m_context.bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_arrayBuffer);
+    m_context.vertexAttribPointer(m_vertexAttribIndex, m_vertexAttribSize, m_vertexAttribType, m_vertexAttribNormalized, m_vertexAttribStride, m_vertexAttribPointer);
+
+    m_context.bindTexture(GraphicsContext3D::TEXTURE_BINDING_2D, m_texture);
+    m_context.bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_framebuffer);
+    m_context.useProgram(m_program);
+    m_context.viewport(m_viewport[0], m_viewport[1], m_viewport[2], m_viewport[3]);
 }
 
+void VideoTextureCopierCV::GC3DStateSaver::saveVertexAttribState(GC3Duint index)
+{
+    m_vertexAttribIndex = index;
+    m_context.getVertexAttribiv(index, GraphicsContext3D::VERTEX_ATTRIB_ARRAY_ENABLED, &m_vertexAttribEnabled);
+    m_context.getVertexAttribiv(index, GraphicsContext3D::VERTEX_ATTRIB_ARRAY_SIZE, &m_vertexAttribSize);
+    m_context.getVertexAttribiv(index, GraphicsContext3D::VERTEX_ATTRIB_ARRAY_TYPE, &m_vertexAttribType);
+    m_context.getVertexAttribiv(index, GraphicsContext3D::VERTEX_ATTRIB_ARRAY_NORMALIZED, &m_vertexAttribNormalized);
+    m_context.getVertexAttribiv(index, GraphicsContext3D::VERTEX_ATTRIB_ARRAY_STRIDE, &m_vertexAttribStride);
+    m_vertexAttribPointer = m_context.getVertexAttribOffset(index, GraphicsContext3D::VERTEX_ATTRIB_ARRAY_POINTER);
+}
 
 }

Modified: trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h (222197 => 222198)


--- trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h	2017-09-19 02:52:38 UTC (rev 222197)
+++ trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h	2017-09-19 02:55:33 UTC (rev 222198)
@@ -52,16 +52,26 @@
 private:
     class GC3DStateSaver {
     public:
-        GC3DStateSaver(GraphicsContext3D*);
+        GC3DStateSaver(GraphicsContext3D&);
         ~GC3DStateSaver();
 
+        void saveVertexAttribState(GC3Duint index);
+
     private:
-        GraphicsContext3D* m_context;
+        GraphicsContext3D& m_context;
         GC3Dint m_texture { 0 };
         GC3Dint m_framebuffer { 0 };
         GC3Dint m_program { 0 };
         GC3Dint m_arrayBuffer { 0 };
         GC3Dint m_viewport[4] { 0, 0, 0, 0 };
+
+        GC3Duint m_vertexAttribIndex { 0 };
+        GC3Dint m_vertexAttribEnabled { 0 };
+        GC3Dint m_vertexAttribSize { 0 };
+        GC3Dint m_vertexAttribType { 0 };
+        GC3Dint m_vertexAttribNormalized { 0 };
+        GC3Dint m_vertexAttribStride { 0 };
+        GC3Dsizeiptr m_vertexAttribPointer { 0 };
     };
 
     bool initializeContextObjects();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to