cc: Don't do CompositeAndReadback when using DelegatingRenderer.

cc/trees/thread_proxy.cc

 // Copyright 2011 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 "cc/trees/thread_proxy.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "cc/base/thread.h"
@@ skipping 895 matching lines @@
   // to produce a frame, as the calling site on main thread is blocked until its
   // request completes, and we signal completion here. If CanDraw() is false, we
   // will indicate success=false to the caller, but we must still signal
   // completion to avoid deadlock.
 
   // We guard PrepareToDraw() with CanDraw() because it always returns a valid
   // frame, so can only be used when such a frame is possible. Since
   // DrawLayers() depends on the result of PrepareToDraw(), it is guarded on
   // CanDraw() as well.
 
-  // If it is a forced draw, make sure we do a draw and swap.
-  gfx::Rect readback_rect;
-  if (readback_request_on_impl_thread_)
-    readback_rect = readback_request_on_impl_thread_->rect;
+  bool drawing_for_readback = !!readback_request_on_impl_thread_;
+  bool can_do_readback = layer_tree_host_impl_->renderer()->CanReadPixels();
 
   LayerTreeHostImpl::FrameData frame;
   bool draw_frame = false;
   bool start_ready_animations = true;
 
-  if (layer_tree_host_impl_->CanDraw()) {
+  if (layer_tree_host_impl_->CanDraw() &&
+      (!drawing_for_readback || can_do_readback)) {
+    // If it is for a readback, make sure we draw the portion being read back.
+    gfx::Rect readback_rect;
+    if (drawing_for_readback)
+      readback_rect = readback_request_on_impl_thread_->rect;
+
     // Do not start animations if we skip drawing the frame to avoid
     // checkerboarding.
     if (layer_tree_host_impl_->PrepareToDraw(&frame, readback_rect) ||
         forced_draw)
       draw_frame = true;
     else
       start_ready_animations = false;
   }
 
   if (draw_frame) {
@@ skipping 17 matching lines @@
   }
 
   // Check for a pending CompositeAndReadback.
   if (readback_request_on_impl_thread_) {
     readback_request_on_impl_thread_->success = false;
     if (draw_frame) {
       layer_tree_host_impl_->Readback(readback_request_on_impl_thread_->pixels,
                                       readback_request_on_impl_thread_->rect);
       readback_request_on_impl_thread_->success =
           !layer_tree_host_impl_->IsContextLost();
     }

[piman, 2013/05/09 22:07:31]

else readback_request_on_impl_thread_->success = false.

Alternatively, and maybe better, we should make sure ReadbackRequest initializes
its fields...

[danakj, 2013/05/09 22:14:51]

This is set on 961 just before the if, so that seems like enough?

[piman, 2013/05/09 22:18:04]

Oh, thanks... missed that.

[danakj, 2013/05/09 22:18:47]

Np :) TBH I didn't see it first round and just thought draw_frame false was
enough :x

     readback_request_on_impl_thread_->completion.Signal();
     readback_request_on_impl_thread_ = NULL;
   } else if (draw_frame) {
     result.did_swap = layer_tree_host_impl_->SwapBuffers(frame);
 
     if (frame.contains_incomplete_tile)
       DidSwapUseIncompleteTileOnImplThread();
   }
 
   // Tell the main thread that the the newly-commited frame was drawn.
@@ skipping 400 matching lines @@
 
 void ThreadProxy::DidReceiveLastInputEventForVSync(
     base::TimeTicks frame_time) {
   if (render_vsync_notification_enabled_) {
     TRACE_EVENT0("cc", "ThreadProxy::DidReceiveLastInputEventForVSync");
     DidVSync(frame_time);
   }
 }
 
 }  // namespace cc