From d3c1156332e85b01e1ec4fc1382766fc754f9839 Mon Sep 17 00:00:00 2001 From: Sergey Ulanov Date: Thu, 17 Feb 2022 06:11:47 +0000 Subject: [PATCH] Revert "[Fuchsia] Hide ImagePipe when the view is detached from the scene" This reverts commit bf61a251bf7e9db90ac1cf3f029a1256b1f56ed1. Reason for revert: b/219533087 Original change's description: > [Fuchsia] Hide ImagePipe when the view is detached from the scene > > Previously ScenicSurface was pushing an empty image to the ImagePipe to > ensure that the outdated content is not displayed when the view is > re-added to the scene. That approach didn't work reliably because > compositor may push new frames after the empty frame. > This change implements a different solution: ImagePipe is detached from > the view until the surface is ready to present a new frame. > > (cherry picked from commit 02544b90f5a9fb9f1b1f2c779c76f68ac5544680) > > Bug: 1280802 > Change-Id: I616c6250682f3aeff43287f7b2cdbf2278574c8a > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456844 > Reviewed-by: Emircan Uysaler > Commit-Queue: Sergey Ulanov > Auto-Submit: Sergey Ulanov > Cr-Original-Commit-Position: refs/heads/main@{#970240} > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3462095 > Bot-Commit: Rubber Stamper > Cr-Commit-Position: refs/branch-heads/4692@{#1516} > Cr-Branched-From: 038cd96142d384c0d2238973f1cb277725a62eba-refs/heads/main@{#938553} Bug: 1280802 Change-Id: I5361e23f368cc0695ab0a96f323a6a934d810e60 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3470846 Bot-Commit: Rubber Stamper Commit-Queue: Sergey Ulanov Cr-Commit-Position: refs/branch-heads/4692@{#1520} Cr-Branched-From: 038cd96142d384c0d2238973f1cb277725a62eba-refs/heads/main@{#938553} --- ui/ozone/platform/scenic/scenic_surface.cc | 97 ++++++++++++++++------ ui/ozone/platform/scenic/scenic_surface.h | 5 +- 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/ui/ozone/platform/scenic/scenic_surface.cc b/ui/ozone/platform/scenic/scenic_surface.cc index 943ab4ec95e68..100adf57f82a5 100644 --- a/ui/ozone/platform/scenic/scenic_surface.cc +++ b/ui/ozone/platform/scenic/scenic_surface.cc @@ -85,7 +85,6 @@ ScenicSurface::ScenicSurface( scenic::SessionPtrAndListenerRequest sesion_and_listener_request) : scenic_session_(std::move(sesion_and_listener_request)), safe_presenter_(&scenic_session_), - root_node_(&scenic_session_), main_shape_(&scenic_session_), main_material_(&scenic_session_), scenic_surface_factory_(scenic_surface_factory), @@ -97,7 +96,6 @@ ScenicSurface::ScenicSurface( main_shape_.SetShape(scenic::Rectangle(&scenic_session_, 1.f, 1.f)); main_shape_.SetMaterial(transparent_material); main_shape_.SetEventMask(fuchsia::ui::gfx::kMetricsEventMask); - root_node_.AddChild(main_shape_); scenic_surface_factory->AddSurface(window, this); scenic_session_.SetDebugName("Chromium ScenicSurface"); scenic_session_.set_event_handler( @@ -136,22 +134,13 @@ void ScenicSurface::OnScenicEvents( main_shape_size_.set_width(metrics.scale_x); main_shape_size_.set_height(metrics.scale_y); UpdateViewHolderScene(); - safe_presenter_.QueuePresent(); - break; - } - case fuchsia::ui::gfx::Event::kViewAttachedToScene: { - DCHECK(event.gfx().view_detached_from_scene().view_id == parent_->id()); - - // `root_node_` will be attached in the next Present(). This ensures - // that outdated content is never displaye don the screen. - attach_root_on_present_ = true; break; } case fuchsia::ui::gfx::Event::kViewDetachedFromScene: { DCHECK(event.gfx().view_detached_from_scene().view_id == parent_->id()); - - root_node_.Detach(); - safe_presenter_.QueuePresent(); + // Present an empty frame to ensure that the outdated content doesn't + // become visible if the view is attached again. + PresentEmptyImage(); break; } default: @@ -227,20 +216,11 @@ void ScenicSurface::Present( } } - if (layout_update_required || attach_root_on_present_) { - if (attach_root_on_present_) { - parent_->AddChild(root_node_); - attach_root_on_present_ = false; - } - - if (layout_update_required) - UpdateViewHolderScene(); - + if (layout_update_required) { for (auto& fence : acquire_fences) { scenic_session_.EnqueueAcquireFence(std::move(fence.Clone().owned_event)); } - - safe_presenter_.QueuePresent(); + UpdateViewHolderScene(); } pending_frames_.emplace_back( @@ -389,7 +369,7 @@ bool ScenicSurface::RemoveOverlayView(gfx::SysmemBufferCollectionId id) { auto it = overlay_views_.find(id); DCHECK(it != overlay_views_.end()); - it->second.entity_node.Detach(); + parent_->DetachChild(it->second.entity_node); safe_presenter_.QueuePresent(); overlay_views_.erase(it); return true; @@ -404,6 +384,7 @@ mojo::PlatformHandle ScenicSurface::CreateView() { auto tokens = scenic::ViewTokenPair::New(); parent_ = std::make_unique( &scenic_session_, std::move(tokens.view_token), "chromium surface"); + parent_->AddChild(main_shape_); // Defer first Present call to SetTextureToNewImagePipe(). return mojo::PlatformHandle(std::move(tokens.view_holder_token.value)); @@ -482,7 +463,7 @@ void ScenicSurface::UpdateViewHolderScene() { } // No-op if the node is already attached. - root_node_.AddChild(overlay_view.entity_node); + parent_->AddChild(overlay_view.entity_node); // Apply view bound clipping around the ImagePipe that has size 1x1 and // centered at (0, 0). @@ -541,6 +522,68 @@ void ScenicSurface::UpdateViewHolderScene() { main_material_.SetColor(255, 255, 255, 0 > min_z_order ? 254 : 255); main_shape_.SetTranslation(0.f, 0.f, min_z_order * kElevationStep); + + safe_presenter_.QueuePresent(); +} + +void ScenicSurface::PresentEmptyImage() { + if (last_frame_present_time_ == base::TimeTicks()) + return; + + fuchsia::sysmem::BufferCollectionTokenSyncPtr dummy_collection_token; + zx_status_t status = + sysmem_buffer_manager_->GetAllocator()->AllocateSharedCollection( + dummy_collection_token.NewRequest()); + if (status != ZX_OK) { + ZX_DLOG(ERROR, status) + << "fuchsia.sysmem.Allocator.AllocateSharedCollection()"; + return; + } + dummy_collection_token->SetName(100u, "DummyImageCollection"); + dummy_collection_token->SetDebugClientInfo("chromium", 0u); + + fuchsia::sysmem::BufferCollectionTokenSyncPtr token_for_scenic; + dummy_collection_token->Duplicate(ZX_RIGHT_SAME_RIGHTS, + token_for_scenic.NewRequest()); + token_for_scenic->SetDebugClientInfo("scenic", 0u); + + const uint32_t image_id = ++next_unique_id_; + image_pipe_->AddBufferCollection(image_id, std::move(token_for_scenic)); + + // Synchroniously wait for the collection to be allocated before proceeding. + fuchsia::sysmem::BufferCollectionSyncPtr dummy_collection; + status = sysmem_buffer_manager_->GetAllocator()->BindSharedCollection( + std::move(dummy_collection_token), dummy_collection.NewRequest()); + if (status != ZX_OK) { + ZX_DLOG(ERROR, status) << "fuchsia.sysmem.Allocator.BindSharedCollection()"; + return; + } + + fuchsia::sysmem::BufferCollectionConstraints constraints; + constraints.usage.none = fuchsia::sysmem::noneUsage; + constraints.min_buffer_count = 1; + constraints.image_format_constraints_count = 0; + status = dummy_collection->SetConstraints(/*has_constraints=*/true, + std::move(constraints)); + zx_status_t wait_status; + fuchsia::sysmem::BufferCollectionInfo_2 buffers_info; + status = + dummy_collection->WaitForBuffersAllocated(&wait_status, &buffers_info); + if (status != ZX_OK) { + ZX_DLOG(ERROR, status) << "fuchsia.sysmem.BufferCollection failed"; + return; + } + dummy_collection->Close(); + + // Present the first image from the collection. + fuchsia::sysmem::ImageFormat_2 image_format; + image_format.coded_width = 1; + image_format.coded_height = 1; + image_pipe_->AddImage(image_id, image_id, 0, image_format); + + image_pipe_->PresentImage(image_id, last_frame_present_time_.ToZxTime(), {}, + {}, [](fuchsia::images::PresentationInfo) {}); + image_pipe_->RemoveBufferCollection(image_id); } ScenicSurface::PresentedFrame::PresentedFrame( diff --git a/ui/ozone/platform/scenic/scenic_surface.h b/ui/ozone/platform/scenic/scenic_surface.h index a09a18d564e33..42c8248e29ab2 100644 --- a/ui/ozone/platform/scenic/scenic_surface.h +++ b/ui/ozone/platform/scenic/scenic_surface.h @@ -142,6 +142,8 @@ class ScenicSurface : public PlatformWindowSurface { void OnPresentComplete(fuchsia::images::PresentationInfo presentation_info); void UpdateViewHolderScene(); + void PresentEmptyImage(); + scenic::Session scenic_session_; std::unique_ptr parent_; @@ -175,7 +177,6 @@ class ScenicSurface : public PlatformWindowSurface { std::vector release_fences_from_last_present_; // Scenic resources used for the primary plane, that is not an overlay. - scenic::EntityNode root_node_; scenic::ShapeNode main_shape_; scenic::Material main_material_; gfx::SizeF main_shape_size_; @@ -184,8 +185,6 @@ class ScenicSurface : public PlatformWindowSurface { SysmemBufferManager* const sysmem_buffer_manager_; const gfx::AcceleratedWidget window_; - bool attach_root_on_present_ = false; - struct OverlayViewInfo { OverlayViewInfo(scenic::ViewHolder holder, scenic::EntityNode node);