cc: Use BeginFrameArgs

cc/trees/layer_tree_host_impl.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/layer_tree_host_impl.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/json/json_writer.h"
@@ skipping 89 matching lines @@
     // construct a DebugScopedSetImplThread object. There is no need to do
     // this in multi-threaded mode since the real thread id's will be
     // correct. In fact, setting fake thread id's interferes with the real
     // thread id's and causes breakage.
     scoped_ptr<DebugScopedSetImplThread> set_impl_thread;
     if (!layer_tree_host_impl_->proxy()->HasImplThread()) {
       set_impl_thread.reset(
           new DebugScopedSetImplThread(layer_tree_host_impl_->proxy()));
     }
 
-    // TODO(enne): This should probably happen post-animate.
     if (layer_tree_host_impl_->pending_tree()) {
       layer_tree_host_impl_->ActivatePendingTreeIfNeeded();
 
       if (layer_tree_host_impl_->pending_tree()) {
         layer_tree_host_impl_->pending_tree()->UpdateDrawProperties();
         layer_tree_host_impl_->ManageTiles();
       }
     }
 
     layer_tree_host_impl_->Animate(
@@ skipping 42 matching lines @@
                             rendering_stats_instrumentation));
 }
 
 LayerTreeHostImpl::LayerTreeHostImpl(
     const LayerTreeSettings& settings,
     LayerTreeHostImplClient* client,
     Proxy* proxy,
     RenderingStatsInstrumentation* rendering_stats_instrumentation)
     : client_(client),
       proxy_(proxy),
+      output_surface_lost_(true),
       input_handler_client_(NULL),
       did_lock_scrolling_layer_(false),
       should_bubble_scrolls_(false),
       wheel_scrolling_(false),
       manage_tiles_needed_(false),
       root_layer_scroll_offset_delegate_(NULL),
       settings_(settings),
       visible_(true),
       cached_managed_memory_policy_(
           PrioritizedResourceManager::DefaultMemoryAllocationLimit(),
@@ skipping 62 matching lines @@
 void LayerTreeHostImpl::CommitComplete() {
   TRACE_EVENT0("cc", "LayerTreeHostImpl::CommitComplete");
 
   // Impl-side painting needs an update immediately post-commit to have the
   // opportunity to create tilings.  Other paths can call UpdateDrawProperties
   // more lazily when needed prior to drawing.
   if (settings_.impl_side_painting) {
     pending_tree_->set_needs_update_draw_properties();
     pending_tree_->UpdateDrawProperties();
     // Start working on newly created tiles immediately if needed.
-    ManageTiles();
+    if (!ManageTiles())

[enne (OOO), 2013/08/06 23:35:16]

It seems like TileManager should be able to activate here via
TileManagerClient::NotifyReadyToActivate without additional logic.

[brianderson, 2013/08/07 01:55:51]

In this patch, I avoid back to back activations by having the Scheduler
explicitly initiate activation. This code eventually notifies the Scheduler
which keeps track of whether or not the current active tree has been drawn.  If
the current active tree hasn't been drawn, it will not activate the pending tree
until after the draw.

+      client_->NotifyReadyToActivate();
   } else {
     active_tree_->set_needs_update_draw_properties();
   }
 
   client_->SendManagedMemoryStats();
 }
 
 bool LayerTreeHostImpl::CanDraw() const {
   // Note: If you are changing this function or any other function that might
   // affect the result of CanDraw, make sure to call
@@ skipping 43 matching lines @@
 void LayerTreeHostImpl::Animate(base::TimeTicks monotonic_time,
                                 base::Time wall_clock_time) {
   if (input_handler_client_)
     input_handler_client_->Animate(monotonic_time);
   AnimatePageScale(monotonic_time);
   AnimateLayers(monotonic_time, wall_clock_time);
   AnimateScrollbars(monotonic_time);
   AnimateTopControls(monotonic_time);
 }
 
-void LayerTreeHostImpl::ManageTiles() {
+bool LayerTreeHostImpl::ManageTiles() {
   if (!tile_manager_)
-    return;
+    return false;
   if (!manage_tiles_needed_)
-    return;
+    return false;

[enne (OOO), 2013/08/06 23:35:16]

This false seems a little sketchy.  If you call ManageTiles and then immediately
call ManageTiles again, it will return false, but that doesn't mean the pending
tree can be activated.

I wonder if this would be conceptually cleaner if ManageTiles didn't return
anything.  The one caller that checks the return value is also guarded by an
impl-side painting setting which makes the tile_manager_ check redundant.  The
one caller is also right after an UpdateDrawProperties call which will always
indirectly call WillModifyTilePriorities.

[brianderson, 2013/08/07 01:55:51]

Ok. It seems like I need to fix some of the corner cases here.

If I do not return a value from this function to notify the Scheduler that we
are ready to activate immediately, then there are cases where
manage_tiles_needed_ is still false even after the call to UpdateDrawProperties.
 I think this is because of layers that aren't tiled (such as texture layers)
and when we are rasterizing on demand.

Is there a better way to check for those cases where we are not expecting a
NotifyReadyToActivate from the TileManager and can activate immediately.

[enne (OOO), 2013/08/07 15:44:19]

Maybe the problem here is just the comment.  ManageTiles returning false does
not mean that you can activate.  That's true during commit, but is not true in
general.  Returning true from ManageTiles here just means that the TileManager
is responsible for notifying about activation.

One option would be to change the comment on ManageTiles.  Alternatively, the
caller of ManageTiles during commit should just check manage_tiles_needed_
directly prior to calling ManageTiles to catch this case, rather than returning
a boolean value any time ManageTiles is called.

[brianderson, 2013/08/07 18:41:57]

Thanks for the background. I would like to avoid calling ManageTiles if we don't
need to because of the overhead
(https://code.google.com/p/chromium/issues/detail?id=269272).

Would another option be to revert changes to LayerTreeHostImpl::ManageTiles and
have pending_tree_->UpdateDrawProperties() somehow trigger the ReadyToActivate
if it doesn't set manage_tiles_needed_ to true?

[enne (OOO), 2013/08/07 19:48:48]

I wasn't suggesting to move the early out, just to add an additional check in
the commit function.

I think what you suggest could work for now, but I have been thinking about
making UpdateDrawProperties not always update tile priorities which would cause
that check to misfire.

I think it might be clearer to do it in the commit function, since not needing
to manage tiles is sort of a property of the tree, and can only be true once,
and so will always be true in commit when we have to add pending priorities for
all tiles on the pending tree.

[brianderson, 2013/08/07 20:12:29]

Ah, I understand your original comment now and it makes a lot of sense. Checking
in the commit works very well.

   manage_tiles_needed_ = false;
   tile_manager_->ManageTiles();
 
   size_t memory_required_bytes;
   size_t memory_nice_to_have_bytes;
   size_t memory_used_bytes;
   tile_manager_->GetMemoryStats(&memory_required_bytes,
                                 &memory_nice_to_have_bytes,
                                 &memory_used_bytes);
   SendManagedMemoryStats(memory_required_bytes,
                          memory_nice_to_have_bytes,
                          memory_used_bytes);
+  return true;
 }
 
 void LayerTreeHostImpl::StartPageScaleAnimation(gfx::Vector2d target_offset,
                                                 bool anchor_point,
                                                 float page_scale,
                                                 base::TimeTicks start_time,
                                                 base::TimeDelta duration) {
   if (!RootScrollLayer())
     return;
 
@@ skipping 709 matching lines @@
 
 void LayerTreeHostImpl::DidInitializeVisibleTile() {
   // TODO(reveman): Determine tiles that changed and only damage
   // what's necessary.
   SetFullRootLayerDamage();
   if (client_)
     client_->DidInitializeVisibleTileOnImplThread();
 }
 
 void LayerTreeHostImpl::NotifyReadyToActivate() {
-  if (pending_tree_) {
-    need_to_update_visible_tiles_before_draw_ = true;
-    ActivatePendingTree();
-  }
+  client_->NotifyReadyToActivate();
 }
 
 bool LayerTreeHostImpl::ShouldClearRootRenderPass() const {
   return settings_.should_clear_root_render_pass;
 }
 
 void LayerTreeHostImpl::SetMemoryPolicy(const ManagedMemoryPolicy& policy) {
   SetManagedMemoryPolicy(policy, zero_budget_);
 }
 
@@ skipping 224 matching lines @@
 bool LayerTreeHostImpl::IsContextLost() {
   DCHECK(proxy_->IsImplThread());
   return renderer_ && renderer_->IsContextLost();
 }
 
 const RendererCapabilities& LayerTreeHostImpl::GetRendererCapabilities() const {
   return renderer_->Capabilities();
 }
 
 bool LayerTreeHostImpl::SwapBuffers(const LayerTreeHostImpl::FrameData& frame) {
-  if (frame.has_no_damage)
+  if (frame.has_no_damage || output_surface_lost_)
     return false;
   renderer_->SwapBuffers();
   active_tree_->ClearLatencyInfo();
   return true;
 }
 
 void LayerTreeHostImpl::SetNeedsBeginFrame(bool enable) {
   if (output_surface_)
     output_surface_->SetNeedsBeginFrame(enable);
 }
@@ skipping 22 matching lines @@
 }
 
 const LayerTreeSettings& LayerTreeHostImpl::Settings() const {
   return settings();
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
   // TODO(jamesr): The renderer_ check is needed to make some of the
   // LayerTreeHostContextTest tests pass, but shouldn't be necessary (or
   // important) in production. We should adjust the test to not need this.
+  output_surface_lost_ = true;
   if (renderer_)
     client_->DidLoseOutputSurfaceOnImplThread();
 }
 
 void LayerTreeHostImpl::Readback(void* pixels,
                                  gfx::Rect rect_in_device_viewport) {
   DCHECK(renderer_);
   renderer_->GetFramebufferPixels(pixels, rect_in_device_viewport);
 }
 
@@ skipping 74 matching lines @@
   TRACE_EVENT_ASYNC_STEP1(
     "cc",
     "PendingTree", pending_tree_.get(), "activate",
     "state", TracedValue::FromValue(ActivationStateAsValue().release()));
 }
 
 void LayerTreeHostImpl::ActivatePendingTree() {
   CHECK(pending_tree_);
   TRACE_EVENT_ASYNC_END0("cc", "PendingTree", pending_tree_.get());
 
+  need_to_update_visible_tiles_before_draw_ = true;
+
   active_tree_->SetRootLayerScrollOffsetDelegate(NULL);
   active_tree_->PushPersistedState(pending_tree_.get());
   if (pending_tree_->needs_full_tree_sync()) {
     active_tree_->SetRootLayer(
         TreeSynchronizer::SynchronizeTrees(pending_tree_->root_layer(),
                                            active_tree_->DetachLayerTree(),
                                            active_tree_.get()));
   }
   TreeSynchronizer::PushProperties(pending_tree_->root_layer(),
                                    active_tree_->root_layer());
   DCHECK(!recycle_tree_);
 
   // Process any requests in the UI resource queue.  The request queue is given
   // in LayerTreeHost::FinishCommitOnImplThread.  This must take place before
   // the swap.
   pending_tree_->ProcessUIResourceRequestQueue();
 
   pending_tree_->PushPropertiesTo(active_tree_.get());
 
   // Now that we've synced everything from the pending tree to the active
   // tree, rename the pending tree the recycle tree so we can reuse it on the
   // next sync.
   pending_tree_.swap(recycle_tree_);
 
   active_tree_->SetRootLayerScrollOffsetDelegate(
       root_layer_scroll_offset_delegate_);
   active_tree_->DidBecomeActive();
 
+  client_->DidActivatePendingTree();
+  if (!tree_activation_callback_.is_null())
+    tree_activation_callback_.Run();
+
   // Reduce wasted memory now that unlinked resources are guaranteed not
   // to be used.
   client_->ReduceWastedContentsTextureMemoryOnImplThread();
 
   client_->OnCanDrawStateChanged(CanDraw());
   client_->OnHasPendingTreeStateChanged(pending_tree_);
   client_->SetNeedsRedrawOnImplThread();
   client_->RenewTreePriority();
 
   if (debug_state_.continuous_painting) {
     const RenderingStats& stats =
         rendering_stats_instrumentation_->GetRenderingStats();
     paint_time_counter_->SavePaintTime(
         stats.total_paint_time + stats.total_record_time +
             stats.total_rasterize_time_for_now_bins_on_pending_tree);
   }
-
-  client_->DidActivatePendingTree();
-  if (!tree_activation_callback_.is_null())
-    tree_activation_callback_.Run();
 }
 
 void LayerTreeHostImpl::SetVisible(bool visible) {
   DCHECK(proxy_->IsImplThread());
 
   if (visible_ == visible)
     return;
   visible_ = visible;
   DidVisibilityChange(this, visible_);
   EnforceManagedMemoryPolicy(ActualManagedMemoryPolicy());
@@ skipping 87 matching lines @@
   // before we destroy the old resource provider.
   ReleaseTreeResources();
   if (resource_provider_)
     resource_provider_->DidLoseOutputSurface();
 
   // Note: order is important here.
   renderer_.reset();
   tile_manager_.reset();
   resource_provider_.reset();
   output_surface_.reset();
+  output_surface_lost_ = true;
 
   if (!output_surface->BindToClient(this))
     return false;
 
   scoped_ptr<ResourceProvider> resource_provider = ResourceProvider::Create(
       output_surface.get(), settings_.highp_threshold_min);
   if (!resource_provider)
     return false;
 
   if (output_surface->capabilities().deferred_gl_initialization)
@@ skipping 25 matching lines @@
   }
 
   int max_frames_pending =
       output_surface->capabilities().max_frames_pending;
   if (max_frames_pending <= 0)
     max_frames_pending = OutputSurface::DEFAULT_MAX_FRAMES_PENDING;
   output_surface->SetMaxFramesPending(max_frames_pending);
 
   resource_provider_ = resource_provider.Pass();
   output_surface_ = output_surface.Pass();
+  output_surface_lost_ = false;
 
   client_->OnCanDrawStateChanged(CanDraw());
 
   // See note in LayerTreeImpl::UpdateDrawProperties.  Renderer needs
   // to be initialized to get max texture size.
   active_tree_->set_needs_update_draw_properties();
   if (pending_tree_)
     pending_tree_->set_needs_update_draw_properties();
 
   return true;
@@ skipping 530 matching lines @@
   for (size_t i = 0; i < layer_impl->children().size(); ++i)
     CollectScrollDeltas(scroll_info, layer_impl->children()[i]);
 }
 
 scoped_ptr<ScrollAndScaleSet> LayerTreeHostImpl::ProcessScrollDeltas() {
   scoped_ptr<ScrollAndScaleSet> scroll_info(new ScrollAndScaleSet());
 
   CollectScrollDeltas(scroll_info.get(), active_tree_->root_layer());
   scroll_info->page_scale_delta = active_tree_->page_scale_delta();
   active_tree_->set_sent_page_scale_delta(scroll_info->page_scale_delta);
+  if (pending_tree_)
+    pending_tree_->set_sent_page_scale_delta(scroll_info->page_scale_delta);

[enne (OOO), 2013/08/06 23:35:16]

Sent page scale delta should always be 1 on the pending tree as it has not sent
anything.  I think this will especially not work with your changes to pass the
sent page scale delta from pending to active tree during activaiton.

[brianderson, 2013/08/07 01:55:51]

What prompted the page scale delta changes was that my changes (either back to
back activates or delaying the first draw after a new output surface until after
activation) were causing LayerTreeHostTestStartPageScaleAnimation to fail. I
think the best course of action will be to undo my changes regarding the page
scale and debug with Enne what's going wrong and the best way to fix it.

 
   return scroll_info.Pass();
 }
 
 void LayerTreeHostImpl::SetFullRootLayerDamage() {
   SetViewportDamage(gfx::Rect(device_viewport_size_));
 }
 
 void LayerTreeHostImpl::AnimatePageScale(base::TimeTicks time) {
   if (!page_scale_animation_ || !RootScrollLayer())
     return;
 
   double monotonic_time = (time - base::TimeTicks()).InSecondsF();
   gfx::Vector2dF scroll_total = RootScrollLayer()->scroll_offset() +
                                 RootScrollLayer()->ScrollDelta();
 
   active_tree_->SetPageScaleDelta(
       page_scale_animation_->PageScaleFactorAtTime(monotonic_time) /
       active_tree_->page_scale_factor());
+  if (pending_tree_) {
+    pending_tree_->SetPageScaleDelta(
+      page_scale_animation_->PageScaleFactorAtTime(monotonic_time) /
+      pending_tree_->page_scale_factor());
+  }
+
   gfx::Vector2dF next_scroll =
       page_scale_animation_->ScrollOffsetAtTime(monotonic_time);
 
   RootScrollLayer()->ScrollBy(next_scroll - scroll_total);
   client_->SetNeedsRedrawOnImplThread();
 
   if (page_scale_animation_->IsAnimationCompleteAtTime(monotonic_time)) {
     page_scale_animation_.reset();
     client_->SetNeedsCommitOnImplThread();
     client_->RenewTreePriority();
@@ skipping 266 matching lines @@
 
 ResourceProvider::ResourceId LayerTreeHostImpl::ResourceIdForUIResource(
     UIResourceId uid) const {
   UIResourceMap::const_iterator iter = ui_resource_map_.find(uid);
   if (iter != ui_resource_map_.end())
     return iter->second;
   return 0;
 }
 
 }  // namespace cc