Mac: Make resize more smooth

gpu/ipc/service/image_transport_surface_overlay_mac.mm

 // Copyright 2015 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/ipc/service/image_transport_surface_overlay_mac.h"
 
 #include <CoreGraphics/CoreGraphics.h>
 #include <IOSurface/IOSurface.h>
 #include <OpenGL/CGLRenderers.h>
 #include <OpenGL/CGLTypes.h>
@@ skipping 25 matching lines @@
 #include "ui/gfx/transform.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_fence.h"
 #include "ui/gl/gl_image_io_surface.h"
 #include "ui/gl/gpu_switching_manager.h"
 #include "ui/gl/scoped_api.h"
 #include "ui/gl/scoped_cgl.h"
 
 namespace {
 
+// Keep previously-used CAContexts around until 5 subsequent swaps have been
+// issued, to ensure that the CAContext is no longer being displayed. This
+// value was determined empirically.
+const size_t kSwapsToKeepDiscardedSizeDependentState = 5;
+
 void CheckGLErrors(const char* msg) {
   GLenum gl_error;
   while ((gl_error = glGetError()) != GL_NO_ERROR) {
     LOG(ERROR) << "OpenGL error hit " << msg << ": " << gl_error;
   }
 }
 
 void IOSurfaceContextNoOp(scoped_refptr<ui::IOSurfaceContext>) {
 }
 
 }  // namespace
 
 namespace gpu {
 
 scoped_refptr<gl::GLSurface> ImageTransportSurfaceCreateNativeSurface(
     GpuChannelManager* manager,
     GpuCommandBufferStub* stub,
     SurfaceHandle handle) {
   return new ImageTransportSurfaceOverlayMac(stub, handle);
 }
 
 ImageTransportSurfaceOverlayMac::ImageTransportSurfaceOverlayMac(
     GpuCommandBufferStub* stub,
     SurfaceHandle handle)
     : stub_(stub->AsWeakPtr()),
       handle_(handle),
       use_remote_layer_api_(ui::RemoteLayerAPISupported()),
-      scale_factor_(1),
       gl_renderer_id_(0) {
   ui::GpuSwitchingManager::GetInstance()->AddObserver(this);
-  ca_layer_tree_coordinator_.reset(
-      new ui::CALayerTreeCoordinator(use_remote_layer_api_));
 }
 
 ImageTransportSurfaceOverlayMac::~ImageTransportSurfaceOverlayMac() {
   ui::GpuSwitchingManager::GetInstance()->RemoveObserver(this);
   if (stub_.get()) {
     stub_->SetLatencyInfoCallback(
         base::Callback<void(const std::vector<ui::LatencyInfo>&)>());
   }
   Destroy();
 }
 
 bool ImageTransportSurfaceOverlayMac::Initialize(gl::GLSurface::Format format) {
   if (!stub_.get() || !stub_->decoder())
     return false;
 
   stub_->SetLatencyInfoCallback(
       base::Bind(&ImageTransportSurfaceOverlayMac::SetLatencyInfo,
                  base::Unretained(this)));
 
-  // Create the CAContext to send this to the GPU process, and the layer for
-  // the context.
-  if (use_remote_layer_api_) {
-    CGSConnectionID connection_id = CGSMainConnectionID();
-    ca_context_.reset([
-        [CAContext contextWithCGSConnection:connection_id options:@{}] retain]);
-    [ca_context_ setLayer:ca_layer_tree_coordinator_->GetCALayerForDisplay()];
-
-    fullscreen_low_power_ca_context_.reset([
-        [CAContext contextWithCGSConnection:connection_id options:@{}] retain]);
-    [fullscreen_low_power_ca_context_ setLayer:
-        ca_layer_tree_coordinator_->GetFullscreenLowPowerLayerForDisplay()];
-  }
+  if (!size_dependent_state_)

[tapted, 2016/06/17 00:23:38]

should this always be true? (e.g. can we DCHECK(!size_dependent_state_)

+    Resize(gfx::Size(1, 1), 1, false);
   return true;
 }
 
 void ImageTransportSurfaceOverlayMac::Destroy() {
-  ca_layer_tree_coordinator_.reset();
+  discarded_size_dependent_state_.push_back(std::move(size_dependent_state_));

[tapted, 2016/06/17 00:23:38]

why not

discarded_size_dependent_state_.clear()
size_dependent_state_.reset()

?

   if (previous_frame_fence_) {
     gl::ScopedSetGLToRealGLApi scoped_set_gl_api;
     // Ensure we are using the context with which the fence was created.
     gl::ScopedCGLSetCurrentContext scoped_set_current(fence_context_obj_);
     CheckGLErrors("Before destroy fence");
     previous_frame_fence_.reset();
     CheckGLErrors("After destroy fence");
   }
 }
 
@@ skipping 100 matching lines @@
     gl::ScopedSetGLToRealGLApi scoped_set_gl_api;
     TRACE_EVENT0("gpu", "ImageTransportSurfaceOverlayMac::glFinish");
     CheckGLErrors("Before finish");
     glFinish();
     CheckGLErrors("After finish");
   }
 
   base::TimeTicks finish_time = base::TimeTicks::Now();
 
   bool fullscreen_low_power_layer_valid = false;
-  ca_layer_tree_coordinator_->CommitPendingTreesToCA(
+  size_dependent_state_->ca_layer_tree_coordinator_->CommitPendingTreesToCA(
       pixel_damage_rect, &fullscreen_low_power_layer_valid);
   // TODO(ccameron): Plumb the fullscreen low power layer through to the
   // appropriate window.
 
   // Update the latency info to reflect the swap time.
   for (auto latency_info : latency_info_) {
     latency_info.AddLatencyNumberWithTimestamp(
         ui::INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT, 0, 0, finish_time, 1);
     latency_info.AddLatencyNumberWithTimestamp(
         ui::INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT, 0, 0,
         finish_time, 1);
   }
 
   // Send acknowledgement to the browser.
   CAContextID ca_context_id = 0;
   CAContextID fullscreen_low_power_ca_context_id = 0;
   gfx::ScopedRefCountedIOSurfaceMachPort io_surface_mach_port;
   if (use_remote_layer_api_) {
-    ca_context_id = [ca_context_ contextId];
+    ca_context_id = [size_dependent_state_->ca_context_ contextId];
     fullscreen_low_power_ca_context_id =
-        [fullscreen_low_power_ca_context_ contextId];
+        [size_dependent_state_->fullscreen_low_power_ca_context_ contextId];
   } else {
-    IOSurfaceRef io_surface =
-        ca_layer_tree_coordinator_->GetIOSurfaceForDisplay();
+    IOSurfaceRef io_surface = size_dependent_state_->ca_layer_tree_coordinator_
+                                  ->GetIOSurfaceForDisplay();
     if (io_surface)
       io_surface_mach_port.reset(IOSurfaceCreateMachPort(io_surface));
   }
   SendAcceleratedSurfaceBuffersSwapped(
       handle_, ca_context_id, fullscreen_low_power_layer_valid,
-      fullscreen_low_power_ca_context_id, io_surface_mach_port, pixel_size_,
-      scale_factor_, std::move(latency_info_));
+      fullscreen_low_power_ca_context_id, io_surface_mach_port,
+      size_dependent_state_->pixel_size_, size_dependent_state_->scale_factor_,
+      std::move(latency_info_));
+
+  // Free state from previous resizes only after several swaps.
+  for (auto& discarded_state : discarded_size_dependent_state_)
+    discarded_state->swaps_since_discarded_ += 1;
+  while (!discarded_size_dependent_state_.empty()) {
+    if (discarded_size_dependent_state_.front()->swaps_since_discarded_ >
+        kSwapsToKeepDiscardedSizeDependentState) {
+      discarded_size_dependent_state_.pop_front();
+    } else {
+      break;

[tapted, 2016/06/17 00:23:38]

optional nit: Maybe just move the `if` into the `while` condition. And (for
readability) perhaps something like

  auto& discards = discarded_size_dependent_state_;
  while (!discards.empty() && discards.front()->swaps_since_discarded_ > 
         kSwapsToKeepDiscardedSizeDependentState)
    discards.pop_front();


(My old-stylee brain wanted to combine these two loops, but it requires knowing
too much about interator-invalidation on std::deque - e.g. the following
probably works, but explaining, e.g., why you need to check against .empty() as
well as .end() makes it feel a little too much like voodoo

  auto& discards = discarded_size_dependent_state_;
  for (auto i = discards.begin(); !discards.empty() && i != discards.end(); ) {
    if (i->swaps_since_discarded_++ == kSwapsToKeepDiscardedSizeDependentState)
      discarded_size_dependent_state_.erase(i++);
    else
      ++i;
  }

+    }
+  }
 
   // Reset all state for the next frame.
   latency_info_.clear();
   return gfx::SwapResult::SWAP_ACK;
 }
 
 gfx::SwapResult ImageTransportSurfaceOverlayMac::SwapBuffers() {
-  return SwapBuffersInternal(
-      gfx::Rect(0, 0, pixel_size_.width(), pixel_size_.height()));
+  return SwapBuffersInternal(gfx::Rect(size_dependent_state_->pixel_size_));
 }
 
 gfx::SwapResult ImageTransportSurfaceOverlayMac::PostSubBuffer(int x,
                                                                int y,
                                                                int width,
                                                                int height) {
   return SwapBuffersInternal(gfx::Rect(x, y, width, height));
 }
 
 bool ImageTransportSurfaceOverlayMac::SupportsPostSubBuffer() {
@@ skipping 23 matching lines @@
     const gfx::Rect& pixel_frame_rect,
     const gfx::RectF& crop_rect) {
   if (transform != gfx::OVERLAY_TRANSFORM_NONE) {
     DLOG(ERROR) << "Invalid overlay plane transform.";
     return false;
   }
   if (z_order) {
     DLOG(ERROR) << "Invalid non-zero Z order.";
     return false;
   }
-  return ca_layer_tree_coordinator_->SetPendingGLRendererBackbuffer(
-      static_cast<gl::GLImageIOSurface*>(image)->io_surface());
+  return size_dependent_state_->ca_layer_tree_coordinator_
+      ->SetPendingGLRendererBackbuffer(
+          static_cast<gl::GLImageIOSurface*>(image)->io_surface());
 }
 
 bool ImageTransportSurfaceOverlayMac::ScheduleCALayer(
     gl::GLImage* contents_image,
     const gfx::RectF& contents_rect,
     float opacity,
     unsigned background_color,
     unsigned edge_aa_mask,
     const gfx::RectF& rect,
     bool is_clipped,
     const gfx::RectF& clip_rect,
     const gfx::Transform& transform,
     int sorting_context_id,
     unsigned filter) {
   base::ScopedCFTypeRef<IOSurfaceRef> io_surface;
   base::ScopedCFTypeRef<CVPixelBufferRef> cv_pixel_buffer;
   if (contents_image) {
     gl::GLImageIOSurface* io_surface_image =
         static_cast<gl::GLImageIOSurface*>(contents_image);
     io_surface = io_surface_image->io_surface();
     cv_pixel_buffer = io_surface_image->cv_pixel_buffer();
   }
-  return ca_layer_tree_coordinator_->GetPendingCARendererLayerTree()
+  return size_dependent_state_->ca_layer_tree_coordinator_
+      ->GetPendingCARendererLayerTree()
       ->ScheduleCALayer(is_clipped, gfx::ToEnclosingRect(clip_rect),
                         sorting_context_id, transform, io_surface,
                         cv_pixel_buffer, contents_rect,
                         gfx::ToEnclosingRect(rect), background_color,
                         edge_aa_mask, opacity, filter);
 }
 
 void ImageTransportSurfaceOverlayMac::ScheduleCALayerInUseQuery(
     std::vector<CALayerInUseQuery> queries) {
   for (auto& query : queries) {
     IOSurfaceInUseQuery io_surface_query;
     io_surface_query.texture = query.texture;
     if (query.image) {
       gl::GLImageIOSurface* io_surface_image =
           static_cast<gl::GLImageIOSurface*>(query.image.get());
       io_surface_query.io_surface = io_surface_image->io_surface();
     }
     io_surface_in_use_queries_.push_back(std::move(io_surface_query));
   }
 }
 
 bool ImageTransportSurfaceOverlayMac::IsSurfaceless() const {
   return true;
 }
 
 bool ImageTransportSurfaceOverlayMac::Resize(const gfx::Size& pixel_size,
                                              float scale_factor,
                                              bool has_alpha) {
-  pixel_size_ = pixel_size;
-  scale_factor_ = scale_factor;
-  ca_layer_tree_coordinator_->Resize(pixel_size, scale_factor);
+  if (size_dependent_state_ &&
+      size_dependent_state_->pixel_size_ == pixel_size &&
+      size_dependent_state_->scale_factor_ == scale_factor) {
+    return true;
+  }
+  if (size_dependent_state_)
+    discarded_size_dependent_state_.push_back(std::move(size_dependent_state_));
+
+  size_dependent_state_.reset(new SizeDependentState);
+  size_dependent_state_->pixel_size_ = pixel_size;
+  size_dependent_state_->scale_factor_ = scale_factor;
+  size_dependent_state_->ca_layer_tree_coordinator_.reset(
+      new ui::CALayerTreeCoordinator(use_remote_layer_api_));
+  size_dependent_state_->ca_layer_tree_coordinator_->Resize(pixel_size,
+                                                            scale_factor);
+
+  // Create the CAContext to send this to the GPU process, and the layer for
+  // the context.
+  if (use_remote_layer_api_) {
+    CGSConnectionID connection_id = CGSMainConnectionID();
+    size_dependent_state_->ca_context_.reset([
+        [CAContext contextWithCGSConnection:connection_id options:@{}] retain]);

[tapted, 2016/06/17 00:23:38]

nit: use base::scoped_policy::RETAIN ?

+    [size_dependent_state_->ca_context_
+        setLayer:size_dependent_state_->ca_layer_tree_coordinator_
+                     ->GetCALayerForDisplay()];
+
+    size_dependent_state_->fullscreen_low_power_ca_context_.reset([
+        [CAContext contextWithCGSConnection:connection_id options:@{}] retain]);

[tapted, 2016/06/17 00:23:38]

nit: base::scoped_policy::RETAIN

+    [size_dependent_state_->fullscreen_low_power_ca_context_
+        setLayer:size_dependent_state_->ca_layer_tree_coordinator_
+                     ->GetFullscreenLowPowerLayerForDisplay()];
+  }

[tapted, 2016/06/17 00:23:38]

I think it would be good to move all this into the SizeDependentState
constructor -- makes it clear what properties it's using from
ImageTransportSurfaceOverlayMac to create the state (I think just
use_remote_layer_api_?), and you get to remove some occurrences of
`size_dependent_state_->`

   return true;
 }
 
 void ImageTransportSurfaceOverlayMac::OnGpuSwitched() {
   // Create a new context, and use the GL renderer ID that the new context gets.
   scoped_refptr<ui::IOSurfaceContext> context_on_new_gpu =
       ui::IOSurfaceContext::Get(ui::IOSurfaceContext::kCALayerContext);
   if (!context_on_new_gpu)
     return;
   GLint context_renderer_id = -1;
@@ skipping 14 matching lines @@
 
 ImageTransportSurfaceOverlayMac::IOSurfaceInUseQuery::IOSurfaceInUseQuery() =
     default;
 ImageTransportSurfaceOverlayMac::IOSurfaceInUseQuery::IOSurfaceInUseQuery(
     const IOSurfaceInUseQuery&) = default;
 ImageTransportSurfaceOverlayMac::IOSurfaceInUseQuery::IOSurfaceInUseQuery(
     IOSurfaceInUseQuery&&) = default;
 ImageTransportSurfaceOverlayMac::IOSurfaceInUseQuery::~IOSurfaceInUseQuery() =
     default;
 
+ImageTransportSurfaceOverlayMac::SizeDependentState::SizeDependentState() =

[tapted, 2016/06/17 00:23:38]

nit: (if you want to keep the declaration in the header) move before
IOSurfaceInUseQuery, to match header order

+    default;
+ImageTransportSurfaceOverlayMac::SizeDependentState::~SizeDependentState() =
+    default;
+
 }  // namespace gpu