Add a glBegin/End pair to make glBindFramebuffer work

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <stdio.h>
 
 #include <algorithm>
 #include <list>
@@ skipping 4251 matching lines @@
   state_.RestoreVertexAttribs();
 }
 
 void GLES2DecoderImpl::SetIgnoreCachedStateForTest(bool ignore) {
   state_.SetIgnoreCachedStateForTest(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.
 void GLES2DecoderImpl::OnUseFramebuffer() const {
   if (state_.fbo_binding_for_scissor_workaround_dirty_) {
     state_.fbo_binding_for_scissor_workaround_dirty_ = false;
     // The driver forgets the correct scissor when modifying the FBO binding.
     glScissor(state_.scissor_x,
               state_.scissor_y,
               state_.scissor_width,
@@ skipping 7250 matching lines @@
   }
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu