- Revision
- 224207
- Author
- jer.no...@apple.com
- Date
- 2017-10-30 15:43:56 -0700 (Mon, 30 Oct 2017)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (224206 => 224207)
--- trunk/Source/WebCore/ChangeLog 2017-10-30 22:42:23 UTC (rev 224206)
+++ trunk/Source/WebCore/ChangeLog 2017-10-30 22:43:56 UTC (rev 224207)
@@ -1,3 +1,42 @@
+2017-10-30 Jer Noble <jer.no...@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 <mcatanz...@igalia.com>
WKBundlePageWillSendSubmitEventCallback is called with incorrect frame parameter
Modified: trunk/Source/WebCore/platform/graphics/GraphicsContext3D.h (224206 => 224207)
--- trunk/Source/WebCore/platform/graphics/GraphicsContext3D.h 2017-10-30 22:42:23 UTC (rev 224206)
+++ trunk/Source/WebCore/platform/graphics/GraphicsContext3D.h 2017-10-30 22:43:56 UTC (rev 224207)
@@ -1285,6 +1285,9 @@
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); }
private:
@@ -1431,7 +1434,7 @@
struct GraphicsContext3DState {
GC3Duint boundFBO { 0 };
- GC3Denum activeTexture { GraphicsContext3D::TEXTURE0 };
+ GC3Denum activeTextureUnit { GraphicsContext3D::TEXTURE0 };
using BoundTextureMap = HashMap<GC3Denum,
std::pair<GC3Duint, GC3Denum>,
@@ -1440,8 +1443,8 @@
WTF::PairHashTraits<WTF::UnsignedWithZeroKeyHashTraits<GC3Duint>, WTF::UnsignedWithZeroKeyHashTraits<GC3Duint>>
>;
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);
if (iterator != boundTextureMap.end())
@@ -1449,7 +1452,8 @@
return 0;
}
- GC3Denum boundTarget(GC3Denum textureUnit)
+ GC3Duint currentBoundTarget() const { return boundTarget(activeTextureUnit); }
+ GC3Denum boundTarget(GC3Denum textureUnit) const
{
auto iterator = boundTextureMap.find(textureUnit);
if (iterator != boundTextureMap.end())
Modified: trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp (224206 => 224207)
--- trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp 2017-10-30 22:42:23 UTC (rev 224206)
+++ trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp 2017-10-30 22:43:56 UTC (rev 224207)
@@ -517,11 +517,10 @@
return false;
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.
return true;
@@ -619,7 +618,7 @@
m_lastSurface = surface;
m_lastSurfaceSeed = newSurfaceSeed;
- m_lastTextureSeed.set(outputTexture, newTextureSeed);
+ m_lastTextureSeed.set(outputTexture, m_context->textureSeed(outputTexture));
m_lastFlipY = flipY;
return true;
@@ -723,7 +722,9 @@
VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver(GraphicsContext3D& context)
: 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);
m_context.getIntegerv(GraphicsContext3D::ARRAY_BUFFER_BINDING, &m_arrayBuffer);
@@ -741,7 +742,8 @@
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.activeTexture(m_activeTextureUnit);
+ m_context.bindTexture(m_boundTarget, m_boundTexture);
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]);
Modified: trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h (224206 => 224207)
--- trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h 2017-10-30 22:42:23 UTC (rev 224206)
+++ trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h 2017-10-30 22:43:56 UTC (rev 224207)
@@ -66,7 +66,9 @@
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 };
GC3Dint m_arrayBuffer { 0 };
Modified: trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp (224206 => 224207)
--- trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp 2017-10-30 22:42:23 UTC (rev 224206)
+++ trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp 2017-10-30 22:43:56 UTC (rev 224207)
@@ -262,7 +262,7 @@
::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);
::glFlush();
@@ -452,7 +452,7 @@
void GraphicsContext3D::activeTexture(GC3Denum texture)
{
makeContextCurrent();
- m_state.activeTexture = texture;
+ m_state.activeTextureUnit = texture;
::glActiveTexture(texture);
}
@@ -505,7 +505,7 @@
void GraphicsContext3D::bindTexture(GC3Denum target, Platform3DObject texture)
{
makeContextCurrent();
- m_state.setBoundTexture(m_state.activeTexture, texture, target);
+ m_state.setBoundTexture(m_state.activeTextureUnit, texture, target);
::glBindTexture(target, texture);
}
