Skip to content

Commit

Permalink
Remove cc::BlockingTaskRunner and cc::ReleaseCallbackImpl
Browse files Browse the repository at this point in the history
BlockingTaskRunner was introduced in order for resources released by
a commit to be returned from the impl thread back to the main thread
(ie have the main thread ReleaseCallback run) before the commit ended.
This allowed cc clients to reuse textures immediately upon commit
ending, allowing for less buffering.

With the implementation of delgated compositing, the value of
BlockingTaskRunner disppeared. Now the cases where the impl thread has
a reference to return to the main thread during commit are very few.
The only one that appeared in cc unittests was:
- Make a layer undrawable (sized 0x0)
- Set a mailbox, commit
- Remove the layer from the tree, dropping the impl thread ref.
- Expect to see mailbox returned inside the commit that removes
  the layer from the tree.

Another case would be a texture layer which is not part of the
on-screen frame. But as soon as the layer became visible, committing
a mailbox would mean by the time the next commit happend, the mailbox
has been sent to the display compositor and *can't* be returned. If
unused in the next frame, the display compositor will return it async
outside of commit, and we'll forward it to the main thread.

So the main thread always needs extra buffering for on-screen texture
layers now. By removing the complexity introduced by BlockingTaskRunner
we will make a cc client animating a texture layer outside the
viewport require extra an extra texture of buffering, just as it would
need once it's in the viewport. So the memory overhead shouldn't be
impactful - offscreen clients shouldn't be animating textures, and we'd
need to allocate another texture as soon as they became visible anyway.

Another concern is that on shutdown/removal from the tree, the
resources should all be returned before the commit ends, so the client
can destroy its own structures immediately. This is no longer the case
with delegated rendering and any clients depending on this have already
been leaking in the common case (layer on screen, and can be drawn).

Removing this class will greatly simplify the effort to split
ResourceProvider into two classes - to service the layer and the
display compositors separately, as there's no BlockingTaskRunner
in DisplayResourceProvider anymoore, which was a layer-tree concept.

R=piman@chromium.org

Bug: 738190
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I5d9ffe70e245cdc763f6672c0e8c561f9992de0c
Reviewed-on: https://chromium-review.googlesource.com/679481
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504110}
  • Loading branch information
danakj authored and Commit Bot committed Sep 25, 2017
1 parent 9ff8517 commit cde8e4c
Show file tree
Hide file tree
Showing 42 changed files with 270 additions and 833 deletions.
5 changes: 0 additions & 5 deletions cc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,6 @@ cc_component("cc") {
"resources/scoped_resource.h",
"resources/scoped_ui_resource.cc",
"resources/scoped_ui_resource.h",
"resources/single_release_callback_impl.cc",
"resources/single_release_callback_impl.h",
"resources/ui_resource_bitmap.cc",
"resources/ui_resource_bitmap.h",
"resources/ui_resource_client.h",
Expand Down Expand Up @@ -258,8 +256,6 @@ cc_component("cc") {
"tiles/tiling_set_raster_queue_all.h",
"tiles/tiling_set_raster_queue_required.cc",
"tiles/tiling_set_raster_queue_required.h",
"trees/blocking_task_runner.cc",
"trees/blocking_task_runner.h",
"trees/clip_expander.cc",
"trees/clip_expander.h",
"trees/clip_node.cc",
Expand Down Expand Up @@ -640,7 +636,6 @@ cc_test("cc_unittests") {
"tiles/software_image_decode_cache_unittest.cc",
"tiles/tile_manager_unittest.cc",
"tiles/tile_priority_unittest.cc",
"trees/blocking_task_runner_unittest.cc",
"trees/damage_tracker_unittest.cc",
"trees/image_animation_controller_unittest.cc",
"trees/layer_tree_frame_sink_unittest.cc",
Expand Down
23 changes: 12 additions & 11 deletions cc/layers/texture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
#include "cc/base/simple_enclosed_region.h"
#include "cc/layers/texture_layer_client.h"
#include "cc/layers/texture_layer_impl.h"
#include "cc/resources/single_release_callback_impl.h"
#include "cc/trees/blocking_task_runner.h"
#include "cc/trees/layer_tree_host.h"
#include "components/viz/common/quads/single_release_callback.h"

Expand Down Expand Up @@ -219,14 +217,15 @@ void TextureLayer::PushPropertiesTo(LayerImpl* layer) {
texture_layer->SetBlendBackgroundColor(blend_background_color_);
if (needs_set_mailbox_) {
viz::TextureMailbox texture_mailbox;
std::unique_ptr<SingleReleaseCallbackImpl> release_callback_impl;
std::unique_ptr<viz::SingleReleaseCallback> release_callback;
if (holder_ref_) {
TextureMailboxHolder* holder = holder_ref_->holder();
texture_mailbox = holder->mailbox();
release_callback_impl = holder->GetCallbackForImplThread();
release_callback = holder->GetCallbackForImplThread(
layer_tree_host()->GetTaskRunnerProvider()->MainThreadTaskRunner());
}
texture_layer->SetTextureMailbox(texture_mailbox,
std::move(release_callback_impl));
std::move(release_callback));
needs_set_mailbox_ = false;
}
}
Expand Down Expand Up @@ -271,14 +270,16 @@ void TextureLayer::TextureMailboxHolder::Return(
is_lost_ = is_lost;
}

std::unique_ptr<SingleReleaseCallbackImpl>
TextureLayer::TextureMailboxHolder::GetCallbackForImplThread() {
std::unique_ptr<viz::SingleReleaseCallback>
TextureLayer::TextureMailboxHolder::GetCallbackForImplThread(
scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner) {
// We can't call GetCallbackForImplThread if we released the main thread
// reference.
DCHECK_GT(internal_references_, 0u);
InternalAddRef();
return SingleReleaseCallbackImpl::Create(
base::Bind(&TextureMailboxHolder::ReturnAndReleaseOnImplThread, this));
return viz::SingleReleaseCallback::Create(
base::Bind(&TextureMailboxHolder::ReturnAndReleaseOnImplThread, this,
std::move(main_thread_task_runner)));
}

void TextureLayer::TextureMailboxHolder::InternalAddRef() {
Expand All @@ -295,9 +296,9 @@ void TextureLayer::TextureMailboxHolder::InternalRelease() {
}

void TextureLayer::TextureMailboxHolder::ReturnAndReleaseOnImplThread(
const scoped_refptr<base::SequencedTaskRunner>& main_thread_task_runner,
const gpu::SyncToken& sync_token,
bool is_lost,
BlockingTaskRunner* main_thread_task_runner) {
bool is_lost) {
Return(sync_token, is_lost);
main_thread_task_runner->PostTask(
FROM_HERE, base::Bind(&TextureMailboxHolder::InternalRelease, this));
Expand Down
10 changes: 5 additions & 5 deletions cc/layers/texture_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ class SingleReleaseCallback;
}

namespace cc {
class BlockingTaskRunner;
class SingleReleaseCallbackImpl;
class SingleReleaseCallback;
class TextureLayerClient;

// A Layer containing a the rendered output of a plugin instance.
Expand All @@ -50,7 +49,8 @@ class CC_EXPORT TextureLayer : public Layer {

// Gets a viz::ReleaseCallback that can be called from another thread. Note:
// the caller must ensure the callback is called.
std::unique_ptr<SingleReleaseCallbackImpl> GetCallbackForImplThread();
std::unique_ptr<viz::SingleReleaseCallback> GetCallbackForImplThread(
scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner);

protected:
friend class TextureLayer;
Expand All @@ -71,9 +71,9 @@ class CC_EXPORT TextureLayer : public Layer {
void InternalAddRef();
void InternalRelease();
void ReturnAndReleaseOnImplThread(
const scoped_refptr<base::SequencedTaskRunner>& main_thread_task_runner,
const gpu::SyncToken& sync_token,
bool is_lost,
BlockingTaskRunner* main_thread_task_runner);
bool is_lost);

// These members are only accessed on the main thread, or on the impl thread
// during commit where the main thread is blocked.
Expand Down
12 changes: 4 additions & 8 deletions cc/layers/texture_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@

#include "base/strings/stringprintf.h"
#include "cc/resources/scoped_resource.h"
#include "cc/resources/single_release_callback_impl.h"
#include "cc/trees/layer_tree_impl.h"
#include "cc/trees/occlusion.h"
#include "components/viz/common/quads/single_release_callback.h"
#include "components/viz/common/quads/solid_color_draw_quad.h"
#include "components/viz/common/quads/texture_draw_quad.h"
#include "components/viz/common/resources/platform_color.h"
Expand Down Expand Up @@ -41,7 +41,7 @@ TextureLayerImpl::~TextureLayerImpl() { FreeTextureMailbox(); }

void TextureLayerImpl::SetTextureMailbox(
const viz::TextureMailbox& mailbox,
std::unique_ptr<SingleReleaseCallbackImpl> release_callback) {
std::unique_ptr<viz::SingleReleaseCallback> release_callback) {
DCHECK_EQ(mailbox.IsValid(), !!release_callback);
FreeTextureMailbox();
texture_mailbox_ = mailbox;
Expand Down Expand Up @@ -248,12 +248,8 @@ const char* TextureLayerImpl::LayerTypeAsString() const {
void TextureLayerImpl::FreeTextureMailbox() {
if (own_mailbox_) {
DCHECK(!external_texture_resource_);
if (release_callback_) {
release_callback_->Run(texture_mailbox_.sync_token(), false,
layer_tree_impl()
->task_runner_provider()
->blocking_main_thread_task_runner());
}
if (release_callback_)
release_callback_->Run(texture_mailbox_.sync_token(), false);
texture_mailbox_ = viz::TextureMailbox();
release_callback_ = nullptr;
} else if (external_texture_resource_) {
Expand Down
9 changes: 6 additions & 3 deletions cc/layers/texture_layer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
#include "cc/cc_export.h"
#include "cc/layers/layer_impl.h"

namespace viz {
class SingleReleaseCallback;
}

namespace cc {
class SingleReleaseCallbackImpl;
class ScopedResource;

class CC_EXPORT TextureLayerImpl : public LayerImpl {
Expand Down Expand Up @@ -55,7 +58,7 @@ class CC_EXPORT TextureLayerImpl : public LayerImpl {

void SetTextureMailbox(
const viz::TextureMailbox& mailbox,
std::unique_ptr<SingleReleaseCallbackImpl> release_callback);
std::unique_ptr<viz::SingleReleaseCallback> release_callback);

private:
TextureLayerImpl(LayerTreeImpl* tree_impl, int id);
Expand All @@ -75,7 +78,7 @@ class CC_EXPORT TextureLayerImpl : public LayerImpl {
std::unique_ptr<ScopedResource> texture_copy_;

viz::TextureMailbox texture_mailbox_;
std::unique_ptr<SingleReleaseCallbackImpl> release_callback_;
std::unique_ptr<viz::SingleReleaseCallback> release_callback_;
bool own_mailbox_;
bool valid_texture_copy_;

Expand Down
17 changes: 7 additions & 10 deletions cc/layers/texture_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
namespace cc {
namespace {

void IgnoreCallback(const gpu::SyncToken& sync_token,
bool lost,
BlockingTaskRunner* main_thread_task_runner) {}
void IgnoreCallback(const gpu::SyncToken& sync_token, bool lost) {}

TEST(TextureLayerImplTest, VisibleOpaqueRegion) {
const gfx::Size layer_bounds(100, 100);
Expand Down Expand Up @@ -71,7 +69,7 @@ TEST(TextureLayerImplTest, Occlusion) {
texture_layer_impl->SetDrawsContent(true);
texture_layer_impl->SetTextureMailbox(
texture_mailbox,
SingleReleaseCallbackImpl::Create(base::Bind(&IgnoreCallback)));
viz::SingleReleaseCallback::Create(base::Bind(&IgnoreCallback)));

impl.CalcDrawProps(viewport_size);

Expand Down Expand Up @@ -131,7 +129,7 @@ TEST(TextureLayerImplTest, OutputIsSecure) {
texture_layer_impl->SetDrawsContent(true);
texture_layer_impl->SetTextureMailbox(
texture_mailbox,
SingleReleaseCallbackImpl::Create(base::Bind(&IgnoreCallback)));
viz::SingleReleaseCallback::Create(base::Bind(&IgnoreCallback)));

impl.CalcDrawProps(viewport_size);

Expand Down Expand Up @@ -173,11 +171,10 @@ TEST(TextureLayerImplTest, ResourceNotFreedOnGpuRasterToggle) {
texture_layer_impl->SetBounds(layer_size);
texture_layer_impl->SetDrawsContent(true);
texture_layer_impl->SetTextureMailbox(
texture_mailbox,
SingleReleaseCallbackImpl::Create(base::Bind(
[](bool* released, const gpu::SyncToken& sync_token, bool lost,
BlockingTaskRunner* main_thread_task_runner) { *released = true; },
base::Unretained(&released))));
texture_mailbox, viz::SingleReleaseCallback::Create(base::Bind(
[](bool* released, const gpu::SyncToken& sync_token,
bool lost) { *released = true; },
base::Unretained(&released))));

impl.CalcDrawProps(viewport_size);

Expand Down
Loading

0 comments on commit cde8e4c

Please sign in to comment.