Commit ff22cdad authored by Kramer Ge's avatar Kramer Ge Committed by Commit Bot

[ozone/wayland]Fix unresponsive picture-in-picture video playback

When exiting picture-in-picture view, the associated wl_surface is not
destroyed, but attaches a nil wl_buffer. When restarting
picture-in-picture mode, skia_output_device_buffer_queue does not know
it has to reschedule the background image, resulting in
picture-in-picture being unresponsive.

Save the background buffer and re-attach it in wayland_window s.t. the
buffer queue does not need to schedule/damage background every frame.

Change-Id: I882e73cecd083d56e360ba61662db9eae566d199
Bug: 1142825
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519607
Commit-Queue: Kramer Ge <fangzhoug@chromium.org>
Commit-Queue: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: default avatarRobert Kroeger <rjkroege@chromium.org>
Reviewed-by: default avatarMaksim Sisov (GMT+2) <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#825052}
parent ffac685b
......@@ -185,17 +185,8 @@ bool SkiaOutputDeviceBufferQueue::IsPrimaryPlaneOverlay() const {
void SkiaOutputDeviceBufferQueue::SchedulePrimaryPlane(
const base::Optional<OverlayProcessorInterface::OutputSurfaceOverlayPlane>&
plane) {
if (background_image_) {
if (!background_image_is_scheduled_)
background_image_->BeginPresent();
// WaylandWindow can attach a null wl_buffer to its surface to hide its
// content so needs to reschedule |background_image_| so that
// the wl_surface has a non-null wl_buffer when the window re-appears.
//
// TODO(fangzhoug): It should not be necessary to schedule
// |background_image_| every frame. Make this a responsibility of
// WaylandWindow instead.
if (background_image_ && !background_image_is_scheduled_) {
background_image_->BeginPresent();
presenter_->ScheduleBackground(background_image_.get());
background_image_is_scheduled_ = true;
}
......
......@@ -291,7 +291,6 @@ void GbmSurfacelessWayland::OnSubmission(BufferId buffer_id,
break;
}
}
DCHECK(erased);
// Following while loop covers below scenario:
// frame_1 submitted a buffer_1 for overlay; frame_2 submitted a buffer_2
......@@ -354,8 +353,6 @@ void GbmSurfacelessWayland::OnPresentation(
}
}
DCHECK(erased);
while (!pending_presentation_frames_.empty() &&
pending_presentation_frames_.front()
->pending_presentation_buffers.empty()) {
......
......@@ -24,6 +24,7 @@ void WaylandAuxiliaryWindow::Show(bool inactive) {
CreateSubsurface();
UpdateBufferScale(false);
WaylandWindow::Show(inactive);
}
void WaylandAuxiliaryWindow::Hide() {
......
......@@ -47,6 +47,7 @@ void WaylandPopup::Show(bool inactive) {
UpdateBufferScale(false);
connection()->ScheduleFlush();
WaylandWindow::Show(inactive);
}
void WaylandPopup::Hide() {
......
......@@ -135,6 +135,8 @@ void WaylandToplevelWindow::Show(bool inactive) {
if (auto* drag_controller = connection()->window_drag_controller())
drag_controller->OnToplevelWindowCreated(this);
WaylandWindow::Show(inactive);
}
void WaylandToplevelWindow::Hide() {
......
......@@ -122,7 +122,8 @@ void WaylandWindow::SetPointerFocus(bool focus) {
}
void WaylandWindow::Show(bool inactive) {
NOTREACHED();
if (background_buffer_id_ != 0u)
should_attach_background_buffer_ = true;
}
void WaylandWindow::Hide() {
......@@ -643,11 +644,17 @@ bool WaylandWindow::CommitOverlays(
}
root_surface_->SetViewportDestination(bounds_px_.size());
if (overlays.front()->z_order == INT32_MIN) {
background_buffer_id_ = overlays.front()->buffer_id;
should_attach_background_buffer_ = true;
}
if (should_attach_background_buffer_) {
connection_->buffer_manager_host()->CommitBufferInternal(
root_surface(), overlays.front()->buffer_id,
/*damage_region=*/gfx::Rect(0, 0, 1, 1),
root_surface(), background_buffer_id_, /*damage_region=*/gfx::Rect(),
/*wait_for_frame_callback=*/true);
should_attach_background_buffer_ = false;
} else {
// Subsurfaces are set to sync, above surface configs will only take effect
// when root_surface is committed.
......
......@@ -232,6 +232,8 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
WaylandWindow* parent_window_ = nullptr;
WaylandWindow* child_window_ = nullptr;
bool should_attach_background_buffer_ = false;
uint32_t background_buffer_id_ = 0u;
// |root_surface_| is a surface for the opaque background. Its z-order is
// INT32_MIN.
std::unique_ptr<WaylandSurface> root_surface_;
......
......@@ -12,6 +12,7 @@
#include <xdg-shell-server-protocol.h>
#include <xdg-shell-unstable-v6-server-protocol.h>
#include "base/files/file_util.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -22,6 +23,7 @@
#include "ui/gfx/native_widget_types.h"
#include "ui/gfx/overlay_transform.h"
#include "ui/ozone/platform/wayland/common/wayland_util.h"
#include "ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h"
#include "ui/ozone/platform/wayland/host/wayland_subsurface.h"
#include "ui/ozone/platform/wayland/test/mock_pointer.h"
#include "ui/ozone/platform/wayland/test/mock_surface.h"
......@@ -81,6 +83,15 @@ class ScopedWlArray {
wl_array array_;
};
base::ScopedFD MakeFD() {
base::FilePath temp_path;
EXPECT_TRUE(base::CreateTemporaryFile(&temp_path));
auto file =
base::File(temp_path, base::File::FLAG_READ | base::File::FLAG_WRITE |
base::File::FLAG_CREATE_ALWAYS);
return base::ScopedFD(file.TakePlatformFile());
}
} // namespace
class WaylandWindowTest : public WaylandTest {
......@@ -1879,6 +1890,87 @@ TEST_P(WaylandWindowTest, DestroysCreatesPopupsOnHideShow) {
EXPECT_TRUE(mock_surface->xdg_surface()->xdg_popup());
}
TEST_P(WaylandWindowTest, RemovesReattachesBackgroundOnHideShow) {
EXPECT_TRUE(connection_->buffer_manager_host());
auto interface_ptr = connection_->buffer_manager_host()->BindInterface();
buffer_manager_gpu_->Initialize(std::move(interface_ptr), {}, false);
// Setup wl_buffers.
constexpr uint32_t buffer_id1 = 1;
constexpr uint32_t buffer_id2 = 2;
gfx::Size buffer_size(1024, 768);
auto length = 1024 * 768 * 4;
buffer_manager_gpu_->CreateShmBasedBuffer(MakeFD(), length, buffer_size,
buffer_id1);
buffer_manager_gpu_->CreateShmBasedBuffer(MakeFD(), length, buffer_size,
buffer_id2);
Sync();
// Create window.
MockPlatformWindowDelegate delegate;
auto window = CreateWaylandWindowWithParams(
PlatformWindowType::kWindow, gfx::kNullAcceleratedWidget,
gfx::Rect(0, 0, 100, 100), &delegate);
ASSERT_TRUE(window);
auto states = InitializeWlArrayWithActivatedState();
Sync();
// Configure window to be ready to attach wl_buffers.
auto* mock_surface = server_.GetObject<wl::MockSurface>(
window->root_surface()->GetSurfaceId());
EXPECT_TRUE(mock_surface->xdg_surface());
EXPECT_TRUE(mock_surface->xdg_surface()->xdg_toplevel());
SendConfigureEvent(mock_surface->xdg_surface(), 100, 100, 1, states.get());
// Commit a frame with only background.
std::vector<ui::ozone::mojom::WaylandOverlayConfigPtr> overlays;
ui::ozone::mojom::WaylandOverlayConfigPtr background{
ui::ozone::mojom::WaylandOverlayConfig::New()};
background->z_order = INT32_MIN;
background->transform = gfx::OVERLAY_TRANSFORM_NONE;
background->buffer_id = buffer_id1;
overlays.push_back(std::move(background));
buffer_manager_gpu_->CommitOverlays(window->GetWidget(), std::move(overlays));
mock_surface->SendFrameCallback();
Sync();
EXPECT_NE(mock_surface->attached_buffer(), nullptr);
// Hiding window attaches a nil wl_buffer as background.
window->Hide();
mock_surface->SendFrameCallback();
Sync();
EXPECT_EQ(mock_surface->attached_buffer(), nullptr);
mock_surface->ReleaseBuffer(mock_surface->prev_attached_buffer());
window->Show(false);
Sync();
SendConfigureEvent(mock_surface->xdg_surface(), 100, 100, 2, states.get());
// Commit a frame with only the primary_plane.
overlays.clear();
ui::ozone::mojom::WaylandOverlayConfigPtr primary{
ui::ozone::mojom::WaylandOverlayConfig::New()};
primary->z_order = 0;
primary->transform = gfx::OVERLAY_TRANSFORM_NONE;
primary->buffer_id = buffer_id2;
overlays.push_back(std::move(primary));
buffer_manager_gpu_->CommitOverlays(window->GetWidget(), std::move(overlays));
Sync();
// WaylandWindow should automatically reattach the background.
EXPECT_NE(mock_surface->attached_buffer(), nullptr);
}
// Tests that if the window gets hidden and shown again, the title, app id and
// size constraints remain the same.
TEST_P(WaylandWindowTest, SetsPropertiesOnShow) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment