UI Resource Manager

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 22 matching lines @@
 #include "cc/output/copy_output_request.h"
 #include "cc/output/delegating_renderer.h"
 #include "cc/output/gl_renderer.h"
 #include "cc/output/software_renderer.h"
 #include "cc/quads/render_pass_draw_quad.h"
 #include "cc/quads/shared_quad_state.h"
 #include "cc/quads/solid_color_draw_quad.h"
 #include "cc/resources/memory_history.h"
 #include "cc/resources/picture_layer_tiling.h"
 #include "cc/resources/prioritized_resource_manager.h"
+#include "cc/resources/ui_resource_bitmap.h"
 #include "cc/scheduler/delay_based_time_source.h"
 #include "cc/scheduler/texture_uploader.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/layer_tree_host.h"
 #include "cc/trees/layer_tree_host_common.h"
 #include "cc/trees/layer_tree_impl.h"
 #include "cc/trees/quad_culler.h"
 #include "cc/trees/single_thread_proxy.h"
 #include "cc/trees/tree_synchronizer.h"
+#include "gpu/GLES2/gl2extchromium.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gfx/vector2d_conversions.h"
 
 namespace {
 
 void DidVisibilityChange(cc::LayerTreeHostImpl* id, bool visible) {
   if (visible) {
     TRACE_EVENT_ASYNC_BEGIN1("webkit",
                              "LayerTreeHostImpl::SetVisible",
                              id,
@@ skipping 1229 matching lines @@
       top_controls_manager_ ? top_controls_manager_->content_top_offset() : 0.f;
   return gfx::SizeF(dip_size.width(),
                     dip_size.height() - top_offset - overdraw_bottom_height_);
 }
 
 const LayerTreeSettings& LayerTreeHostImpl::Settings() const {
   return settings();
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
+  // When output surface is lost, notify the client of the lost and remove
+  // exisiting entries.

[enne (OOO), 2013/07/22 23:09:15]

typo: exisiting => existing

[powei, 2013/07/24 02:28:29]

Done.

+  for (UIResourceMap::iterator iter = ui_resource_map_.begin();
+      iter != ui_resource_map_.end();
+      iter++) {
+    client_->UIResourceLostOnImplThread(iter->first);

[enne (OOO), 2013/07/22 23:09:15]

I am still a little confused about this path.  It still seems to me that losing
one implies losing all.  Can LayerTreeHost::DidLoseOutputSurface drop all these
resources without needing all of the UIResourceLost message bouncing?

[aelias_OOO_until_Jul13, 2013/07/23 00:06:48]

I agree with Enne, the loop should be in LayerTreeHost.

[powei, 2013/07/24 02:28:29]

Done.

[powei, 2013/07/24 02:28:29]

Done.

+  }
+  // Remove all map from existing UIResourceId to ResourceId.
+  ui_resource_map_.clear();
+  // Remove all pending UI resource requests because there might be
+  // deletion requests, which can no longer be honored (or already
+  // gone anyways).
+  ui_resource_request_queue_.clear();
+
   // 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.
   if (renderer_)
     client_->DidLoseOutputSurfaceOnImplThread();
 }
 
 void LayerTreeHostImpl::Readback(void* pixels,
                                  gfx::Rect rect_in_device_viewport) {
   DCHECK(renderer_);
@@ skipping 112 matching lines @@
   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);
   }
 
+  while (ui_resource_request_queue_.size() > 0) {
+    UIResourceRequest req = ui_resource_request_queue_.front();
+    ui_resource_request_queue_.pop_front();
+    switch (req.type) {
+     case UIResourceCreate:
+       CreateUIResource(req.id, req.bitmap);

[enne (OOO), 2013/07/22 23:09:15]

In general activation is gated on having all of your resources uploaded and
ready, so it's a little odd to create ui resources as a part of activation.

Is the assumption that these resources are not going to be so costly that this
will cause jank? Are these resources always going to be synchronously uploaded?

Is there any way to (at least for impl-side painting) reuse the tile manager's
asynchronous upload and also not make this memory usage invisible to it?

[aelias_OOO_until_Jul13, 2013/07/23 00:06:48]

Yes, the assumption is that for v1 of this patch, it will only be used for small
resources that are unlikely to cause jank or cause memory problems.  In the
future, we'll improve this to use the async upload system (this is one of the
requirements for the Android thumbnail use case).

[powei, 2013/07/24 02:28:29]

Done.

[powei, 2013/07/24 02:28:29]

Done.

+       break;
+     case UIResourceDelete:
+       DeleteUIResource(req.id);
+       break;
+    }
+  }
+
   client_->DidActivatePendingTree();
 }
 
 void LayerTreeHostImpl::SetVisible(bool visible) {
   DCHECK(proxy_->IsImplThread());
 
   if (visible_ == visible)
     return;
   visible_ = visible;
   DidVisibilityChange(this, visible_);
@@ skipping 912 matching lines @@
   return layer ? layer->GetPicture() : skia::RefPtr<SkPicture>();
 }
 
 void LayerTreeHostImpl::SetDebugState(const LayerTreeDebugState& debug_state) {
   if (debug_state_.continuous_painting != debug_state.continuous_painting)
     paint_time_counter_->ClearHistory();
 
   debug_state_ = debug_state;
 }
 
+void LayerTreeHostImpl::CreateUIResource(
+    UIResourceId uid,
+    scoped_refptr<UIResourceBitmap> bitmap) {
+  ResourceProvider::ResourceId id =
+    resource_provider_->CreateGLTexture(bitmap->GetSize(),

[enne (OOO), 2013/07/22 23:09:15]

CreateGLTexture in LayerTreeHostImpl? What if there's a software renderer?

[powei, 2013/07/24 02:28:29]

Done.

+                                        GL_RGBA,

[enne (OOO), 2013/07/22 23:09:15]

Should you DCHECK that bitmap's format is RGBA8 and not ETC1?

[powei, 2013/07/24 02:28:29]

Done.

+                                        GL_TEXTURE_POOL_UNMANAGED_CHROMIUM,
+                                        ResourceProvider::TextureUsageAny);
+
+  ui_resource_map_[uid] = id;

[enne (OOO), 2013/07/22 23:09:15]

Can you DCHECK that it doesn't previously exist?

[powei, 2013/07/24 02:28:29]

Done.

+  gfx::Size bitmap_size = bitmap->GetSize();
+  resource_provider_->SetPixels(id,
+                                reinterpret_cast<uint8_t*>(bitmap->GetPixels()),
+                                gfx::Rect(bitmap_size),
+                                gfx::Rect(bitmap_size),
+                                gfx::Vector2d(0, 0));
+}
+
+void LayerTreeHostImpl::DeleteUIResource(UIResourceId uid) {
+  ResourceProvider::ResourceId id = ResourceIdForUIResource(uid);
+  if (id) {
+    resource_provider_->DeleteResource(id);
+    ui_resource_map_.erase(uid);
+  }
+}
+
+ResourceProvider::ResourceId
+LayerTreeHostImpl::ResourceIdForUIResource(UIResourceId uid) const {
+  if (ui_resource_map_.find(uid) != ui_resource_map_.end()) {

[enne (OOO), 2013/07/22 23:09:15]

If you're going to find and then do something with the results, please keep the
resulting iterator around rather than doing a second find.

[powei, 2013/07/24 02:28:29]

Done.

+    return ui_resource_map_.find(uid)->second;
+  }
+  return 0;
+}
+
+
 }  // namespace cc