Aura: Resize locks with --ui-enable-threaded-compositing

content/common/gpu/texture_image_transport_surface.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 "content/common/gpu/texture_image_transport_surface.h"
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 26 matching lines @@
       : fbo_id_(0),
         front_(0),
         stub_destroyed_(false),
         backbuffer_suggested_allocation_(true),
         frontbuffer_suggested_allocation_(true),
         frontbuffer_is_protected_(true),
         protection_state_id_(0),
         handle_(handle),
         parent_stub_(NULL),
         is_swap_buffers_pending_(false),
-        did_unschedule_(false) {
+        did_unschedule_(false),
+        did_flip_(false) {
   helper_.reset(new ImageTransportHelper(this,
                                          manager,
                                          stub,
                                          gfx::kNullPluginWindow));
 }
 
 TextureImageTransportSurface::~TextureImageTransportSurface() {
   DCHECK(stub_destroyed_);
   Destroy();
 }
@@ skipping 256 matching lines @@
     content::ScopedTextureBinder texture_binder(back_texture_service_id);
 
     for (size_t i = 0; i < regions_to_copy.size(); ++i) {
       const gfx::Rect& region_to_copy = regions_to_copy[i];
       if (!region_to_copy.IsEmpty()) {
         glCopyTexSubImage2D(GL_TEXTURE_2D, 0, region_to_copy.x(),
             region_to_copy.y(), region_to_copy.x(), region_to_copy.y(),
             region_to_copy.width(), region_to_copy.height());
       }
     }
-  } else {
+  } else if (!surfaces_same_size && did_flip_) {
     DCHECK(new_damage_rect == gfx::Rect(expected_size));
   }
 
   glFlush();
   front_ = back();
   previous_damage_rect_ = new_damage_rect;
 
   DCHECK(textures_[front()].client_id);
 
   GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params params;
@@ skipping 47 matching lines @@
   if (is_protected && textures_[front()].info->service_id() &&
       textures_[front()].sent_to_client) {
     GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
     params.surface_handle = textures_[front()].client_id;
     params.protection_state_id = protection_state_id_;
     params.skip_ack = true;
     helper_->SendAcceleratedSurfaceBuffersSwapped(params);
   }
 }
 
-void TextureImageTransportSurface::OnBufferPresented(uint32 sync_point) {
+void TextureImageTransportSurface::OnBufferPresented(bool presented,
+                                                     uint32 sync_point) {
   if (sync_point == 0) {
-    BufferPresentedImpl();
+    BufferPresentedImpl(presented);
   } else {
     helper_->manager()->sync_point_manager()->AddSyncPointCallback(
         sync_point,
         base::Bind(&TextureImageTransportSurface::BufferPresentedImpl,
-                   this->AsWeakPtr()));
+                   this->AsWeakPtr(),
+                   presented));
   }
 }
 
-void TextureImageTransportSurface::BufferPresentedImpl() {
+void TextureImageTransportSurface::BufferPresentedImpl(bool presented) {
   DCHECK(is_swap_buffers_pending_);
   is_swap_buffers_pending_ = false;
 
+  if (presented) {
+    // If we had not flipped, the two frame damage tracking is inconsistent.
+    // So conservatively take the whole frame.
+    if (!did_flip_)
+      previous_damage_rect_ = gfx::Rect(textures_[front()].size);
+  } else {
+    front_ = back();
+    previous_damage_rect_ = gfx::Rect(0, 0, 0, 0);
+  }

[piman, 2012/10/17 21:26:32]

Just a suggestion: would it help to move the 2 lines from PostSubBuffer:

front_ = back();
previous_damage_rect_ = new_damage_rect;

(and corresponding from SwapBuffers) onto here?

It kinda feels we're trying to "undo" whatever was done there in some cases.

[jonathan.backer, 2012/10/18 20:20:35]

I was motivated by a comment you made on an earlier version of this CL where I
did the flip here: "Mmh, is it ok to move this [the flip] here wrt e.g. dropping
front surface? If AdjustFrontBufferAllocation gets called between the swap and
the swap ack, then it'll change the allocation of the old-front-now-back buffer
instead of the old-back-now-front buffer."

I'm not sure how big a concern this is. I don't think that the front buffer
allocation can change like this (mid swap), but I really don't know.

I don't think that deferring the flip until we know that it goes through would
help substantially with the previous_damage_rect_ logic here. When we fast ACK,
we don't need to copy on the next post sub buffer. When we go from fast ACKing
to flipping, our two frame damage tracking is screwed up. We could do better by
accumulating damage with a union, but this is such an uncommon case that I'd
rather go suboptimal and obviously correct.

+
+  did_flip_ = presented;
+
   // We're relying on the fact that the parent context is
   // finished with it's context when it inserts the sync point that
   // triggers this callback.
   if (helper_->MakeCurrent()) {
     if (textures_[front()].size != textures_[back()].size ||
         !textures_[back()].info->service_id() ||
         !textures_[back()].sent_to_client) {
       // We may get an ACK from a stale swap just to reschedule.  In that case,
       // we may not have a backbuffer suggestion and should not recreate one.
       if (backbuffer_suggested_allocation_)
@@ skipping 122 matching lines @@
     Texture& texture = textures_[i];
     texture.info = NULL;
     if (!texture.sent_to_client)
       continue;
     GpuHostMsg_AcceleratedSurfaceRelease_Params params;
     params.identifier = texture.client_id;
     helper_->SendAcceleratedSurfaceRelease(params);
   }
   parent_stub_ = NULL;
 }