Add a glBegin/End pair to make glBindFramebuffer work

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index f97710e86a61009ce7b6b54517f63040d5f8cb3b..fcce245ebb0af236a61e715e6964ebb495ce5bbf 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -4269,6 +4269,28 @@ 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) {
+    // Sometimes calling glBindFramebuffer doesn't seem to be enough to get
+    // rendered contents to show up in the color attachment, if it's the
+    // backbuffer. It appears that doing a glBegin/glEnd pair is enough to
+    // tickle the driver into actually effect the binding.
+    // http://crbug.com/435786
+    GLuint bound_fbo = -1;
+    glGetIntegerv(GL_FRAMEBUFFER_BINDING_EXT, (GLint*)&bound_fbo);
+    if (bound_fbo == GetBackbufferServiceId()) {
+      GLuint old_program = -1;
+      glGetIntegerv(GL_CURRENT_PROGRAM, (GLint*)&old_program);
+      glUseProgram(0);
+      GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
+      if (status == GL_FRAMEBUFFER_COMPLETE) {
+        glBegin(GL_TRIANGLES);
+        glEnd();

[Ken Russell (switch to Gerrit), 2014/12/11 07:06:37]

I'm surprised this compiles. This file is supposed to be making all of its calls
through the GL bindings in ui/gl/ and they only include the OpenGL ES entry
points, which don't include glBegin or glEnd. It's likely that some header is
pulling in either the system OpenGL headers or the Mesa headers. Could you
please rebuild this file via ninja with -v, get the compiler's command line for
it, and capture the preprocessor's output to figure out how this is happening?
It shouldn't be possible to call these functions from here.

[ccameron, 2014/12/11 09:57:31]

Interesting! The command line is at the bottom. I've worked around this by
moving the action to ImageTransportSurfaceFBO.

[1/109] clang++ -MMD -MF
obj/gpu/command_buffer/service/gpu.gles2_cmd_decoder.o.d
-DV8_DEPRECATION_WARNINGS -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0
-DGOOGLE_CHROME_BUILD -DCR_CLANG_REVISION=223109 -DENABLE_RLZ -DCOMPONENT_BUILD
-DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_PRE_SYNC_BACKUP
-DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DUSE_PROPRIETARY_CODECS
-DENABLE_PEPPER_CDMS -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS
-DENABLE_HIDPI=1 -DDISCARDABLE_MEMORY_ALWAYS_SUPPORTED_NATIVELY
-DSYSTEM_NATIVELY_SIGNALS_MEMORY_PRESSURE -DENABLE_TASK_MANAGER=1
-DENABLE_EXTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PLUGINS=1
-DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1
-DENABLE_PROD_WALLET_SERVICE=1 -DENABLE_BACKGROUND=1 -DENABLE_GOOGLE_NOW=1
-DCLD_VERSION=2 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
-DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1
-DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 -DENABLE_SETTINGS_APP=1
-DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1
-DENABLE_WIFI_BOOTSTRAPPING=1 -DENABLE_LOAD_COMPLETION_HACKS=1
-DV8_USE_EXTERNAL_STARTUP_DATA -DGPU_IMPLEMENTATION -DPROTOBUF_USE_DLLS
-DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DSKIA_DLL
-DGR_GL_IGNORE_ES3_MSAA=0 -DSK_ENABLE_INST_COUNT=0 -DSK_SUPPORT_GPU=1
'-DGR_GL_CUSTOM_SETUP_HEADER="GrGLConfig_chrome.h"'
-DSK_ENABLE_LEGACY_API_ALIASING=1 -DSK_ATTR_DEPRECATED=SK_NOTHING_ARG1
-DSK_WILL_NEVER_DRAW_PERSPECTIVE_TEXT -DSK_SUPPORT_LEGACY_TEXTRENDERMODE
-DSK_IGNORE_GPU_LAYER_HOISTING -DSK_USE_POSIX_THREADS -DU_USING_ICU_NAMESPACE=0
-DU_ENABLE_DYLOAD=0 -DMESA_EGL_NO_X11_HEADERS -DUSE_LIBPCI=1 -DUSE_OPENSSL=1
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DNDEBUG -DOFFICIAL_BUILD
-DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -D_FORTIFY_SOURCE=2 -Igen -I../..
-I../../third_party/khronos -I../../gpu -I../../skia/config -Igen/protoc_out
-I../../third_party/protobuf -I../../third_party/protobuf/src
-I../../third_party/re2 -I../../third_party/skia/src/core
-I../../third_party/skia/include/core -I../../third_party/skia/include/effects
-I../../third_party/skia/include/pdf -I../../third_party/skia/include/gpu
-I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops
-I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports
-I../../third_party/skia/include/utils -I../../skia/ext
-I../../third_party/skia/include/utils/mac -I../../third_party/icu/source/i18n
-I../../third_party/icu/source/common -Igen/ui/gl
-I../../third_party/mesa/src/include -isysroot
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.6.sdk
-O2 -gdwarf-2 -fvisibility=hidden -Werror -Wnewline-eof
-mmacosx-version-min=10.6 -arch x86_64 -Wall -Wendif-labels -Wextra
-Wno-unused-parameter -Wno-missing-field-initializers
-Wno-selector-type-mismatch -Wheader-hygiene -Wno-char-subscripts
-Wno-unneeded-internal-declaration -Wno-covered-switch-default
-Wstring-conversion -Wno-c++11-narrowing -Wno-deprecated-register
-Wno-inconsistent-missing-override -std=c++11 -fno-rtti -fno-exceptions
-fvisibility-inlines-hidden -fno-threadsafe-statics -Xclang -load -Xclang
/Users/ccameron/chrome-dev-official/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib
-Xclang -add-plugin -Xclang find-bad-constructs -fcolor-diagnostics
-fno-strict-aliasing  -c ../../gpu/command_buffer/service/gles2_cmd_decoder.cc
-o obj/gpu/command_buffer/service/gpu.gles2_cmd_decoder.o

+      }
+      if (glIsProgram(old_program))
+        glUseProgram(old_program);

[Ken Russell (switch to Gerrit), 2014/12/11 07:06:37]

Note: this isn't guaranteed to be able to fully restore the previous state. If
it happens the user recently unsuccessfully relinked the program that's current
(defined by glUseProgram), then it won't be valid to call glUseProgram again on
the program object. This will have user-visible side effects.

Is your intent to only do this workaround in the "compositor's context"? Is the
test of the bound FBO against GetBackbufferServiceId() sufficient to do that? A
way to further restrict this workaround would be to only run it if
(!force_webgl_glsl_validation_) -- but that wouldn't take into account contexts
for Pepper 3D.

[ccameron, 2014/12/11 09:57:31]

Okay, I was curious about how this could fail. I've updated this to only call
glUseProgram(0) if the previously-bound program
- has a successful last-link status
- does not have a true deleted status
Empirically those were the only ways I was able to hit errors -- is this enough
to ensure no user-visible changes?
By moving this to the ImageTransportSurfaceFBO, I've effectively limited its
scope to compositor contexts (AFAIK). 

[Ken Russell (switch to Gerrit), 2014/12/12 02:30:44]

I think that should be sufficient.
OK.

+    }
+  }
 }
 
 // Called after the FBO is checked for completeness.