cc: split UpdateGpuRasterizationStatus() into two parts.

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 <limits>
 #include <map>
 #include <set>
@@ skipping 177 matching lines @@
     gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
     TaskGraphRunner* task_graph_runner,
     int id)
     : client_(client),
       proxy_(proxy),
       content_is_suitable_for_gpu_rasterization_(true),
       has_gpu_rasterization_trigger_(false),
       use_gpu_rasterization_(false),
       use_msaa_(false),
       gpu_rasterization_status_(GpuRasterizationStatus::OFF_DEVICE),
+      tree_resources_dirty_(false),
       input_handler_client_(NULL),
       did_lock_scrolling_layer_(false),
       should_bubble_scrolls_(false),
       wheel_scrolling_(false),
       scroll_affects_scroll_handler_(false),
       scroll_layer_id_when_mouse_over_scrollbar_(0),
       tile_priorities_dirty_(false),
       root_layer_scroll_offset_delegate_(NULL),
       settings_(settings),
       visible_(true),
@@ skipping 93 matching lines @@
   if (!settings_.impl_side_painting && output_surface_)
     output_surface_->ForceReclaimResources();
 
   if (settings_.impl_side_painting && !proxy_->CommitToActiveTree())
     CreatePendingTree();
 }
 
 void LayerTreeHostImpl::CommitComplete() {
   TRACE_EVENT0("cc", "LayerTreeHostImpl::CommitComplete");
 
-  UpdateGpuRasterizationStatus();
+  UpdateTreeResourcesIfNeeded();

[danakj, 2015/05/13 19:50:58]

I would UpdateGpuRasterizationStatus() here then UpdateTreeResourcesIfNeeded
right after, rather than doing it in the setters.

[Stephen White, 2015/05/13 20:15:20]

Having to remember to call UpdateGpuRasterizationStatus() seems 
error-prone. It seems safer to keep it automatically up-to-date, so 
that (e.g.) Victor can rely on gpu_rasterization_enabled_ being valid.

   sync_tree()->set_needs_update_draw_properties();
 
   if (settings_.impl_side_painting) {
     // 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.  Because invalidations may
     // be coming from the main thread, it's safe to do an update for lcd text
     // at this point and see if lcd text needs to be disabled on any layers.
     bool update_lcd_text = true;
     sync_tree()->UpdateDrawProperties(update_lcd_text);
@@ skipping 1292 matching lines @@
   }
 
   if (use_gpu == use_gpu_rasterization_ && use_msaa == use_msaa_)
     return;
 
   // Note that this must happen first, in case the rest of the calls want to
   // query the new state of |use_gpu_rasterization_|.
   use_gpu_rasterization_ = use_gpu;
   use_msaa_ = use_msaa;
 
+  tree_resources_dirty_ = true;
+}
+
+void LayerTreeHostImpl::UpdateTreeResourcesIfNeeded() {
+  if (!tree_resources_dirty_) {

[danakj, 2015/05/13 19:50:58]

We need to do the following stuff unconditionally when we change renderer. And
conditionally when we CommitComplete (if gpu raster changed).

[danakj, 2015/05/13 19:53:45]

Oh no I'm mistaken, the unconditional of the following happens in
InitializeRenderer, this just does it a second time (which we could actually
avoid some of this if we made the decision earlier?)

[Stephen White, 2015/05/13 20:15:20]

Yeah, it was conditional before in both cases. I've removed it from
CreateAndSetRenderer(), since it seems superfluous (see below).

+    return;
+  }
+
   // Clean up and replace existing tile manager with another one that uses
   // appropriate rasterizer.
   ReleaseTreeResources();
   if (tile_manager_) {
     DestroyTileManager();
     CreateAndSetTileManager();
   }
   RecreateTreeResources();
 
   // We have released tilings for both active and pending tree.
   // We would not have any content to draw until the pending tree is activated.
   // Prevent the active tree from drawing until activation.
   SetRequiresHighResToDraw();
+
+  tree_resources_dirty_ = false;
 }
 
 const RendererCapabilitiesImpl&
 LayerTreeHostImpl::GetRendererCapabilities() const {
   CHECK(renderer_);
   return renderer_->Capabilities();
 }
 
 bool LayerTreeHostImpl::SwapBuffers(const LayerTreeHostImpl::FrameData& frame) {
   ResetRequiresHighResToDraw();
@@ skipping 337 matching lines @@
         resource_provider_.get(), texture_mailbox_deleter_.get(),
         settings_.renderer_settings.highp_threshold_min);
   } else if (output_surface_->software_device()) {
     renderer_ = SoftwareRenderer::Create(this, &settings_.renderer_settings,
                                          output_surface_.get(),
                                          resource_provider_.get());
   }
   DCHECK(renderer_);
 
   // Since the new renderer may be capable of MSAA, update status here.
   UpdateGpuRasterizationStatus();

[danakj, 2015/05/13 19:50:58]

Move this out to InitializeRenderer?

[Stephen White, 2015/05/13 20:15:20]

Done.

+  UpdateTreeResourcesIfNeeded();

[Stephen White, 2015/05/13 20:15:20]

I see now that InitializeRenderer() is doing ReleaseTreeResoures() /
RecreateTreeResources() already, so this call seems superfluous. I've removed it
and all tests pass; let me know if I've missed something.

+
   renderer_->SetVisible(visible_);
   SetFullRootLayerDamage();
 
   // See note in LayerTreeImpl::UpdateDrawProperties.  Renderer needs to be
   // initialized to get max texture size.  Also, after releasing resources,
   // trees need another update to generate new ones.
   active_tree_->set_needs_update_draw_properties();
   if (pending_tree_)
     pending_tree_->set_needs_update_draw_properties();
   client_->UpdateRendererCapabilitiesOnImplThread();
@@ skipping 1394 matching lines @@
   new_target.SetToMin(layer_impl->MaxScrollOffset());
 
   curve->UpdateTarget(
       animation->TrimTimeToCurrentIteration(CurrentBeginFrameArgs().frame_time)
           .InSecondsF(),
       new_target);
 
   return true;
 }
 }  // namespace cc