Commit f8c96db7 authored by vmpstr's avatar vmpstr Committed by Commit bot

cc: Move (partially) the predecode image tracking to ImageManager.

This patch moves some of the predecode image tracking to ImageManager.
The other part depends on crbug.com/647402. Once the TileTaskRunner
can keep a ref on scheduled tiles, then we can remove
TileManager::locked_image_tasks_ as well.

R=enne
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2345833002
Cr-Commit-Position: refs/heads/master@{#419539}
parent 4d7456e2
...@@ -916,6 +916,7 @@ test("cc_unittests") { ...@@ -916,6 +916,7 @@ test("cc_unittests") {
"test/ordered_simple_task_runner_unittest.cc", "test/ordered_simple_task_runner_unittest.cc",
"test/test_web_graphics_context_3d_unittest.cc", "test/test_web_graphics_context_3d_unittest.cc",
"tiles/gpu_image_decode_controller_unittest.cc", "tiles/gpu_image_decode_controller_unittest.cc",
"tiles/image_manager_unittest.cc",
"tiles/mipmap_util_unittest.cc", "tiles/mipmap_util_unittest.cc",
"tiles/picture_layer_tiling_set_unittest.cc", "tiles/picture_layer_tiling_set_unittest.cc",
"tiles/picture_layer_tiling_unittest.cc", "tiles/picture_layer_tiling_unittest.cc",
......
...@@ -10,6 +10,14 @@ ImageManager::ImageManager() = default; ...@@ -10,6 +10,14 @@ ImageManager::ImageManager() = default;
ImageManager::~ImageManager() = default; ImageManager::~ImageManager() = default;
void ImageManager::SetImageDecodeController(ImageDecodeController* controller) { void ImageManager::SetImageDecodeController(ImageDecodeController* controller) {
// We can only switch from null to non-null and back.
DCHECK(controller || controller_);
DCHECK(!controller || !controller_);
if (!controller) {
SetPredecodeImages(std::vector<DrawImage>(),
ImageDecodeController::TracingInfo());
}
controller_ = controller; controller_ = controller;
} }
...@@ -43,4 +51,14 @@ void ImageManager::ReduceMemoryUsage() { ...@@ -43,4 +51,14 @@ void ImageManager::ReduceMemoryUsage() {
controller_->ReduceCacheUsage(); controller_->ReduceCacheUsage();
} }
std::vector<scoped_refptr<TileTask>> ImageManager::SetPredecodeImages(
std::vector<DrawImage> images,
const ImageDecodeController::TracingInfo& tracing_info) {
std::vector<scoped_refptr<TileTask>> new_tasks;
GetTasksForImagesAndRef(&images, &new_tasks, tracing_info);
UnrefImages(predecode_locked_images_);
predecode_locked_images_ = std::move(images);
return new_tasks;
}
} // namespace cc } // namespace cc
...@@ -28,9 +28,13 @@ class CC_EXPORT ImageManager { ...@@ -28,9 +28,13 @@ class CC_EXPORT ImageManager {
const ImageDecodeController::TracingInfo& tracing_info); const ImageDecodeController::TracingInfo& tracing_info);
void UnrefImages(const std::vector<DrawImage>& images); void UnrefImages(const std::vector<DrawImage>& images);
void ReduceMemoryUsage(); void ReduceMemoryUsage();
std::vector<scoped_refptr<TileTask>> SetPredecodeImages(
std::vector<DrawImage> predecode_images,
const ImageDecodeController::TracingInfo& tracing_info);
private: private:
ImageDecodeController* controller_; ImageDecodeController* controller_ = nullptr;
std::vector<DrawImage> predecode_locked_images_;
DISALLOW_COPY_AND_ASSIGN(ImageManager); DISALLOW_COPY_AND_ASSIGN(ImageManager);
}; };
......
// Copyright 2016 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/tiles/image_decode_controller.h"
#include "cc/tiles/image_manager.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace cc {
class TestableController : public ImageDecodeController {
public:
bool GetTaskForImageAndRef(const DrawImage& image,
const TracingInfo& tracing_info,
scoped_refptr<TileTask>* task) override {
*task = nullptr;
++number_of_refs_;
return true;
}
void UnrefImage(const DrawImage& image) override {
ASSERT_GT(number_of_refs_, 0);
--number_of_refs_;
}
DecodedDrawImage GetDecodedImageForDraw(const DrawImage& image) override {
return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
}
void DrawWithImageFinished(const DrawImage& image,
const DecodedDrawImage& decoded_image) override {}
void ReduceCacheUsage() override {}
void SetShouldAggressivelyFreeResources(
bool aggressively_free_resources) override {}
int number_of_refs() const { return number_of_refs_; }
private:
int number_of_refs_ = 0;
};
TEST(ImageManagerTest, NullControllerUnrefsImages) {
TestableController controller;
ImageManager manager;
manager.SetImageDecodeController(&controller);
std::vector<DrawImage> images(10);
ImageDecodeController::TracingInfo tracing_info;
ASSERT_EQ(10u, images.size());
auto tasks = manager.SetPredecodeImages(std::move(images), tracing_info);
EXPECT_EQ(0u, tasks.size());
EXPECT_EQ(10, controller.number_of_refs());
manager.SetImageDecodeController(nullptr);
EXPECT_EQ(0, controller.number_of_refs());
}
} // namespace cc
...@@ -361,8 +361,7 @@ void TileManager::FinishTasksAndCleanUp() { ...@@ -361,8 +361,7 @@ void TileManager::FinishTasksAndCleanUp() {
signals_check_notifier_.Cancel(); signals_check_notifier_.Cancel();
task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs(); task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
image_manager_.UnrefImages(locked_images_); image_manager_.SetImageDecodeController(nullptr);
locked_images_.clear();
locked_image_tasks_.clear(); locked_image_tasks_.clear();
} }
...@@ -845,21 +844,25 @@ void TileManager::ScheduleTasks( ...@@ -845,21 +844,25 @@ void TileManager::ScheduleTasks(
const std::vector<PrioritizedTile>& tiles_to_process_for_images = const std::vector<PrioritizedTile>& tiles_to_process_for_images =
work_to_schedule.tiles_to_process_for_images; work_to_schedule.tiles_to_process_for_images;
std::vector<DrawImage> new_locked_images; std::vector<DrawImage> new_locked_images;
std::vector<scoped_refptr<TileTask>> new_locked_image_tasks;
for (const PrioritizedTile& prioritized_tile : tiles_to_process_for_images) { for (const PrioritizedTile& prioritized_tile : tiles_to_process_for_images) {
Tile* tile = prioritized_tile.tile(); Tile* tile = prioritized_tile.tile();
std::vector<DrawImage> images; std::vector<DrawImage> images;
prioritized_tile.raster_source()->GetDiscardableImagesInRect( prioritized_tile.raster_source()->GetDiscardableImagesInRect(
tile->enclosing_layer_rect(), tile->contents_scale(), &images); tile->enclosing_layer_rect(), tile->contents_scale(), &images);
ImageDecodeController::TracingInfo tracing_info(
prepare_tiles_count_, prioritized_tile.priority().priority_bin);
image_manager_.GetTasksForImagesAndRef(&images, &new_locked_image_tasks,
tracing_info);
new_locked_images.insert(new_locked_images.end(), images.begin(), new_locked_images.insert(new_locked_images.end(), images.begin(),
images.end()); images.end());
} }
// TODO(vmpstr): SOON is misleading here, but these images can come from
// several diffent tiles. Rethink what we actually want to trace here. Note
// that I'm using SOON, since it can't be NOW (these are prepaint).
ImageDecodeController::TracingInfo tracing_info(prepare_tiles_count_,
TilePriority::SOON);
std::vector<scoped_refptr<TileTask>> new_locked_image_tasks =
image_manager_.SetPredecodeImages(std::move(new_locked_images),
tracing_info);
for (auto& task : new_locked_image_tasks) { for (auto& task : new_locked_image_tasks) {
auto decode_it = std::find_if(graph_.nodes.begin(), graph_.nodes.end(), auto decode_it = std::find_if(graph_.nodes.begin(), graph_.nodes.end(),
[&task](const TaskGraph::Node& node) { [&task](const TaskGraph::Node& node) {
...@@ -874,10 +877,11 @@ void TileManager::ScheduleTasks( ...@@ -874,10 +877,11 @@ void TileManager::ScheduleTasks(
graph_.edges.push_back(TaskGraph::Edge(task.get(), all_done_task.get())); graph_.edges.push_back(TaskGraph::Edge(task.get(), all_done_task.get()));
} }
image_manager_.UnrefImages(locked_images_); // The old locked images tasks have to stay around until past the
// The old locked images have to stay around until past the ScheduleTasks call // ScheduleTasks call below, so we do a swap instead of a move.
// below, so we do a swap instead of a move. // TODO(crbug.com/647402): Have the tile_task_manager keep a ref on the tasks,
locked_images_.swap(new_locked_images); // since it makes it awkward for the callers to keep refs on tasks that only
// exist within the task graph runner.
locked_image_tasks_.swap(new_locked_image_tasks); locked_image_tasks_.swap(new_locked_image_tasks);
// We must reduce the amount of unused resources before calling // We must reduce the amount of unused resources before calling
...@@ -997,8 +1001,7 @@ void TileManager::OnRasterTaskCompleted( ...@@ -997,8 +1001,7 @@ void TileManager::OnRasterTaskCompleted(
// Unref all the images. // Unref all the images.
auto images_it = scheduled_draw_images_.find(tile->id()); auto images_it = scheduled_draw_images_.find(tile->id());
const std::vector<DrawImage>& images = images_it->second; image_manager_.UnrefImages(images_it->second);
image_manager_.UnrefImages(images);
scheduled_draw_images_.erase(images_it); scheduled_draw_images_.erase(images_it);
if (was_canceled) { if (was_canceled) {
...@@ -1148,8 +1151,8 @@ void TileManager::CheckIfMoreTilesNeedToBePrepared() { ...@@ -1148,8 +1151,8 @@ void TileManager::CheckIfMoreTilesNeedToBePrepared() {
// If we're not in SMOOTHNESS_TAKES_PRIORITY mode, we should unlock all // If we're not in SMOOTHNESS_TAKES_PRIORITY mode, we should unlock all
// images since we're technically going idle here at least for this frame. // images since we're technically going idle here at least for this frame.
if (global_state_.tree_priority != SMOOTHNESS_TAKES_PRIORITY) { if (global_state_.tree_priority != SMOOTHNESS_TAKES_PRIORITY) {
image_manager_.UnrefImages(locked_images_); image_manager_.SetPredecodeImages(std::vector<DrawImage>(),
locked_images_.clear(); ImageDecodeController::TracingInfo());
locked_image_tasks_.clear(); locked_image_tasks_.clear();
} }
......
...@@ -343,7 +343,6 @@ class CC_EXPORT TileManager { ...@@ -343,7 +343,6 @@ class CC_EXPORT TileManager {
uint64_t next_tile_id_; uint64_t next_tile_id_;
std::unordered_map<Tile::Id, std::vector<DrawImage>> scheduled_draw_images_; std::unordered_map<Tile::Id, std::vector<DrawImage>> scheduled_draw_images_;
std::vector<DrawImage> locked_images_;
std::vector<scoped_refptr<TileTask>> locked_image_tasks_; std::vector<scoped_refptr<TileTask>> locked_image_tasks_;
base::WeakPtrFactory<TileManager> task_set_finished_weak_ptr_factory_; base::WeakPtrFactory<TileManager> task_set_finished_weak_ptr_factory_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment