From 95d8f62132f3e19dfb9916ea0be3a327e276ec57 Mon Sep 17 00:00:00 2001 From: Piotr Bialecki Date: Thu, 29 Jul 2021 18:58:44 +0000 Subject: [PATCH] CopyOutputRequest: refactor format to separate out the destination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: François Beaufort Reviewed-by: Ahmed Fakhry Reviewed-by: mark a. foltz Reviewed-by: Jeffrey Kardatzke Reviewed-by: Ted Choc Reviewed-by: Mitsuru Oshima Reviewed-by: Dominick Ng Reviewed-by: Alex Moshchuk Reviewed-by: kylechar Reviewed-by: David Tseng Reviewed-by: Joe Downing Commit-Queue: Piotr Bialecki Cr-Commit-Position: refs/heads/master@{#906789} --- ash/rotator/screen_rotation_animator.cc | 6 +- ash/utility/layer_util.cc | 10 +- .../desks/root_window_desk_switch_animator.cc | 3 +- cc/layers/layer_unittest.cc | 18 +- cc/test/layer_tree_pixel_test.cc | 7 +- cc/test/pixel_test.cc | 10 +- cc/trees/layer_tree_host_impl_unittest.cc | 3 +- .../layer_tree_host_pixeltest_readback.cc | 10 +- .../layer_tree_host_unittest_copyrequest.cc | 59 ++++-- .../arc_screen_capture_session.cc | 3 +- ...n_picture_window_controller_browsertest.cc | 7 +- ...cessibility_focus_highlight_browsertest.cc | 3 +- components/exo/drag_drop_operation.cc | 3 +- components/exo/pointer.cc | 3 +- .../common/frame_sinks/copy_output_request.cc | 13 +- .../common/frame_sinks/copy_output_request.h | 15 +- .../common/frame_sinks/copy_output_result.cc | 39 ++-- .../common/frame_sinks/copy_output_result.h | 57 ++++-- .../display/copy_output_scaling_pixeltest.cc | 13 +- .../viz/service/display/display_unittest.cc | 6 +- .../viz/service/display/gl_renderer_copier.cc | 63 +++--- .../viz/service/display/gl_renderer_copier.h | 12 +- .../display/gl_renderer_copier_perftest.cc | 30 ++- .../display/gl_renderer_copier_pixeltest.cc | 16 +- .../display/gl_renderer_copier_unittest.cc | 3 +- .../display/skia_readback_pixeltest.cc | 11 +- .../viz/service/display/software_renderer.cc | 19 +- .../display/software_renderer_unittest.cc | 7 +- .../skia_output_surface_impl_on_gpu.cc | 15 +- .../skia_output_surface_impl_unittest.cc | 9 +- .../compositor_frame_sink_support_unittest.cc | 27 ++- .../frame_sink_video_capturer_impl.cc | 21 +- ...frame_sink_video_capturer_impl_unittest.cc | 5 +- .../service/surfaces/surface_saved_frame.cc | 15 +- .../viz/service/surfaces/surface_unittest.cc | 3 +- .../viz/test/fake_skia_output_surface.cc | 6 +- .../capture/slow_window_capturer_chromeos.cc | 3 +- .../renderer_host/delegated_frame_host.cc | 18 +- .../renderer_host/delegated_frame_host.h | 1 + .../render_widget_host_view_child_frame.cc | 3 +- .../host/chromeos/aura_desktop_capturer.cc | 3 +- .../copy_output_request_mojom_traits.cc | 16 +- .../copy_output_request_mojom_traits.h | 7 + .../copy_output_result_mojom_traits.cc | 181 +++++++++++------- .../copy_output_result_mojom_traits.h | 13 ++ .../cpp/compositing/mojom_traits_unittest.cc | 47 +++-- .../compositing/copy_output_request.mojom | 1 + .../compositing/copy_output_result.mojom | 18 +- ui/android/delegated_frame_host_android.cc | 3 +- ui/android/delegated_frame_host_android.h | 4 +- ui/compositor/layer_unittest.cc | 3 +- ui/snapshot/snapshot_android.cc | 3 +- ui/snapshot/snapshot_aura.cc | 4 +- 53 files changed, 579 insertions(+), 299 deletions(-) diff --git a/ash/rotator/screen_rotation_animator.cc b/ash/rotator/screen_rotation_animator.cc index b9f1d6b701c03..a4b18c72d5604 100644 --- a/ash/rotator/screen_rotation_animator.cc +++ b/ash/rotator/screen_rotation_animator.cc @@ -211,7 +211,8 @@ void ScreenRotationAnimator::StartRotationAnimation( current_async_rotation_request_ = ScreenRotationRequest(*rotation_request); RequestCopyScreenRotationContainerLayer( std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kNativeTextures, CreateAfterCopyCallbackBeforeRotation( std::move(rotation_request)))); screen_rotation_state_ = COPY_REQUESTED; @@ -320,7 +321,8 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedBeforeRotation( RequestCopyScreenRotationContainerLayer( std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kNativeTextures, CreateAfterCopyCallbackAfterRotation(std::move(rotation_request)))); } diff --git a/ash/utility/layer_util.cc b/ash/utility/layer_util.cc index def523cc715be..da6da8edc41e9 100644 --- a/ash/utility/layer_util.cc +++ b/ash/utility/layer_util.cc @@ -18,7 +18,9 @@ void CopyCopyOutputResultToLayer( std::unique_ptr 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( @@ -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::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); @@ -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::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); diff --git a/ash/wm/desks/root_window_desk_switch_animator.cc b/ash/wm/desks/root_window_desk_switch_animator.cc index 682fc08809219..2eb3a354af6de 100644 --- a/ash/wm/desks/root_window_desk_switch_animator.cc +++ b/ash/wm/desks/root_window_desk_switch_animator.cc @@ -77,7 +77,8 @@ void TakeScreenshot( const gfx::Rect request_bounds(screenshot_layer->size()); auto screenshot_request = std::make_unique( - 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( diff --git a/cc/layers/layer_unittest.cc b/cc/layers/layer_unittest.cc index 61b4890e82c87..6ae8e295a231b 100644 --- a/cc/layers/layer_unittest.cc +++ b/cc/layers/layer_unittest.cc @@ -1395,7 +1395,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) { // layer does not abort either one. std::unique_ptr request = std::make_unique( - 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)); @@ -1404,7 +1405,8 @@ TEST_F(LayerTest, DedupesCopyOutputRequestsBySource) { CCTestSuite::RunUntilIdle(); EXPECT_EQ(0, result_count.load()); request = std::make_unique( - 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)); @@ -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::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); @@ -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::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&ReceiveCopyOutputResult, &did_receive_result_from_different_source)); request->set_source(kArbitrarySourceId2); @@ -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::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)); @@ -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::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); diff --git a/cc/test/layer_tree_pixel_test.cc b/cc/test/layer_tree_pixel_test.cc index 645dcba869fe1..c4e7342f8d3f6 100644 --- a/cc/test/layer_tree_pixel_test.cc +++ b/cc/test/layer_tree_pixel_test.cc @@ -176,7 +176,8 @@ LayerTreePixelTest::CreateDisplayOutputSurfaceOnThread( std::unique_ptr LayerTreePixelTest::CreateCopyOutputRequest() { return std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputResult::Destination::kSystemMemory, base::BindOnce(&LayerTreePixelTest::ReadbackResult, base::Unretained(this))); } @@ -184,7 +185,9 @@ LayerTreePixelTest::CreateCopyOutputRequest() { void LayerTreePixelTest::ReadbackResult( std::unique_ptr 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(scoped_bitmap.GetOutScopedBitmap()); diff --git a/cc/test/pixel_test.cc b/cc/test/pixel_test.cc index 24050baaf48aa..ccda26f5dba4a 100644 --- a/cc/test/pixel_test.cc +++ b/cc/test/pixel_test.cc @@ -118,7 +118,8 @@ bool PixelTest::RunPixelTestWithReadbackTargetAndArea( std::unique_ptr request = std::make_unique( - 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) @@ -156,7 +157,8 @@ bool PixelTest::RunPixelTest(viz::AggregatedRenderPassList* pass_list, std::unique_ptr request = std::make_unique( - 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)); @@ -203,7 +205,9 @@ bool PixelTest::RunPixelTest(viz::AggregatedRenderPassList* pass_list, void PixelTest::ReadbackResult(base::OnceClosure quit_run_loop, std::unique_ptr 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(scoped_sk_bitmap.GetOutScopedBitmap()); diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc index c499105c9617a..9c8a3ccf30250 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -11680,7 +11680,8 @@ TEST_P(LayerTreeHostImplTestWithRenderer, ShutdownReleasesContext) { GetPropertyTrees(root)->effect_tree.AddCopyRequest( root->effect_tree_index(), std::make_unique( - 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(); diff --git a/cc/trees/layer_tree_host_pixeltest_readback.cc b/cc/trees/layer_tree_host_pixeltest_readback.cc index 55767ae27d160..8f8867f019d29 100644 --- a/cc/trees/layer_tree_host_pixeltest_readback.cc +++ b/cc/trees/layer_tree_host_pixeltest_readback.cc @@ -66,14 +66,16 @@ class LayerTreeHostReadbackPixelTest if (readback_type() == TestReadBackType::kBitmap) { request = std::make_unique( - 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::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kNativeTextures, base::BindOnce( &LayerTreeHostReadbackPixelTest::ReadbackResultAsTexture, base::Unretained(this))); @@ -114,7 +116,9 @@ class LayerTreeHostReadbackPixelTest void ReadbackResultAsTexture(std::unique_ptr 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; diff --git a/cc/trees/layer_tree_host_unittest_copyrequest.cc b/cc/trees/layer_tree_host_unittest_copyrequest.cc index 07f3476fd4c3e..b9079a9e8d70a 100644 --- a/cc/trees/layer_tree_host_unittest_copyrequest.cc +++ b/cc/trees/layer_tree_host_unittest_copyrequest.cc @@ -95,7 +95,8 @@ class LayerTreeHostCopyRequestTestMultipleRequests switch (frame) { case 1: child->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&LayerTreeHostCopyRequestTestMultipleRequests:: CopyOutputCallback, base::Unretained(this), 0))); @@ -113,18 +114,21 @@ class LayerTreeHostCopyRequestTestMultipleRequests EXPECT_EQ(gfx::Size(10, 10).ToString(), callbacks_[0].ToString()); child->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&LayerTreeHostCopyRequestTestMultipleRequests:: CopyOutputCallback, base::Unretained(this), 1))); root->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&LayerTreeHostCopyRequestTestMultipleRequests:: CopyOutputCallback, base::Unretained(this), 2))); grand_child->RequestCopyOfOutput( std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&LayerTreeHostCopyRequestTestMultipleRequests:: CopyOutputCallback, base::Unretained(this), 3))); @@ -260,7 +264,8 @@ class LayerTreeHostCopyRequestCompletionCausesCommit switch (frame) { case 1: layer_->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&LayerTreeHostCopyRequestCompletionCausesCommit:: CopyOutputCallback))); break; @@ -326,13 +331,15 @@ class LayerTreeHostCopyRequestTestLayerDestroyed case 1: main_destroyed_->RequestCopyOfOutput(std::make_unique< viz::CopyOutputRequest>( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostCopyRequestTestLayerDestroyed::CopyOutputCallback, base::Unretained(this), &main_destroyed_event_))); impl_destroyed_->RequestCopyOfOutput(std::make_unique< viz::CopyOutputRequest>( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostCopyRequestTestLayerDestroyed::CopyOutputCallback, base::Unretained(this), &impl_destroyed_event_))); @@ -437,7 +444,8 @@ class LayerTreeHostCopyRequestTestInHiddenSubtree void AddCopyRequest(Layer* layer) { layer->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostCopyRequestTestInHiddenSubtree::CopyOutputCallback, base::Unretained(this)))); @@ -563,7 +571,8 @@ class LayerTreeHostTestHiddenSurfaceNotAllocatedForSubtreeCopyRequest PostSetNeedsCommitToMainThread(); copy_layer_->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostTestHiddenSurfaceNotAllocatedForSubtreeCopyRequest:: CopyOutputCallback, @@ -670,7 +679,8 @@ class LayerTreeHostCopyRequestTestClippedOut PostSetNeedsCommitToMainThread(); copy_layer_->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostCopyRequestTestClippedOut::CopyOutputCallback, base::Unretained(this)))); @@ -733,7 +743,8 @@ class LayerTreeHostCopyRequestTestScaledLayer std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostCopyRequestTestScaledLayer::CopyOutputCallback, base::Unretained(this))); @@ -786,7 +797,8 @@ class LayerTreeHostTestAsyncTwoReadbacksWithoutDraw void AddCopyRequest(Layer* layer) { layer->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostTestAsyncTwoReadbacksWithoutDraw::CopyOutputCallback, base::Unretained(this)))); @@ -913,7 +925,9 @@ class LayerTreeHostCopyRequestTestDeleteSharedImage std::unique_ptr result) { EXPECT_TRUE(layer_tree_host()->GetTaskRunnerProvider()->IsMainThread()); EXPECT_EQ(gfx::Size(10, 10).ToString(), result->size().ToString()); - EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_TEXTURE); + EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA); + EXPECT_EQ(result->destination(), + viz::CopyOutputResult::Destination::kNativeTextures); EXPECT_NE(result->GetTextureResult(), nullptr); // Save the result for later. @@ -926,7 +940,8 @@ class LayerTreeHostCopyRequestTestDeleteSharedImage void InsertCopyRequest() { copy_layer_->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputResult::Destination::kNativeTextures, base::BindOnce(&LayerTreeHostCopyRequestTestDeleteSharedImage:: ReceiveCopyRequestOutputAndCommit, base::Unretained(this)))); @@ -1151,7 +1166,8 @@ class LayerTreeHostCopyRequestTestCreatesSharedImage void RequestCopy(Layer* layer) override { // Request a normal texture copy. This should create a new shared image. copy_layer_->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputResult::Destination::kNativeTextures, base::BindOnce( &LayerTreeHostCopyRequestTestCreatesSharedImage::CopyOutputCallback, base::Unretained(this)))); @@ -1159,7 +1175,9 @@ class LayerTreeHostCopyRequestTestCreatesSharedImage void CopyOutputCallback(std::unique_ptr result) { EXPECT_FALSE(result->IsEmpty()); - EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_TEXTURE); + EXPECT_EQ(result->format(), viz::CopyOutputResult::Format::RGBA); + EXPECT_EQ(result->destination(), + viz::CopyOutputResult::Destination::kNativeTextures); ASSERT_NE(nullptr, result->GetTextureResult()); release_ = result->TakeTextureOwnership(); EXPECT_TRUE(release_); @@ -1232,7 +1250,8 @@ class LayerTreeHostCopyRequestTestDestroyBeforeCopy // drawing to take place. std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputResult::Destination::kNativeTextures, base::BindOnce(&LayerTreeHostCopyRequestTestDestroyBeforeCopy:: CopyOutputCallback, base::Unretained(this))); @@ -1324,7 +1343,8 @@ class LayerTreeHostCopyRequestTestShutdownBeforeCopy // drawing to take place. std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputResult::Destination::kNativeTextures, base::BindOnce(&LayerTreeHostCopyRequestTestShutdownBeforeCopy:: CopyOutputCallback, base::Unretained(this))); @@ -1397,7 +1417,8 @@ class LayerTreeHostCopyRequestTestMultipleDrawsHiddenCopyRequest // Send a copy request after the first commit. if (layer_tree_host()->SourceFrameNumber() == 1) { child_->RequestCopyOfOutput(std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &LayerTreeHostCopyRequestTestMultipleDrawsHiddenCopyRequest:: CopyOutputCallback, diff --git a/chrome/browser/ash/arc/screen_capture/arc_screen_capture_session.cc b/chrome/browser/ash/arc/screen_capture/arc_screen_capture_session.cc index afa7e39615775..5ed99850fcdfc 100644 --- a/chrome/browser/ash/arc/screen_capture/arc_screen_capture_session.cc +++ b/chrome/browser/ash/arc/screen_capture/arc_screen_capture_session.cc @@ -364,7 +364,8 @@ void ArcScreenCaptureSession::OnAnimationStep(base::TimeTicks timestamp) { } std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kNativeTextures, base::BindOnce(&ArcScreenCaptureSession::OnDesktopCaptured, weak_ptr_factory_.GetWeakPtr())); // Clip the requested area to the desktop area. See b/118675936. diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc index 0d321b695f187..81b058c8db45a 100644 --- a/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc +++ b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc @@ -373,7 +373,9 @@ class PictureInPicturePixelComparisonBrowserTest void ReadbackResult(base::RepeatingClosure quit_run_loop, std::unique_ptr result) { ASSERT_FALSE(result->IsEmpty()); - EXPECT_EQ(viz::CopyOutputResult::Format::RGBA_BITMAP, result->format()); + EXPECT_EQ(viz::CopyOutputResult::Format::RGBA, result->format()); + EXPECT_EQ(viz::CopyOutputResult::Destination::kSystemMemory, + result->destination()); auto scoped_sk_bitmap = result->ScopedAccessSkBitmap(); result_bitmap_ = std::make_unique(scoped_sk_bitmap.GetOutScopedBitmap()); @@ -413,7 +415,8 @@ class PictureInPicturePixelComparisonBrowserTest base::RunLoop run_loop; std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( &PictureInPicturePixelComparisonBrowserTest::ReadbackResult, base::Unretained(this), run_loop.QuitClosure())); diff --git a/chrome/browser/ui/views/accessibility/accessibility_focus_highlight_browsertest.cc b/chrome/browser/ui/views/accessibility/accessibility_focus_highlight_browsertest.cc index 212280d76c15b..21bea10754395 100644 --- a/chrome/browser/ui/views/accessibility/accessibility_focus_highlight_browsertest.cc +++ b/chrome/browser/ui/views/accessibility/accessibility_focus_highlight_browsertest.cc @@ -273,7 +273,8 @@ IN_PROC_BROWSER_TEST_F(AccessibilityFocusHighlightBrowserTest, scoped_refptr holder(new ReadbackHolder); std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&ReadbackHolder::OutputRequestCallback, holder)); request->set_area(source_rect); layer->RequestCopyOfOutput(std::move(request)); diff --git a/components/exo/drag_drop_operation.cc b/components/exo/drag_drop_operation.cc index d0291e67f930a..97cb9d6caed1e 100644 --- a/components/exo/drag_drop_operation.cc +++ b/components/exo/drag_drop_operation.cc @@ -125,7 +125,8 @@ class DragDropOperation::IconSurface final : public SurfaceTreeHost, std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&IconSurface::OnCaptured, weak_ptr_factory_.GetWeakPtr())); request->set_result_task_runner(base::SequencedTaskRunnerHandle::Get()); diff --git a/components/exo/pointer.cc b/components/exo/pointer.cc index cbbed10ece337..f7e62fff71365 100644 --- a/components/exo/pointer.cc +++ b/components/exo/pointer.cc @@ -717,7 +717,8 @@ void Pointer::CaptureCursor(const gfx::Point& hotspot) { std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&Pointer::OnCursorCaptured, cursor_capture_weak_ptr_factory_.GetWeakPtr(), hotspot)); diff --git a/components/viz/common/frame_sinks/copy_output_request.cc b/components/viz/common/frame_sinks/copy_output_request.cc index cb0ffdc8f5427..34a82dddfdeb8 100644 --- a/components/viz/common/frame_sinks/copy_output_request.cc +++ b/components/viz/common/frame_sinks/copy_output_request.cc @@ -17,11 +17,18 @@ namespace viz { CopyOutputRequest::CopyOutputRequest(ResultFormat result_format, + ResultDestination result_destination, CopyOutputRequestCallback result_callback) : result_format_(result_format), + result_destination_(result_destination), result_callback_(std::move(result_callback)), scale_from_(1, 1), scale_to_(1, 1) { + // If format is I420_PLANES, the result must be in system memory. Returning + // I420_PLANES via textures is currently not supported. + DCHECK(result_format != ResultFormat::I420_PLANES || + result_destination == ResultDestination::kSystemMemory); + DCHECK(!result_callback_.is_null()); TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("viz", "CopyOutputRequest", this); } @@ -29,8 +36,8 @@ CopyOutputRequest::CopyOutputRequest(ResultFormat result_format, CopyOutputRequest::~CopyOutputRequest() { if (!result_callback_.is_null()) { // Send an empty result to indicate the request was never satisfied. - SendResult( - std::make_unique(result_format_, gfx::Rect(), false)); + SendResult(std::make_unique( + result_format_, result_destination_, gfx::Rect(), false)); } } @@ -82,7 +89,7 @@ bool CopyOutputRequest::SendsResultsInCurrentSequence() const { // static std::unique_ptr CopyOutputRequest::CreateStubForTesting() { return std::make_unique( - ResultFormat::RGBA_BITMAP, + ResultFormat::RGBA, ResultDestination::kSystemMemory, base::BindOnce([](std::unique_ptr) {})); } diff --git a/components/viz/common/frame_sinks/copy_output_request.h b/components/viz/common/frame_sinks/copy_output_request.h index 22d6576a9bdd6..7bf8e969fbebd 100644 --- a/components/viz/common/frame_sinks/copy_output_request.h +++ b/components/viz/common/frame_sinks/copy_output_request.h @@ -6,6 +6,7 @@ #define COMPONENTS_VIZ_COMMON_FRAME_SINKS_COPY_OUTPUT_REQUEST_H_ #include +#include #include "base/callback.h" #include "base/sequenced_task_runner.h" @@ -43,17 +44,27 @@ class CopyOutputRequestDataView; class VIZ_COMMON_EXPORT CopyOutputRequest { public: using ResultFormat = CopyOutputResult::Format; + // Specifies intended destination for the results. For software compositing, + // only the system-memory results are supported - even if the + // CopyOutputRequest is issued with ResultDestination::kNativeTextures, the + // results will still be returned via ResultDestination::kSystemMemory. + using ResultDestination = CopyOutputResult::Destination; using CopyOutputRequestCallback = base::OnceCallback result)>; + // Creates new CopyOutputRequest. I420_PLANES format returned via + // kNativeTextures is currently not supported. CopyOutputRequest(ResultFormat result_format, + ResultDestination result_destination, CopyOutputRequestCallback result_callback); ~CopyOutputRequest(); // Returns the requested result format. ResultFormat result_format() const { return result_format_; } + // Returns the requested result destination. + ResultDestination result_destination() const { return result_destination_; } // Requests that the result callback be run as a task posted to the given // |task_runner|. If this is not set, the result callback could be run from @@ -115,7 +126,8 @@ class VIZ_COMMON_EXPORT CopyOutputRequest { // same TaskRunner as that to which the current task was posted. bool SendsResultsInCurrentSequence() const; - // Creates a RGBA_BITMAP request that ignores results, for testing purposes. + // Creates a RGBA request with ResultDestination::kSystemMemory that ignores + // results, for testing purposes. static std::unique_ptr CreateStubForTesting(); private: @@ -126,6 +138,7 @@ class VIZ_COMMON_EXPORT CopyOutputRequest { std::unique_ptr>; const ResultFormat result_format_; + const ResultDestination result_destination_; CopyOutputRequestCallback result_callback_; scoped_refptr result_task_runner_; gfx::Vector2d scale_from_; diff --git a/components/viz/common/frame_sinks/copy_output_result.cc b/components/viz/common/frame_sinks/copy_output_result.cc index 2910ed82564e7..43425104eb96d 100644 --- a/components/viz/common/frame_sinks/copy_output_result.cc +++ b/components/viz/common/frame_sinks/copy_output_result.cc @@ -15,13 +15,16 @@ namespace viz { CopyOutputResult::CopyOutputResult(Format format, + Destination destination, const gfx::Rect& rect, bool needs_lock_for_bitmap) : format_(format), + destination_(destination), rect_(rect), needs_lock_for_bitmap_(needs_lock_for_bitmap) { - DCHECK(format_ == Format::RGBA_BITMAP || format_ == Format::RGBA_TEXTURE || - format_ == Format::I420_PLANES); + DCHECK(format_ == Format::RGBA || format_ == Format::I420_PLANES); + DCHECK(destination_ == Destination::kSystemMemory || + destination_ == Destination::kNativeTextures); } CopyOutputResult::~CopyOutputResult() = default; @@ -29,16 +32,18 @@ CopyOutputResult::~CopyOutputResult() = default; bool CopyOutputResult::IsEmpty() const { if (rect_.IsEmpty()) return true; - switch (format_) { - case Format::RGBA_BITMAP: - case Format::I420_PLANES: - return false; - case Format::RGBA_TEXTURE: - if (const TextureResult* result = GetTextureResult()) + + switch (destination_) { + case Destination::kNativeTextures: + if (const TextureResult* result = GetTextureResult()) { return result->mailbox.IsZero(); - else + } else { return true; + } + case Destination::kSystemMemory: + return false; } + NOTREACHED(); return true; } @@ -126,14 +131,14 @@ gfx::ColorSpace CopyOutputResult::GetRGBAColorSpace() const { CopyOutputSkBitmapResult::CopyOutputSkBitmapResult(const gfx::Rect& rect, SkBitmap bitmap) - : CopyOutputSkBitmapResult(Format::RGBA_BITMAP, rect, std::move(bitmap)) {} + : CopyOutputSkBitmapResult(Format::RGBA, rect, std::move(bitmap)) {} + +CopyOutputSkBitmapResult::CopyOutputSkBitmapResult(Format format, + const gfx::Rect& rect, + SkBitmap bitmap) + : CopyOutputResult(format, Destination::kSystemMemory, rect, false) { + DCHECK(format == Format::RGBA || format == Format::I420_PLANES); -CopyOutputSkBitmapResult::CopyOutputSkBitmapResult( - CopyOutputResult::Format format, - const gfx::Rect& rect, - SkBitmap bitmap) - : CopyOutputResult(format, rect, false) { - DCHECK(format == Format::RGBA_BITMAP || format == Format::I420_PLANES); if (!rect.IsEmpty()) { DCHECK(!bitmap.pixelRef() || bitmap.pixelRef()->unique()); DCHECK(!bitmap.readyToDraw() || bitmap.colorSpace()); @@ -177,7 +182,7 @@ CopyOutputTextureResult::CopyOutputTextureResult( const gpu::SyncToken& sync_token, const gfx::ColorSpace& color_space, ReleaseCallback release_callback) - : CopyOutputResult(Format::RGBA_TEXTURE, rect, false), + : CopyOutputResult(Format::RGBA, Destination::kNativeTextures, rect, false), texture_result_(mailbox, sync_token, color_space), release_callback_(std::move(release_callback)) { DCHECK_EQ(rect.IsEmpty(), mailbox.IsZero()); diff --git a/components/viz/common/frame_sinks/copy_output_result.h b/components/viz/common/frame_sinks/copy_output_result.h index f21f3ebbdebce..209e5babf000c 100644 --- a/components/viz/common/frame_sinks/copy_output_result.h +++ b/components/viz/common/frame_sinks/copy_output_result.h @@ -25,22 +25,39 @@ namespace viz { class VIZ_COMMON_EXPORT CopyOutputResult { public: enum class Format : uint8_t { - // A normal bitmap in system memory. AsSkBitmap() will return a bitmap in - // "N32Premul" form. - RGBA_BITMAP, - // A GL_RGBA texture, referred to by a gpu::Mailbox. Client code can - // optionally take ownership of the texture (via TakeTextureOwnership()), if - // it is needed beyond the lifetime of CopyOutputResult. - RGBA_TEXTURE, - // I420 format planes in system memory. This is intended to be used - // internally within the VIZ component to support video capture. When - // requesting this format, results can only be delivered on the same task - // runner sequence that runs the DirectRenderer implementation. + // A normal bitmap. When the results are returned in system memory, the + // AsSkBitmap() will return a bitmap in "N32Premul" form. When the results + // are returned in a texture, it will be an GL_RGBA texture referred to by + // a gpu::Mailbox. Client code can optionally take ownership of the texture + // (via a call to |TakeTextureOwnership()|) if it is needed beyond the + // lifetime of the CopyOutputResult. + RGBA, + // I420 format planes. This is intended to be used internally within the VIZ + // component to support video capture. When requesting this format, results + // can only be delivered on the same task runner sequence that runs the + // DirectRenderer implementation. For now, I420 format can be requested only + // for system memory. I420_PLANES, }; - CopyOutputResult( - Format format, const gfx::Rect& rect, bool needs_lock_for_bitmap); + // Specifies how the results are delivered to the issuer of the request. + // This should usually (but not always!) correspond to the value found in + // CopyOutputRequest::result_destination() of the request that caused this + // result to be produced. For details, see the comment on + // CopyOutputRequest::ResultDestination. + enum class Destination : uint8_t { + // Place the results in system memory. + kSystemMemory, + // Place the results in native textures. The GPU textures are returned via a + // mailbox. The caller can use |GetTextureResult()| and + // |TakeTextureOwnership()| to access the results. + kNativeTextures, + }; + + CopyOutputResult(Format format, + Destination destination, + const gfx::Rect& rect, + bool needs_lock_for_bitmap); virtual ~CopyOutputResult(); @@ -50,6 +67,8 @@ class VIZ_COMMON_EXPORT CopyOutputResult { // Returns the format of this result. Format format() const { return format_; } + // Returns the destination of this result. + Destination destination() const { return destination_; } // Returns the result Rect, which is the position and size of the image data // within the surface/layer (see CopyOutputRequest::set_area()). If a scale @@ -113,9 +132,9 @@ class VIZ_COMMON_EXPORT CopyOutputResult { uint8_t* v_out, int v_out_stride) const; - // Copies the result of an RGBA_BITMAP into |dest|. The result is in N32Premul - // form. Returns true if successful, or false if: 1) the result is empty, or - // 2) the result format is not RGBA_BITMAP and conversion is not implemented. + // Copies the result into |dest|. The result is in N32Premul form. Returns + // true if successful, or false if: 1) the result is empty, or 2) the result + // format is not RGBA and conversion is not implemented. virtual bool ReadRGBAPlane(uint8_t* dest, int stride) const; // Returns the color space of the image data returned by ReadRGBAPlane(). @@ -141,6 +160,7 @@ class VIZ_COMMON_EXPORT CopyOutputResult { private: const Format format_; + const Destination destination_; const gfx::Rect rect_; const bool needs_lock_for_bitmap_; @@ -150,8 +170,9 @@ class VIZ_COMMON_EXPORT CopyOutputResult { DISALLOW_COPY_AND_ASSIGN(CopyOutputResult); }; -// Subclass of CopyOutputResult that provides a RGBA_BITMAP result from an -// SkBitmap (or an I420_PLANES result based on a SkBitmap). +// Subclass of CopyOutputResult that provides a RGBA result from an +// SkBitmap (or an I420_PLANES result based on a SkBitmap). Implies that the +// destination is kSystemMemory. class VIZ_COMMON_EXPORT CopyOutputSkBitmapResult : public CopyOutputResult { public: CopyOutputSkBitmapResult(Format format, diff --git a/components/viz/service/display/copy_output_scaling_pixeltest.cc b/components/viz/service/display/copy_output_scaling_pixeltest.cc index 89b4251d5b499..c8b8d38217c67 100644 --- a/components/viz/service/display/copy_output_scaling_pixeltest.cc +++ b/components/viz/service/display/copy_output_scaling_pixeltest.cc @@ -69,8 +69,11 @@ class CopyOutputScalingPixelTest // the resulting bitmap is compared against an expected bitmap. void RunTest() { const char* result_format_as_str = ""; - if (result_format_ == CopyOutputResult::Format::RGBA_BITMAP) - result_format_as_str = "RGBA_BITMAP"; + + // Tests only issue requests for system-memory destinations, no need to + // take the destination into account: + if (result_format_ == CopyOutputResult::Format::RGBA) + result_format_as_str = "RGBA"; else if (result_format_ == CopyOutputResult::Format::I420_PLANES) result_format_as_str = "I420_PLANES"; else @@ -137,7 +140,7 @@ class CopyOutputScalingPixelTest // http://crbug.com/792734 bool dummy_ran = false; auto request = std::make_unique( - result_format_, + result_format_, CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](bool* dummy_ran, std::unique_ptr result) { EXPECT_TRUE(!result->IsEmpty()); @@ -156,7 +159,7 @@ class CopyOutputScalingPixelTest // Add a copy request to the root RenderPass, to capture the results of // drawing all passes for this frame. request = std::make_unique( - result_format_, + result_format_, CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](bool* dummy_ran, std::unique_ptr* test_result, @@ -308,7 +311,7 @@ const auto kParameters = testing::Values(gfx::Vector2d(1, 1), gfx::Vector2d(2, 1), gfx::Vector2d(1, 2)), - testing::Values(CopyOutputResult::Format::RGBA_BITMAP, + testing::Values(CopyOutputResult::Format::RGBA, CopyOutputResult::Format::I420_PLANES)); TEST_P(CopyOutputScalingPixelTest, ScaledCopyOfDrawnFrame) { diff --git a/components/viz/service/display/display_unittest.cc b/components/viz/service/display/display_unittest.cc index 7b40269c3334b..e6770dfafc9e1 100644 --- a/components/viz/service/display/display_unittest.cc +++ b/components/viz/service/display/display_unittest.cc @@ -454,7 +454,8 @@ TEST_F(DisplayTest, DisplayDamaged) { base::RunLoop copy_run_loop; bool copy_called = false; pass->copy_requests.push_back(std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyCallback, ©_called, copy_run_loop.QuitClosure()))); pass->id = CompositorRenderPassId{1u}; @@ -4469,7 +4470,8 @@ TEST_F(DisplayTest, DisplaySizeMismatch) { base::RunLoop copy_run_loop; bool copy_called = false; pass->copy_requests.push_back(std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyCallback, ©_called, copy_run_loop.QuitClosure()))); pass->id = CompositorRenderPassId{1u}; diff --git a/components/viz/service/display/gl_renderer_copier.cc b/components/viz/service/display/gl_renderer_copier.cc index a5bd4013a9a90..5f38e59d6e9b1 100644 --- a/components/viz/service/display/gl_renderer_copier.cc +++ b/components/viz/service/display/gl_renderer_copier.cc @@ -38,6 +38,7 @@ namespace viz { using ResultFormat = CopyOutputRequest::ResultFormat; +using ResultDestination = CopyOutputRequest::ResultDestination; namespace { @@ -130,12 +131,14 @@ void GLRendererCopier::CopyFromTextureOrFramebuffer( // before returning the copy result. gfx::ColorSpace dest_color_space = framebuffer_color_space; if (!framebuffer_color_space.ToSkColorSpace() && - request->result_format() == ResultFormat::RGBA_BITMAP) { + request->result_format() == ResultFormat::RGBA && + request->result_destination() == ResultDestination::kSystemMemory) { dest_color_space = gfx::ColorSpace::CreateSRGB(); } // Fast-Path: If no transformation is necessary and no new textures need to be // generated, read-back directly from the currently-bound framebuffer. - if (request->result_format() == ResultFormat::RGBA_BITMAP && + if (request->result_format() == ResultFormat::RGBA && + request->result_destination() == ResultDestination::kSystemMemory && framebuffer_color_space == dest_color_space && !request->is_scaled()) { StartReadbackFromFramebuffer(std::move(request), geometry.readback_offset, flipped_source, false, result_rect, @@ -199,23 +202,28 @@ void GLRendererCopier::CopyFromTextureOrFramebuffer( } switch (request->result_format()) { - case ResultFormat::RGBA_BITMAP: - EnsureTextureDefinedWithSize(context_provider_->ContextGL(), - result_rect.size(), &things->result_texture, - &things->result_texture_size); - RenderResultTexture(*request, flipped_source, framebuffer_color_space, - dest_color_space, source_texture, source_texture_size, - sampling_rect, result_rect, things->result_texture, - things.get()); - StartReadbackFromTexture(std::move(request), result_rect, - dest_color_space, things.get()); - break; + case ResultFormat::RGBA: + + switch (request->result_destination()) { + case ResultDestination::kSystemMemory: + EnsureTextureDefinedWithSize( + context_provider_->ContextGL(), result_rect.size(), + &things->result_texture, &things->result_texture_size); + RenderResultTexture(*request, flipped_source, framebuffer_color_space, + dest_color_space, source_texture, + source_texture_size, sampling_rect, result_rect, + things->result_texture, things.get()); + StartReadbackFromTexture(std::move(request), result_rect, + dest_color_space, things.get()); + break; + case ResultDestination::kNativeTextures: + RenderAndSendTextureResult(std::move(request), flipped_source, + framebuffer_color_space, dest_color_space, + source_texture, source_texture_size, + sampling_rect, result_rect, things.get()); + break; + } - case ResultFormat::RGBA_TEXTURE: - RenderAndSendTextureResult(std::move(request), flipped_source, - framebuffer_color_space, dest_color_space, - source_texture, source_texture_size, - sampling_rect, result_rect, things.get()); break; case ResultFormat::I420_PLANES: @@ -271,7 +279,9 @@ void GLRendererCopier::RenderResultTexture( GLScaler::Parameters params; PopulateScalerParameters(request, source_color_space, dest_color_space, flipped_source, ¶ms); - if (request.result_format() == ResultFormat::RGBA_BITMAP) { + + DCHECK_EQ(request.result_format(), ResultFormat::RGBA); + if (request.result_destination() == ResultDestination::kSystemMemory) { // Render the result in top-down row order, and swizzle, within the GPU so // these things don't have to be done, less efficiently, on the CPU later. params.flip_output = flipped_source; @@ -279,7 +289,7 @@ void GLRendererCopier::RenderResultTexture( ShouldSwapRedAndBlueForBitmapReadback() ? GL_BGRA_EXT : GL_RGBA; } else { // Texture results are always in bottom-up row order. - DCHECK_EQ(request.result_format(), ResultFormat::RGBA_TEXTURE); + DCHECK_EQ(request.result_destination(), ResultDestination::kNativeTextures); params.flip_output = !flipped_source; DCHECK_EQ(params.swizzle[0], static_cast(GL_RGBA)); } @@ -364,7 +374,8 @@ void GLRendererCopier::StartReadbackFromTexture( const gfx::Rect& result_rect, const gfx::ColorSpace& color_space, ReusableThings* things) { - DCHECK_EQ(request->result_format(), ResultFormat::RGBA_BITMAP); + DCHECK_EQ(request->result_format(), ResultFormat::RGBA); + DCHECK_EQ(request->result_destination(), ResultDestination::kSystemMemory); auto* const gl = context_provider_->ContextGL(); if (things->readback_framebuffer == 0) { @@ -396,7 +407,8 @@ class GLPixelBufferRGBAResult final : public CopyOutputResult { GLuint transfer_buffer, bool is_upside_down, bool swap_red_and_blue) - : CopyOutputResult(CopyOutputResult::Format::RGBA_BITMAP, + : CopyOutputResult(CopyOutputResult::Format::RGBA, + CopyOutputResult::Destination::kSystemMemory, result_rect, /*needs_lock_for_bitmap=*/false), color_space_(color_space), @@ -577,7 +589,8 @@ void GLRendererCopier::StartReadbackFromFramebuffer( bool swapped_red_and_blue, const gfx::Rect& result_rect, const gfx::ColorSpace& color_space) { - DCHECK_EQ(request->result_format(), ResultFormat::RGBA_BITMAP); + DCHECK_EQ(request->result_format(), ResultFormat::RGBA); + DCHECK_EQ(request->result_destination(), ResultDestination::kSystemMemory); read_pixels_workflows_.emplace_back(std::make_unique( std::move(request), readback_offset, flipped_source, @@ -600,7 +613,8 @@ void GLRendererCopier::RenderAndSendTextureResult( const gfx::Rect& sampling_rect, const gfx::Rect& result_rect, ReusableThings* things) { - DCHECK_EQ(request->result_format(), ResultFormat::RGBA_TEXTURE); + DCHECK_EQ(request->result_format(), ResultFormat::RGBA); + DCHECK_EQ(request->result_destination(), ResultDestination::kNativeTextures); auto* sii = context_provider_->SharedImageInterface(); gpu::Mailbox mailbox = sii->CreateSharedImage( @@ -649,6 +663,7 @@ class GLPixelBufferI420Result final : public CopyOutputResult { ContextProvider* context_provider, GLuint transfer_buffer) : CopyOutputResult(CopyOutputResult::Format::I420_PLANES, + CopyOutputResult::Destination::kSystemMemory, result_rect, /*needs_lock_for_bitmap=*/false), aligned_rect_(aligned_rect), diff --git a/components/viz/service/display/gl_renderer_copier.h b/components/viz/service/display/gl_renderer_copier.h index aa457e9a95d97..5fab5bd879553 100644 --- a/components/viz/service/display/gl_renderer_copier.h +++ b/components/viz/service/display/gl_renderer_copier.h @@ -184,10 +184,11 @@ class VIZ_SERVICE_EXPORT GLRendererCopier { // Renders a scaled/transformed copy of a source texture according to the // |request| parameters and other source characteristics. |result_texture| - // must be allocated/sized by the caller. For RGBA_BITMAP requests, the image - // content will be rendered in top-down row order and maybe red-blue swapped, - // to support efficient readback later on. For RGBA_TEXTURE requests, the - // image content is always rendered Y-flipped (bottom-up row order). + // must be allocated/sized by the caller. For RGBA requests with destination + // set to system memory, the image content will be rendered in top-down row + // order and maybe red-blue swapped, to support efficient readback later on. + // For RGBA requests with ResultDestination::kNativeTextures set, the image + // content is always rendered Y-flipped (bottom-up row order). void RenderResultTexture(const CopyOutputRequest& request, bool flipped_source, const gfx::ColorSpace& source_color_space, @@ -257,7 +258,8 @@ class VIZ_SERVICE_EXPORT GLRendererCopier { ReusableThings* things); // Binds the |things->result_texture| to a framebuffer and calls - // StartReadbackFromFramebuffer(). This is for RGBA_BITMAP requests only. + // StartReadbackFromFramebuffer(). This is only for RGBA requests with + // destination set to kSystemMemory. // Assumes the image content is in top-down row order (and is red-blue swapped // iff RenderResultTexture() would have done that). void StartReadbackFromTexture(std::unique_ptr request, diff --git a/components/viz/service/display/gl_renderer_copier_perftest.cc b/components/viz/service/display/gl_renderer_copier_perftest.cc index ecfd624705e33..c1102e6d05084 100644 --- a/components/viz/service/display/gl_renderer_copier_perftest.cc +++ b/components/viz/service/display/gl_renderer_copier_perftest.cc @@ -169,11 +169,13 @@ class GLRendererCopierPerfTest : public testing::Test { } } - void CopyFromTextureOrFramebuffer(bool have_source_texture, - CopyOutputResult::Format result_format, - bool scale_by_half, - bool flipped_source, - const std::string& story) { + void CopyFromTextureOrFramebuffer( + bool have_source_texture, + CopyOutputResult::Format result_format, + CopyOutputResult::Destination result_destination, + bool scale_by_half, + bool flipped_source, + const std::string& story) { std::unique_ptr result; gfx::Rect result_selection(kRequestArea); @@ -193,7 +195,7 @@ class GLRendererCopierPerfTest : public testing::Test { do { base::RunLoop loop; auto request = std::make_unique( - result_format, + result_format, result_destination, base::BindOnce( [](std::unique_ptr* result, base::OnceClosure quit_closure, @@ -222,7 +224,8 @@ class GLRendererCopierPerfTest : public testing::Test { else ASSERT_EQ(kRequestArea, result->rect()); - if (result_format == CopyOutputResult::Format::RGBA_BITMAP) { + if (result_format == CopyOutputResult::Format::RGBA && + result_destination == CopyOutputResult::Destination::kSystemMemory) { auto scoped_bitmap = result->ScopedAccessSkBitmap(); const SkBitmap& result_bitmap = scoped_bitmap.bitmap(); ASSERT_TRUE(result_bitmap.readyToDraw()); @@ -274,7 +277,8 @@ class GLRendererCopierPerfTest : public testing::Test { // generated, read-back directly from the currently-bound framebuffer. TEST_F(GLRendererCopierPerfTest, NoTransformNoNewTextures) { CopyFromTextureOrFramebuffer( - /*have_source_texture=*/false, CopyOutputResult::Format::RGBA_BITMAP, + /*have_source_texture=*/false, CopyOutputResult::Format::RGBA, + CopyOutputResult::Destination::kSystemMemory, /*scale_by_half=*/false, /*flipped_source=*/false, "no_transformation_and_no_new_textures"); } @@ -283,19 +287,22 @@ TEST_F(GLRendererCopierPerfTest, NoTransformNoNewTextures) { // than having to make a copy of the framebuffer. TEST_F(GLRendererCopierPerfTest, HaveTextureResultRGBABitmap) { CopyFromTextureOrFramebuffer( - /*have_source_texture=*/true, CopyOutputResult::Format::RGBA_BITMAP, + /*have_source_texture=*/true, CopyOutputResult::Format::RGBA, + CopyOutputResult::Destination::kSystemMemory, /*scale_by_half=*/true, /*flipped_source=*/false, "framebuffer_has_texture_and_result_is_RGBA_BITMAP"); } TEST_F(GLRendererCopierPerfTest, HaveTextureResultRGBATexture) { CopyFromTextureOrFramebuffer( - /*have_source_texture=*/true, CopyOutputResult::Format::RGBA_TEXTURE, + /*have_source_texture=*/true, CopyOutputResult::Format::RGBA, + CopyOutputResult::Destination::kNativeTextures, /*scale_by_half=*/true, /*flipped_source=*/false, "framebuffer_has_texture_and_result_is_RGBA_TEXTURE"); } TEST_F(GLRendererCopierPerfTest, HaveTextureResultI420Planes) { CopyFromTextureOrFramebuffer( /*have_source_texture=*/true, CopyOutputResult::Format::I420_PLANES, + CopyOutputResult::Destination::kSystemMemory, /*scale_by_half=*/true, /*flipped_source=*/false, "framebuffer_has_texture_and_result_is_I420_PLANES"); } @@ -304,6 +311,7 @@ TEST_F(GLRendererCopierPerfTest, HaveTextureResultI420Planes) { TEST_F(GLRendererCopierPerfTest, NoTextureResultI420Planes) { CopyFromTextureOrFramebuffer( /*have_source_texture=*/false, CopyOutputResult::Format::I420_PLANES, + CopyOutputResult::Destination::kSystemMemory, /*scale_by_half=*/true, /*flipped_source=*/false, "framebuffer_doesn't_have_texture_and_result_is_I420_PLANES"); } @@ -312,6 +320,7 @@ TEST_F(GLRendererCopierPerfTest, NoTextureResultI420Planes) { TEST_F(GLRendererCopierPerfTest, SourceContentVerticallyFlipped) { CopyFromTextureOrFramebuffer(/*have_source_texture=*/true, CopyOutputResult::Format::I420_PLANES, + CopyOutputResult::Destination::kSystemMemory, /*scale_by_half=*/true, /*flipped_source=*/true, "source_content_is_vertically_flipped"); } @@ -320,6 +329,7 @@ TEST_F(GLRendererCopierPerfTest, SourceContentVerticallyFlipped) { TEST_F(GLRendererCopierPerfTest, ResultNotScaled) { CopyFromTextureOrFramebuffer(/*have_source_texture=*/true, CopyOutputResult::Format::I420_PLANES, + CopyOutputResult::Destination::kSystemMemory, /*scale_by_half=*/false, /*flipped_source=*/true, "result_is_not_scaled_by_half"); } diff --git a/components/viz/service/display/gl_renderer_copier_pixeltest.cc b/components/viz/service/display/gl_renderer_copier_pixeltest.cc index 1f090f446ed22..df4c6203469ee 100644 --- a/components/viz/service/display/gl_renderer_copier_pixeltest.cc +++ b/components/viz/service/display/gl_renderer_copier_pixeltest.cc @@ -60,7 +60,7 @@ base::FilePath GetTestFilePath(const base::FilePath::CharType* basename) { class GLRendererCopierPixelTest : public cc::PixelTest, public testing::WithParamInterface< - std::tuple> { + std::tuple> { public: void SetUp() override { SetUpGLWithoutRenderer(gfx::SurfaceOrigin::kBottomLeft); @@ -70,7 +70,7 @@ class GLRendererCopierPixelTest source_gl_format_ = std::get<0>(GetParam()); have_source_texture_ = std::get<1>(GetParam()); - result_format_ = std::get<2>(GetParam()); + result_destination_ = std::get<2>(GetParam()); scale_by_half_ = std::get<3>(GetParam()); flipped_source_ = std::get<4>(GetParam()); @@ -221,7 +221,7 @@ class GLRendererCopierPixelTest protected: GLenum source_gl_format_; bool have_source_texture_; - CopyOutputResult::Format result_format_; + CopyOutputResult::Destination result_destination_; bool scale_by_half_; bool flipped_source_; SkBitmap source_bitmap_; @@ -249,7 +249,7 @@ TEST_P(GLRendererCopierPixelTest, ExecutesCopyRequest) { { base::RunLoop loop; auto request = std::make_unique( - result_format_, + CopyOutputRequest::ResultFormat::RGBA, result_destination_, base::BindOnce( [](std::unique_ptr* result, base::OnceClosure quit_closure, @@ -293,7 +293,7 @@ TEST_P(GLRendererCopierPixelTest, ExecutesCopyRequest) { // Examine the image in the |result|, and compare it to the baseline PNG file. absl::optional scoped_bitmap; SkBitmap actual; - if (result_format_ == CopyOutputResult::Format::RGBA_BITMAP) { + if (result_destination_ == CopyOutputResult::Destination::kSystemMemory) { scoped_bitmap = result->ScopedAccessSkBitmap(); actual = scoped_bitmap->bitmap(); } else { @@ -327,9 +327,9 @@ INSTANTIATE_TEST_SUITE_P( static_cast(GL_RGB)), // Source: Have texture too? testing::Values(false, true), - // Result format. - testing::Values(CopyOutputResult::Format::RGBA_BITMAP, - CopyOutputResult::Format::RGBA_TEXTURE), + // Result destination. + testing::Values(CopyOutputResult::Destination::kSystemMemory, + CopyOutputResult::Destination::kNativeTextures), // Result scaling: Scale by half? testing::Values(false, true), // Source content is vertically flipped? diff --git a/components/viz/service/display/gl_renderer_copier_unittest.cc b/components/viz/service/display/gl_renderer_copier_unittest.cc index 6b10304fc1e0e..8ff8bca30b99c 100644 --- a/components/viz/service/display/gl_renderer_copier_unittest.cc +++ b/components/viz/service/display/gl_renderer_copier_unittest.cc @@ -193,7 +193,8 @@ TEST_F(GLRendererCopierTest, FallsBackToSRGBForInvalidSkColorSpaces) { std::unique_ptr result; base::RunLoop loop; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](std::unique_ptr* result_out, base::OnceClosure quit_closure, diff --git a/components/viz/service/display/skia_readback_pixeltest.cc b/components/viz/service/display/skia_readback_pixeltest.cc index 0ab88ecb955e4..092565def8824 100644 --- a/components/viz/service/display/skia_readback_pixeltest.cc +++ b/components/viz/service/display/skia_readback_pixeltest.cc @@ -75,11 +75,16 @@ void DeleteSharedImage(scoped_refptr context_provider, class SkiaReadbackPixelTest : public cc::PixelTest, public testing::WithParamInterface { public: + CopyOutputResult::Format RequestFormat() const { + return CopyOutputResult::Format::RGBA; + } + // TODO(kylechar): Add parameter to also test RGBA_TEXTURE when it's // supported with the Skia readback API. - CopyOutputResult::Format RequestFormat() const { - return CopyOutputResult::Format::RGBA_BITMAP; + CopyOutputResult::Destination RequestDestination() const { + return CopyOutputResult::Destination::kSystemMemory; } + bool ScaleByHalf() const { return GetParam(); } void SetUp() override { @@ -167,7 +172,7 @@ TEST_P(SkiaReadbackPixelTest, ExecutesCopyRequest) { std::unique_ptr result; base::RunLoop loop; auto request = std::make_unique( - RequestFormat(), + RequestFormat(), RequestDestination(), base::BindOnce( [](std::unique_ptr* result_out, base::OnceClosure quit_closure, diff --git a/components/viz/service/display/software_renderer.cc b/components/viz/service/display/software_renderer.cc index 64c2f6bdef412..0ef8e12fe088d 100644 --- a/components/viz/service/display/software_renderer.cc +++ b/components/viz/service/display/software_renderer.cc @@ -651,20 +651,15 @@ void SoftwareRenderer::CopyDrawnRenderPass( return; } - // Deliver the result. SoftwareRenderer supports RGBA_BITMAP and I420_PLANES - // only. For legacy reasons, if a RGBA_TEXTURE request is being made, clients - // are prepared to accept RGBA_BITMAP results. - // - // TODO(crbug/754872): Get rid of the legacy behavior and send empty results - // for RGBA_TEXTURE requests once tab capture is moved into VIZ. - const CopyOutputResult::Format result_format = - (request->result_format() == CopyOutputResult::Format::RGBA_TEXTURE) - ? CopyOutputResult::Format::RGBA_BITMAP - : request->result_format(); - // Note: The CopyOutputSkBitmapResult automatically provides I420 format + // Deliver the result. SoftwareRenderer supports system memory destinations + // only. For legacy reasons, if a RGBA texture request is being made, clients + // are prepared to accept system memory results. + + // Note: The CopyOutputSkBitmapResult already implies that results are + // returned in system memory and automatically provides I420 format // conversion, if needed. request->SendResult(std::make_unique( - result_format, geometry.result_selection, std::move(bitmap))); + request->result_format(), geometry.result_selection, std::move(bitmap))); } void SoftwareRenderer::DidChangeVisibility() { diff --git a/components/viz/service/display/software_renderer_unittest.cc b/components/viz/service/display/software_renderer_unittest.cc index fd23f9f32921f..2bbee232aeedc 100644 --- a/components/viz/service/display/software_renderer_unittest.cc +++ b/components/viz/service/display/software_renderer_unittest.cc @@ -104,7 +104,8 @@ class SoftwareRendererTest : public testing::Test { base::RunLoop loop; list->back()->copy_requests.push_back(std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&SoftwareRendererTest::SaveBitmapResult, base::Unretained(&bitmap_result), loop.QuitClosure()))); @@ -120,7 +121,9 @@ class SoftwareRendererTest : public testing::Test { base::OnceClosure quit_closure, std::unique_ptr result) { DCHECK(!result->IsEmpty()); - DCHECK_EQ(result->format(), CopyOutputResult::Format::RGBA_BITMAP); + DCHECK_EQ(result->format(), CopyOutputResult::Format::RGBA); + DCHECK_EQ(result->destination(), + CopyOutputResult::Destination::kSystemMemory); auto scoped_bitmap = result->ScopedAccessSkBitmap(); (*bitmap_result) = std::make_unique(scoped_bitmap.GetOutScopedBitmap()); diff --git a/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc b/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc index bfb5d93c68cf1..56182176ac986 100644 --- a/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc +++ b/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc @@ -225,6 +225,7 @@ class SkiaOutputSurfaceImplOnGpu::CopyOutputResultYUV const gfx::Rect& rect, std::unique_ptr result) : CopyOutputResult(Format::I420_PLANES, + Destination::kSystemMemory, rect, /*needs_lock_for_bitmap=*/false), result_(impl, std::move(result)) { @@ -307,7 +308,8 @@ class SkiaOutputSurfaceImplOnGpu::CopyOutputResultRGBA const gfx::Rect& rect, std::unique_ptr result, const gfx::ColorSpace& color_space) - : CopyOutputResult(Format::RGBA_BITMAP, + : CopyOutputResult(Format::RGBA, + Destination::kSystemMemory, rect, /*needs_lock_for_bitmap=*/true), result_(impl, std::move(result)), @@ -846,7 +848,8 @@ void SkiaOutputSurfaceImplOnGpu::CopyOutput( std::unique_ptr request, const gpu::Mailbox& mailbox) { TRACE_EVENT0("viz", "SkiaOutputSurfaceImplOnGpu::CopyOutput"); - // TODO(crbug.com/898595): Do this on the GPU instead of CPU with Vulkan. + // TODO(https://crbug.com/898595): Do this on the GPU instead of CPU with + // Vulkan. DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); if (context_is_lost_) @@ -958,7 +961,9 @@ void SkiaOutputSurfaceImplOnGpu::CopyOutput( SkSurface::RescaleGamma::kSrc, rescale_mode, &CopyOutputResultYUV::OnReadbackDone, context.release()); } else if (request->result_format() == - CopyOutputRequest::ResultFormat::RGBA_BITMAP) { + CopyOutputRequest::ResultFormat::RGBA && + request->result_destination() == + CopyOutputRequest::ResultDestination::kSystemMemory) { // Perform swizzle during readback. const bool skbitmap_is_bgra = (kN32_SkColorType == kBGRA_8888_SkColorType); // If we can't convert |color_space| to a SkColorSpace @@ -984,7 +989,9 @@ void SkiaOutputSurfaceImplOnGpu::CopyOutput( dst_info, src_rect, SkSurface::RescaleGamma::kSrc, rescale_mode, &CopyOutputResultRGBA::OnReadbackDone, context.release()); } else if (request->result_format() == - CopyOutputRequest::ResultFormat::RGBA_TEXTURE) { + CopyOutputRequest::ResultFormat::RGBA && + request->result_destination() == + CopyOutputRequest::ResultDestination::kNativeTextures) { gpu::Mailbox mailbox = gpu::Mailbox::GenerateForSharedImage(); constexpr auto kUsage = gpu::SHARED_IMAGE_USAGE_GLES2 | gpu::SHARED_IMAGE_USAGE_GLES2_FRAMEBUFFER_HINT | diff --git a/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc b/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc index 4947f51bff5c9..610d7eebfefd8 100644 --- a/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc +++ b/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc @@ -152,7 +152,8 @@ TEST_F(SkiaOutputSurfaceImplTest, EndPaint) { // Copy the output const gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB(); auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&SkiaOutputSurfaceImplTest::CopyRequestCallbackOnGpuThread, base::Unretained(this), output_rect, color_space)); request->set_result_task_runner( @@ -212,7 +213,8 @@ TEST_F(SkiaOutputSurfaceImplTest, CopyOutputBitmapSupportedColorSpace) { base::RunLoop run_loop; std::unique_ptr result; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](std::unique_ptr* result_out, base::OnceClosure quit_closure, @@ -251,7 +253,8 @@ TEST_F(SkiaOutputSurfaceImplTest, CopyOutputBitmapUnsupportedColorSpace) { base::RunLoop run_loop; std::unique_ptr result; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](std::unique_ptr* result_out, base::OnceClosure quit_closure, diff --git a/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc b/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc index 1776ff29220bb..1708b528bea7d 100644 --- a/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc +++ b/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc @@ -660,7 +660,8 @@ TEST_F(CompositorFrameSinkSupportTest, ProhibitsUnprivilegedCopyRequests) { bool did_receive_aborted_copy_result = false; base::RunLoop aborted_copy_run_loop; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](bool* got_nothing, base::OnceClosure finished, std::unique_ptr result) { @@ -828,7 +829,8 @@ TEST_F(CompositorFrameSinkSupportTest, CopyRequestOnSubtree) { bool called1 = false; base::RunLoop called1_run_loop; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyRequestTestCallback, &called1, called1_run_loop.QuitClosure())); support_->RequestCopyOfOutput( @@ -841,7 +843,8 @@ TEST_F(CompositorFrameSinkSupportTest, CopyRequestOnSubtree) { bool called2 = false; base::RunLoop called2_run_loop; request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyRequestTestCallback, &called2, called2_run_loop.QuitClosure())); support_->RequestCopyOfOutput( @@ -875,7 +878,8 @@ TEST_F(CompositorFrameSinkSupportTest, DuplicateCopyRequest) { bool called1 = false; base::RunLoop called1_run_loop; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyRequestTestCallback, &called1, called1_run_loop.QuitClosure())); request->set_source(kArbitrarySourceId1); @@ -888,7 +892,8 @@ TEST_F(CompositorFrameSinkSupportTest, DuplicateCopyRequest) { bool called2 = false; base::RunLoop called2_run_loop; request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyRequestTestCallback, &called2, called2_run_loop.QuitClosure())); request->set_source(kArbitrarySourceId2); @@ -903,7 +908,8 @@ TEST_F(CompositorFrameSinkSupportTest, DuplicateCopyRequest) { bool called3 = false; base::RunLoop called3_run_loop; request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&CopyRequestTestCallback, &called3, called3_run_loop.QuitClosure())); request->set_source(kArbitrarySourceId1); @@ -1141,7 +1147,8 @@ TEST_F(CompositorFrameSinkSupportTest, // Send a CopyOutputRequest. auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(StubResultCallback)); support_->RequestCopyOfOutput( {local_surface_id1, SubtreeCaptureId(), std::move(request)}); @@ -1184,7 +1191,8 @@ TEST_F(CompositorFrameSinkSupportTest, // Send a CopyOutputRequest. auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(StubResultCallback)); support_->RequestCopyOfOutput( {local_surface_id2, SubtreeCaptureId(), std::move(request)}); @@ -1226,7 +1234,8 @@ TEST_F(CompositorFrameSinkSupportTest, // Send a CopyOutputRequest. Note that the second surface doesn't even exist // yet. auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(StubResultCallback)); support_->RequestCopyOfOutput( {local_surface_id1, SubtreeCaptureId(), std::move(request)}); diff --git a/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc b/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc index 745fbefbf1b2a..94eab1bdbb6dd 100644 --- a/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc +++ b/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc @@ -668,7 +668,8 @@ void FrameSinkVideoCapturerImpl::MaybeCaptureFrame( auto request = std::make_unique( pixel_format_ == media::PIXEL_FORMAT_I420 ? CopyOutputRequest::ResultFormat::I420_PLANES - : CopyOutputRequest::ResultFormat::RGBA_BITMAP, + : CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&FrameSinkVideoCapturerImpl::DidCopyFrame, capture_weak_factory_.GetWeakPtr(), capture_frame_number, oracle_frame_number, content_version_, content_rect, @@ -730,15 +731,19 @@ void FrameSinkVideoCapturerImpl::DidCopyFrame( frame->stride(VideoFrame::kUPlane), frame->stride(VideoFrame::kVPlane)); break; - case CopyOutputResult::Format::RGBA_BITMAP: - format = "RGBA_Bitmap"; - strides = base::StringPrintf("strideRGBA:%d", - frame->stride(VideoFrame::kARGBPlane)); - break; - case CopyOutputResult::Format::RGBA_TEXTURE: - format = "RGBA_Texture"; + case CopyOutputResult::Format::RGBA: + strides = base::StringPrintf("strideRGBA:%d", frame->stride(VideoFrame::kARGBPlane)); + + switch (result->destination()) { + case CopyOutputResult::Destination::kSystemMemory: + format = "RGBA_Bitmap"; + break; + case CopyOutputResult::Destination::kNativeTextures: + format = "RGBA_Texture"; + break; + } break; } consumer_->OnLog(base::StringPrintf( diff --git a/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc b/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc index 18e9070d2335d..ee47cbf4ee8bf 100644 --- a/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc +++ b/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc @@ -195,7 +195,10 @@ class MockConsumer : public mojom::FrameSinkVideoConsumer { class SolidColorI420Result : public CopyOutputResult { public: SolidColorI420Result(const gfx::Rect rect, YUVColor color) - : CopyOutputResult(CopyOutputResult::Format::I420_PLANES, rect, false), + : CopyOutputResult(CopyOutputResult::Format::I420_PLANES, + CopyOutputResult::Destination::kNativeTextures, + rect, + false), color_(color) {} bool ReadI420Planes(uint8_t* y_out, diff --git a/components/viz/service/surfaces/surface_saved_frame.cc b/components/viz/service/surfaces/surface_saved_frame.cc index 4b3ed5596ffab..dd22b7570ffe6 100644 --- a/components/viz/service/surfaces/surface_saved_frame.cc +++ b/components/viz/service/surfaces/surface_saved_frame.cc @@ -4,6 +4,7 @@ #include "components/viz/service/surfaces/surface_saved_frame.h" +#include #include #include @@ -59,15 +60,14 @@ bool SurfaceSavedFrame::IsValid() const { void SurfaceSavedFrame::RequestCopyOfOutput(Surface* surface) { DCHECK(surface->HasActiveFrame()); - // RGBA_TEXTURE will become RGBA_BITMAP with SoftwareCompositing path. - // TODO(kylechar): Add RGBA_NATIVE that returns either RGBA_TEXTURE or - // RGBA_BITMAP depending on what is native. - constexpr auto result_format = CopyOutputRequest::ResultFormat::RGBA_TEXTURE; + constexpr auto result_format = CopyOutputRequest::ResultFormat::RGBA; + constexpr auto result_destination = + CopyOutputRequest::ResultDestination::kNativeTextures; const auto& root_draw_data = GetRootRenderPassDrawData(surface); // Bind kRoot and root geometry information to the callback. auto root_request = std::make_unique( - result_format, + result_format, result_destination, base::BindOnce(&SurfaceSavedFrame::NotifyCopyOfOutputComplete, weak_factory_.GetWeakPtr(), ResultType::kRoot, 0, root_draw_data)); @@ -152,7 +152,7 @@ void SurfaceSavedFrame::RequestCopyOfOutput(Surface* surface) { active_frame.render_pass_list, original_render_pass_id)); int index = std::distance(shared_pass_ids.begin(), shared_pass_it); auto request = std::make_unique( - result_format, + result_format, result_destination, base::BindOnce(&SurfaceSavedFrame::NotifyCopyOfOutputComplete, weak_factory_.GetWeakPtr(), ResultType::kShared, index, draw_data)); @@ -277,7 +277,8 @@ void SurfaceSavedFrame::NotifyCopyOfOutputComplete( DCHECK(slot); DCHECK_EQ(output_copy->size(), data.size); - if (output_copy->format() == CopyOutputResult::Format::RGBA_BITMAP) { + if (output_copy->destination() == + CopyOutputResult::Destination::kSystemMemory) { slot->bitmap = output_copy->ScopedAccessSkBitmap().GetOutScopedBitmap(); slot->is_software = true; } else { diff --git a/components/viz/service/surfaces/surface_unittest.cc b/components/viz/service/surfaces/surface_unittest.cc index 6fce1d1d5e22d..99057415b577b 100644 --- a/components/viz/service/surfaces/surface_unittest.cc +++ b/components/viz/service/surfaces/surface_unittest.cc @@ -105,7 +105,8 @@ TEST(SurfaceTest, CopyRequestLifetime) { support->RequestCopyOfOutput(PendingCopyOutputRequest{ local_surface_id, SubtreeCaptureId(), std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&TestCopyResultCallback, ©_called, copy_runloop.QuitClosure()))}); surface->TakeCopyOutputRequestsFromClient(); diff --git a/components/viz/test/fake_skia_output_surface.cc b/components/viz/test/fake_skia_output_surface.cc index 2af359c37b09f..5219d97a39c67 100644 --- a/components/viz/test/fake_skia_output_surface.cc +++ b/components/viz/test/fake_skia_output_surface.cc @@ -257,8 +257,7 @@ void FakeSkiaOutputSurface::CopyOutput( DCHECK(sk_surfaces_.find(id) != sk_surfaces_.end()); auto* surface = sk_surfaces_[id].get(); - if ((request->result_format() != CopyOutputResult::Format::RGBA_BITMAP && - request->result_format() != CopyOutputResult::Format::RGBA_TEXTURE) || + if (request->result_format() != CopyOutputResult::Format::RGBA || request->is_scaled() || geometry.result_bounds != geometry.result_selection) { // TODO(crbug.com/644851): Complete the implementation for all request @@ -267,7 +266,8 @@ void FakeSkiaOutputSurface::CopyOutput( return; } - if (request->result_format() == CopyOutputResult::Format::RGBA_TEXTURE) { + if (request->result_destination() == + CopyOutputResult::Destination::kNativeTextures) { // TODO(rivr): This implementation is incomplete and doesn't copy // anything into the mailbox, but currently the only tests that use this // don't actually check the returned texture data. diff --git a/content/browser/media/capture/slow_window_capturer_chromeos.cc b/content/browser/media/capture/slow_window_capturer_chromeos.cc index b6d3726bf6404..2e7cfd3641a7e 100644 --- a/content/browser/media/capture/slow_window_capturer_chromeos.cc +++ b/content/browser/media/capture/slow_window_capturer_chromeos.cc @@ -293,7 +293,8 @@ void SlowWindowCapturerChromeOS::CaptureNextFrame() { // Request a copy of the Layer associated with the |target_| aura::Window. auto request = std::make_unique( // Note: As of this writing, I420_PLANES is not supported external to VIZ. - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&SlowWindowCapturerChromeOS::DidCopyFrame, weak_factory_.GetWeakPtr(), std::move(in_flight_frame))); request->set_source(copy_request_source_); diff --git a/content/browser/renderer_host/delegated_frame_host.cc b/content/browser/renderer_host/delegated_frame_host.cc index ec767d4374f46..e08b194c0a38c 100644 --- a/content/browser/renderer_host/delegated_frame_host.cc +++ b/content/browser/renderer_host/delegated_frame_host.cc @@ -126,8 +126,8 @@ void DelegatedFrameHost::CopyFromCompositingSurface( const gfx::Size& output_size, base::OnceCallback callback) { CopyFromCompositingSurfaceInternal( - src_subrect, output_size, - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + src_subrect, output_size, viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](base::OnceCallback callback, std::unique_ptr result) { @@ -142,17 +142,19 @@ void DelegatedFrameHost::CopyFromCompositingSurfaceAsTexture( const gfx::Size& output_size, viz::CopyOutputRequest::CopyOutputRequestCallback callback) { CopyFromCompositingSurfaceInternal( - src_subrect, output_size, - viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, std::move(callback)); + src_subrect, output_size, viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kNativeTextures, + std::move(callback)); } void DelegatedFrameHost::CopyFromCompositingSurfaceInternal( const gfx::Rect& src_subrect, const gfx::Size& output_size, viz::CopyOutputRequest::ResultFormat format, + viz::CopyOutputRequest::ResultDestination destination, viz::CopyOutputRequest::CopyOutputRequestCallback callback) { - auto request = - std::make_unique(format, std::move(callback)); + auto request = std::make_unique(format, destination, + std::move(callback)); // It is possible for us to not have a valid surface to copy from. Such as // if a navigation fails to complete. In such a case do not attempt to request @@ -378,7 +380,9 @@ void DelegatedFrameHost::DidCopyStaleContent( if (frame_evictor_->visible() || result->IsEmpty()) return; - DCHECK_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_TEXTURE); + DCHECK_EQ(result->format(), viz::CopyOutputResult::Format::RGBA); + DCHECK_EQ(result->destination(), + viz::CopyOutputResult::Destination::kNativeTextures); DCHECK_NE(frame_eviction_state_, FrameEvictionState::kNotStarted); SetFrameEvictionStateAndNotifyObservers(FrameEvictionState::kNotStarted); diff --git a/content/browser/renderer_host/delegated_frame_host.h b/content/browser/renderer_host/delegated_frame_host.h index 46ed0c43eb5b3..0cec90466b91a 100644 --- a/content/browser/renderer_host/delegated_frame_host.h +++ b/content/browser/renderer_host/delegated_frame_host.h @@ -199,6 +199,7 @@ class CONTENT_EXPORT DelegatedFrameHost const gfx::Rect& src_subrect, const gfx::Size& output_size, viz::CopyOutputRequest::ResultFormat format, + viz::CopyOutputRequest::ResultDestination destination, viz::CopyOutputRequest::CopyOutputRequestCallback callback); void SetFrameEvictionStateAndNotifyObservers( diff --git a/content/browser/renderer_host/render_widget_host_view_child_frame.cc b/content/browser/renderer_host/render_widget_host_view_child_frame.cc index eec35bffe4ac4..8b3eac22d1ad7 100644 --- a/content/browser/renderer_host/render_widget_host_view_child_frame.cc +++ b/content/browser/renderer_host/render_widget_host_view_child_frame.cc @@ -796,7 +796,8 @@ void RenderWidgetHostViewChildFrame::CopyFromSurface( std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](base::OnceCallback callback, std::unique_ptr result) { diff --git a/remoting/host/chromeos/aura_desktop_capturer.cc b/remoting/host/chromeos/aura_desktop_capturer.cc index 445b9e146e482..9f564875f2ffe 100644 --- a/remoting/host/chromeos/aura_desktop_capturer.cc +++ b/remoting/host/chromeos/aura_desktop_capturer.cc @@ -44,7 +44,8 @@ void AuraDesktopCapturer::Start(webrtc::DesktopCapturer::Callback* callback) { void AuraDesktopCapturer::CaptureFrame() { std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&AuraDesktopCapturer::OnFrameCaptured, weak_factory_.GetWeakPtr())); diff --git a/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.cc b/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.cc index 8731bdddda98f..3b914e0d57d71 100644 --- a/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.cc +++ b/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.cc @@ -26,9 +26,11 @@ class CopyOutputResultSenderImpl : public viz::mojom::CopyOutputResultSender { public: CopyOutputResultSenderImpl( viz::CopyOutputRequest::ResultFormat result_format, + viz::CopyOutputRequest::ResultDestination result_destination, viz::CopyOutputRequest::CopyOutputRequestCallback result_callback, scoped_refptr callback_task_runner) : result_format_(result_format), + result_destination_(result_destination), result_callback_(std::move(result_callback)), result_callback_task_runner_(std::move(callback_task_runner)) { DCHECK(result_callback_); @@ -40,7 +42,8 @@ class CopyOutputResultSenderImpl : public viz::mojom::CopyOutputResultSender { result_callback_task_runner_->PostTask( FROM_HERE, base::BindOnce(std::move(result_callback_), std::make_unique( - result_format_, gfx::Rect(), false))); + result_format_, result_destination_, + gfx::Rect(), false))); } } @@ -56,6 +59,7 @@ class CopyOutputResultSenderImpl : public viz::mojom::CopyOutputResultSender { private: const viz::CopyOutputRequest::ResultFormat result_format_; + const viz::CopyOutputRequest::ResultDestination result_destination_; viz::CopyOutputRequest::CopyOutputRequestCallback result_callback_; scoped_refptr result_callback_task_runner_; }; @@ -84,7 +88,8 @@ StructTraits( - request->result_format(), std::move(request->result_callback_), + request->result_format(), request->result_destination(), + std::move(request->result_callback_), base::SequencedTaskRunnerHandle::Get()); auto runner = base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()}); runner->PostTask( @@ -108,11 +113,16 @@ bool StructTraits>(); auto request = std::make_unique( - result_format, base::BindOnce(SendResult, std::move(result_sender))); + result_format, result_destination, + base::BindOnce(SendResult, std::move(result_sender))); gfx::Vector2d scale_from; if (!data.ReadScaleFrom(&scale_from)) diff --git a/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.h b/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.h index a8427ffb9effc..ac490d4396f06 100644 --- a/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.h +++ b/services/viz/public/cpp/compositing/copy_output_request_mojom_traits.h @@ -5,6 +5,8 @@ #ifndef SERVICES_VIZ_PUBLIC_CPP_COMPOSITING_COPY_OUTPUT_REQUEST_MOJOM_TRAITS_H_ #define SERVICES_VIZ_PUBLIC_CPP_COMPOSITING_COPY_OUTPUT_REQUEST_MOJOM_TRAITS_H_ +#include + #include "components/viz/common/frame_sinks/copy_output_request.h" #include "mojo/public/cpp/base/unguessable_token_mojom_traits.h" #include "mojo/public/cpp/bindings/pending_remote.h" @@ -23,6 +25,11 @@ struct StructTraitsresult_format(); } + static viz::CopyOutputRequest::ResultDestination result_destination( + const std::unique_ptr& request) { + return request->result_destination(); + } + static const gfx::Vector2d& scale_from( const std::unique_ptr& request) { return request->scale_from(); diff --git a/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.cc b/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.cc index fded376355254..7c98b53cede5f 100644 --- a/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.cc +++ b/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.cc @@ -49,15 +49,13 @@ viz::mojom::CopyOutputResultFormat EnumTraits:: ToMojom(viz::CopyOutputResult::Format format) { switch (format) { - case viz::CopyOutputResult::Format::RGBA_BITMAP: - return viz::mojom::CopyOutputResultFormat::RGBA_BITMAP; - case viz::CopyOutputResult::Format::RGBA_TEXTURE: - return viz::mojom::CopyOutputResultFormat::RGBA_TEXTURE; + case viz::CopyOutputResult::Format::RGBA: + return viz::mojom::CopyOutputResultFormat::RGBA; case viz::CopyOutputResult::Format::I420_PLANES: break; // Not intended for transport across service boundaries. } NOTREACHED(); - return viz::mojom::CopyOutputResultFormat::RGBA_BITMAP; + return viz::mojom::CopyOutputResultFormat::RGBA; } // static @@ -66,11 +64,37 @@ bool EnumTraits:: + ToMojom(viz::CopyOutputResult::Destination destination) { + switch (destination) { + case viz::CopyOutputResult::Destination::kSystemMemory: + return viz::mojom::CopyOutputResultDestination::kSystemMemory; + case viz::CopyOutputResult::Destination::kNativeTextures: + return viz::mojom::CopyOutputResultDestination::kNativeTextures; + } +} + +// static +bool EnumTraits:: + FromMojom(viz::mojom::CopyOutputResultDestination input, + viz::CopyOutputResult::Destination* out) { + switch (input) { + case viz::mojom::CopyOutputResultDestination::kSystemMemory: + *out = viz::CopyOutputResult::Destination::kSystemMemory; + return true; + case viz::mojom::CopyOutputResultDestination::kNativeTextures: + *out = viz::CopyOutputResult::Destination::kNativeTextures; return true; } return false; @@ -84,6 +108,14 @@ StructTraitsformat(); } +// static +viz::CopyOutputResult::Destination +StructTraits>:: + destination(const std::unique_ptr& result) { + return result->destination(); +} + // static const gfx::Rect& StructTraits>:: @@ -96,7 +128,8 @@ absl::optional StructTraits>:: bitmap(const std::unique_ptr& result) { - if (result->format() != viz::CopyOutputResult::Format::RGBA_BITMAP) + if (result->destination() != + viz::CopyOutputResult::Destination::kSystemMemory) return absl::nullopt; auto scoped_bitmap = result->ScopedAccessSkBitmap(); if (!scoped_bitmap.bitmap().readyToDraw()) { @@ -113,7 +146,8 @@ absl::optional StructTraits>:: mailbox(const std::unique_ptr& result) { - if (result->format() != viz::CopyOutputResult::Format::RGBA_TEXTURE || + if (result->destination() != + viz::CopyOutputResult::Destination::kNativeTextures || result->IsEmpty()) { return absl::nullopt; } @@ -125,7 +159,8 @@ absl::optional StructTraits>:: sync_token(const std::unique_ptr& result) { - if (result->format() != viz::CopyOutputResult::Format::RGBA_TEXTURE || + if (result->destination() != + viz::CopyOutputResult::Destination::kNativeTextures || result->IsEmpty()) { return absl::nullopt; } @@ -137,7 +172,8 @@ absl::optional StructTraits>:: color_space(const std::unique_ptr& result) { - if (result->format() != viz::CopyOutputResult::Format::RGBA_TEXTURE || + if (result->destination() != + viz::CopyOutputResult::Destination::kNativeTextures || result->IsEmpty()) { return absl::nullopt; } @@ -149,7 +185,8 @@ mojo::PendingRemote StructTraits>:: releaser(const std::unique_ptr& result) { - if (result->format() != viz::CopyOutputResult::Format::RGBA_TEXTURE) + if (result->destination() != + viz::CopyOutputResult::Destination::kNativeTextures) return mojo::NullRemote(); mojo::PendingRemote releaser; @@ -167,73 +204,79 @@ bool StructTraits(format, gfx::Rect(), false); + *out_p = std::make_unique(format, destination, + gfx::Rect(), false); return true; } switch (format) { - case viz::CopyOutputResult::Format::RGBA_BITMAP: { - absl::optional bitmap_opt; - if (!data.ReadBitmap(&bitmap_opt)) - return false; - if (!bitmap_opt) { - // During shutdown or switching to background on Android, Chrome will - // release GPU context, it will release mapped GPU memory which is used - // in SkBitmap, in that case, the sender will send a null bitmap. So we - // should consider the copy output result is empty. - *out_p = - std::make_unique(format, gfx::Rect(), false); - return true; - } - if (!bitmap_opt->readyToDraw()) - return false; - - *out_p = std::make_unique( - rect, std::move(*bitmap_opt)); - return true; - } - - case viz::CopyOutputResult::Format::RGBA_TEXTURE: { - absl::optional mailbox; - if (!data.ReadMailbox(&mailbox) || !mailbox) - return false; - absl::optional sync_token; - if (!data.ReadSyncToken(&sync_token) || !sync_token) - return false; - absl::optional color_space; - if (!data.ReadColorSpace(&color_space) || !color_space) - return false; - - if (mailbox->IsZero()) { - // Returns an empty result. - *out_p = std::make_unique( - viz::CopyOutputResult::Format::RGBA_TEXTURE, gfx::Rect(), false); - return true; + case viz::CopyOutputResult::Format::RGBA: + switch (destination) { + case viz::CopyOutputResult::Destination::kSystemMemory: { + absl::optional bitmap_opt; + if (!data.ReadBitmap(&bitmap_opt)) + return false; + if (!bitmap_opt) { + // During shutdown or switching to background on Android, Chrome + // will release GPU context, it will release mapped GPU memory which + // is used in SkBitmap, in that case, the sender will send a null + // bitmap. So we should consider the copy output result is empty. + *out_p = std::make_unique( + format, destination, gfx::Rect(), false); + return true; + } + if (!bitmap_opt->readyToDraw()) + return false; + + *out_p = std::make_unique( + rect, std::move(*bitmap_opt)); + return true; + } + + case viz::CopyOutputResult::Destination::kNativeTextures: { + absl::optional mailbox; + if (!data.ReadMailbox(&mailbox) || !mailbox) + return false; + absl::optional sync_token; + if (!data.ReadSyncToken(&sync_token) || !sync_token) + return false; + absl::optional color_space; + if (!data.ReadColorSpace(&color_space) || !color_space) + return false; + + if (mailbox->IsZero()) { + // Returns an empty result. + *out_p = std::make_unique( + format, destination, gfx::Rect(), false); + return true; + } + + auto releaser = data.TakeReleaser< + mojo::PendingRemote>(); + if (!releaser) + return false; // Illegal to provide texture without Releaser. + + // Returns a result with a ReleaseCallback that will return here and + // proxy the callback over mojo to the CopyOutputResult's origin via a + // mojo::Remote remote. + *out_p = std::make_unique( + rect, *mailbox, *sync_token, *color_space, + + base::BindOnce(&Release, std::move(releaser))); + return true; + } } - auto releaser = - data.TakeReleaser>(); - if (!releaser) - return false; // Illegal to provide texture without Releaser. - - // Returns a result with a ReleaseCallback that will return here and proxy - // the callback over mojo to the CopyOutputResult's origin via a - // mojo::Remote remote. - *out_p = std::make_unique( - rect, *mailbox, *sync_token, *color_space, - - base::BindOnce(&Release, std::move(releaser))); - return true; - } - case viz::CopyOutputResult::Format::I420_PLANES: break; // Not intended for transport across service boundaries. } diff --git a/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.h b/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.h index 54f50463850ca..da1946a19bd19 100644 --- a/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.h +++ b/services/viz/public/cpp/compositing/copy_output_result_mojom_traits.h @@ -31,12 +31,25 @@ struct EnumTraits +struct EnumTraits { + static viz::mojom::CopyOutputResultDestination ToMojom( + viz::CopyOutputResult::Destination destination); + + static bool FromMojom(viz::mojom::CopyOutputResultDestination input, + viz::CopyOutputResult::Destination* out); +}; + template <> struct StructTraits> { static viz::CopyOutputResult::Format format( const std::unique_ptr& result); + static viz::CopyOutputResult::Destination destination( + const std::unique_ptr& result); + static const gfx::Rect& rect( const std::unique_ptr& result); diff --git a/services/viz/public/cpp/compositing/mojom_traits_unittest.cc b/services/viz/public/cpp/compositing/mojom_traits_unittest.cc index d9317611877c3..ccf30fc385826 100644 --- a/services/viz/public/cpp/compositing/mojom_traits_unittest.cc +++ b/services/viz/public/cpp/compositing/mojom_traits_unittest.cc @@ -231,7 +231,9 @@ TEST_F(StructTraitsTest, LocalSurfaceId) { TEST_F(StructTraitsTest, CopyOutputRequest_BitmapRequest) { base::test::TaskEnvironment task_environment; - const auto result_format = CopyOutputRequest::ResultFormat::RGBA_BITMAP; + const auto result_format = CopyOutputRequest::ResultFormat::RGBA; + const auto result_destination = + CopyOutputRequest::ResultDestination::kSystemMemory; const gfx::Rect area(5, 7, 44, 55); const auto source = base::UnguessableToken::Deserialize(0xdeadbeef, 0xdeadf00d); @@ -242,7 +244,7 @@ TEST_F(StructTraitsTest, CopyOutputRequest_BitmapRequest) { base::RunLoop run_loop; std::unique_ptr input(new CopyOutputRequest( - result_format, + result_format, result_destination, base::BindOnce( [](base::OnceClosure quit_closure, const gfx::Rect& expected_rect, std::unique_ptr result) { @@ -263,6 +265,7 @@ TEST_F(StructTraitsTest, CopyOutputRequest_BitmapRequest) { mojo::test::SerializeAndDeserialize(input, output); EXPECT_EQ(result_format, output->result_format()); + EXPECT_EQ(result_destination, output->result_destination()); EXPECT_TRUE(output->is_scaled()); EXPECT_EQ(scale_from, output->scale_from()); EXPECT_EQ(scale_to, output->scale_to()); @@ -288,7 +291,8 @@ TEST_F(StructTraitsTest, CopyOutputRequest_MessagePipeBroken) { base::RunLoop run_loop; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](base::OnceClosure quit_closure, std::unique_ptr result) { @@ -308,7 +312,10 @@ TEST_F(StructTraitsTest, CopyOutputRequest_MessagePipeBroken) { TEST_F(StructTraitsTest, CopyOutputRequest_TextureRequest) { base::test::TaskEnvironment task_environment; - const auto result_format = CopyOutputRequest::ResultFormat::RGBA_TEXTURE; + const auto result_format = CopyOutputRequest::ResultFormat::RGBA; + const auto result_destination = + CopyOutputRequest::ResultDestination::kNativeTextures; + const int8_t mailbox_name[GL_MAILBOX_SIZE_CHROMIUM] = { 0, 9, 8, 7, 6, 5, 4, 3, 2, 1, 9, 7, 5, 3, 1, 3}; gpu::Mailbox mailbox; @@ -318,7 +325,7 @@ TEST_F(StructTraitsTest, CopyOutputRequest_TextureRequest) { base::RunLoop run_loop_for_result; std::unique_ptr input(new CopyOutputRequest( - result_format, + result_format, result_destination, base::BindOnce( [](base::OnceClosure quit_closure, const gfx::Rect& expected_rect, std::unique_ptr result) { @@ -333,6 +340,7 @@ TEST_F(StructTraitsTest, CopyOutputRequest_TextureRequest) { mojo::test::SerializeAndDeserialize(input, output); EXPECT_EQ(output->result_format(), result_format); + EXPECT_EQ(output->result_destination(), result_destination); EXPECT_FALSE(output->is_scaled()); EXPECT_FALSE(output->has_source()); EXPECT_FALSE(output->has_area()); @@ -366,7 +374,8 @@ TEST_F(StructTraitsTest, CopyOutputRequest_CallbackRunsOnce) { int n_called = 0; auto request = std::make_unique( - CopyOutputRequest::ResultFormat::RGBA_BITMAP, + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](int* n_called, std::unique_ptr result) { ++*n_called; @@ -380,7 +389,8 @@ TEST_F(StructTraitsTest, CopyOutputRequest_CallbackRunsOnce) { std::move(result_sender_pending_remote)); for (int i = 0; i < 10; i++) result_sender_remote->SendResult(std::make_unique( - request->result_format(), gfx::Rect(), false)); + request->result_format(), request->result_destination(), gfx::Rect(), + false)); EXPECT_EQ(0, n_called); result_sender_remote.FlushForTesting(); EXPECT_EQ(1, n_called); @@ -1185,12 +1195,15 @@ TEST_F(StructTraitsTest, YUVDrawQuad) { TEST_F(StructTraitsTest, CopyOutputResult_EmptyBitmap) { auto input = std::make_unique( - CopyOutputResult::Format::RGBA_BITMAP, gfx::Rect(), false); + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kSystemMemory, gfx::Rect(), false); std::unique_ptr output; mojo::test::SerializeAndDeserialize(input, output); EXPECT_TRUE(output->IsEmpty()); - EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA_BITMAP); + EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA); + EXPECT_EQ(output->destination(), + CopyOutputResult::Destination::kSystemMemory); EXPECT_TRUE(output->rect().IsEmpty()); auto scoped_bitmap = output->ScopedAccessSkBitmap(); auto bitmap = scoped_bitmap.bitmap(); @@ -1202,14 +1215,18 @@ TEST_F(StructTraitsTest, CopyOutputResult_EmptyTexture) { base::test::TaskEnvironment task_environment; auto input = std::make_unique( - CopyOutputResult::Format::RGBA_TEXTURE, gfx::Rect(), false); + CopyOutputRequest::ResultFormat::RGBA, + CopyOutputRequest::ResultDestination::kNativeTextures, gfx::Rect(), + false); EXPECT_TRUE(input->IsEmpty()); std::unique_ptr output; mojo::test::SerializeAndDeserialize(input, output); EXPECT_TRUE(output->IsEmpty()); - EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA_TEXTURE); + EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA); + EXPECT_EQ(output->destination(), + CopyOutputResult::Destination::kNativeTextures); EXPECT_TRUE(output->rect().IsEmpty()); EXPECT_EQ(output->GetTextureResult(), nullptr); } @@ -1229,7 +1246,9 @@ TEST_F(StructTraitsTest, CopyOutputResult_Bitmap) { mojo::test::SerializeAndDeserialize(input, output); EXPECT_FALSE(output->IsEmpty()); - EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA_BITMAP); + EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA); + EXPECT_EQ(output->destination(), + CopyOutputResult::Destination::kSystemMemory); EXPECT_EQ(output->rect(), result_rect); EXPECT_EQ(output->GetTextureResult(), nullptr); @@ -1284,7 +1303,9 @@ TEST_F(StructTraitsTest, CopyOutputResult_Texture) { mojo::test::SerializeAndDeserialize(input, output); EXPECT_FALSE(output->IsEmpty()); - EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA_TEXTURE); + EXPECT_EQ(output->format(), CopyOutputResult::Format::RGBA); + EXPECT_EQ(output->destination(), + CopyOutputResult::Destination::kNativeTextures); EXPECT_EQ(output->rect(), result_rect); ASSERT_NE(output->GetTextureResult(), nullptr); EXPECT_EQ(output->GetTextureResult()->mailbox, mailbox); diff --git a/services/viz/public/mojom/compositing/copy_output_request.mojom b/services/viz/public/mojom/compositing/copy_output_request.mojom index 655d6a07130e1..570271520c00d 100644 --- a/services/viz/public/mojom/compositing/copy_output_request.mojom +++ b/services/viz/public/mojom/compositing/copy_output_request.mojom @@ -13,6 +13,7 @@ import "ui/gfx/geometry/mojom/geometry.mojom"; // See components/viz/common/frame_sinks/copy_output_request.h. struct CopyOutputRequest { CopyOutputResultFormat result_format; + CopyOutputResultDestination result_destination; // Set both to (1,1) for no scaling. Both must have positive X and Y values. gfx.mojom.Vector2d scale_from; diff --git a/services/viz/public/mojom/compositing/copy_output_result.mojom b/services/viz/public/mojom/compositing/copy_output_result.mojom index 92d8e9bbfb4bf..5036e0c8cc29c 100644 --- a/services/viz/public/mojom/compositing/copy_output_result.mojom +++ b/services/viz/public/mojom/compositing/copy_output_result.mojom @@ -12,24 +12,32 @@ import "ui/gfx/geometry/mojom/geometry.mojom"; import "ui/gfx/mojom/color_space.mojom"; // See components/viz/common/frame_sinks/copy_output_result.h. +// Only the RGBA format is supported for cross-process requests. +// CopyOutputRequests with other formats are issued internally to viz/. enum CopyOutputResultFormat { - RGBA_BITMAP, - RGBA_TEXTURE, + RGBA, +}; + +// See components/viz/common/frame_sinks/copy_output_result.h. +enum CopyOutputResultDestination { + kSystemMemory, + kNativeTextures, }; // See components/viz/common/frame_sinks/copy_output_result.h. struct CopyOutputResult { CopyOutputResultFormat format; + CopyOutputResultDestination destination; gfx.mojom.Rect rect; - // Present when the format is RGBA_BITMAP. + // Present when the destination is kSystemMemory. BitmapInSharedMemory? bitmap; - // Present when the format is RGBA_TEXTURE. + // Present when the destination is kNativeTextures. // If the result is empty, then the |mailbox| will be present but empty. gpu.mojom.Mailbox? mailbox; gpu.mojom.SyncToken? sync_token; gfx.mojom.ColorSpace? color_space; - // Present when the format is RGBA_TEXTURE and |mailbox| is non-empty. + // Present when the destination is kNativeTextures and |mailbox| is non-empty. pending_remote? releaser; }; diff --git a/ui/android/delegated_frame_host_android.cc b/ui/android/delegated_frame_host_android.cc index a483dbbb6797d..1757415575f8e 100644 --- a/ui/android/delegated_frame_host_android.cc +++ b/ui/android/delegated_frame_host_android.cc @@ -117,7 +117,8 @@ void DelegatedFrameHostAndroid::CopyFromCompositingSurface( } std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce( [](base::OnceCallback callback, std::unique_ptr diff --git a/ui/android/delegated_frame_host_android.h b/ui/android/delegated_frame_host_android.h index 758a8f6c55f3d..dcc5407e40920 100644 --- a/ui/android/delegated_frame_host_android.h +++ b/ui/android/delegated_frame_host_android.h @@ -91,8 +91,8 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid const viz::FrameSinkId& GetFrameSinkId() const; // Should only be called when the host has a content layer. Use this for one- - // off screen capture, not for video. Always provides RGBA_BITMAP - // CopyOutputResults. + // off screen capture, not for video. Always provides ResultFormat::RGBA, + // ResultDestination::kSystemMemory CopyOutputResults. void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& output_size, diff --git a/ui/compositor/layer_unittest.cc b/ui/compositor/layer_unittest.cc index 62aa365fdf204..e4f3b926bfd86 100644 --- a/ui/compositor/layer_unittest.cc +++ b/ui/compositor/layer_unittest.cc @@ -177,7 +177,8 @@ class LayerWithRealCompositorTest : public testing::Test { scoped_refptr holder(new ReadbackHolder); std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, base::BindOnce(&ReadbackHolder::OutputRequestCallback, holder)); request->set_area(source_rect); diff --git a/ui/snapshot/snapshot_android.cc b/ui/snapshot/snapshot_android.cc index b6596a13f5664..ae97915c66b41 100644 --- a/ui/snapshot/snapshot_android.cc +++ b/ui/snapshot/snapshot_android.cc @@ -40,7 +40,8 @@ static std::unique_ptr CreateCopyRequest( viz::CopyOutputRequest::CopyOutputRequestCallback callback) { std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, std::move(callback)); float scale = ui::GetScaleFactorForNativeView(view); request->set_area(gfx::ScaleToEnclosingRect(source_rect, scale)); diff --git a/ui/snapshot/snapshot_aura.cc b/ui/snapshot/snapshot_aura.cc index 1c60d741cd649..e09bc73c22efe 100644 --- a/ui/snapshot/snapshot_aura.cc +++ b/ui/snapshot/snapshot_aura.cc @@ -11,6 +11,7 @@ #include "base/callback.h" #include "base/task_runner_util.h" #include "base/threading/sequenced_task_runner_handle.h" +#include "build/build_config.h" #include "components/viz/common/frame_sinks/copy_output_request.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/aura/window.h" @@ -34,7 +35,8 @@ static void MakeAsyncCopyRequest( viz::CopyOutputRequest::CopyOutputRequestCallback callback) { std::unique_ptr request = std::make_unique( - viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP, + viz::CopyOutputRequest::ResultFormat::RGBA, + viz::CopyOutputRequest::ResultDestination::kSystemMemory, std::move(callback)); request->set_area(source_rect); request->set_result_task_runner(