From 9af0a97c8d0dd9aac55a579735b127844c3d9056 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 18 Mar 2020 14:46:33 -0700 Subject: [PATCH] remove debugging prints and update comments --- flow/layers/container_layer.cc | 14 +++++++------- flow/layers/container_layer.h | 30 +++++++++++++++++++++++------- flow/layers/image_filter_layer.cc | 8 ++------ flow/raster_cache.cc | 1 - 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 0c98b002c1b9c..ddc6763a64092 100644 --- a/flow/layers/container_layer.cc +++ b/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()); } @@ -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; } diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 19981d34764cb..66e6dc6cc5c7f 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -47,17 +47,33 @@ class MergedContainerLayer : public ContainerLayer { void Add(std::shared_ptr 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); }; diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc index fcf1338c34cdc..7d2f1cb5ff848 100644 --- a/flow/layers/image_filter_layer.cc +++ b/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 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); diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index a31e5349548f4..f179905ed8e53 100644 --- a/flow/raster_cache.cc +++ b/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(),