From 318c00d6d093d6d0da2687ad51880578a49899dd Mon Sep 17 00:00:00 2001 From: Maximilian Seidler <78690852+PaideiaDilemma@users.noreply.github.com> Date: Wed, 26 Jun 2024 20:31:15 +0200 Subject: [PATCH] core: stabilize label updates and revision locking in the asyncResourceGatherer (#384) * core: handle rerendering when frameCallback is pending * core: log when skipping label updates * asyncResourceGatherer: remove busy and use loopMutex Makes getAssetById fail less often and thus labels get more stable updates * asyncResourceGatherer: revision locking `assetsMutex` was not needed, since `apply` only gets called from the main thread and resources are also only aquired via the main thread. `preloadTargets`, previously kinda guarded by the `busy` flag are now locked as suggested in #367 (but via a copy of `peloadTargets`). `apply` now returns a boolean so that the locking of preloadTargets in combination with checking `preloadTargets.empty()` is a bit nicer. * asyncResourceGatherer: remove explicit template arg for unique lock --- src/core/LockSurface.cpp | 4 +- src/renderer/AsyncResourceGatherer.cpp | 67 ++++++++++++-------------- src/renderer/AsyncResourceGatherer.hpp | 12 ++--- src/renderer/widgets/Label.cpp | 9 ++-- 4 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/core/LockSurface.cpp b/src/core/LockSurface.cpp index 3ffe2a4..beb522b 100644 --- a/src/core/LockSurface.cpp +++ b/src/core/LockSurface.cpp @@ -129,8 +129,10 @@ static const wl_callback_listener callbackListener = { void CSessionLockSurface::render() { Debug::log(TRACE, "render lock"); - if (frameCallback || !readyForFrame) + if (frameCallback || !readyForFrame) { + needsFrame = true; return; + } const auto FEEDBACK = g_pRenderer->renderLock(*this); frameCallback = wl_surface_frame(surface); diff --git a/src/renderer/AsyncResourceGatherer.cpp b/src/renderer/AsyncResourceGatherer.cpp index f7f46ee..465511c 100644 --- a/src/renderer/AsyncResourceGatherer.cpp +++ b/src/renderer/AsyncResourceGatherer.cpp @@ -11,8 +11,6 @@ #include "../helpers/Jpeg.hpp" #include "../helpers/Webp.hpp" -std::mutex cvmtx; - CAsyncResourceGatherer::CAsyncResourceGatherer() { asyncLoopThread = std::thread([this]() { this->gather(); /* inital gather */ @@ -83,23 +81,17 @@ void CAsyncResourceGatherer::recheckDMAFramesFor(COutput* output) { } SPreloadedAsset* CAsyncResourceGatherer::getAssetByID(const std::string& id) { - if (asyncLoopState.busy) - return nullptr; - - std::lock_guard lg(asyncLoopState.loopMutex); - for (auto& a : assets) { if (a.first == id) return &a.second; } - if (!preloadTargets.empty()) { - apply(); + if (apply()) { for (auto& a : assets) { if (a.first == id) return &a.second; } - } + }; for (auto& dma : dmas) { if (id == "dma:" + dma->name) @@ -210,18 +202,27 @@ void CAsyncResourceGatherer::gather() { ready = true; } -void CAsyncResourceGatherer::apply() { +bool CAsyncResourceGatherer::apply() { + preloadTargetsMutex.lock(); - for (auto& t : preloadTargets) { + if (preloadTargets.empty()) { + preloadTargetsMutex.unlock(); + return false; + } + + auto currentPreloadTargets = preloadTargets; + preloadTargets.clear(); + preloadTargetsMutex.unlock(); + + for (auto& t : currentPreloadTargets) { if (t.type == TARGET_IMAGE) { - std::lock_guard lg(asyncLoopState.assetsMutex); - const auto ASSET = &assets[t.id]; + const auto ASSET = &assets[t.id]; - const auto SURFACESTATUS = cairo_surface_status((cairo_surface_t*)t.cairosurface); - const auto CAIROFORMAT = cairo_image_surface_get_format((cairo_surface_t*)t.cairosurface); - const GLint glIFormat = CAIROFORMAT == CAIRO_FORMAT_RGB96F ? GL_RGB32F : GL_RGBA; - const GLint glFormat = CAIROFORMAT == CAIRO_FORMAT_RGB96F ? GL_RGB : GL_RGBA; - const GLint glType = CAIROFORMAT == CAIRO_FORMAT_RGB96F ? GL_FLOAT : GL_UNSIGNED_BYTE; + const auto SURFACESTATUS = cairo_surface_status((cairo_surface_t*)t.cairosurface); + const auto CAIROFORMAT = cairo_image_surface_get_format((cairo_surface_t*)t.cairosurface); + const GLint glIFormat = CAIROFORMAT == CAIRO_FORMAT_RGB96F ? GL_RGB32F : GL_RGBA; + const GLint glFormat = CAIROFORMAT == CAIRO_FORMAT_RGB96F ? GL_RGB : GL_RGBA; + const GLint glType = CAIROFORMAT == CAIRO_FORMAT_RGB96F ? GL_FLOAT : GL_UNSIGNED_BYTE; if (SURFACESTATUS != CAIRO_STATUS_SUCCESS) { Debug::log(ERR, "Resource {} invalid ({})", t.id, cairo_status_to_string(SURFACESTATUS)); @@ -248,9 +249,8 @@ void CAsyncResourceGatherer::apply() { } } - preloadTargets.clear(); - applied = true; + return true; } void CAsyncResourceGatherer::renderImage(const SPreloadRequest& rq) { @@ -269,6 +269,7 @@ void CAsyncResourceGatherer::renderImage(const SPreloadRequest& rq) { target.data = cairo_image_surface_get_data(CAIROISURFACE); target.size = {(double)cairo_image_surface_get_width(CAIROISURFACE), (double)cairo_image_surface_get_height(CAIROISURFACE)}; + std::lock_guard lg{preloadTargetsMutex}; preloadTargets.push_back(target); } @@ -363,6 +364,7 @@ void CAsyncResourceGatherer::renderText(const SPreloadRequest& rq) { target.data = cairo_image_surface_get_data(CAIROSURFACE); target.size = {layoutWidth / PANGO_SCALE, layoutHeight / PANGO_SCALE}; + std::lock_guard lg{preloadTargetsMutex}; preloadTargets.push_back(target); } @@ -380,27 +382,23 @@ static void timerCallback(std::shared_ptr self, void* data_) { void CAsyncResourceGatherer::asyncAssetSpinLock() { while (!g_pHyprlock->m_bTerminate) { - std::unique_lock lk(cvmtx); + std::unique_lock lk(asyncLoopState.requestsMutex); if (asyncLoopState.pending == false) // avoid a lock if a thread managed to request something already since we .unlock()ed - asyncLoopState.loopGuard.wait_for(lk, std::chrono::seconds(5), [this] { return asyncLoopState.pending; }); // wait for events - - asyncLoopState.requestMutex.lock(); + asyncLoopState.requestsCV.wait_for(lk, std::chrono::seconds(5), [this] { return asyncLoopState.pending; }); // wait for events asyncLoopState.pending = false; if (asyncLoopState.requests.empty()) { - asyncLoopState.requestMutex.unlock(); + lk.unlock(); continue; } auto requests = asyncLoopState.requests; asyncLoopState.requests.clear(); - asyncLoopState.requestMutex.unlock(); + lk.unlock(); // process requests - - asyncLoopState.busy = true; for (auto& r : requests) { if (r.type == TARGET_TEXT) { renderText(r); @@ -415,29 +413,26 @@ void CAsyncResourceGatherer::asyncAssetSpinLock() { if (r.callback) g_pHyprlock->addTimer(std::chrono::milliseconds(0), timerCallback, new STimerCallbackData{r.callback, r.callbackData}); } - - asyncLoopState.busy = false; } dmas.clear(); } void CAsyncResourceGatherer::requestAsyncAssetPreload(const SPreloadRequest& request) { - std::lock_guard lg(asyncLoopState.requestMutex); + std::lock_guard lg(asyncLoopState.requestsMutex); asyncLoopState.requests.push_back(request); asyncLoopState.pending = true; - asyncLoopState.loopGuard.notify_all(); + asyncLoopState.requestsCV.notify_all(); } void CAsyncResourceGatherer::unloadAsset(SPreloadedAsset* asset) { - std::lock_guard lg(asyncLoopState.assetsMutex); - std::erase_if(assets, [asset](const auto& a) { return &a.second == asset; }); } void CAsyncResourceGatherer::notify() { + std::lock_guard lg(asyncLoopState.requestsMutex); asyncLoopState.pending = true; - asyncLoopState.loopGuard.notify_all(); + asyncLoopState.requestsCV.notify_all(); } void CAsyncResourceGatherer::await() { diff --git a/src/renderer/AsyncResourceGatherer.hpp b/src/renderer/AsyncResourceGatherer.hpp index 2fa81a8..d94cb85 100644 --- a/src/renderer/AsyncResourceGatherer.hpp +++ b/src/renderer/AsyncResourceGatherer.hpp @@ -24,7 +24,7 @@ class CAsyncResourceGatherer { /* only call from ogl thread */ SPreloadedAsset* getAssetByID(const std::string& id); - void apply(); + bool apply(); enum eTargetType { TARGET_IMAGE = 0, @@ -59,12 +59,8 @@ class CAsyncResourceGatherer { void renderImage(const SPreloadRequest& rq); struct { - std::condition_variable loopGuard; - std::mutex loopMutex; - - std::mutex requestMutex; - - std::mutex assetsMutex; + std::condition_variable requestsCV; + std::mutex requestsMutex; std::vector requests; bool pending = false; @@ -86,6 +82,8 @@ class CAsyncResourceGatherer { std::vector> dmas; std::vector preloadTargets; + std::mutex preloadTargetsMutex; + std::unordered_map assets; void gather(); diff --git a/src/renderer/widgets/Label.cpp b/src/renderer/widgets/Label.cpp index 79665b9..29c335b 100644 --- a/src/renderer/widgets/Label.cpp +++ b/src/renderer/widgets/Label.cpp @@ -39,8 +39,10 @@ void CLabel::onTimerUpdate() { if (label.formatted == oldFormatted && !label.alwaysUpdate) return; - if (!pendingResourceID.empty()) - return; // too many updates, we'll miss some. Shouldn't happen tbh + if (!pendingResourceID.empty()) { + Debug::log(WARN, "Trying to update label, but resource {} is still pending! Skipping update.", pendingResourceID); + return; + } // request new request.id = getUniqueResourceId(); @@ -141,7 +143,4 @@ static void onAssetCallbackTimer(std::shared_ptr self, void* data) { void CLabel::renderSuper() { g_pHyprlock->renderOutput(outputStringPort); - - if (!pendingResourceID.empty()) /* did not consume the pending resource */ - g_pHyprlock->addTimer(std::chrono::milliseconds(100), onAssetCallbackTimer, this); }