diff options
author | ccameron <ccameron@chromium.org> | 2014-12-12 13:38:19 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-12-12 21:38:53 +0000 |
commit | 4ff12a73e3ec88084f1803f3fbbe998e079d5c8e (patch) | |
tree | ebb1939659a4cc3e80972c5955bc1dd20dce3839 | |
parent | c19a06b763dc61eb0f36734680bbbc3fdc5c67b7 (diff) | |
download | chromium_src-4ff12a73e3ec88084f1803f3fbbe998e079d5c8e.zip chromium_src-4ff12a73e3ec88084f1803f3fbbe998e079d5c8e.tar.gz chromium_src-4ff12a73e3ec88084f1803f3fbbe998e079d5c8e.tar.bz2 |
Add a glBegin/End pair to make glBindFramebuffer work
It appears that this fixes the issue where rendered contents wouldn't
appear on the screen. I say "it appears" because this bug is very
difficult to repro, and so I can never say it's actually gone, just
that I wasn't able to make it happen anymore.
I can come up with a line of reasoning that the glBegin/End causes
the driver to go and validate its internal state. Perhaps sometimes the
dirty bit for the FBO is missed (and doing the glBegin/End immediately
after changing the FBO ensure it's picked up). Ultimately it is just a
guess.
This workaround was discovered by accident through the following
sequence of steps when debugging:
1. Draw a triangle to the CAOpenGLLayer after drawing the texture, to
ensure that we are actually getting draw calls
- This appeared and kept updating even when the texture stopped
updating
2. Draw a triangle to the FBO's texture from the CAOpenGLLayer's
context, to make sure changes to the texture would go through
- This appeared and kept updating even when the textures topped
updating (so it's probably a problem with the command buffer
context).
3. Draw a triangle to the FBO's texture from the command buffer's
context just before glSwapBuffers
- This triangle never appeared, so I tried the next experiment.
4. Draw a triangle to the FBO's texture right after it is bound using
glBindFramebufferEXT
- Suddenly the bug went away (and I never saw the triangle, because
it was drawn over).
5. Just do a glBegin/End with program 0, since we found this was enough to
work around driver bugs in the past.
- Still couldn't repro the bug with this.
BUG=435786
Review URL: https://codereview.chromium.org/797533002
Cr-Commit-Position: refs/heads/master@{#308166}
-rw-r--r-- | content/common/gpu/image_transport_surface_fbo_mac.h | 1 | ||||
-rw-r--r-- | content/common/gpu/image_transport_surface_fbo_mac.mm | 36 | ||||
-rw-r--r-- | gpu/command_buffer/service/gles2_cmd_decoder.cc | 8 | ||||
-rw-r--r-- | gpu/config/gpu_driver_bug_list_json.cc | 13 | ||||
-rw-r--r-- | gpu/config/gpu_driver_bug_workaround_type.h | 2 | ||||
-rw-r--r-- | ui/gl/gl_surface.cc | 3 | ||||
-rw-r--r-- | ui/gl/gl_surface.h | 4 |
7 files changed, 66 insertions, 1 deletions
diff --git a/content/common/gpu/image_transport_surface_fbo_mac.h b/content/common/gpu/image_transport_surface_fbo_mac.h index dbcdc83..f9f8b0e 100644 --- a/content/common/gpu/image_transport_surface_fbo_mac.h +++ b/content/common/gpu/image_transport_surface_fbo_mac.h @@ -74,6 +74,7 @@ class ImageTransportSurfaceFBO void* GetHandle() override; void* GetDisplay() override; bool OnMakeCurrent(gfx::GLContext* context) override; + void NotifyWasBound() override; unsigned int GetBackingFrameBufferObject() override; bool SetBackbufferAllocation(bool allocated) override; void SetFrontbufferAllocation(bool allocated) override; diff --git a/content/common/gpu/image_transport_surface_fbo_mac.mm b/content/common/gpu/image_transport_surface_fbo_mac.mm index 6af794d..e081633 100644 --- a/content/common/gpu/image_transport_surface_fbo_mac.mm +++ b/content/common/gpu/image_transport_surface_fbo_mac.mm @@ -81,6 +81,42 @@ bool ImageTransportSurfaceFBO::OnMakeCurrent(gfx::GLContext* context) { return true; } +void ImageTransportSurfaceFBO::NotifyWasBound() { + // Sometimes calling glBindFramebuffer doesn't seem to be enough to get + // rendered contents to show up in the color attachment. It appears that doing + // a glBegin/End pair with program 0 is enough to tickle the driver into + // actually effecting the binding. + // http://crbug.com/435786 + DCHECK(has_complete_framebuffer_); + + // We will restore the current program after the dummy glBegin/End pair. + // Ensure that we will be able to restore this state before attempting to + // change it. + GLint old_program_signed = 0; + glGetIntegerv(GL_CURRENT_PROGRAM, &old_program_signed); + GLuint old_program = static_cast<GLuint>(old_program_signed); + if (old_program && glIsProgram(old_program)) { + // A deleted program cannot be re-bound. + GLint delete_status = GL_FALSE; + glGetProgramiv(old_program, GL_DELETE_STATUS, &delete_status); + if (delete_status == GL_TRUE) + return; + // A program which has had the most recent link fail cannot be re-bound. + GLint link_status = GL_FALSE; + glGetProgramiv(old_program, GL_LINK_STATUS, &link_status); + if (link_status != GL_TRUE) + return; + } + + // Issue the dummy call and then restore the state. + glUseProgram(0); + GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT); + DCHECK(status == GL_FRAMEBUFFER_COMPLETE); + glBegin(GL_TRIANGLES); + glEnd(); + glUseProgram(old_program); +} + unsigned int ImageTransportSurfaceFBO::GetBackingFrameBufferObject() { return fbo_id_; } diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc index 6d37175..7ea1a06 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc @@ -4278,6 +4278,14 @@ void GLES2DecoderImpl::SetIgnoreCachedStateForTest(bool ignore) { void GLES2DecoderImpl::OnFboChanged() const { if (workarounds().restore_scissor_on_fbo_change) state_.fbo_binding_for_scissor_workaround_dirty_ = true; + + if (workarounds().gl_begin_gl_end_on_fbo_change_to_backbuffer) { + GLint bound_fbo_unsigned = -1; + glGetIntegerv(GL_FRAMEBUFFER_BINDING_EXT, &bound_fbo_unsigned); + GLuint bound_fbo = static_cast<GLuint>(bound_fbo_unsigned); + if (surface_ && surface_->GetBackingFrameBufferObject() == bound_fbo) + surface_->NotifyWasBound(); + } } // Called after the FBO is checked for completeness. diff --git a/gpu/config/gpu_driver_bug_list_json.cc b/gpu/config/gpu_driver_bug_list_json.cc index b447c1a..80147469 100644 --- a/gpu/config/gpu_driver_bug_list_json.cc +++ b/gpu/config/gpu_driver_bug_list_json.cc @@ -19,7 +19,7 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST( { "name": "gpu driver bug list", // Please update the version number whenever you change this file. - "version": "7.11", + "version": "7.12", "entries": [ { "id": 1, @@ -1081,6 +1081,17 @@ LONG_STRING_CONST( "features": [ "gl_clear_broken" ] + }, + { + "id": 96, + "description": "glBindFramebuffer sometimes requires a glBegin/End to take effect", + "cr_bugs": [435786], + "os": { + "type": "macosx" + }, + "features": [ + "gl_begin_gl_end_on_fbo_change_to_backbuffer" + ] } ] } diff --git a/gpu/config/gpu_driver_bug_workaround_type.h b/gpu/config/gpu_driver_bug_workaround_type.h index 841a470..77d721b 100644 --- a/gpu/config/gpu_driver_bug_workaround_type.h +++ b/gpu/config/gpu_driver_bug_workaround_type.h @@ -54,6 +54,8 @@ force_gl_finish_after_compositing) \ GPU_OP(FORCE_INTEGRATED_GPU, \ force_integrated_gpu) \ + GPU_OP(GL_BEGIN_GL_END_ON_FBO_CHANGE_TO_BACKBUFFER, \ + gl_begin_gl_end_on_fbo_change_to_backbuffer) \ GPU_OP(GL_CLEAR_BROKEN, \ gl_clear_broken) \ GPU_OP(INIT_GL_POSITION_IN_VERTEX_SHADER, \ diff --git a/ui/gl/gl_surface.cc b/ui/gl/gl_surface.cc index e2f1717..3de560b 100644 --- a/ui/gl/gl_surface.cc +++ b/ui/gl/gl_surface.cc @@ -199,6 +199,9 @@ bool GLSurface::OnMakeCurrent(GLContext* context) { return true; } +void GLSurface::NotifyWasBound() { +} + bool GLSurface::SetBackbufferAllocation(bool allocated) { return true; } diff --git a/ui/gl/gl_surface.h b/ui/gl/gl_surface.h index 1302cce..ec04c96 100644 --- a/ui/gl/gl_surface.h +++ b/ui/gl/gl_surface.h @@ -88,6 +88,10 @@ class GL_EXPORT GLSurface : public base::RefCounted<GLSurface> { // on error. virtual bool OnMakeCurrent(GLContext* context); + // Called when the surface is bound as the current framebuffer for the + // current context. + virtual void NotifyWasBound(); + // Used for explicit buffer management. virtual bool SetBackbufferAllocation(bool allocated); virtual void SetFrontbufferAllocation(bool allocated); |