Skip to content

Commit

Permalink
CopyOutputRequest: refactor format to separate out the destination
Browse files Browse the repository at this point in the history
Currently, CopyOutputRequest & Result use Format both to express the
pixel format of the result, and the medium via which the result will be
returned. This CL introduces a separate enum for the result destination.
The change will allow additions of more formats to CopyOutputRequest
without causing exponential growth in enum values.

There should be no behavior change introduced by this CL.

Part of zero-copy capture work, (internal) design doc:
go/zero-copy-capture

Bug: 1233108
Change-Id: I0b3b54305606b8e144bd338c2be5cf4227399a92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3044550
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906789}
  • Loading branch information
bialpio authored and Chromium LUCI CQ committed Jul 29, 2021
1 parent 63a9421 commit 95d8f62
Show file tree
Hide file tree
Showing 53 changed files with 579 additions and 299 deletions.
6 changes: 4 additions & 2 deletions ash/rotator/screen_rotation_animator.cc
Expand Up @@ -211,7 +211,8 @@ void ScreenRotationAnimator::StartRotationAnimation(
current_async_rotation_request_ = ScreenRotationRequest(*rotation_request);
RequestCopyScreenRotationContainerLayer(
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
CreateAfterCopyCallbackBeforeRotation(
std::move(rotation_request))));
screen_rotation_state_ = COPY_REQUESTED;
Expand Down Expand Up @@ -320,7 +321,8 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedBeforeRotation(

RequestCopyScreenRotationContainerLayer(
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
CreateAfterCopyCallbackAfterRotation(std::move(rotation_request))));
}

Expand Down
10 changes: 7 additions & 3 deletions ash/utility/layer_util.cc
Expand Up @@ -18,7 +18,9 @@ void CopyCopyOutputResultToLayer(
std::unique_ptr<viz::CopyOutputResult> copy_result,
ui::Layer* target_layer) {
DCHECK(!copy_result->IsEmpty());
DCHECK_EQ(copy_result->format(), viz::CopyOutputResult::Format::RGBA_TEXTURE);
DCHECK_EQ(copy_result->format(), viz::CopyOutputResult::Format::RGBA);
DCHECK_EQ(copy_result->destination(),
viz::CopyOutputResult::Destination::kNativeTextures);

viz::TransferableResource transferable_resource =
viz::TransferableResource::MakeGL(
Expand Down Expand Up @@ -78,7 +80,8 @@ void CopyLayerContentToNewLayer(ui::Layer* layer, LayerCopyCallback callback) {
auto new_callback = base::BindOnce(&CopyToNewLayerOnCopyRequestFinished,
std::move(callback), layer->size());
auto copy_request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
std::move(new_callback));
gfx::Rect bounds(layer->size());
copy_request->set_area(bounds);
Expand All @@ -91,7 +94,8 @@ void CopyLayerContentToLayer(ui::Layer* layer,
auto new_callback =
base::BindOnce(&CopyToLayerOnCopyRequestFinished, std::move(callback));
auto copy_request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
std::move(new_callback));
gfx::Rect bounds(layer->size());
copy_request->set_area(bounds);
Expand Down
3 changes: 2 additions & 1 deletion ash/wm/desks/root_window_desk_switch_animator.cc
Expand Up @@ -77,7 +77,8 @@ void TakeScreenshot(

const gfx::Rect request_bounds(screenshot_layer->size());
auto screenshot_request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
std::move(on_screenshot_taken));
screenshot_request->set_area(request_bounds);
screenshot_request->set_result_task_runner(
Expand Down
18 changes: 12 additions & 6 deletions cc/layers/layer_unittest.cc
Expand Up @@ -1395,7 +1395,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) {
// layer does not abort either one.
std::unique_ptr<viz::CopyOutputRequest> request =
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&ReceiveCopyOutputResultAtomic,
base::Unretained(&result_count)));
layer->RequestCopyOfOutput(std::move(request));
Expand All @@ -1404,7 +1405,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) {
CCTestSuite::RunUntilIdle();
EXPECT_EQ(0, result_count.load());
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&ReceiveCopyOutputResultAtomic,
base::Unretained(&result_count)));
layer->RequestCopyOfOutput(std::move(request));
Expand All @@ -1426,7 +1428,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) {
// the second request using |kArbitrarySourceId1| is made.
int did_receive_first_result_from_this_source = 0;
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&ReceiveCopyOutputResult,
&did_receive_first_result_from_this_source));
request->set_source(kArbitrarySourceId1);
Expand All @@ -1438,7 +1441,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) {
// Make a request from a different source.
int did_receive_result_from_different_source = 0;
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&ReceiveCopyOutputResult,
&did_receive_result_from_different_source));
request->set_source(kArbitrarySourceId2);
Expand All @@ -1450,7 +1454,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) {
// Make a request without specifying the source.
int did_receive_result_from_anonymous_source = 0;
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&ReceiveCopyOutputResult,
&did_receive_result_from_anonymous_source));
layer->RequestCopyOfOutput(std::move(request));
Expand All @@ -1461,7 +1466,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) {
// Make the second request from |kArbitrarySourceId1|.
int did_receive_second_result_from_this_source = 0;
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&ReceiveCopyOutputResult,
&did_receive_second_result_from_this_source));
request->set_source(kArbitrarySourceId1);
Expand Down
7 changes: 5 additions & 2 deletions cc/test/layer_tree_pixel_test.cc
Expand Up @@ -176,15 +176,18 @@ LayerTreePixelTest::CreateDisplayOutputSurfaceOnThread(
std::unique_ptr<viz::CopyOutputRequest>
LayerTreePixelTest::CreateCopyOutputRequest() {
return std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputResult::Destination::kSystemMemory,
base::BindOnce(&LayerTreePixelTest::ReadbackResult,
base::Unretained(this)));
}

void LayerTreePixelTest::ReadbackResult(
std::unique_ptr<viz::CopyOutputResult> result) {
ASSERT_FALSE(result->IsEmpty());
EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_BITMAP);
EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA);
EXPECT_EQ(result->destination(),
viz::CopyOutputResult::Destination::kSystemMemory);
auto scoped_bitmap = result->ScopedAccessSkBitmap();
result_bitmap_ =
std::make_unique<SkBitmap>(scoped_bitmap.GetOutScopedBitmap());
Expand Down
10 changes: 7 additions & 3 deletions cc/test/pixel_test.cc
Expand Up @@ -118,7 +118,8 @@ bool PixelTest::RunPixelTestWithReadbackTargetAndArea(

std::unique_ptr<viz::CopyOutputRequest> request =
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&PixelTest::ReadbackResult, base::Unretained(this),
run_loop.QuitClosure()));
if (copy_rect)
Expand Down Expand Up @@ -156,7 +157,8 @@ bool PixelTest::RunPixelTest(viz::AggregatedRenderPassList* pass_list,

std::unique_ptr<viz::CopyOutputRequest> request =
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(&PixelTest::ReadbackResult, base::Unretained(this),
run_loop.QuitClosure()));
target->copy_requests.push_back(std::move(request));
Expand Down Expand Up @@ -203,7 +205,9 @@ bool PixelTest::RunPixelTest(viz::AggregatedRenderPassList* pass_list,
void PixelTest::ReadbackResult(base::OnceClosure quit_run_loop,
std::unique_ptr<viz::CopyOutputResult> result) {
ASSERT_FALSE(result->IsEmpty());
EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_BITMAP);
EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA);
EXPECT_EQ(result->destination(),
viz::CopyOutputResult::Destination::kSystemMemory);
auto scoped_sk_bitmap = result->ScopedAccessSkBitmap();
result_bitmap_ =
std::make_unique<SkBitmap>(scoped_sk_bitmap.GetOutScopedBitmap());
Expand Down
3 changes: 2 additions & 1 deletion cc/trees/layer_tree_host_impl_unittest.cc
Expand Up @@ -11680,7 +11680,8 @@ TEST_P(LayerTreeHostImplTestWithRenderer, ShutdownReleasesContext) {
GetPropertyTrees(root)->effect_tree.AddCopyRequest(
root->effect_tree_index(),
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
base::BindOnce(&Helper::OnResult, base::Unretained(&helper),
copy_request_run_loop.QuitClosure())));
DrawFrame();
Expand Down
10 changes: 7 additions & 3 deletions cc/trees/layer_tree_host_pixeltest_readback.cc
Expand Up @@ -66,14 +66,16 @@ class LayerTreeHostReadbackPixelTest

if (readback_type() == TestReadBackType::kBitmap) {
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kSystemMemory,
base::BindOnce(
&LayerTreeHostReadbackPixelTest::ReadbackResultAsBitmap,
base::Unretained(this)));
} else {
DCHECK_NE(renderer_type_, viz::RendererType::kSoftware);
request = std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
viz::CopyOutputRequest::ResultFormat::RGBA,
viz::CopyOutputRequest::ResultDestination::kNativeTextures,
base::BindOnce(
&LayerTreeHostReadbackPixelTest::ReadbackResultAsTexture,
base::Unretained(this)));
Expand Down Expand Up @@ -114,7 +116,9 @@ class LayerTreeHostReadbackPixelTest

void ReadbackResultAsTexture(std::unique_ptr<viz::CopyOutputResult> result) {
EXPECT_TRUE(task_runner_provider()->IsMainThread());
ASSERT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_TEXTURE);
ASSERT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA);
ASSERT_EQ(result->destination(),
viz::CopyOutputResult::Destination::kNativeTextures);

gpu::Mailbox mailbox = result->GetTextureResult()->mailbox;
gpu::SyncToken sync_token = result->GetTextureResult()->sync_token;
Expand Down

0 comments on commit 95d8f62

Please sign in to comment.