gpu: Cleanup 'black-screen on Huawei' work-around.

We got a response from their driver team. The surface is only corrupted if an FBO is bound when it is first made-current. So we can avoid re-creating the surface and just unbind the Fbo instead. The work-around is still needed in the context itself for two reasons: - Virtual context indirections - The first make-current for new surfaces is not in the decoder. BUG=235935 NOTRY=true No try, since this doesn't run on mac, so mac_asan bot isn't relevant. Review URL: https://chromiumcodereview.appspot.com/14021014 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196915 0039d316-1c4b-4281-b951-d872f2087c98
qweri0p · Apr 27, 2013 · 9b75399 · 9b75399
1 parent 64ee67e
commit 9b75399
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 64 deletions.
diff --git a/gpu/command_buffer/service/feature_info.cc b/gpu/command_buffer/service/feature_info.cc
@@ -191,7 +191,7 @@ void FeatureInfo::AddFeatures(const CommandLine& command_line) {
   bool is_qualcomm = false;
   bool is_imagination = false;
   bool is_arm = false;
-  bool is_hisilicon = false;
+  bool is_vivante = false;
   const char* gl_strings[2];
   gl_strings[0] = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
   gl_strings[1] = reinterpret_cast<const char*>(glGetString(GL_RENDERER));
@@ -209,10 +209,14 @@ void FeatureInfo::AddFeatures(const CommandLine& command_line) {
         is_qualcomm |= string_set.Contains("qualcomm");
         is_imagination |= string_set.Contains("imagination");
         is_arm |= string_set.Contains("arm");
-        is_hisilicon |= string_set.Contains("hisilicon");
+        is_vivante |= string_set.Contains("vivante");
+        is_vivante |= string_set.Contains("hisilicon");
       }
     }
 
+    if (extensions.Contains("GL_VIV_shader_binary"))
+      is_vivante = true;
+
     workarounds_.set_texture_filter_before_generating_mipmap = true;
     workarounds_.clear_alpha_in_readpixels = true;
     if (is_nvidia) {
@@ -223,8 +227,8 @@ void FeatureInfo::AddFeatures(const CommandLine& command_line) {
       workarounds_.flush_on_context_switch = true;
       workarounds_.delete_instead_of_resize_fbo = true;
     }
-    if (is_hisilicon) {
-      workarounds_.makecurrent_recreates_surfaces = true;
+    if (is_vivante) {
+      workarounds_.unbind_fbo_on_context_switch = true;
     }
 #if defined(OS_MACOSX)
     workarounds_.needs_offscreen_buffer_workaround = is_nvidia;

diff --git a/gpu/command_buffer/service/gl_context_virtual.cc b/gpu/command_buffer/service/gl_context_virtual.cc
@@ -103,8 +103,8 @@ bool GLContextVirtual::WasAllocatedUsingRobustnessExtension() {
   return shared_context_->WasAllocatedUsingRobustnessExtension();
 }
 
-void GLContextVirtual::SetRecreateSurfaceOnMakeCurrent() {
-  shared_context_->SetRecreateSurfaceOnMakeCurrent();
+void GLContextVirtual::SetUnbindFboOnMakeCurrent() {
+  shared_context_->SetUnbindFboOnMakeCurrent();
 }
 
 GLContextVirtual::~GLContextVirtual() {

diff --git a/gpu/command_buffer/service/gl_context_virtual.h b/gpu/command_buffer/service/gl_context_virtual.h
@@ -48,7 +48,7 @@ class GPU_EXPORT GLContextVirtual : public gfx::GLContext {
   virtual bool GetTotalGpuMemory(size_t* bytes) OVERRIDE;
   virtual void SetSafeToForceGpuSwitch() OVERRIDE;
   virtual bool WasAllocatedUsingRobustnessExtension() OVERRIDE;
-  virtual void SetRecreateSurfaceOnMakeCurrent() OVERRIDE;
+  virtual void SetUnbindFboOnMakeCurrent() OVERRIDE;
 
  protected:
   virtual ~GLContextVirtual();

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2532,11 +2532,9 @@ bool GLES2DecoderImpl::Initialize(
     glPointParameteri(GL_POINT_SPRITE_COORD_ORIGIN, GL_LOWER_LEFT);
   }
 
-#if defined(OS_ANDROID)
-  if (feature_info_->workarounds().makecurrent_recreates_surfaces) {
-    context_->SetRecreateSurfaceOnMakeCurrent();
+  if (feature_info_->workarounds().unbind_fbo_on_context_switch) {
+    context_->SetUnbindFboOnMakeCurrent();
   }
-#endif
 
   // Only compositor contexts are known to use only the subset of GL
   // that can be safely migrated between the iGPU and the dGPU. Mark
@@ -2841,6 +2839,10 @@ bool GLES2DecoderImpl::MakeCurrent() {
   if (workarounds().flush_on_context_switch)
     glFlush();
 
+  // Rebind the FBO if it was unbound by the context.
+  if (workarounds().unbind_fbo_on_context_switch)
+    RestoreFramebufferBindings();
+
   return true;
 }
 

diff --git a/gpu/command_buffer/service/gpu_driver_bug_workaround_type.h b/gpu/command_buffer/service/gpu_driver_bug_workaround_type.h
@@ -28,8 +28,8 @@
          exit_on_context_lost)                        \
   GPU_OP(FLUSH_ON_CONTEXT_SWITCH,                     \
          flush_on_context_switch)                     \
-  GPU_OP(MAKECURRENT_RECREATES_SURFACES,              \
-         makecurrent_recreates_surfaces)              \
+  GPU_OP(UNBIND_FBO_ON_CONTEXT_SWITCH,                \
+         unbind_fbo_on_context_switch)                \
   GPU_OP(MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_1024,        \
          max_cube_map_texture_size_limit_1024)        \
   GPU_OP(MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_4096,        \

diff --git a/ui/gl/gl_context.cc b/ui/gl/gl_context.cc
@@ -45,7 +45,7 @@ bool GLContext::GetTotalGpuMemory(size_t* bytes) {
 void GLContext::SetSafeToForceGpuSwitch() {
 }
 
-void GLContext::SetRecreateSurfaceOnMakeCurrent() {
+void GLContext::SetUnbindFboOnMakeCurrent() {
   NOTIMPLEMENTED();
 }
 

diff --git a/ui/gl/gl_context.h b/ui/gl/gl_context.h
@@ -65,9 +65,9 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
   // transitioning can cause corruption and hangs (OS X only).
   virtual void SetSafeToForceGpuSwitch();
 
-  // Indicate that the real context switches should recreate the surface
+  // Indicate that the real context switches should unbind the FBO first
   // (For an Android work-around only).
-  virtual void SetRecreateSurfaceOnMakeCurrent();
+  virtual void SetUnbindFboOnMakeCurrent();
 
   // Returns whether the current context supports the named extension. The
   // context must be current.

diff --git a/ui/gl/gl_context_egl.cc b/ui/gl/gl_context_egl.cc
@@ -29,7 +29,7 @@ GLContextEGL::GLContextEGL(GLShareGroup* share_group)
       context_(NULL),
       display_(NULL),
       config_(NULL),
-      recreate_surface_on_makecurrent_(false) {
+      unbind_fbo_on_makecurrent_(false) {
 }
 
 bool GLContextEGL::Initialize(
@@ -98,6 +98,9 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
                "context", context_,
                "surface", surface);
 
+  if (unbind_fbo_on_makecurrent_)
+    glBindFramebufferEXT(GL_FRAMEBUFFER, 0);
+
   if (!eglMakeCurrent(display_,
                       surface->GetHandle(),
                       surface->GetHandle(),
@@ -113,11 +116,6 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
     return false;
   }
 
-#if defined(OS_ANDROID)
-  if (!RecreateSurfaceIfNeeded(surface))
-    return false;
-#endif
-
   if (!surface->OnMakeCurrent(this)) {
     LOG(ERROR) << "Could not make current.";
     return false;
@@ -127,44 +125,8 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
   return true;
 }
 
-void GLContextEGL::SetRecreateSurfaceOnMakeCurrent() {
-  recreate_surface_on_makecurrent_ = true;
-}
-
-bool GLContextEGL::RecreateSurfaceIfNeeded(GLSurface* surface) {
-  if (!recreate_surface_on_makecurrent_ ||
-      !surface ||
-      surface->IsOffscreen() ||
-      surface->GetBackingFrameBufferObject())
-    return true;
-
-  // This is specifically needed for Vivante GPU's on Android.
-  // A native view surface will not be configured correctly
-  // unless we do all of the following steps after making the
-  // surface current.
-  GLint fbo = 0;
-  glGetIntegerv(GL_FRAMEBUFFER_BINDING, &fbo);
-  glBindFramebufferEXT(GL_FRAMEBUFFER, 0);
-
-  eglMakeCurrent(display_,
-                 EGL_NO_SURFACE,
-                 EGL_NO_SURFACE,
-                 EGL_NO_CONTEXT);
-  if (!surface->Recreate()) {
-    LOG(ERROR) << "Failed to recreate surface";
-    return false;
-  }
-  if (!eglMakeCurrent(display_,
-                      surface->GetHandle(),
-                      surface->GetHandle(),
-                      context_)) {
-    LOG(ERROR) << "eglMakeCurrent failed with error "
-               << GetLastEGLErrorString();
-    return false;
-  }
-
-  glBindFramebufferEXT(GL_FRAMEBUFFER, fbo);
-  return true;
+void GLContextEGL::SetUnbindFboOnMakeCurrent() {
+  unbind_fbo_on_makecurrent_ = true;
 }
 
 void GLContextEGL::ReleaseCurrent(GLSurface* surface) {

diff --git a/ui/gl/gl_context_egl.h b/ui/gl/gl_context_egl.h
@@ -35,9 +35,7 @@ class GLContextEGL : public GLContext {
   virtual std::string GetExtensions() OVERRIDE;
   virtual bool WasAllocatedUsingRobustnessExtension() OVERRIDE;
   virtual bool GetTotalGpuMemory(size_t* bytes) OVERRIDE;
-  virtual void SetRecreateSurfaceOnMakeCurrent() OVERRIDE;
-
-  bool RecreateSurfaceIfNeeded(GLSurface* surface);
+  virtual void SetUnbindFboOnMakeCurrent() OVERRIDE;
 
  protected:
   virtual ~GLContextEGL();
@@ -46,7 +44,7 @@ class GLContextEGL : public GLContext {
   EGLContext context_;
   EGLDisplay display_;
   EGLConfig config_;
-  bool recreate_surface_on_makecurrent_;
+  bool unbind_fbo_on_makecurrent_;
 
   DISALLOW_COPY_AND_ASSIGN(GLContextEGL);
 };