Diff
Modified: branches/safari-604-branch/LayoutTests/ChangeLog (223244 => 223245)
--- branches/safari-604-branch/LayoutTests/ChangeLog 2017-10-12 18:31:15 UTC (rev 223244)
+++ branches/safari-604-branch/LayoutTests/ChangeLog 2017-10-12 18:31:19 UTC (rev 223245)
@@ -1,5 +1,19 @@
2017-10-12 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-10-12 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: branches/safari-604-branch/LayoutTests/fast/canvas/webgl/texImage2D-video-flipY-true.html (223244 => 223245)
--- branches/safari-604-branch/LayoutTests/fast/canvas/webgl/texImage2D-video-flipY-true.html 2017-10-12 18:31:15 UTC (rev 223244)
+++ branches/safari-604-branch/LayoutTests/fast/canvas/webgl/texImage2D-video-flipY-true.html 2017-10-12 18:31:19 UTC (rev 223245)
@@ -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: branches/safari-604-branch/Source/WebCore/ChangeLog (223244 => 223245)
--- branches/safari-604-branch/Source/WebCore/ChangeLog 2017-10-12 18:31:15 UTC (rev 223244)
+++ branches/safari-604-branch/Source/WebCore/ChangeLog 2017-10-12 18:31:19 UTC (rev 223245)
@@ -1,5 +1,32 @@
2017-10-12 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-10-12 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: branches/safari-604-branch/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp (223244 => 223245)
--- branches/safari-604-branch/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp 2017-10-12 18:31:15 UTC (rev 223244)
+++ branches/safari-604-branch/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp 2017-10-12 18:31:19 UTC (rev 223245)
@@ -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: branches/safari-604-branch/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h (223244 => 223245)
--- branches/safari-604-branch/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h 2017-10-12 18:31:15 UTC (rev 223244)
+++ branches/safari-604-branch/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h 2017-10-12 18:31:19 UTC (rev 223245)
@@ -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();
