Skip to content
Permalink
Browse files
[WebGL] Optimization to skip painting if texture and source surface h…
…asn'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):


Canonical link: https://commits.webkit.org/195171@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224207 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
jernoble committed Oct 30, 2017
1 parent c0aec4d commit c643ca5be8a09c4eb274185c9adfb7569f67c54e
Showing 5 changed files with 60 additions and 13 deletions.
@@ -1,3 +1,42 @@
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>

WKBundlePageWillSendSubmitEventCallback is called with incorrect frame parameter
@@ -1285,6 +1285,9 @@ class GraphicsContext3D : public RefCounted<GraphicsContext3D> {

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 @@ class GraphicsContext3D : public RefCounted<GraphicsContext3D> {

struct GraphicsContext3DState {
GC3Duint boundFBO { 0 };
GC3Denum activeTexture { GraphicsContext3D::TEXTURE0 };
GC3Denum activeTextureUnit { GraphicsContext3D::TEXTURE0 };

using BoundTextureMap = HashMap<GC3Denum,
std::pair<GC3Duint, GC3Denum>,
@@ -1440,16 +1443,17 @@ class GraphicsContext3D : public RefCounted<GraphicsContext3D> {
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())
return iterator->value.first;
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())
@@ -517,11 +517,10 @@ bool VideoTextureCopierCV::copyImageToPlatformTexture(CVPixelBufferRef image, si
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 @@ bool VideoTextureCopierCV::copyImageToPlatformTexture(CVPixelBufferRef image, si

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 @@ bool VideoTextureCopierCV::copyVideoTextureToPlatformTexture(Platform3DObject vi
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 @@ VideoTextureCopierCV::GC3DStateSaver::~GC3DStateSaver()
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]);
@@ -66,7 +66,9 @@ class VideoTextureCopierCV {

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 };
@@ -262,7 +262,7 @@ void GraphicsContext3D::prepareTexture()

::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 @@ IntSize GraphicsContext3D::getInternalFramebufferSize() const
void GraphicsContext3D::activeTexture(GC3Denum texture)
{
makeContextCurrent();
m_state.activeTexture = texture;
m_state.activeTextureUnit = texture;
::glActiveTexture(texture);
}

@@ -505,7 +505,7 @@ void GraphicsContext3D::bindRenderbuffer(GC3Denum target, Platform3DObject rende
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);
}

0 comments on commit c643ca5

Please sign in to comment.