diff options
author | reveman@chromium.org <reveman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-14 07:18:53 +0000 |
---|---|---|
committer | reveman@chromium.org <reveman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-14 07:18:53 +0000 |
commit | 282dc515172f46f41fc8511e29a9a5b0a92e48fb (patch) | |
tree | 5dfe3129a8861ca1fd99d7af716cdad6f78c9b03 /cc | |
parent | 103994e87aa95172b922dbbf6be9fe4ce0e08585 (diff) | |
download | chromium_src-282dc515172f46f41fc8511e29a9a5b0a92e48fb.zip chromium_src-282dc515172f46f41fc8511e29a9a5b0a92e48fb.tar.gz chromium_src-282dc515172f46f41fc8511e29a9a5b0a92e48fb.tar.bz2 |
cc: Refactor force upload mechanism to allow proper resource ownership passing.
Makes ResourceProvider changes required to allow reuse of resources
immediately after forcing "set pixels" to complete and removes
AbortSetPixels() which is no longer needed.
Improves the efficiency of forced uploads by issuing the "wait-for"
command as soon as we know that the upload can be forced. It also
provides a significant cleanup to RasterWorkerPool interface and
tile management.
BUG=245767
Review URL: https://chromiumcodereview.appspot.com/16926002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206352 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'cc')
-rw-r--r-- | cc/layers/picture_layer_impl.cc | 6 | ||||
-rw-r--r-- | cc/resources/managed_tile_state.cc | 7 | ||||
-rw-r--r-- | cc/resources/managed_tile_state.h | 29 | ||||
-rw-r--r-- | cc/resources/pixel_buffer_raster_worker_pool.cc | 83 | ||||
-rw-r--r-- | cc/resources/pixel_buffer_raster_worker_pool.h | 4 | ||||
-rw-r--r-- | cc/resources/raster_worker_pool.cc | 77 | ||||
-rw-r--r-- | cc/resources/raster_worker_pool.h | 52 | ||||
-rw-r--r-- | cc/resources/raster_worker_pool_unittest.cc | 28 | ||||
-rw-r--r-- | cc/resources/resource_provider.cc | 38 | ||||
-rw-r--r-- | cc/resources/resource_provider.h | 1 | ||||
-rw-r--r-- | cc/resources/resource_provider_unittest.cc | 49 | ||||
-rw-r--r-- | cc/resources/tile_manager.cc | 53 | ||||
-rw-r--r-- | cc/resources/tile_manager.h | 6 |
13 files changed, 182 insertions, 251 deletions
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc index 73ec9d8..84a7d9c 100644 --- a/cc/layers/picture_layer_impl.cc +++ b/cc/layers/picture_layer_impl.cc @@ -263,7 +263,11 @@ void PictureLayerImpl::AppendQuads(QuadSink* quad_sink, opaque_rect, texture_rect, iter.texture_size(), - tile_version.contents_swizzled(), + // TODO(reveman): This assumes the renderer will use + // GL_RGBA as format of temporary resource. The need + // to swizzle should instead be determined by the + // renderer. + !PlatformColor::SameComponentOrder(GL_RGBA), iter->content_rect(), iter->contents_scale(), draw_direct_to_backbuffer, diff --git a/cc/resources/managed_tile_state.cc b/cc/resources/managed_tile_state.cc index 1b2c2d0..578fb31 100644 --- a/cc/resources/managed_tile_state.cc +++ b/cc/resources/managed_tile_state.cc @@ -25,10 +25,7 @@ ManagedTileState::ManagedTileState() ManagedTileState::TileVersion::TileVersion() : mode_(RESOURCE_MODE), - has_text_(false), - resource_id_(0), - resource_format_(GL_RGBA), - forced_upload_(false) { + has_text_(false) { } ManagedTileState::TileVersion::~TileVersion() { @@ -38,7 +35,7 @@ ManagedTileState::TileVersion::~TileVersion() { bool ManagedTileState::TileVersion::IsReadyToDraw() const { switch (mode_) { case RESOURCE_MODE: - return resource_id_ && (resource_ || forced_upload_); + return !!resource_; case SOLID_COLOR_MODE: case PICTURE_PILE_MODE: return true; diff --git a/cc/resources/managed_tile_state.h b/cc/resources/managed_tile_state.h index 200997b..4effe1d2 100644 --- a/cc/resources/managed_tile_state.h +++ b/cc/resources/managed_tile_state.h @@ -38,15 +38,9 @@ class CC_EXPORT ManagedTileState { ResourceProvider::ResourceId get_resource_id() const { DCHECK(mode_ == RESOURCE_MODE); + DCHECK(resource_); - // We have to have a resource ID here. - DCHECK(resource_id_); - // If we have a resource, it implies IDs are equal. - DCHECK(!resource_ || (resource_id_ == resource_->id())); - // If we don't have a resource, it implies that we're in forced upload. - DCHECK(resource_ || (resource_id_ && forced_upload_)); - - return resource_id_; + return resource_->id(); } SkColor get_solid_color() const { @@ -56,7 +50,8 @@ class CC_EXPORT ManagedTileState { } bool contents_swizzled() const { - return !PlatformColor::SameComponentOrder(resource_format_); + DCHECK(resource_); + return !PlatformColor::SameComponentOrder(resource_->format()); } bool requires_resource() const { @@ -68,11 +63,10 @@ class CC_EXPORT ManagedTileState { void SetResourceForTesting(scoped_ptr<ResourcePool::Resource> resource) { resource_ = resource.Pass(); - resource_id_ = resource_->id(); } - scoped_ptr<ResourcePool::Resource>& GetResourceForTesting() { - return resource_; + const ResourcePool::Resource* GetResourceForTesting() const { + return resource_.get(); } private: @@ -87,7 +81,6 @@ class CC_EXPORT ManagedTileState { void set_solid_color(const SkColor& color) { mode_ = SOLID_COLOR_MODE; solid_color_ = color; - resource_id_ = 0; } void set_has_text(bool has_text) { @@ -96,25 +89,15 @@ class CC_EXPORT ManagedTileState { void set_rasterize_on_demand() { mode_ = PICTURE_PILE_MODE; - resource_id_ = 0; } Mode mode_; SkColor solid_color_; bool has_text_; - - // TODO(reveman): Eliminate the need for |resource_id_| - // and |resource_format_| and | forced_upload_| by re-factoring - // the "force upload" mechanism. crbug.com/245767 - ResourceProvider::ResourceId resource_id_; - GLenum resource_format_; - bool forced_upload_; - scoped_ptr<ResourcePool::Resource> resource_; RasterWorkerPool::RasterTask raster_task_; }; - ManagedTileState(); ~ManagedTileState(); diff --git a/cc/resources/pixel_buffer_raster_worker_pool.cc b/cc/resources/pixel_buffer_raster_worker_pool.cc index e036b42..dcee900 100644 --- a/cc/resources/pixel_buffer_raster_worker_pool.cc +++ b/cc/resources/pixel_buffer_raster_worker_pool.cc @@ -84,6 +84,7 @@ const int kCheckForCompletedRasterTasksDelayMs = 6; PixelBufferRasterWorkerPool::PixelBufferRasterWorkerPool( ResourceProvider* resource_provider, size_t num_threads) : RasterWorkerPool(resource_provider, num_threads), + shutdown_(false), bytes_pending_upload_(0), has_performed_uploads_since_last_flush_(false), check_for_completed_raster_tasks_pending_(false), @@ -92,6 +93,7 @@ PixelBufferRasterWorkerPool::PixelBufferRasterWorkerPool( } PixelBufferRasterWorkerPool::~PixelBufferRasterWorkerPool() { + DCHECK(shutdown_); DCHECK(!check_for_completed_raster_tasks_pending_); DCHECK_EQ(0u, pixel_buffer_tasks_.size()); DCHECK_EQ(0u, tasks_with_pending_upload_.size()); @@ -99,9 +101,9 @@ PixelBufferRasterWorkerPool::~PixelBufferRasterWorkerPool() { } void PixelBufferRasterWorkerPool::Shutdown() { + shutdown_ = true; RasterWorkerPool::Shutdown(); CheckForCompletedRasterTasks(); - AbortPendingUploads(); for (TaskMap::iterator it = pixel_buffer_tasks_.begin(); it != pixel_buffer_tasks_.end(); ++it) { internal::RasterWorkerPoolTask* task = it->first; @@ -168,9 +170,8 @@ void PixelBufferRasterWorkerPool::CheckForCompletedTasks() { TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::CheckForCompletedTasks"); RasterWorkerPool::CheckForCompletedTasks(); - FlushUploads(); - CheckForCompletedUploads(); + FlushUploads(); while (!completed_tasks_.empty()) { internal::RasterWorkerPoolTask* task = completed_tasks_.front().get(); @@ -185,20 +186,6 @@ void PixelBufferRasterWorkerPool::CheckForCompletedTasks() { } } -bool PixelBufferRasterWorkerPool::ForceUploadToComplete( - const RasterTask& raster_task) { - for (TaskDeque::iterator it = tasks_with_pending_upload_.begin(); - it != tasks_with_pending_upload_.end(); ++it) { - internal::RasterWorkerPoolTask* task = it->get(); - if (task == raster_task.internal_.get()) { - resource_provider()->ForceSetPixelsToComplete(task->resource()->id()); - return true; - } - } - - return false; -} - void PixelBufferRasterWorkerPool::FlushUploads() { if (!has_performed_uploads_since_last_flush_) return; @@ -208,15 +195,52 @@ void PixelBufferRasterWorkerPool::FlushUploads() { } void PixelBufferRasterWorkerPool::CheckForCompletedUploads() { + TaskDeque tasks_with_completed_uploads; + + // First check if any have completed. while (!tasks_with_pending_upload_.empty()) { internal::RasterWorkerPoolTask* task = tasks_with_pending_upload_.front().get(); - DCHECK(pixel_buffer_tasks_.find(task) != pixel_buffer_tasks_.end()); // Uploads complete in the order they are issued. if (!resource_provider()->DidSetPixelsComplete(task->resource()->id())) break; + tasks_with_completed_uploads.push_back(task); + tasks_with_pending_upload_.pop_front(); + } + + DCHECK(client()); + bool should_force_some_uploads_to_complete = + shutdown_ || client()->ShouldForceTasksRequiredForActivationToComplete(); + + if (should_force_some_uploads_to_complete) { + TaskDeque::iterator it = tasks_with_pending_upload_.begin(); + while (it != tasks_with_pending_upload_.end()) { + internal::RasterWorkerPoolTask* task = *it; + DCHECK(pixel_buffer_tasks_.find(task) != pixel_buffer_tasks_.end()); + + // Force all uploads required for activation to complete. + // During shutdown, force all pending uploads to complete. + if (shutdown_ || IsRasterTaskRequiredForActivation(task)) { + resource_provider()->ForceSetPixelsToComplete(task->resource()->id()); + has_performed_uploads_since_last_flush_ = true; + + tasks_with_completed_uploads.push_back(task); + it = tasks_with_pending_upload_.erase(it); + continue; + } + + ++it; + } + } + + // Release shared memory and move tasks with completed uploads + // to |completed_tasks_|. + while (!tasks_with_completed_uploads.empty()) { + internal::RasterWorkerPoolTask* task = + tasks_with_completed_uploads.front().get(); + // It's now safe to release the pixel buffer and the shared memory. resource_provider()->ReleasePixelBuffer(task->resource()->id()); @@ -225,7 +249,7 @@ void PixelBufferRasterWorkerPool::CheckForCompletedUploads() { task->DidRun(); completed_tasks_.push_back(task); - tasks_with_pending_upload_.pop_front(); + tasks_with_completed_uploads.pop_front(); } } @@ -251,9 +275,8 @@ void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() { check_for_completed_raster_tasks_pending_ = false; RasterWorkerPool::CheckForCompletedTasks(); - FlushUploads(); - CheckForCompletedUploads(); + FlushUploads(); ScheduleMoreTasks(); } @@ -379,24 +402,6 @@ void PixelBufferRasterWorkerPool::OnRasterTaskCompleted( tasks_with_pending_upload_.push_back(task); } -void PixelBufferRasterWorkerPool::AbortPendingUploads() { - while (!tasks_with_pending_upload_.empty()) { - internal::RasterWorkerPoolTask* task = - tasks_with_pending_upload_.front().get(); - DCHECK(pixel_buffer_tasks_.find(task) != pixel_buffer_tasks_.end()); - - resource_provider()->AbortSetPixels(task->resource()->id()); - resource_provider()->ReleasePixelBuffer(task->resource()->id()); - - bytes_pending_upload_ -= task->resource()->bytes(); - - // Need to run the reply callback even though task was aborted. - completed_tasks_.push_back(task); - - tasks_with_pending_upload_.pop_front(); - } -} - void PixelBufferRasterWorkerPool::OnRasterFinished( int64 schedule_more_tasks_count) { TRACE_EVENT1("cc", diff --git a/cc/resources/pixel_buffer_raster_worker_pool.h b/cc/resources/pixel_buffer_raster_worker_pool.h index 269b2f0..2a89d4b 100644 --- a/cc/resources/pixel_buffer_raster_worker_pool.h +++ b/cc/resources/pixel_buffer_raster_worker_pool.h @@ -28,7 +28,6 @@ class CC_EXPORT PixelBufferRasterWorkerPool : public RasterWorkerPool { // Overridden from RasterWorkerPool: virtual void ScheduleTasks(RasterTask::Queue* queue) OVERRIDE; - virtual bool ForceUploadToComplete(const RasterTask& raster_task) OVERRIDE; private: PixelBufferRasterWorkerPool(ResourceProvider* resource_provider, @@ -43,7 +42,6 @@ class CC_EXPORT PixelBufferRasterWorkerPool : public RasterWorkerPool { scoped_refptr<internal::RasterWorkerPoolTask> task, bool was_canceled, bool needs_upload); - void AbortPendingUploads(); void DidCompleteRasterTask(internal::RasterWorkerPoolTask* task); void OnRasterFinished(int64 schedule_more_tasks_count); @@ -51,6 +49,8 @@ class CC_EXPORT PixelBufferRasterWorkerPool : public RasterWorkerPool { scoped_refptr<base::MessageLoopProxy> origin_loop, const base::Closure& on_raster_finished_callback); + bool shutdown_; + TaskMap pixel_buffer_tasks_; typedef std::deque<scoped_refptr<internal::RasterWorkerPoolTask> > TaskDeque; diff --git a/cc/resources/raster_worker_pool.cc b/cc/resources/raster_worker_pool.cc index 22d3b7d..1713ee7 100644 --- a/cc/resources/raster_worker_pool.cc +++ b/cc/resources/raster_worker_pool.cc @@ -316,9 +316,12 @@ RasterWorkerPool::RasterTask::Queue::Queue() { RasterWorkerPool::RasterTask::Queue::~Queue() { } -void RasterWorkerPool::RasterTask::Queue::Append(const RasterTask& task) { +void RasterWorkerPool::RasterTask::Queue::Append( + const RasterTask& task, bool required_for_activation) { DCHECK(!task.is_null()); tasks_.push_back(task.internal_); + if (required_for_activation) + tasks_required_for_activation_.insert(task.internal_.get()); } RasterWorkerPool::RasterTask::RasterTask() { @@ -353,20 +356,7 @@ RasterWorkerPool::RootTask::RootTask( RasterWorkerPool::RootTask::~RootTask() { } -RasterWorkerPool::RasterWorkerPool(ResourceProvider* resource_provider, - size_t num_threads) - : WorkerPool(num_threads, kWorkerThreadNamePrefix), - resource_provider_(resource_provider) { -} - -RasterWorkerPool::~RasterWorkerPool() { -} - -void RasterWorkerPool::Shutdown() { - raster_tasks_.clear(); - WorkerPool::Shutdown(); -} - +// static RasterWorkerPool::RasterTask RasterWorkerPool::CreateRasterTask( const Resource* resource, PicturePileImpl* picture_pile, @@ -379,32 +369,52 @@ RasterWorkerPool::RasterTask RasterWorkerPool::CreateRasterTask( const RasterTask::Reply& reply, Task::Set& dependencies) { return RasterTask(new RasterWorkerPoolTaskImpl(resource, - picture_pile, - content_rect, - contents_scale, - raster_mode, - use_color_estimator, - metadata, - rendering_stats, - reply, - &dependencies.tasks_)); -} - + picture_pile, + content_rect, + contents_scale, + raster_mode, + use_color_estimator, + metadata, + rendering_stats, + reply, + &dependencies.tasks_)); +} + +// static RasterWorkerPool::Task RasterWorkerPool::CreateImageDecodeTask( skia::LazyPixelRef* pixel_ref, int layer_id, RenderingStatsInstrumentation* stats_instrumentation, const Task::Reply& reply) { - return Task(new ImageDecodeWorkerPoolTaskImpl( - pixel_ref, layer_id, stats_instrumentation, reply)); + return Task(new ImageDecodeWorkerPoolTaskImpl(pixel_ref, + layer_id, + stats_instrumentation, + reply)); } -bool RasterWorkerPool::ForceUploadToComplete(const RasterTask& raster_task) { - return false; +RasterWorkerPool::RasterWorkerPool(ResourceProvider* resource_provider, + size_t num_threads) + : WorkerPool(num_threads, kWorkerThreadNamePrefix), + client_(NULL), + resource_provider_(resource_provider) { +} + +RasterWorkerPool::~RasterWorkerPool() { +} + +void RasterWorkerPool::SetClient(RasterWorkerPoolClient* client) { + client_ = client; +} + +void RasterWorkerPool::Shutdown() { + raster_tasks_.clear(); + WorkerPool::Shutdown(); } void RasterWorkerPool::SetRasterTasks(RasterTask::Queue* queue) { raster_tasks_.swap(queue->tasks_); + raster_tasks_required_for_activation_.swap( + queue->tasks_required_for_activation_); } void RasterWorkerPool::ScheduleRasterTasks(const RootTask& root) { @@ -417,4 +427,11 @@ void RasterWorkerPool::ScheduleRasterTasks(const RootTask& root) { root_.swap(new_root); } +bool RasterWorkerPool::IsRasterTaskRequiredForActivation( + internal::RasterWorkerPoolTask* task) const { + return + raster_tasks_required_for_activation_.find(task) != + raster_tasks_required_for_activation_.end(); +} + } // namespace cc diff --git a/cc/resources/raster_worker_pool.h b/cc/resources/raster_worker_pool.h index 6ae8812..5485fec 100644 --- a/cc/resources/raster_worker_pool.h +++ b/cc/resources/raster_worker_pool.h @@ -100,6 +100,14 @@ struct RasterTaskMetadata { int source_frame_number; }; +class CC_EXPORT RasterWorkerPoolClient { + public: + virtual bool ShouldForceTasksRequiredForActivationToComplete() const = 0; + + protected: + virtual ~RasterWorkerPoolClient() {} +}; + // A worker thread pool that runs raster tasks. class CC_EXPORT RasterWorkerPool : public WorkerPool { public: @@ -109,8 +117,6 @@ class CC_EXPORT RasterWorkerPool : public WorkerPool { class CC_EXPORT Set { public: - typedef internal::WorkerPoolTask::TaskVector TaskVector; - Set(); ~Set(); @@ -120,6 +126,7 @@ class CC_EXPORT RasterWorkerPool : public WorkerPool { friend class RasterWorkerPool; friend class RasterWorkerPoolTest; + typedef internal::WorkerPoolTask::TaskVector TaskVector; TaskVector tasks_; }; @@ -153,12 +160,14 @@ class CC_EXPORT RasterWorkerPool : public WorkerPool { Queue(); ~Queue(); - void Append(const RasterTask& task); + void Append(const RasterTask& task, bool required_for_activation); private: friend class RasterWorkerPool; TaskVector tasks_; + typedef base::hash_set<internal::RasterWorkerPoolTask*> TaskSet; + TaskSet tasks_required_for_activation_; }; RasterTask(); @@ -180,6 +189,21 @@ class CC_EXPORT RasterWorkerPool : public WorkerPool { scoped_refptr<internal::RasterWorkerPoolTask> internal_; }; + virtual ~RasterWorkerPool(); + + void SetClient(RasterWorkerPoolClient* client); + + // Tells the worker pool to shutdown after canceling all previously + // scheduled tasks. Reply callbacks are still guaranteed to run. + virtual void Shutdown() OVERRIDE; + + // Schedule running of raster tasks in |queue| and all dependencies. + // Previously scheduled tasks that are no longer needed to run + // raster tasks in |queue| will be canceled unless already running. + // Once scheduled, reply callbacks are guaranteed to run for all tasks + // even if they later get canceled by another call to ScheduleTasks(). + virtual void ScheduleTasks(RasterTask::Queue* queue) = 0; + // TODO(vmpstr): Try to elimiate some variables. static RasterTask CreateRasterTask( const Resource* resource, @@ -199,23 +223,6 @@ class CC_EXPORT RasterWorkerPool : public WorkerPool { RenderingStatsInstrumentation* stats_instrumentation, const Task::Reply& reply); - virtual ~RasterWorkerPool(); - - // Tells the worker pool to shutdown after canceling all previously - // scheduled tasks. Reply callbacks are still guaranteed to run. - virtual void Shutdown() OVERRIDE; - - // Schedule running of raster tasks in |queue| and all dependencies. - // Previously scheduled tasks that are no longer needed to run - // raster tasks in |queue| will be canceled unless already running. - // Once scheduled, reply callbacks are guaranteed to run for all tasks - // even if they later get canceled by another call to ScheduleTasks(). - virtual void ScheduleTasks(RasterTask::Queue* queue) = 0; - - // Tells the raster worker pool to force any pending uploads for - // |raster_task| to complete. Returns true when successful. - virtual bool ForceUploadToComplete(const RasterTask& raster_task); - protected: class RootTask { public: @@ -239,15 +246,20 @@ class CC_EXPORT RasterWorkerPool : public WorkerPool { void SetRasterTasks(RasterTask::Queue* queue); void ScheduleRasterTasks(const RootTask& root); + bool IsRasterTaskRequiredForActivation( + internal::RasterWorkerPoolTask* task) const; + RasterWorkerPoolClient* client() const { return client_; } ResourceProvider* resource_provider() const { return resource_provider_; } const RasterTask::Queue::TaskVector& raster_tasks() const { return raster_tasks_; } private: + RasterWorkerPoolClient* client_; ResourceProvider* resource_provider_; RasterTask::Queue::TaskVector raster_tasks_; + RasterTask::Queue::TaskSet raster_tasks_required_for_activation_; // The root task that is a dependent of all other tasks. scoped_refptr<internal::WorkerPoolTask> root_; diff --git a/cc/resources/raster_worker_pool_unittest.cc b/cc/resources/raster_worker_pool_unittest.cc index 136488b..4a0af4e 100644 --- a/cc/resources/raster_worker_pool_unittest.cc +++ b/cc/resources/raster_worker_pool_unittest.cc @@ -17,11 +17,12 @@ namespace cc { -class TestRasterTaskImpl : public internal::RasterWorkerPoolTask { +class TestRasterWorkerPoolTaskImpl : public internal::RasterWorkerPoolTask { public: - TestRasterTaskImpl(const Resource* resource, - const RasterWorkerPool::RasterTask::Reply& reply, - internal::WorkerPoolTask::TaskVector* dependencies) + TestRasterWorkerPoolTaskImpl( + const Resource* resource, + const RasterWorkerPool::RasterTask::Reply& reply, + internal::WorkerPoolTask::TaskVector* dependencies) : internal::RasterWorkerPoolTask(resource, dependencies), reply_(reply) {} @@ -33,13 +34,14 @@ class TestRasterTaskImpl : public internal::RasterWorkerPoolTask { } protected: - virtual ~TestRasterTaskImpl() {} + virtual ~TestRasterWorkerPoolTaskImpl() {} private: const RasterWorkerPool::RasterTask::Reply reply_; }; -class RasterWorkerPoolTest : public testing::Test { +class RasterWorkerPoolTest : public testing::Test, + public RasterWorkerPoolClient { public: RasterWorkerPoolTest() : output_surface_(FakeOutputSurface::Create3d()), @@ -61,6 +63,12 @@ class RasterWorkerPoolTest : public testing::Test { raster_worker_pool_->CheckForCompletedTasks(); } + // Overriden from RasterWorkerPoolClient: + virtual bool ShouldForceTasksRequiredForActivationToComplete() const + OVERRIDE { + return false; + } + virtual void BeginTest() = 0; virtual void AfterTest() = 0; @@ -77,7 +85,9 @@ class RasterWorkerPoolTest : public testing::Test { const RasterWorkerPool::RasterTask::Reply& reply, RasterWorkerPool::Task::Set& dependencies) { return RasterWorkerPool::RasterTask( - new TestRasterTaskImpl(resource, reply, &dependencies.tasks_)); + new TestRasterWorkerPoolTaskImpl(resource, + reply, + &dependencies.tasks_)); } void RunTest(bool use_map_image) { @@ -89,6 +99,8 @@ class RasterWorkerPoolTest : public testing::Test { resource_provider(), 1); } + raster_worker_pool_->SetClient(this); + BeginTest(); ScheduleCheckForCompletedTasks(); @@ -208,7 +220,7 @@ class BasicRasterWorkerPoolTest : public RasterWorkerPoolTest { for (std::vector<RasterWorkerPool::RasterTask>::iterator it = tasks_.begin(); it != tasks_.end(); ++it) - tasks.Append(*it); + tasks.Append(*it, false); worker_pool()->ScheduleTasks(&tasks); } diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc index d0b4de7..da90a44 100644 --- a/cc/resources/resource_provider.cc +++ b/cc/resources/resource_provider.cc @@ -958,6 +958,18 @@ void ResourceProvider::ReleasePixelBuffer(ResourceId id) { DCHECK(!resource->exported); DCHECK(!resource->image_id); + // The pixel buffer can be released while there is a pending "set pixels" + // if completion has been forced. Any shared memory associated with this + // pixel buffer will not be freed until the waitAsyncTexImage2DCHROMIUM + // command has been processed on the service side. It is also safe to + // reuse any query id associated with this resource before they complete + // as each new query has a unique submit count. + if (resource->pending_set_pixels) { + DCHECK(resource->set_pixels_completion_forced); + resource->pending_set_pixels = false; + UnlockForWrite(id); + } + if (resource->type == GLTexture) { if (!resource->gl_pixel_buffer_id) return; @@ -1210,32 +1222,6 @@ bool ResourceProvider::DidSetPixelsComplete(ResourceId id) { return true; } -void ResourceProvider::AbortSetPixels(ResourceId id) { - DCHECK(thread_checker_.CalledOnValidThread()); - ResourceMap::iterator it = resources_.find(id); - CHECK(it != resources_.end()); - Resource* resource = &it->second; - DCHECK(resource->locked_for_write); - DCHECK(resource->pending_set_pixels); - - if (resource->gl_id) { - WebGraphicsContext3D* context3d = output_surface_->context3d(); - DCHECK(context3d); - DCHECK(resource->gl_upload_query_id); - // CHROMIUM_async_pixel_transfers currently doesn't have a way to - // abort an upload. The best we can do is delete the query and - // the texture. - context3d->deleteQueryEXT(resource->gl_upload_query_id); - resource->gl_upload_query_id = 0; - context3d->deleteTexture(resource->gl_id); - resource->gl_id = CreateTextureId(context3d); - resource->allocated = false; - } - - resource->pending_set_pixels = false; - UnlockForWrite(id); -} - void ResourceProvider::CreateForTesting(ResourceId id) { ResourceMap::iterator it = resources_.find(id); CHECK(it != resources_.end()); diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h index d8b89c24..5bce6f7 100644 --- a/cc/resources/resource_provider.h +++ b/cc/resources/resource_provider.h @@ -290,7 +290,6 @@ class CC_EXPORT ResourceProvider { void BeginSetPixels(ResourceId id); void ForceSetPixelsToComplete(ResourceId id); bool DidSetPixelsComplete(ResourceId id); - void AbortSetPixels(ResourceId id); // Acquire and release an image. The image allows direct // manipulation of texture memory. diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc index e763d0e..5790774 100644 --- a/cc/resources/resource_provider_unittest.cc +++ b/cc/resources/resource_provider_unittest.cc @@ -1393,6 +1393,7 @@ TEST_P(ResourceProviderTest, TextureAllocation) { EXPECT_CALL(*context, asyncTexImage2DCHROMIUM(_, _, _, 2, 2, _, _, _, _)) .Times(1); resource_provider->BeginSetPixels(id); + ASSERT_TRUE(resource_provider->DidSetPixelsComplete(id)); resource_provider->ReleasePixelBuffer(id); @@ -1445,54 +1446,6 @@ TEST_P(ResourceProviderTest, ForcingAsyncUploadToComplete) { Mock::VerifyAndClearExpectations(context); } -TEST_P(ResourceProviderTest, AbortForcedAsyncUpload) { - // Only for GL textures. - if (GetParam() != ResourceProvider::GLTexture) - return; - scoped_ptr<WebKit::WebGraphicsContext3D> mock_context( - static_cast<WebKit::WebGraphicsContext3D*>( - new StrictMock<AllocationTrackingContext3D>)); - scoped_ptr<OutputSurface> output_surface( - FakeOutputSurface::Create3d(mock_context.Pass())); - - gfx::Size size(2, 2); - WGC3Denum format = GL_RGBA; - ResourceProvider::ResourceId id = 0; - int texture_id = 123; - - AllocationTrackingContext3D* context = - static_cast<AllocationTrackingContext3D*>(output_surface->context3d()); - scoped_ptr<ResourceProvider> resource_provider( - ResourceProvider::Create(output_surface.get(), 0)); - - id = resource_provider->CreateResource( - size, format, ResourceProvider::TextureUsageAny); - resource_provider->AcquirePixelBuffer(id); - - EXPECT_CALL(*context, createTexture()).WillOnce(Return(texture_id)); - EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, texture_id)).Times(2); - EXPECT_CALL(*context, asyncTexImage2DCHROMIUM(_, _, _, 2, 2, _, _, _, _)) - .Times(1); - resource_provider->BeginSetPixels(id); - - EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, texture_id)).Times(1); - EXPECT_CALL(*context, waitAsyncTexImage2DCHROMIUM(GL_TEXTURE_2D)).Times(1); - EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, 0)).Times(1); - resource_provider->ForceSetPixelsToComplete(id); - - EXPECT_CALL(*context, deleteTexture(texture_id)).Times(1); - EXPECT_CALL(*context, createTexture()).WillOnce(Return(texture_id)); - EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, texture_id)).Times(1); - resource_provider->AbortSetPixels(id); - - resource_provider->ReleasePixelBuffer(id); - - EXPECT_CALL(*context, deleteTexture(texture_id)).Times(1); - resource_provider->DeleteResource(id); - - Mock::VerifyAndClearExpectations(context); -} - TEST_P(ResourceProviderTest, PixelBufferLostContext) { scoped_ptr<WebKit::WebGraphicsContext3D> mock_context( static_cast<WebKit::WebGraphicsContext3D*>( diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc index f3319af..9d499e9 100644 --- a/cc/resources/tile_manager.cc +++ b/cc/resources/tile_manager.cc @@ -120,6 +120,7 @@ TileManager::TileManager( use_color_estimator_(use_color_estimator), did_initialize_visible_tile_(false), texture_format_(texture_format) { + raster_worker_pool_->SetClient(this); } TileManager::~TileManager() { @@ -163,6 +164,10 @@ void TileManager::UnregisterTile(Tile* tile) { tiles_.erase(std::remove(tiles_.begin(), tiles_.end(), tile)); } +bool TileManager::ShouldForceTasksRequiredForActivationToComplete() const { + return client_->ShouldForceTileUploadsRequiredForActivationToComplete(); +} + class BinComparator { public: bool operator() (const Tile* a, const Tile* b) const { @@ -297,43 +302,6 @@ void TileManager::ManageTiles() { void TileManager::CheckForCompletedTileUploads() { raster_worker_pool_->CheckForCompletedTasks(); - - if (!client_->ShouldForceTileUploadsRequiredForActivationToComplete()) - return; - - TileSet initialized_tiles; - for (TileSet::iterator it = - tiles_that_need_to_be_initialized_for_activation_.begin(); - it != tiles_that_need_to_be_initialized_for_activation_.end(); - ++it) { - Tile* tile = *it; - ManagedTileState& mts = tile->managed_state(); - - for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) { - ManagedTileState::TileVersion& pending_version = - mts.tile_versions[mode]; - if (!pending_version.raster_task_.is_null() && - !pending_version.forced_upload_) { - if (!raster_worker_pool_->ForceUploadToComplete( - pending_version.raster_task_)) { - continue; - } - // Setting |forced_upload_| to true makes this tile version - // ready to draw. - pending_version.forced_upload_ = true; - initialized_tiles.insert(tile); - break; - } - } - } - - for (TileSet::iterator it = initialized_tiles.begin(); - it != initialized_tiles.end(); - ++it) { - Tile* tile = *it; - DidFinishTileInitialization(tile); - DCHECK(tile->IsReadyToDraw(NULL)); - } } void TileManager::GetMemoryStats( @@ -599,8 +567,6 @@ void TileManager::FreeResourceForTile(Tile* tile, RasterMode mode) { resource_pool_->ReleaseResource( mts.tile_versions[mode].resource_.Pass()); } - mts.tile_versions[mode].resource_id_ = 0; - mts.tile_versions[mode].forced_upload_ = false; } void TileManager::FreeResourcesForTile(Tile* tile) { @@ -639,7 +605,7 @@ void TileManager::ScheduleTasks() { if (tile_version.raster_task_.is_null()) tile_version.raster_task_ = CreateRasterTask(tile, &decoded_images); - tasks.Append(tile_version.raster_task_); + tasks.Append(tile_version.raster_task_, tile->required_for_activation()); } // Schedule running of |tasks|. This replaces any previously @@ -695,11 +661,6 @@ RasterWorkerPool::RasterTask TileManager::CreateRasterTask( texture_format_); const Resource* const_resource = resource.get(); - DCHECK(!mts.tile_versions[mts.raster_mode].resource_id_); - DCHECK(!mts.tile_versions[mts.raster_mode].forced_upload_); - mts.tile_versions[mts.raster_mode].resource_id_ = resource->id(); - mts.tile_versions[mts.raster_mode].resource_format_ = texture_format_; - // Create and queue all image decode tasks that this tile depends on. RasterWorkerPool::Task::Set decode_tasks; for (PicturePileImpl::PixelRefIterator iter(tile->content_rect(), @@ -768,7 +729,6 @@ void TileManager::OnRasterTaskCompleted( if (was_canceled) { resource_pool_->ReleaseResource(resource.Pass()); - tile_version.resource_id_ = 0; return; } @@ -778,7 +738,6 @@ void TileManager::OnRasterTaskCompleted( resource_pool_->ReleaseResource(resource.Pass()); } else { tile_version.resource_ = resource.Pass(); - tile_version.forced_upload_ = false; } FreeUnusedResourcesForTile(tile.get()); diff --git a/cc/resources/tile_manager.h b/cc/resources/tile_manager.h index 7c22814..4c87af0 100644 --- a/cc/resources/tile_manager.h +++ b/cc/resources/tile_manager.h @@ -60,7 +60,7 @@ scoped_ptr<base::Value> TileManagerBinPriorityAsValue( // should no longer have any memory assigned to them. Tile objects are "owned" // by layers; they automatically register with the manager when they are // created, and unregister from the manager when they are deleted. -class CC_EXPORT TileManager { +class CC_EXPORT TileManager : public RasterWorkerPoolClient { public: typedef base::hash_set<uint32_t> PixelRefSet; @@ -109,6 +109,10 @@ class CC_EXPORT TileManager { void RegisterTile(Tile* tile); void UnregisterTile(Tile* tile); + // Overriden from RasterWorkerPoolClient: + virtual bool ShouldForceTasksRequiredForActivationToComplete() const + OVERRIDE; + // Virtual for test virtual void ScheduleTasks(); |