remove debugging prints and update comments

Author: flar

flow/layers/container_layer.cc

@@ -88,12 +88,12 @@ void ContainerLayer::UpdateSceneChildren(SceneUpdateContext& context) {
 MergedContainerLayer::MergedContainerLayer() {
   // Ensure the layer has only one direct child.
   //
-  // This intermediary container helps container layers that want or need
-  // to cache the rendering of their children to do so more easily.
-  // TBD (flar) - mention caveat and use of GetCacheableChild() here...
-  //
-  // Any children will be actually added as children of this empty
-  // ContainerLayer.
+  // Any children will actually be added as children of this empty
+  // ContainerLayer.  If only one child is ever added to this layer then
+  // that child will become the layer returned from ::GetCacheableChild().
+  // If multiple child layers are added, then this implicit container
+  // child becomes the cacheable child, but at the potential cost of
+  // not being as stable in the raster cache from frame to frame.
   ContainerLayer::Add(std::make_shared<ContainerLayer>());
 }
 
@@ -113,7 +113,7 @@ Layer* MergedContainerLayer::GetCacheableChild() const {
     return child_container->layers()[0].get();
   }
 
-  FML_LOG(INFO) << "Single child layer does not contain a single child";
+  FML_LOG(INFO) << "Single child layer contains multiple children";
   return child_container;
 }
 

flow/layers/container_layer.h

@@ -47,17 +47,33 @@ class MergedContainerLayer : public ContainerLayer {
   void Add(std::shared_ptr<Layer> layer) override;
 
  protected:
+  // Return the implicit container layer created by the constructor that
+  // is guaranteed to contain all children of this layer. This particular
+  // layer may not be the best choice for caching the children in the
+  // raster cache, though. See ::GetCacheableChild() for more details.
   ContainerLayer* GetChildContainer() const;
 
-  // The ChildContainer will be created anew whenever the MergedContainerLayer
-  // subclass is created even though the real child of the container may be
-  // the same child. If the MergedContainerLayer wants to cache the rendering
-  // of its "child" then the cache will need to be redrawn on every frame that
-  // the MergedContainer is different, defeating the purpose of caching. This
-  // method will attempt to return the real child (if there is only one) to
-  // improve the chance of caching.
+  // Return a single layer that both represents all children of this layer
+  // and which is most likely to remain stable from frame to frame for
+  // purposes of keeping it in the raster cache.
+  //
+  // Most every use of this utility container subclass will involve only a
+  // single actual child, but this utility exists to enforce that condition
+  // so that we always have a single child even when the scene builders do
+  // not follow that guideline.
+  //
+  // If the guidelines are followed then the single actual child is the best
+  // choice for caching since it will often be reused from scene to scene and
+  // will remain in the cache from frame to frame. If the guidelines are not
+  // followed and some agent building scenes has added multiple children to
+  // this layer, then the only "single child" we have for caching is the
+  // implicit ChildContainer created in the constructor. That implicit child
+  // can still be cacheable as long as this layer is not recreated on every
+  // frame, but it is less likely to be so, especially if this layer is
+  // actively being animated (Opacity animation for example).
   Layer* GetCacheableChild() const;
 
+ private:
   FML_DISALLOW_COPY_AND_ASSIGN(MergedContainerLayer);
 };
 

flow/layers/image_filter_layer.cc

@@ -59,7 +59,6 @@ void ImageFilterLayer::Paint(PaintContext& context) const {
     RasterCacheResult layer_cache =
         context.raster_cache->Get((Layer*)this, ctm);
     if (layer_cache.is_valid()) {
-      FML_LOG(ERROR) << "Rendering filtered output from cache";
       layer_cache.draw(*context.leaf_nodes_canvas);
       return;
     }
@@ -68,8 +67,6 @@ void ImageFilterLayer::Paint(PaintContext& context) const {
       sk_sp<SkImageFilter> transformed_filter =
           filter_->makeWithLocalMatrix(ctm);
       if (transformed_filter) {
-        FML_LOG(ERROR) << "Filtering from cached child";
-
         SkPaint paint;
         paint.setImageFilter(transformed_filter);
 
@@ -79,14 +76,13 @@ void ImageFilterLayer::Paint(PaintContext& context) const {
     }
   }
 
-  FML_LOG(ERROR) << "Applying filter to child on the fly";
   SkPaint paint;
   paint.setImageFilter(filter_);
 
   // Normally a save_layer is sized to the current layer bounds, but in this
   // case the bounds of the child may not be the same as the filtered version
-  // so we use the child_paint_bounds_ which were snapshotted from the
-  // Preroll on the children before we adjusted them based on the filter.
+  // so we use the bounds of the child container which do not include any
+  // modifications that the filter might apply.
   Layer::AutoSaveLayer save_layer = Layer::AutoSaveLayer::Create(
       context, GetChildContainer()->paint_bounds(), &paint);
   PaintChildren(context);

flow/raster_cache.cc

@@ -137,7 +137,6 @@ void RasterCache::Prepare(PrerollContext* context,
   entry.access_count++;
   entry.used_this_frame = true;
   if (!entry.image.is_valid()) {
-    FML_LOG(ERROR) << "Rasterizing " << layer->unique_id();
     entry.image = Rasterize(
         context->gr_context, ctm, context->dst_color_space,
         checkerboard_images_, layer->paint_bounds(),