Skip to content

Commit

Permalink
Revert "[Fuchsia] Hide ImagePipe when the view is detached from the s…
Browse files Browse the repository at this point in the history 
…cene"

This reverts commit bf61a25.

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 02544b9)
>
> Bug: 1280802
> Change-Id: I616c6250682f3aeff43287f7b2cdbf2278574c8a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456844
> Reviewed-by: Emircan Uysaler <emircan@chromium.org>
> Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
> Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
> Cr-Original-Commit-Position: refs/heads/main@{#970240}
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3462095
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/branch-heads/4692@{#1516}
> Cr-Branched-From: 038cd96-refs/heads/main@{#938553}

Bug: 1280802
Change-Id: I5361e23f368cc0695ab0a96f323a6a934d810e60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3470846
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/branch-heads/4692@{#1520}
Cr-Branched-From: 038cd96-refs/heads/main@{#938553}
  • Loading branch information
@SergeyUlanov
SergeyUlanov authored and Chromium LUCI CQ committed Feb 17, 2022
1 parent 523048f commit d3c1156
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 30 deletions.
97 changes: 70 additions & 27 deletions ui/ozone/platform/scenic/scenic_surface.cc
View file
Expand Up @@ -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),
Expand All @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand All @@ -404,6 +384,7 @@ mojo::PlatformHandle ScenicSurface::CreateView() {
auto tokens = scenic::ViewTokenPair::New();
parent_ = std::make_unique<scenic::View>(
&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));
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 2 additions & 3 deletions ui/ozone/platform/scenic/scenic_surface.h
View file
Expand Up @@ -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<scenic::View> parent_;

Expand Down Expand Up @@ -175,7 +177,6 @@ class ScenicSurface : public PlatformWindowSurface {
std::vector<zx::event> 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_;
Expand All @@ -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);

Expand Down

0 comments on commit d3c1156

Please sign in to comment.