Keeping track of descendants that draw content instead of recalcualting

cc/layers/image_layer.cc

 // Copyright 2010 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/layers/image_layer.h"
 
 #include "base/compiler_specific.h"
 #include "cc/resources/image_layer_updater.h"
 #include "cc/resources/layer_updater.h"
 #include "cc/resources/prioritized_resource.h"
@@ skipping 12 matching lines @@
 
 void ImageLayer::SetBitmap(const SkBitmap& bitmap) {
   // SetBitmap() currently gets called whenever there is any
   // style change that affects the layer even if that change doesn't
   // affect the actual contents of the image (e.g. a CSS animation).
   // With this check in place we avoid unecessary texture uploads.
   if (bitmap.pixelRef() && bitmap.pixelRef() == bitmap_.pixelRef())
     return;
 
   bitmap_ = bitmap;
+  UpdateDrawsContent(true);
   SetNeedsDisplay();
 }
 
+void ImageLayer::UpdateDrawsContent(bool drawsContent) {

[danakj, 2014/07/14 14:56:31]

draws_content thoughout

[awoloszyn, 2014/07/14 19:38:32]

Done.

+  TiledLayer::UpdateDrawsContent(drawsContent && !bitmap_.isNull());

[danakj, 2014/07/14 14:56:31]

so I think what you're trying to do here, you want DrawsContent to be true only
if someone did SetDrawsContent(true) externally on the layer? But to get that
effect you should be && with the current DrawsContent() value, not with |true|
as you would here. Do I have that right?

However, can be simplify this a bit? We have 2 things:
1. SetIsDrawable() which is an external flag saying whether the layer should
draw anything at all.
2. Layer-type-specific internal flag saying whether the layer has anything to
draw.

Can we just use 2 bools on Layer to represent these two things instead of doing
boolean logic to store that in one?

Then Layer::DrawsContent() would be is_drawable_ && has_drawable_content_ or
whatever. WDYT?

[awoloszyn, 2014/07/14 15:25:09]

So how I had set UpateDrawsContent up, currently, is that it is originally
called with true, and then every class in the hierarchy has the ability to veto
the fact that before delegating onto its super-class. Once we hit
Layer::UpdateDrawsContent we will have the actual value of draws_content (with
respect to every subclass on the way down), and set the value.

This could probably be made clearer with a "UpdateDrawsContent" function with no
parameters, and then a helper PropagateDrawsContent(bool) that does this work.
My hope with how this was set up was that we can keep the "truth" if you will,
of whether or not we draw content, in a single variable. This was so that we
could do the required book-keeping to notify our ancestors of our drawing in a
single place, SetDrawsContent. Otherwise we will have to track changes to the
result of (is_drawable_ && has_drawable_content_), both whenever is_drawable_
changes and when has_drawable_content_ changes.

I am certainly amenable to change, I just felt it would be nice to keep the
book-keeping self-contained.

+}
+
 void ImageLayer::SetTexturePriorities(const PriorityCalculator& priority_calc) {
   // Update the tile data before creating all the layer's tiles.
   UpdateTileSizeAndTilingOption();
 
   TiledLayer::SetTexturePriorities(priority_calc);
 }
 
 bool ImageLayer::Update(ResourceUpdateQueue* queue,
                         const OcclusionTracker<Layer>* occlusion) {
   CreateUpdaterIfNeeded();
@@ skipping 24 matching lines @@
                                         float maximum_animation_contents_scale,
                                         bool animating_transform_to_screen,
                                         float* contents_scale_x,
                                         float* contents_scale_y,
                                         gfx::Size* content_bounds) {
   *contents_scale_x = ImageContentsScaleX();
   *contents_scale_y = ImageContentsScaleY();
   *content_bounds = gfx::Size(bitmap_.width(), bitmap_.height());
 }
 
-bool ImageLayer::DrawsContent() const {
-  return !bitmap_.isNull() && TiledLayer::DrawsContent();
-}
-
 void ImageLayer::OnOutputSurfaceCreated() {
   SetTextureFormat(
       layer_tree_host()->GetRendererCapabilities().best_texture_format);
   TiledLayer::OnOutputSurfaceCreated();
 }
 
 float ImageLayer::ImageContentsScaleX() const {
   if (bounds().IsEmpty() || bitmap_.width() == 0)
     return 1;
   return static_cast<float>(bitmap_.width()) / bounds().width();
 }
 
 float ImageLayer::ImageContentsScaleY() const {
   if (bounds().IsEmpty() || bitmap_.height() == 0)
     return 1;
   return static_cast<float>(bitmap_.height()) / bounds().height();
 }
 
 }  // namespace cc