From ecf13e70d29a1c640985345e933c81ba403884ec Mon Sep 17 00:00:00 2001 From: jbauman Date: Tue, 24 Nov 2015 23:54:24 -0800 Subject: Revert of List all child surfaces (including occluded) in CompositorFrame (patchset #7 id:120001 of https://codereview.chromium.org/1430363002/ ) Reason for revert: Seems to have broken resize of some OOPIF. BUG=560237 Original issue's description: > List all child surfaces (including occluded) in CompositorFrame > > Then any occluded child surface with a copy request can be aggregated > (and have its copy done) whenever its parent draws. > > BUG=529378 > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel > > Committed: https://crrev.com/915e16b393182f9ccc0ddc4caadd5f7744b25fd4 > Cr-Commit-Position: refs/heads/master@{#360896} TBR=danakj@chromium.org,kenrb@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=529378 Review URL: https://codereview.chromium.org/1475643006 Cr-Commit-Position: refs/heads/master@{#361604} --- cc/surfaces/surface.cc | 7 +- cc/surfaces/surface_aggregator.cc | 66 ++----------------- cc/surfaces/surface_aggregator.h | 12 +--- cc/surfaces/surface_aggregator_unittest.cc | 102 ----------------------------- cc/surfaces/surface_factory_unittest.cc | 4 +- 5 files changed, 14 insertions(+), 177 deletions(-) (limited to 'cc/surfaces') diff --git a/cc/surfaces/surface.cc b/cc/surfaces/surface.cc index 563415c..8faff98 100644 --- a/cc/surfaces/surface.cc +++ b/cc/surfaces/surface.cc @@ -66,7 +66,12 @@ void Surface::QueueFrame(scoped_ptr frame, std::vector new_referenced_surfaces; if (current_frame_) { - new_referenced_surfaces = current_frame_->metadata.referenced_surfaces; + for (auto& render_pass : + current_frame_->delegated_frame_data->render_pass_list) { + new_referenced_surfaces.insert(new_referenced_surfaces.end(), + render_pass->referenced_surfaces.begin(), + render_pass->referenced_surfaces.end()); + } } if (previous_frame) { diff --git a/cc/surfaces/surface_aggregator.cc b/cc/surfaces/surface_aggregator.cc index 181b07f..4c1441a 100644 --- a/cc/surfaces/surface_aggregator.cc +++ b/cc/surfaces/surface_aggregator.cc @@ -58,10 +58,6 @@ SurfaceAggregator::~SurfaceAggregator() { ProcessAddedAndRemovedSurfaces(); } -SurfaceAggregator::PrewalkResult::PrewalkResult() {} - -SurfaceAggregator::PrewalkResult::~PrewalkResult() {} - // Create a clip rect for an aggregated quad from the original clip rect and // the clip rect from the surface it's on. SurfaceAggregator::ClipData SurfaceAggregator::CalculateClipRect( @@ -495,8 +491,7 @@ void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() { // Walk the Surface tree from surface_id. Validate the resources of the current // surface and its descendants, check if there are any copy requests, and // return the combined damage rect. -gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id, - PrewalkResult* result) { +gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id) { if (referenced_surfaces_.count(surface_id)) return gfx::Rect(); Surface* surface = manager_->GetSurfaceForId(surface_id); @@ -574,7 +569,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id, provider_->DeclareUsedResourcesFromChild(child_id, referenced_resources); for (const auto& render_pass : frame_data->render_pass_list) - result->has_copy_requests |= !render_pass->copy_requests.empty(); + has_copy_requests_ |= !render_pass->copy_requests.empty(); gfx::Rect damage_rect; if (!frame_data->render_pass_list.empty()) { @@ -588,63 +583,14 @@ gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id, SurfaceSet::iterator it = referenced_surfaces_.insert(surface->surface_id()).first; for (const auto& surface_info : child_surfaces) { - gfx::Rect surface_damage = PrewalkTree(surface_info.first, result); + gfx::Rect surface_damage = PrewalkTree(surface_info.first); damage_rect.Union( MathUtil::MapEnclosingClippedRect(surface_info.second, surface_damage)); } - - for (const auto& surface_id : surface_frame->metadata.referenced_surfaces) { - if (!contained_surfaces_.count(surface_id)) { - result->undrawn_surfaces.insert(surface_id); - PrewalkTree(surface_id, result); - } - } - referenced_surfaces_.erase(it); return damage_rect; } -void SurfaceAggregator::CopyUndrawnSurfaces(PrewalkResult* prewalk_result) { - // undrawn_surfaces are Surfaces that were identified by prewalk as being - // referenced by a drawn Surface, but aren't contained in a SurfaceDrawQuad. - // They need to be iterated over to ensure that any copy requests on them - // (or on Surfaces they reference) are executed. - std::vector surfaces_to_copy( - prewalk_result->undrawn_surfaces.begin(), - prewalk_result->undrawn_surfaces.end()); - - for (size_t i = 0; i < surfaces_to_copy.size(); i++) { - SurfaceId surface_id = surfaces_to_copy[i]; - Surface* surface = manager_->GetSurfaceForId(surface_id); - const CompositorFrame* surface_frame = surface->GetEligibleFrame(); - if (!surface_frame) - continue; - bool surface_has_copy_requests = false; - for (const auto& render_pass : - surface_frame->delegated_frame_data->render_pass_list) { - surface_has_copy_requests |= !render_pass->copy_requests.empty(); - } - if (!surface_has_copy_requests) { - // Children are not necessarily included in undrawn_surfaces (because - // they weren't referenced directly from a drawn surface), but may have - // copy requests, so make sure to check them as well. - for (const auto& child_id : surface_frame->metadata.referenced_surfaces) { - // Don't iterate over the child Surface if it was already listed as a - // child of a different Surface, or in the case where there's infinite - // recursion. - if (!prewalk_result->undrawn_surfaces.count(child_id)) { - surfaces_to_copy.push_back(child_id); - prewalk_result->undrawn_surfaces.insert(child_id); - } - } - } else { - SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first; - CopyPasses(surface_frame->delegated_frame_data.get(), surface); - referenced_surfaces_.erase(it); - } - } -} - scoped_ptr SurfaceAggregator::Aggregate(SurfaceId surface_id) { Surface* surface = manager_->GetSurfaceForId(surface_id); DCHECK(surface); @@ -663,11 +609,9 @@ scoped_ptr SurfaceAggregator::Aggregate(SurfaceId surface_id) { dest_pass_list_ = &frame->delegated_frame_data->render_pass_list; valid_surfaces_.clear(); - PrewalkResult prewalk_result; - root_damage_rect_ = PrewalkTree(surface_id, &prewalk_result); - has_copy_requests_ = prewalk_result.has_copy_requests; + has_copy_requests_ = false; + root_damage_rect_ = PrewalkTree(surface_id); - CopyUndrawnSurfaces(&prewalk_result); SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first; CopyPasses(root_surface_frame->delegated_frame_data.get(), surface); referenced_surfaces_.erase(it); diff --git a/cc/surfaces/surface_aggregator.h b/cc/surfaces/surface_aggregator.h index 6df7e6b..738aeec 100644 --- a/cc/surfaces/surface_aggregator.h +++ b/cc/surfaces/surface_aggregator.h @@ -60,15 +60,6 @@ class CC_SURFACES_EXPORT SurfaceAggregator { gfx::Rect rect; }; - struct PrewalkResult { - PrewalkResult(); - ~PrewalkResult(); - bool has_copy_requests = false; - // This is the set of Surfaces that were referenced by another Surface, but - // not included in a SurfaceDrawQuad. - std::set undrawn_surfaces; - }; - ClipData CalculateClipRect(const ClipData& surface_clip, const ClipData& quad_clip, const gfx::Transform& target_transform); @@ -92,8 +83,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator { const ClipData& clip_rect, RenderPass* dest_pass, SurfaceId surface_id); - gfx::Rect PrewalkTree(SurfaceId surface_id, PrewalkResult* result); - void CopyUndrawnSurfaces(PrewalkResult* prewalk); + gfx::Rect PrewalkTree(SurfaceId surface_id); void CopyPasses(const DelegatedFrameData* frame_data, Surface* surface); // Remove Surfaces that were referenced before but aren't currently diff --git a/cc/surfaces/surface_aggregator_unittest.cc b/cc/surfaces/surface_aggregator_unittest.cc index 86eedf6..53b01e0 100644 --- a/cc/surfaces/surface_aggregator_unittest.cc +++ b/cc/surfaces/surface_aggregator_unittest.cc @@ -477,108 +477,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, RootCopyRequest) { factory_.Destroy(embedded_surface_id); } -TEST_F(SurfaceAggregatorValidSurfaceTest, UnreferencedSurface) { - SurfaceId embedded_surface_id = allocator_.GenerateId(); - factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - - test::Quad embedded_quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN)}; - test::Pass embedded_passes[] = { - test::Pass(embedded_quads, arraysize(embedded_quads))}; - - SubmitCompositorFrame(embedded_passes, arraysize(embedded_passes), - embedded_surface_id); - scoped_ptr copy_request( - CopyOutputRequest::CreateEmptyRequest()); - CopyOutputRequest* copy_request_ptr = copy_request.get(); - factory_.RequestCopyOfSurface(embedded_surface_id, copy_request.Pass()); - - SurfaceId parent_surface_id = allocator_.GenerateId(); - factory_.Create(parent_surface_id); - Surface* parent_surface = manager_.GetSurfaceForId(parent_surface_id); - - test::Quad parent_quads[] = { - test::Quad::SolidColorQuad(SK_ColorWHITE), - test::Quad::SurfaceQuad(embedded_surface_id, 1.f), - test::Quad::SolidColorQuad(SK_ColorBLACK)}; - test::Pass parent_passes[] = { - test::Pass(parent_quads, arraysize(parent_quads))}; - - { - scoped_ptr frame_data(new DelegatedFrameData); - AddPasses(&frame_data->render_pass_list, gfx::Rect(SurfaceSize()), - parent_passes, arraysize(parent_passes)); - - scoped_ptr frame(new CompositorFrame); - frame->delegated_frame_data = frame_data.Pass(); - frame->metadata.referenced_surfaces.push_back(embedded_surface_id); - - factory_.SubmitCompositorFrame(parent_surface_id, frame.Pass(), - SurfaceFactory::DrawCallback()); - } - - test::Quad root_quads[] = {test::Quad::SolidColorQuad(SK_ColorWHITE), - test::Quad::SolidColorQuad(SK_ColorBLACK)}; - test::Pass root_passes[] = {test::Pass(root_quads, arraysize(root_quads))}; - - { - scoped_ptr frame_data(new DelegatedFrameData); - AddPasses(&frame_data->render_pass_list, gfx::Rect(SurfaceSize()), - root_passes, arraysize(root_passes)); - - scoped_ptr frame(new CompositorFrame); - frame->delegated_frame_data = frame_data.Pass(); - frame->metadata.referenced_surfaces.push_back(parent_surface_id); - - factory_.SubmitCompositorFrame(root_surface_id_, frame.Pass(), - SurfaceFactory::DrawCallback()); - } - - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(parent_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - - scoped_ptr aggregated_frame = - aggregator_.Aggregate(root_surface_id_); - - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - - ASSERT_TRUE(aggregated_frame); - ASSERT_TRUE(aggregated_frame->delegated_frame_data); - - DelegatedFrameData* frame_data = aggregated_frame->delegated_frame_data.get(); - - // First pass should come from surface that had a copy request but was not - // referenced directly. The second pass comes from the root surface. - // parent_quad should be ignored because it is neither referenced through a - // SurfaceDrawQuad nor has a copy request on it. - test::Pass expected_passes[] = { - test::Pass(embedded_quads, arraysize(embedded_quads)), - test::Pass(root_quads, arraysize(root_quads))}; - TestPassesMatchExpectations(expected_passes, arraysize(expected_passes), - &frame_data->render_pass_list); - ASSERT_EQ(2u, frame_data->render_pass_list.size()); - ASSERT_EQ(1u, frame_data->render_pass_list[0]->copy_requests.size()); - DCHECK_EQ(copy_request_ptr, - frame_data->render_pass_list[0]->copy_requests[0].get()); - - SurfaceId surface_ids[] = {root_surface_id_, parent_surface_id, - embedded_surface_id}; - EXPECT_EQ(arraysize(surface_ids), - aggregator_.previous_contained_surfaces().size()); - for (size_t i = 0; i < arraysize(surface_ids); i++) { - EXPECT_TRUE( - aggregator_.previous_contained_surfaces().find(surface_ids[i]) != - aggregator_.previous_contained_surfaces().end()); - } - - factory_.Destroy(parent_surface_id); - factory_.Destroy(embedded_surface_id); -} - // This tests referencing a surface that has multiple render passes. TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSurfaceReference) { SurfaceId embedded_surface_id = child_allocator_.GenerateId(); diff --git a/cc/surfaces/surface_factory_unittest.cc b/cc/surfaces/surface_factory_unittest.cc index 749d7c5..fa05c2e 100644 --- a/cc/surfaces/surface_factory_unittest.cc +++ b/cc/surfaces/surface_factory_unittest.cc @@ -490,10 +490,10 @@ TEST_F(SurfaceFactoryTest, DestroyCycle) { // Give id2 a frame that references surface_id_. { scoped_ptr render_pass(RenderPass::Create()); + render_pass->referenced_surfaces.push_back(surface_id_); scoped_ptr frame_data(new DelegatedFrameData); frame_data->render_pass_list.push_back(std::move(render_pass)); scoped_ptr frame(new CompositorFrame); - frame->metadata.referenced_surfaces.push_back(surface_id_); frame->delegated_frame_data = std::move(frame_data); factory_->SubmitCompositorFrame(id2, std::move(frame), SurfaceFactory::DrawCallback()); @@ -503,10 +503,10 @@ TEST_F(SurfaceFactoryTest, DestroyCycle) { // Give surface_id_ a frame that references id2. { scoped_ptr render_pass(RenderPass::Create()); + render_pass->referenced_surfaces.push_back(id2); scoped_ptr frame_data(new DelegatedFrameData); frame_data->render_pass_list.push_back(std::move(render_pass)); scoped_ptr frame(new CompositorFrame); - frame->metadata.referenced_surfaces.push_back(id2); frame->delegated_frame_data = std::move(frame_data); factory_->SubmitCompositorFrame(surface_id_, std::move(frame), SurfaceFactory::DrawCallback()); -- cgit v1.1