Changeset 224207 in webkit

Timestamp:

Oct 30, 2017 3:43:56 PM (6 years ago)

Author:

jer.noble@apple.com

Message:

[WebGL] Optimization to skip painting if texture and source surface hasn't changed isn't working; re-optimize.
​https://bugs.webkit.org/show_bug.cgi?id=178953

Reviewed by Dean Jackson.

The "seed" value of the current bound texture never matches the last saved value in
VideoTextureCopierCV::copyImageToPlatformTexture(). The value is modified by the function
itself, so a fresh value needs to be re-queried after the image's surface is attached to the
texture.

Once this fix is in, however, the <canvas> being painted will flash when no new image is
available. This is because the wrong texture target is being restored by the GC3DStateSaver
at the end of copyImageToPlatformTexture(). While we're fixing that, we may as well use the
texture state saved by the GraphicsContext3D itself to restore the correct texture unit,
texture target, and texture.

• platform/graphics/GraphicsContext3D.h:

(WebCore::GraphicsContext3D::activeTextureUnit const):
(WebCore::GraphicsContext3D::currentBoundTexture const):
(WebCore::GraphicsContext3D::currentBoundTarget const):
(WebCore::GraphicsContext3D::GraphicsContext3DState::currentBoundTexture const):
(WebCore::GraphicsContext3D::GraphicsContext3DState::boundTexture const):
(WebCore::GraphicsContext3D::GraphicsContext3DState::currentBoundTarget const):
(WebCore::GraphicsContext3D::GraphicsContext3DState::boundTarget const):
(WebCore::GraphicsContext3D::GraphicsContext3DState::currentBoundTexture): Deleted.
(WebCore::GraphicsContext3D::GraphicsContext3DState::boundTexture): Deleted.
(WebCore::GraphicsContext3D::GraphicsContext3DState::boundTarget): Deleted.

• platform/graphics/cv/VideoTextureCopierCV.cpp:

(WebCore::VideoTextureCopierCV::copyImageToPlatformTexture):
(WebCore::VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver):
(WebCore::VideoTextureCopierCV::GC3DStateSaver::~GC3DStateSaver):

• platform/graphics/cv/VideoTextureCopierCV.h:
• platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:

(WebCore::GraphicsContext3D::prepareTexture):
(WebCore::GraphicsContext3D::activeTexture):
(WebCore::GraphicsContext3D::bindTexture):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/GraphicsContext3D.h (modified) (4 diffs)
• platform/graphics/cv/VideoTextureCopierCV.cpp (modified) (4 diffs)
• platform/graphics/cv/VideoTextureCopierCV.h (modified) (1 diff)
• platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-30  Jer Noble  <jer.noble@apple.com>
+
+        [WebGL] Optimization to skip painting if texture and source surface hasn't changed isn't working; re-optimize.
+        https://bugs.webkit.org/show_bug.cgi?id=178953
+
+        Reviewed by Dean Jackson.
+
+        The "seed" value of the current bound texture never matches the last saved value in
+        VideoTextureCopierCV::copyImageToPlatformTexture(). The value is modified by the function
+        itself, so a fresh value needs to be re-queried after the image's surface is attached to the
+        texture.
+
+        Once this fix is in, however, the <canvas> being painted will flash when no new image is
+        available. This is because the wrong texture target is being restored by the GC3DStateSaver
+        at the end of copyImageToPlatformTexture(). While we're fixing that, we may as well use the
+        texture state saved by the GraphicsContext3D itself to restore the correct texture unit,
+        texture target, and texture.
+
+        * platform/graphics/GraphicsContext3D.h:
+        (WebCore::GraphicsContext3D::activeTextureUnit const):
+        (WebCore::GraphicsContext3D::currentBoundTexture const):
+        (WebCore::GraphicsContext3D::currentBoundTarget const):
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::currentBoundTexture const):
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::boundTexture const):
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::currentBoundTarget const):
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::boundTarget const):
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::currentBoundTexture): Deleted.
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::boundTexture): Deleted.
+        (WebCore::GraphicsContext3D::GraphicsContext3DState::boundTarget): Deleted.
+        * platform/graphics/cv/VideoTextureCopierCV.cpp:
+        (WebCore::VideoTextureCopierCV::copyImageToPlatformTexture):
+        (WebCore::VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver):
+        (WebCore::VideoTextureCopierCV::GC3DStateSaver::~GC3DStateSaver):
+        * platform/graphics/cv/VideoTextureCopierCV.h:
+        * platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:
+        (WebCore::GraphicsContext3D::prepareTexture):
+        (WebCore::GraphicsContext3D::activeTexture):
+        (WebCore::GraphicsContext3D::bindTexture):
+
 2017-10-30  Michael Catanzaro  <mcatanzaro@igalia.com>
 

trunk/Source/WebCore/platform/graphics/GraphicsContext3D.h

@@ -1286 +1286 @@
     void setFailNextGPUStatusCheck() { m_failNextStatusCheck = true; }
 
+    GC3Denum activeTextureUnit() const { return m_state.activeTextureUnit; }
+    GC3Denum currentBoundTexture() const { return m_state.currentBoundTexture(); }
+    GC3Denum currentBoundTarget() const { return m_state.currentBoundTarget(); }
     unsigned textureSeed(GC3Duint texture) { return m_state.textureSeedCount.count(texture); }
 
@@ -1432 +1435 @@
     struct GraphicsContext3DState {
         GC3Duint boundFBO { 0 };
-        GC3Denum activeTexture { GraphicsContext3D::TEXTURE0 };
+        GC3Denum activeTextureUnit { GraphicsContext3D::TEXTURE0 };
 
         using BoundTextureMap = HashMap<GC3Denum,
@@ -1441 +1444 @@
         >;
         BoundTextureMap boundTextureMap;
-        GC3Duint currentBoundTexture() { return boundTexture(activeTexture); }
-        GC3Duint boundTexture(GC3Denum textureUnit)
+        GC3Duint currentBoundTexture() const { return boundTexture(activeTextureUnit); }
+        GC3Duint boundTexture(GC3Denum textureUnit) const
         {
             auto iterator = boundTextureMap.find(textureUnit);
@@ -1450 +1453 @@
         }
 
-        GC3Denum boundTarget(GC3Denum textureUnit)
+        GC3Duint currentBoundTarget() const { return boundTarget(activeTextureUnit); }
+        GC3Denum boundTarget(GC3Denum textureUnit) const
         {
             auto iterator = boundTextureMap.find(textureUnit);

trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp

@@ -518 +518 @@
 
     auto newSurfaceSeed = IOSurfaceGetSeed(surface);
-    auto newTextureSeed = m_context->textureSeed(outputTexture);
     if (flipY == m_lastFlipY
         && surface == m_lastSurface 
         && newSurfaceSeed == m_lastSurfaceSeed
-        && lastTextureSeed(outputTexture) == newTextureSeed) {
+        && lastTextureSeed(outputTexture) == m_context->textureSeed(outputTexture)) {
         // If the texture hasn't been modified since the last time we copied to it, and the
         // image hasn't been modified since the last time it was copied, this is a no-op.
@@ -620 +619 @@
     m_lastSurface = surface;
     m_lastSurfaceSeed = newSurfaceSeed;
-    m_lastTextureSeed.set(outputTexture, newTextureSeed);
+    m_lastTextureSeed.set(outputTexture, m_context->textureSeed(outputTexture));
     m_lastFlipY = flipY;
 
@@ -724 +723 @@
     : m_context(context)
 {
-    m_context.getIntegerv(GraphicsContext3D::TEXTURE_BINDING_2D, &m_texture);
+    m_activeTextureUnit = m_context.activeTextureUnit();
+    m_boundTarget = m_context.currentBoundTarget();
+    m_boundTexture = m_context.currentBoundTexture();
     m_context.getIntegerv(GraphicsContext3D::FRAMEBUFFER_BINDING, &m_framebuffer);
     m_context.getIntegerv(GraphicsContext3D::CURRENT_PROGRAM, &m_program);
@@ -742 +743 @@
     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.activeTexture(m_activeTextureUnit);
+    m_context.bindTexture(m_boundTarget, m_boundTexture);
     m_context.bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_framebuffer);
     m_context.useProgram(m_program);

trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h

@@ -67 +67 @@
     private:
         GraphicsContext3D& m_context;
-        GC3Dint m_texture { 0 };
+        GC3Denum m_activeTextureUnit { 0 };
+        GC3Denum m_boundTarget { 0 };
+        GC3Denum m_boundTexture { 0 };
         GC3Dint m_framebuffer { 0 };
         GC3Dint m_program { 0 };

trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp

@@ -263 +263 @@
     ::glActiveTexture(GL_TEXTURE0);
     ::glBindTexture(GL_TEXTURE_2D, m_state.boundTarget(GL_TEXTURE0) == GL_TEXTURE_2D ? m_state.boundTexture(GL_TEXTURE0) : 0);
-    ::glActiveTexture(m_state.activeTexture);
+    ::glActiveTexture(m_state.activeTextureUnit);
     if (m_state.boundFBO != m_fbo)
         ::glBindFramebufferEXT(GraphicsContext3D::FRAMEBUFFER, m_state.boundFBO);
@@ -453 +453 @@
 {
     makeContextCurrent();
-    m_state.activeTexture = texture;
+    m_state.activeTextureUnit = texture;
     ::glActiveTexture(texture);
 }
@@ -506 +506 @@
 {
     makeContextCurrent();
-    m_state.setBoundTexture(m_state.activeTexture, texture, target);
+    m_state.setBoundTexture(m_state.activeTextureUnit, texture, target);
     ::glBindTexture(target, texture);
 }