IOSurface CGL context current lifetime cleanup

content/browser/compositor/io_surface_texture_mac.mm

 // 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 "content/browser/compositor/io_surface_texture_mac.h"
 
 #include <OpenGL/CGLIOSurface.h>
 #include <OpenGL/CGLRenderers.h>
 #include <OpenGL/OpenGL.h>
 #include <OpenGL/gl.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
+#include "base/mac/bind_objc_block.h"
 #include "base/mac/mac_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/threading/platform_thread.h"
 #include "content/browser/compositor/io_surface_context_mac.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_mac.h"
 #include "content/common/content_constants_internal.h"
 #include "gpu/config/gpu_driver_bug_workaround_type.h"
 #include "media/base/video_util.h"
@@ skipping 24 matching lines @@
       texture_(0),
       gl_error_(GL_NO_ERROR),
       eviction_queue_iterator_(eviction_queue_.Get().end()),
       eviction_has_been_drawn_since_updated_(false) {
   CHECK(offscreen_context_.get());
 }
 
 IOSurfaceTexture::~IOSurfaceTexture() {
   {
     gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
         offscreen_context_->cgl_context());

[Ken Russell (switch to Gerrit), 2014/10/07 22:09:44]

Per our over-the-shoulder review, this ScopedCGLSetCurrentContext is redundant
since this is done inside ReleaseIOSurfaceAndTexture.

[ccameron, 2014/10/07 22:33:45]

Done.

-    UnrefIOSurfaceWithContextCurrent();
+    ReleaseIOSurfaceAndTexture();
   }
   offscreen_context_ = NULL;
   DCHECK(eviction_queue_iterator_ == eviction_queue_.Get().end());
 }
 
-int IOSurfaceTexture::GetRendererID() {
-  GLint current_renderer_id = -1;
-  if (CGLGetParameter(offscreen_context_->cgl_context(),
-                      kCGLCPCurrentRendererID,
-                      &current_renderer_id) == kCGLNoError)
-    return current_renderer_id & kCGLRendererIDMatchingMask;
-  return -1;
-}
-
 bool IOSurfaceTexture::DrawIOSurface() {
   TRACE_EVENT0("browser", "IOSurfaceTexture::DrawIOSurface");
-  DCHECK(HasIOSurface());
+
+  // If we have release the IOSurface, clear the screen to light grey and
+  // early-out.
+  if (!io_surface_) {
+    glClearColor(0.9, 0.9, 0.9, 1);
+    glClear(GL_COLOR_BUFFER_BIT);
+    return false;
+  }
 
   // The viewport is the size of the CALayer, which should always match the
   // IOSurface pixel size.
   GLint viewport[4];
   glGetIntegerv(GL_VIEWPORT, viewport);
   gfx::Rect viewport_rect(viewport[0], viewport[1], viewport[2], viewport[3]);
   DCHECK_EQ(pixel_size_.ToString(), viewport_rect.size().ToString());
 
   // Set the projection matrix to match 1 unit to 1 pixel.
   glMatrixMode(GL_PROJECTION);
@@ skipping 43 matching lines @@
     // rendering artifacts.
     glClearColor(0.8, 0.8, 0.8, 1.0);
     glClear(GL_COLOR_BUFFER_BIT);
   }
 
   eviction_has_been_drawn_since_updated_ = true;
   return result;
 }
 
 bool IOSurfaceTexture::SetIOSurface(
-    scoped_refptr<IOSurfaceContext> context,
     IOSurfaceID io_surface_id,
     const gfx::Size& pixel_size) {
   TRACE_EVENT0("browser", "IOSurfaceTexture::MapIOSurfaceToTexture");
 
-  gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
-      context->cgl_context());
-
   // Destroy the old IOSurface and texture if it is no longer needed.
   bool needs_new_iosurface =
       !io_surface_ || io_surface_id != IOSurfaceGetID(io_surface_);
   if (needs_new_iosurface)
-    UnrefIOSurfaceWithContextCurrent();
+    ReleaseIOSurfaceAndTexture();
 
   // Note that because IOSurface sizes are rounded, the same IOSurface may have
   // two different sizes associated with it, so update the sizes before the
   // early-out.
   pixel_size_ = pixel_size;
 
   // Early-out if the IOSurface has not changed.
   if (!needs_new_iosurface)
     return true;
 
+  // If we early-out at any point from now on, it's because of an error, and we
+  // should destroy the texture and release the IOSurface.
+  base::ScopedClosureRunner error_runner(base::BindBlock(^{
+      ReleaseIOSurfaceAndTexture();
+  }));
+
   // Open the IOSurface handle.
   io_surface_.reset(IOSurfaceLookup(io_surface_id));
-  if (!io_surface_) {
-    UnrefIOSurfaceWithContextCurrent();
+  if (!io_surface_)
     return false;
-  }
 
   // Actual IOSurface size is rounded up to reduce reallocations during window
   // resize. Get the actual size to properly map the texture.
   gfx::Size rounded_size(IOSurfaceGetWidth(io_surface_),
                          IOSurfaceGetHeight(io_surface_));
 
   // Create the GL texture and set it to be backed by the IOSurface.
-  glGenTextures(1, &texture_);
-  glBindTexture(GL_TEXTURE_RECTANGLE_ARB, texture_);
-  glTexParameterf(GL_TEXTURE_RECTANGLE_ARB, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
-  glTexParameterf(GL_TEXTURE_RECTANGLE_ARB, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
-  GLuint plane = 0;
-  CGLError cgl_error = CGLTexImageIOSurface2D(
-      context->cgl_context(),
-      GL_TEXTURE_RECTANGLE_ARB,
-      GL_RGBA,
-      rounded_size.width(),
-      rounded_size.height(),
-      GL_BGRA,
-      GL_UNSIGNED_INT_8_8_8_8_REV,
-      io_surface_.get(),
-      plane);
-  glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
-  GetAndSaveGLError();
+  CGLError cgl_error = kCGLNoError;
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        offscreen_context_->cgl_context());

[Ken Russell (switch to Gerrit), 2014/10/04 00:33:39]

I'm sure I'm being dense, but am finding it difficult to prove to myself that
offscreen_context_ is in the same share group as the context which is current
when IOSurfaceTexture::DrawIOSurface is called. IOSurfaceLayer initializes its
context from IOSurfaceContext::Get(IOSurfaceContext::kCALayerContext), and
that's the context current when drawing this texture_ to the screen in
IOSurfaceTexture::DrawIOSurface. However, offscreen_context_ is initialized from
IOSurfaceContext::Get(IOSurfaceContext::kOffscreenContext), and as far as I can
tell, IOSurfaceContext deliberately puts those contexts in disjoint share
groups. Am I missing something fundamental?

[ccameron, 2014/10/04 00:49:37]

The types (kCALayerContext v kOffscreenContext) actually identify specific
contexts -- not parameters for creating the context. This is the part of the
guesswork where it seems that I need to keep the 2 types of contexts separate to
avoid crashes (the follow-on CL will show if that is the case or not).

The bit that puts the contexts in the same group is in IOSurfaceContext::Get,
where all contexts retrieved by Get are put in the same sharegroup (the
share_context there just pulls an arbitrary context out of the type->context
map).

When an error is encountered (or the GPU is changed), all of the contexts in the
sharegroup are poisoned and the map is cleared, and from then on a new
sharegroup is created.

+    glGenTextures(1, &texture_);
+    glBindTexture(GL_TEXTURE_RECTANGLE_ARB, texture_);
+    glTexParameterf(
+        GL_TEXTURE_RECTANGLE_ARB, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameterf(
+        GL_TEXTURE_RECTANGLE_ARB, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    cgl_error = CGLTexImageIOSurface2D(
+        offscreen_context_->cgl_context(),
+        GL_TEXTURE_RECTANGLE_ARB,
+        GL_RGBA,
+        rounded_size.width(),
+        rounded_size.height(),
+        GL_BGRA,
+        GL_UNSIGNED_INT_8_8_8_8_REV,
+        io_surface_.get(),
+        0 /* plane */);
+    glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
+    GetAndSaveGLError();
+  }
 
   // Return failure if an error was encountered by CGL or GL.
   if (cgl_error != kCGLNoError) {
     LOG(ERROR) << "CGLTexImageIOSurface2D failed with CGL error: " << cgl_error;
-    UnrefIOSurfaceWithContextCurrent();
     return false;
   }
   if (gl_error_ != GL_NO_ERROR) {
     LOG(ERROR) << "Hit GL error in SetIOSurface: " << gl_error_;
-    UnrefIOSurfaceWithContextCurrent();
     return false;
   }
 
+  ignore_result(error_runner.Release());
   return true;
 }
 
-void IOSurfaceTexture::UnrefIOSurface() {
+void IOSurfaceTexture::ReleaseIOSurfaceAndTexture() {
   gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
       offscreen_context_->cgl_context());
-  UnrefIOSurfaceWithContextCurrent();
-}
 
-void IOSurfaceTexture::UnrefIOSurfaceWithContextCurrent() {
   if (texture_) {
     glDeleteTextures(1, &texture_);
     texture_ = 0;
   }
   pixel_size_ = gfx::Size();
   io_surface_.reset();
 
   EvictionMarkEvicted();
 }
 
@@ skipping 50 matching lines @@
     // If the number of IOSurfaces allocated is less than the threshold,
     // stop walking the list of surfaces.
     if (eviction_queue_.Get().size() <= kMaximumUnevictedSurfaces)
       break;
 
     // Don't evict anything that has not yet been drawn.
     if (!surface->eviction_has_been_drawn_since_updated_)
       continue;
 
     // Evict the surface.
-    surface->UnrefIOSurface();
+    surface->ReleaseIOSurfaceAndTexture();
   }
 }
 
 // static
 base::LazyInstance<IOSurfaceTexture::EvictionQueue>
     IOSurfaceTexture::eviction_queue_;
 
 // static
 bool IOSurfaceTexture::eviction_scheduled_ = false;
 
 }  // namespace content