From 79d5a70c01312fe3086ac2286fb13010d96af072 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 14 Nov 2023 08:58:00 +0200 Subject: [PATCH] wayland: Make SurfaceInterface::{frameRendered,takePresentationFeedback} not touch subsurface tree This gives us greater flexibility and in general more cleaner code. --- .../client/test_wayland_subsurface.cpp | 5 ---- src/pointer_input.cpp | 7 +++-- src/scene/dndiconitem.cpp | 11 ++----- src/scene/dndiconitem.h | 3 +- src/scene/workspacescene.cpp | 20 ++++++++----- src/wayland/surface.cpp | 29 ++++++++----------- src/wayland/surface.h | 5 ++++ src/window.cpp | 13 +++++---- 8 files changed, 45 insertions(+), 48 deletions(-) diff --git a/autotests/wayland/client/test_wayland_subsurface.cpp b/autotests/wayland/client/test_wayland_subsurface.cpp index 6bfd26c8e5..491439e20a 100644 --- a/autotests/wayland/client/test_wayland_subsurface.cpp +++ b/autotests/wayland/client/test_wayland_subsurface.cpp @@ -580,11 +580,6 @@ void TestSubSurface::testSyncMode() QCOMPARE(childDamagedSpy.count(), 1); QVERIFY(childSurface->isMapped()); QVERIFY(parentSurface->isMapped()); - - // sending frame rendered to parent should also send it to child - QSignalSpy frameRenderedSpy(surface.get(), &KWayland::Client::Surface::frameRendered); - parentSurface->frameRendered(100); - QVERIFY(frameRenderedSpy.wait()); } void TestSubSurface::testDeSyncMode() diff --git a/src/pointer_input.cpp b/src/pointer_input.cpp index 7d798a608a..d5ff3c4e46 100644 --- a/src/pointer_input.cpp +++ b/src/pointer_input.cpp @@ -941,9 +941,10 @@ void CursorImage::updateCursorOutputs(const QPointF &pos) void CursorImage::markAsRendered(std::chrono::milliseconds timestamp) { if (m_currentSource == m_serverCursor.surface.get()) { - auto cursorSurface = m_serverCursor.surface->surface(); - if (cursorSurface) { - cursorSurface->frameRendered(timestamp.count()); + if (auto cursorSurface = m_serverCursor.surface->surface()) { + cursorSurface->traverseTree([×tamp](SurfaceInterface *surface) { + surface->frameRendered(timestamp.count()); + }); } } } diff --git a/src/scene/dndiconitem.cpp b/src/scene/dndiconitem.cpp index 2a9e51e731..1976afa5d4 100644 --- a/src/scene/dndiconitem.cpp +++ b/src/scene/dndiconitem.cpp @@ -30,16 +30,9 @@ DragAndDropIconItem::~DragAndDropIconItem() { } -void DragAndDropIconItem::frameRendered(quint32 timestamp) +SurfaceInterface *DragAndDropIconItem::surface() const { - if (m_surfaceItem) { - m_surfaceItem->surface()->frameRendered(timestamp); - } -} - -std::unique_ptr DragAndDropIconItem::takePresentationFeedback(Output *output) -{ - return m_surfaceItem ? m_surfaceItem->surface()->takePresentationFeedback(output) : nullptr; + return m_surfaceItem ? m_surfaceItem->surface() : nullptr; } void DragAndDropIconItem::setOutput(Output *output) diff --git a/src/scene/dndiconitem.h b/src/scene/dndiconitem.h index 6de0327670..ff11c7a9a0 100644 --- a/src/scene/dndiconitem.h +++ b/src/scene/dndiconitem.h @@ -25,8 +25,7 @@ public: explicit DragAndDropIconItem(DragAndDropIcon *icon, Scene *scene, Item *parent = nullptr); ~DragAndDropIconItem() override; - void frameRendered(quint32 timestamp); - std::unique_ptr takePresentationFeedback(Output *output); + SurfaceInterface *surface() const; void setOutput(Output *output); diff --git a/src/scene/workspacescene.cpp b/src/scene/workspacescene.cpp index a1a07a1c88..b26400b21d 100644 --- a/src/scene/workspacescene.cpp +++ b/src/scene/workspacescene.cpp @@ -201,17 +201,23 @@ void WorkspaceScene::frame(SceneDelegate *delegate, OutputFrame *frame) continue; } if (auto surface = window->surface()) { - surface->frameRendered(frameTime.count()); - if (auto feedback = surface->takePresentationFeedback(output)) { - frame->addFeedback(std::move(feedback)); - } + surface->traverseTree([&frameTime, &frame, &output](SurfaceInterface *surface) { + surface->frameRendered(frameTime.count()); + if (auto feedback = surface->takePresentationFeedback(output)) { + frame->addFeedback(std::move(feedback)); + } + }); } } if (m_dndIcon) { - m_dndIcon->frameRendered(frameTime.count()); - if (auto feedback = m_dndIcon->takePresentationFeedback(output)) { - frame->addFeedback(std::move(feedback)); + if (auto surface = m_dndIcon->surface()) { + surface->traverseTree([&frameTime, &frame, &output](SurfaceInterface *surface) { + surface->frameRendered(frameTime.count()); + if (auto feedback = surface->takePresentationFeedback(output)) { + frame->addFeedback(std::move(feedback)); + } + }); } } } diff --git a/src/wayland/surface.cpp b/src/wayland/surface.cpp index bed6132987..d974668ca9 100644 --- a/src/wayland/surface.cpp +++ b/src/wayland/surface.cpp @@ -467,13 +467,6 @@ void SurfaceInterface::frameRendered(quint32 msec) wl_callback_send_done(resource, msec); wl_resource_destroy(resource); } - - for (SubSurfaceInterface *subsurface : std::as_const(d->current->subsurface.below)) { - subsurface->surface()->frameRendered(msec); - } - for (SubSurfaceInterface *subsurface : std::as_const(d->current->subsurface.above)) { - subsurface->surface()->frameRendered(msec); - } } std::unique_ptr SurfaceInterface::takePresentationFeedback(Output *output) @@ -484,16 +477,6 @@ std::unique_ptr SurfaceInterface::takePresentationFeedback std::vector> feedbacks; std::move(d->current->presentationFeedbacks.begin(), d->current->presentationFeedbacks.end(), std::back_inserter(feedbacks)); d->current->presentationFeedbacks.clear(); - for (SubSurfaceInterface *subsurface : std::as_const(d->current->subsurface.below)) { - auto &subSurfaceFeedbacks = subsurface->surface()->d->current->presentationFeedbacks; - std::move(subSurfaceFeedbacks.begin(), subSurfaceFeedbacks.end(), std::back_inserter(feedbacks)); - subSurfaceFeedbacks.clear(); - } - for (SubSurfaceInterface *subsurface : std::as_const(d->current->subsurface.above)) { - auto &subSurfaceFeedbacks = subsurface->surface()->d->current->presentationFeedbacks; - std::move(subSurfaceFeedbacks.begin(), subSurfaceFeedbacks.end(), std::back_inserter(feedbacks)); - subSurfaceFeedbacks.clear(); - } return std::make_unique(std::move(feedbacks)); } @@ -1260,6 +1243,18 @@ void SurfaceInterface::setLastTransaction(Transaction *transaction) d->lastTransaction = transaction; } +void SurfaceInterface::traverseTree(std::function callback) +{ + callback(this); + + for (SubSurfaceInterface *subsurface : std::as_const(d->current->subsurface.below)) { + subsurface->surface()->traverseTree(callback); + } + for (SubSurfaceInterface *subsurface : std::as_const(d->current->subsurface.above)) { + subsurface->surface()->traverseTree(callback); + } +} + } // namespace KWin #include "moc_surface.cpp" diff --git a/src/wayland/surface.h b/src/wayland/surface.h index ab2ee81528..1d7c06a3f7 100644 --- a/src/wayland/surface.h +++ b/src/wayland/surface.h @@ -379,6 +379,11 @@ public: void setPreferredColorDescription(const ColorDescription &descr); + /** + * Traverses the surface sub-tree with this surface as the root. + */ + void traverseTree(std::function callback); + Q_SIGNALS: /** * This signal is emitted when the underlying wl_surface resource is about to be freed. diff --git a/src/window.cpp b/src/window.cpp index 695a2f6cf1..a80f9aff5f 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -4197,11 +4197,14 @@ void Window::unrefOffscreenRendering() void Window::maybeSendFrameCallback() { if (m_surface && !m_windowItem->isVisible()) { - m_surface->frameRendered(std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count()); - const auto feedback = m_surface->takePresentationFeedback(nullptr); - if (feedback) { - feedback->presented(std::chrono::nanoseconds(1'000'000'000'000 / output()->refreshRate()), std::chrono::steady_clock::now().time_since_epoch(), PresentationMode::VSync); - } + const auto timestamp = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + m_surface->traverseTree([this, ×tamp](SurfaceInterface *surface) { + surface->frameRendered(timestamp); + const auto feedback = surface->takePresentationFeedback(nullptr); + if (feedback) { + feedback->presented(std::chrono::nanoseconds(1'000'000'000'000 / output()->refreshRate()), std::chrono::steady_clock::now().time_since_epoch(), PresentationMode::VSync); + } + }); // update refresh rate, it might have changed m_offscreenFramecallbackTimer.start(1'000'000 / output()->refreshRate()); }