diff options
Diffstat (limited to '0007-ozone-wayland-Stop-using-wl_display_roundtrip.patch')
| -rw-r--r-- | 0007-ozone-wayland-Stop-using-wl_display_roundtrip.patch | 472 |
1 files changed, 472 insertions, 0 deletions
diff --git a/0007-ozone-wayland-Stop-using-wl_display_roundtrip.patch b/0007-ozone-wayland-Stop-using-wl_display_roundtrip.patch new file mode 100644 index 000000000000..40da4077c1e1 --- /dev/null +++ b/0007-ozone-wayland-Stop-using-wl_display_roundtrip.patch @@ -0,0 +1,472 @@ +From baa6e8592ccb0f4975676e9d1cac4ffc8c1b6e2f Mon Sep 17 00:00:00 2001 +From: Maksim Sisov <msisov@igalia.com> +Date: Thu, 8 Aug 2019 11:32:57 +0000 +Subject: [PATCH 07/11] [ozone/wayland] Stop using wl_display_roundtrip + +According to the Wayland documentation, wl_display_roundtrip is a +blocking call that can block if the event queue is empty. That is, +to be able to process buffer swap requests synchronously, +that call blocks until a frame callback comes or a wl_buffer +is attached. This results in blocking the CrBrowserMain thread. + +This change alters the logic so that the buffer manager processes +one pending buffer at a time asynchronously avoiding blocking +the thread if synchronous processing is not possible due to +two conditions: +1) a frame callback is not received, +2) a wl_frame buffer is not created. + +That is, on a frame no 1, the manager does not have a +frame callback installed and it may not process the buffer +synchronously if a wl_buffer is not created. +(note that wl_buffers can be created synchronously or asynchronously +depending on the protocol version of the zwp_linux_dmabuf[1]). +If such condition satisfies (there is no wl_buffer created), that +buffer is stored as a pending one and is processed on a +AttachWlBuffer call. Otherwise, the manager just processes +the buffer swap for the frame no 1 synchronously. + +On frames no 2 and onwards, the manager may not process +buffer swaps synchronously if a frame callback is set, but not +received. That is, whenever the manager attaches +buffers to surfaces and commits them, it sets frame callbacks, +which Wayland compositor runs whenever it is time to send next +frame. If such condition satisfies, the manager stores the buffer +and processes it on next OnFrameCallback call. Otherwise, the swap +request processed synchronously. + +[1] https://cs.chromium.org/chromium/src/ui/ozone/platform/wayland/host/wayland_zwp_linux_dmabuf.cc?g=0&l=67 + +Bug: 987821 +Test: WaylandBufferManagerTest.TestCommitBufferConditions +Change-Id: I153b2071a453856a40e23a7f0a15f61dbc01d27f +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726051 +Reviewed-by: Robert Kroeger <rjkroege@chromium.org> +Commit-Queue: Maksim Sisov <msisov@igalia.com> +Cr-Commit-Position: refs/heads/master@{#685143} +--- + .../host/wayland_buffer_manager_host.cc | 123 ++++++++++++------ + .../test/mock_zwp_linux_buffer_params.cc | 50 +++++-- + .../test/mock_zwp_linux_buffer_params.h | 17 +++ + .../wayland/test/mock_zwp_linux_dmabuf.cc | 34 ++++- + .../wayland/test/mock_zwp_linux_dmabuf.h | 12 ++ + 5 files changed, 175 insertions(+), 61 deletions(-) + +diff --git a/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc b/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc +index a6759bb798f4..f58a9f1ed8fb 100644 +--- a/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc ++++ b/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc +@@ -63,10 +63,22 @@ class WaylandBufferManagerHost::Surface { + ~Surface() = default; + + bool CommitBuffer(uint32_t buffer_id, const gfx::Rect& damage_region) { ++ DCHECK(!pending_buffer_); ++ + WaylandBuffer* buffer = GetBuffer(buffer_id); + if (!buffer) + return false; + ++ buffer->damage_region = damage_region; ++ ++ // If the wl_buffer has been attached, but the wl_buffer still is null, it ++ // means the Wayland server failed to create the buffer and we have to fail ++ // here. ++ // ++ // TODO(msisov): should we ask to recreate buffers instead of failing? ++ if (buffer->attached && !buffer->wl_buffer) ++ return false; ++ + // This request may come earlier than the Wayland compositor has imported a + // wl_buffer. Wait until the buffer is created. The wait takes place only + // once. Though, the case when a request to attach a buffer comes earlier +@@ -78,46 +90,12 @@ class WaylandBufferManagerHost::Surface { + // Another case, which always happen is waiting until the frame callback is + // completed. Thus, wait here when the Wayland compositor fires the frame + // callback. +- while (!buffer->wl_buffer || !!wl_frame_callback_) { +- // If the wl_buffer has been attached, but the wl_buffer still has been +- // null, it means the Wayland server failed to create the buffer and we +- // have to fail here. +- if ((buffer->attached && !buffer->wl_buffer) || +- wl_display_roundtrip(connection_->display()) == -1) +- return false; ++ if (!buffer->wl_buffer || wl_frame_callback_) { ++ pending_buffer_ = buffer; ++ return true; + } + +- // Once the BufferRelease is called, the buffer will be released. +- DCHECK(buffer->released); +- buffer->released = false; +- +- DCHECK(!submitted_buffer_); +- submitted_buffer_ = buffer; +- +- AttachAndDamageBuffer(buffer, damage_region); +- +- SetupFrameCallback(); +- SetupPresentationFeedback(buffer_id); +- +- CommitSurface(); +- +- connection_->ScheduleFlush(); +- +- // If the contents were reset, there is no buffer attached. It means we have +- // to behave the same way as if it was the very first frame. Check the +- // comment below where the |contents_reset_| is declared. +- if (contents_reset_) { +- prev_submitted_buffer_ = nullptr; +- contents_reset_ = false; +- } +- +- // If it was the very first frame, the surface has not had a back buffer +- // before, and Wayland won't release the front buffer until next buffer is +- // attached. Thus, notify about successful submission immediately. +- if (!prev_submitted_buffer_) +- CompleteSubmission(); +- +- return true; ++ return CommitBufferInternal(buffer); + } + + bool CreateBuffer(const gfx::Size& size, uint32_t buffer_id) { +@@ -150,6 +128,9 @@ class WaylandBufferManagerHost::Surface { + if (prev_submitted_buffer_ == buffer) + prev_submitted_buffer_ = nullptr; + ++ if (pending_buffer_ == buffer) ++ pending_buffer_ = nullptr; ++ + auto result = buffers_.erase(buffer_id); + return result; + } +@@ -167,6 +148,9 @@ class WaylandBufferManagerHost::Surface { + + if (buffer->wl_buffer) + SetupBufferReleaseListener(buffer); ++ ++ if (pending_buffer_ == buffer) ++ ProcessPendingBuffer(); + } + + void ClearState() { +@@ -213,6 +197,10 @@ class WaylandBufferManagerHost::Surface { + // Actual buffer size. + const gfx::Size size; + ++ // Damage region this buffer describes. Must be emptied once buffer is ++ // submitted. ++ gfx::Rect damage_region; ++ + // The id of this buffer. + const uint32_t buffer_id; + +@@ -239,9 +227,45 @@ class WaylandBufferManagerHost::Surface { + DISALLOW_COPY_AND_ASSIGN(WaylandBuffer); + }; + +- void AttachAndDamageBuffer(WaylandBuffer* buffer, +- const gfx::Rect& damage_region) { +- gfx::Rect pending_damage_region = damage_region; ++ bool CommitBufferInternal(WaylandBuffer* buffer) { ++ DCHECK(buffer); ++ DCHECK(!pending_buffer_); ++ ++ // Once the BufferRelease is called, the buffer will be released. ++ DCHECK(buffer->released); ++ buffer->released = false; ++ ++ DCHECK(!submitted_buffer_); ++ submitted_buffer_ = buffer; ++ ++ AttachAndDamageBuffer(buffer); ++ ++ SetupFrameCallback(); ++ SetupPresentationFeedback(buffer->buffer_id); ++ ++ CommitSurface(); ++ ++ connection_->ScheduleFlush(); ++ ++ // If the contents were reset, there is no buffer attached. It means we have ++ // to behave the same way as if it was the very first frame. Check the ++ // comment below where the |contents_reset_| is declared. ++ if (contents_reset_) { ++ prev_submitted_buffer_ = nullptr; ++ contents_reset_ = false; ++ } ++ ++ // If it was the very first frame, the surface has not had a back buffer ++ // before, and Wayland won't release the front buffer until next buffer is ++ // attached. Thus, notify about successful submission immediately. ++ if (!prev_submitted_buffer_) ++ CompleteSubmission(); ++ ++ return true; ++ } ++ ++ void AttachAndDamageBuffer(WaylandBuffer* buffer) { ++ gfx::Rect pending_damage_region = std::move(buffer->damage_region); + // If the size of the damage region is empty, wl_surface_damage must be + // supplied with the actual size of the buffer, which is going to be + // committed. +@@ -299,6 +323,8 @@ class WaylandBufferManagerHost::Surface { + void OnFrameCallback(struct wl_callback* callback) { + DCHECK(wl_frame_callback_.get() == callback); + wl_frame_callback_.reset(); ++ ++ ProcessPendingBuffer(); + } + + // wl_callback_listener +@@ -390,7 +416,8 @@ class WaylandBufferManagerHost::Surface { + const gfx::PresentationFeedback& feedback) { + // The order of submission and presentation callbacks cannot be controlled. + // Some Wayland compositors may fire presentation callbacks earlier than we +- // are able to send submission callbacks and this is bad. Thus, handle it here. ++ // are able to send submission callbacks and this is bad. Thus, handle it ++ // here. + if (submitted_buffer_ && submitted_buffer_->buffer_id == buffer_id) { + submitted_buffer_->needs_send_feedback = true; + submitted_buffer_->feedback = feedback; +@@ -441,6 +468,15 @@ class WaylandBufferManagerHost::Surface { + gfx::PresentationFeedback::Failure()); + } + ++ void ProcessPendingBuffer() { ++ if (!pending_buffer_) ++ return; ++ ++ auto* buffer = pending_buffer_; ++ pending_buffer_ = nullptr; ++ CommitBufferInternal(buffer); ++ } ++ + // Widget this helper surface backs and has 1:1 relationship with the + // WaylandWindow. + +@@ -465,6 +501,9 @@ class WaylandBufferManagerHost::Surface { + // shown. + PresentationFeedbackQueue presentation_feedbacks_; + ++ // A buffer, which is pending to be submitted (look the comment in the ++ // CommitBuffer method). ++ WaylandBuffer* pending_buffer_ = nullptr; + // Current submitted buffer. + WaylandBuffer* submitted_buffer_ = nullptr; + // Previous submitted buffer. +diff --git a/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.cc b/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.cc +index ca7fa6add5b9..8ebe4e6444db 100644 +--- a/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.cc ++++ b/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.cc +@@ -5,6 +5,7 @@ + #include "ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.h" + + #include "ui/ozone/platform/wayland/test/mock_buffer.h" ++#include "ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.h" + + namespace wl { + +@@ -25,6 +26,20 @@ void Add(wl_client* client, + modifier_hi, modifier_lo); + } + ++void CreateCommon(MockZwpLinuxBufferParamsV1* buffer_params, ++ wl_client* client, ++ int32_t width, ++ int32_t height, ++ uint32_t format, ++ uint32_t flags) { ++ wl_resource* buffer_resource = ++ CreateResourceWithImpl<::testing::NiceMock<MockBuffer>>( ++ client, &wl_buffer_interface, 1, &kMockWlBufferImpl, 0, ++ std::move(buffer_params->fds_)); ++ ++ buffer_params->SetBufferResource(buffer_resource); ++} ++ + void Create(wl_client* client, + wl_resource* buffer_params_resource, + int32_t width, +@@ -33,28 +48,23 @@ void Create(wl_client* client, + uint32_t flags) { + auto* buffer_params = + GetUserDataAs<MockZwpLinuxBufferParamsV1>(buffer_params_resource); +- +- wl_resource* buffer_resource = +- CreateResourceWithImpl<::testing::NiceMock<MockBuffer>>( +- client, &wl_buffer_interface, 1, &kMockWlBufferImpl, 0, +- std::move(buffer_params->fds_)); +- +- zwp_linux_buffer_params_v1_send_created(buffer_params_resource, +- buffer_resource); +- ++ CreateCommon(buffer_params, client, width, height, format, flags); + buffer_params->Create(client, buffer_params_resource, width, height, format, + flags); + } + + void CreateImmed(wl_client* client, +- wl_resource* resource, ++ wl_resource* buffer_params_resource, + uint32_t buffer_id, + int32_t width, + int32_t height, + uint32_t format, + uint32_t flags) { +- GetUserDataAs<MockZwpLinuxBufferParamsV1>(resource)->CreateImmed( +- client, resource, buffer_id, width, height, format, flags); ++ auto* buffer_params = ++ GetUserDataAs<MockZwpLinuxBufferParamsV1>(buffer_params_resource); ++ CreateCommon(buffer_params, client, width, height, format, flags); ++ buffer_params->CreateImmed(client, buffer_params_resource, buffer_id, width, ++ height, format, flags); + } + + } // namespace +@@ -66,6 +76,20 @@ const struct zwp_linux_buffer_params_v1_interface + MockZwpLinuxBufferParamsV1::MockZwpLinuxBufferParamsV1(wl_resource* resource) + : ServerObject(resource) {} + +-MockZwpLinuxBufferParamsV1::~MockZwpLinuxBufferParamsV1() {} ++MockZwpLinuxBufferParamsV1::~MockZwpLinuxBufferParamsV1() { ++ DCHECK(linux_dmabuf_); ++ linux_dmabuf_->OnBufferParamsDestroyed(this); ++} ++ ++void MockZwpLinuxBufferParamsV1::SetZwpLinuxDmabuf( ++ MockZwpLinuxDmabufV1* linux_dmabuf) { ++ DCHECK(!linux_dmabuf_); ++ linux_dmabuf_ = linux_dmabuf; ++} ++ ++void MockZwpLinuxBufferParamsV1::SetBufferResource(wl_resource* resource) { ++ DCHECK(!buffer_resource_); ++ buffer_resource_ = resource; ++} + + } // namespace wl +diff --git a/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.h b/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.h +index 57b4b9299a31..7da794a92c69 100644 +--- a/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.h ++++ b/ui/ozone/platform/wayland/test/mock_zwp_linux_buffer_params.h +@@ -20,6 +20,8 @@ namespace wl { + extern const struct zwp_linux_buffer_params_v1_interface + kMockZwpLinuxBufferParamsV1Impl; + ++class MockZwpLinuxDmabufV1; ++ + // Manage zwp_linux_buffer_params_v1 + class MockZwpLinuxBufferParamsV1 : public ServerObject { + public: +@@ -52,9 +54,24 @@ class MockZwpLinuxBufferParamsV1 : public ServerObject { + uint32_t format, + uint32_t flags)); + ++ wl_resource* buffer_resource() const { return buffer_resource_; } ++ ++ void SetZwpLinuxDmabuf(MockZwpLinuxDmabufV1* linux_dmabuf); ++ ++ void SetBufferResource(wl_resource* resource); ++ + std::vector<base::ScopedFD> fds_; + + private: ++ // Non-owned pointer to the linux dmabuf object, which created this params ++ // resource and holds a pointer to it. On destruction, must notify it about ++ // going out of scope. ++ MockZwpLinuxDmabufV1* linux_dmabuf_ = nullptr; ++ ++ // A buffer resource, which is created on Create or CreateImmed call. Can be ++ // null if not created/failed to be created. ++ wl_resource* buffer_resource_ = nullptr; ++ + DISALLOW_COPY_AND_ASSIGN(MockZwpLinuxBufferParamsV1); + }; + +diff --git a/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.cc b/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.cc +index c44d41a40bb7..006d55ff40f4 100644 +--- a/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.cc ++++ b/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.cc +@@ -17,12 +17,20 @@ namespace { + constexpr uint32_t kLinuxDmabufVersion = 1; + + void CreateParams(wl_client* client, wl_resource* resource, uint32_t id) { +- CreateResourceWithImpl<::testing::NiceMock<MockZwpLinuxBufferParamsV1>>( +- client, &zwp_linux_buffer_params_v1_interface, +- wl_resource_get_version(resource), &kMockZwpLinuxBufferParamsV1Impl, id); ++ wl_resource* params_resource = ++ CreateResourceWithImpl<::testing::NiceMock<MockZwpLinuxBufferParamsV1>>( ++ client, &zwp_linux_buffer_params_v1_interface, ++ wl_resource_get_version(resource), &kMockZwpLinuxBufferParamsV1Impl, ++ id); + +- GetUserDataAs<MockZwpLinuxDmabufV1>(resource)->CreateParams(client, resource, +- id); ++ auto* zwp_linux_dmabuf = GetUserDataAs<MockZwpLinuxDmabufV1>(resource); ++ auto* buffer_params = ++ GetUserDataAs<MockZwpLinuxBufferParamsV1>(params_resource); ++ ++ DCHECK(buffer_params); ++ zwp_linux_dmabuf->StoreBufferParams(buffer_params); ++ buffer_params->SetZwpLinuxDmabuf(zwp_linux_dmabuf); ++ zwp_linux_dmabuf->CreateParams(client, resource, id); + } + + } // namespace +@@ -37,6 +45,20 @@ MockZwpLinuxDmabufV1::MockZwpLinuxDmabufV1() + &kMockZwpLinuxDmabufV1Impl, + kLinuxDmabufVersion) {} + +-MockZwpLinuxDmabufV1::~MockZwpLinuxDmabufV1() {} ++MockZwpLinuxDmabufV1::~MockZwpLinuxDmabufV1() { ++ DCHECK(buffer_params_.empty()); ++} ++ ++void MockZwpLinuxDmabufV1::StoreBufferParams( ++ MockZwpLinuxBufferParamsV1* params) { ++ buffer_params_.push_back(params); ++} ++ ++void MockZwpLinuxDmabufV1::OnBufferParamsDestroyed( ++ MockZwpLinuxBufferParamsV1* params) { ++ auto it = std::find(buffer_params_.begin(), buffer_params_.end(), params); ++ DCHECK(it != buffer_params_.end()); ++ buffer_params_.erase(it); ++} + + } // namespace wl +diff --git a/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.h b/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.h +index f63b92d34a50..9d2e1077d79f 100644 +--- a/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.h ++++ b/ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.h +@@ -18,6 +18,8 @@ namespace wl { + + extern const struct zwp_linux_dmabuf_v1_interface kMockZwpLinuxDmabufV1Impl; + ++class MockZwpLinuxBufferParamsV1; ++ + // Manage zwp_linux_dmabuf_v1 object. + class MockZwpLinuxDmabufV1 : public GlobalObject { + public: +@@ -30,7 +32,17 @@ class MockZwpLinuxDmabufV1 : public GlobalObject { + wl_resource* resource, + uint32_t params_id)); + ++ const std::vector<MockZwpLinuxBufferParamsV1*>& buffer_params() const { ++ return buffer_params_; ++ } ++ ++ void StoreBufferParams(MockZwpLinuxBufferParamsV1* params); ++ ++ void OnBufferParamsDestroyed(MockZwpLinuxBufferParamsV1* params); ++ + private: ++ std::vector<MockZwpLinuxBufferParamsV1*> buffer_params_; ++ + DISALLOW_COPY_AND_ASSIGN(MockZwpLinuxDmabufV1); + }; + +-- +2.22.0 + |