From 2eff0c338322add29027e0d08dc53441065361df Mon Sep 17 00:00:00 2001 From: Leonardo Zide Date: Thu, 22 Oct 2026 10:17:05 -0700 Subject: [PATCH] Simplify asset completion handling and piece waits Keep the final piece error on its request and retain that request directly for deferred notification delivery. This removes duplicate notification storage while preserving UI-thread reference cleanup and cancellation behavior. Inline the single-use piece wait helper into the batch wait, restoring the caller's OpenGL context once per batch. Remove the unused completion progress return value. --- common/lc_assetloader.cpp | 81 +++++++++++++++++---------------------- common/lc_assetloader.h | 17 ++------ 2 files changed, 40 insertions(+), 58 deletions(-) diff --git a/common/lc_assetloader.cpp b/common/lc_assetloader.cpp index d8038417..bbeb7c10 100644 --- a/common/lc_assetloader.cpp +++ b/common/lc_assetloader.cpp @@ -231,12 +231,11 @@ void lcAssetLoader::DispatchJobs() } } -bool lcAssetLoader::ProcessCompletions() +void lcAssetLoader::ProcessCompletions() { // Bounded UI-thread publication also runs during scoped waits, without public callbacks. Q_ASSERT(QThread::currentThread() == thread()); - bool Progress = false; constexpr int MaxCompletionsPerPump = 16; for (int Processed = 0; Processed < MaxCompletionsPerPump; Processed++) @@ -285,8 +284,6 @@ bool lcAssetLoader::ProcessCompletions() } } - Progress = true; - if (Retry) { UploadTextureRequest(Retry); @@ -312,14 +309,13 @@ bool lcAssetLoader::ProcessCompletions() { return Entry.second->RetryAt && Entry.second->RetryAt <= Now; }); + mCompletionMutex.lock(); const bool NeedsPump = !mCompletions.empty() || RetryDue; mCompletionMutex.unlock(); if (NeedsPump) QCoreApplication::postEvent(this, new QEvent(lcAssetLoaderCompletionEvent)); - - return Progress; } void lcAssetLoader::ProcessPieceCompletion(Completion& Completed) @@ -607,7 +603,7 @@ void lcAssetLoader::CompletePieceRequest(const std::shared_ptr& Re Q_ASSERT(QThread::currentThread() == thread()); lcPieceData* PieceData = RequestedPiece->PieceData.Get(); - QString LoadError = RequestedPiece->Error; + QString& LoadError = RequestedPiece->Error; if (LoadError.isEmpty() && !RequestedPiece->StagedMesh && !RequestedPiece->EmptyGeometry) LoadError = tr("Could not load part %1.").arg(RequestedPiece->Source.FileName); @@ -625,7 +621,7 @@ void lcAssetLoader::CompletePieceRequest(const std::shared_ptr& Re mPieceRequests.erase(It); RequestedPiece->State = Loaded ? RequestState::Succeeded : RequestState::Failed; - mPieceNotifications.push_back({ RequestedPiece, std::move(LoadError) }); + mPieceNotifications.push_back(RequestedPiece); QCoreApplication::postEvent(this, new QEvent(lcAssetLoaderCompletionEvent)); } @@ -662,15 +658,15 @@ void lcAssetLoader::DispatchNotifications() for (int Delivered = 0; Delivered < MaxNotificationsPerPump && !mPieceNotifications.empty(); Delivered++) { - PieceNotification Notification = std::move(mPieceNotifications.front()); + const std::shared_ptr RequestedPiece = std::move(mPieceNotifications.front()); mPieceNotifications.pop_front(); - lcPieceData* PieceData = Notification.RequestedPiece->PieceData.Get(); + lcPieceData* PieceData = RequestedPiece->PieceData.Get(); - if (Notification.RequestedPiece->State == RequestState::Cancelled || !Notification.RequestedPiece->PieceData.HasOtherReferences()) + if (RequestedPiece->State == RequestState::Cancelled || !RequestedPiece->PieceData.HasOtherReferences()) { - Notification.RequestedPiece->PieceData.Reset(); + RequestedPiece->PieceData.Reset(); continue; } @@ -678,19 +674,19 @@ void lcAssetLoader::DispatchNotifications() QPointer LoaderGuard(this); QPointer LibraryGuard(mLibrary); - if (Notification.RequestedPiece->State == RequestState::Succeeded) + if (RequestedPiece->State == RequestState::Succeeded) emit mLibrary->PartLoaded(PieceData); else - emit mLibrary->PartLoadFailed(PieceData, Notification.Error); + emit mLibrary->PartLoadFailed(PieceData, RequestedPiece->Error); if (!LoaderGuard || !LibraryGuard) { // Library teardown can delete the definition while delivering this callback. - Notification.RequestedPiece->PieceData.Release(); + RequestedPiece->PieceData.Release(); return; } - Notification.RequestedPiece->PieceData.Reset(); + RequestedPiece->PieceData.Reset(); } std::unordered_set SettledSynths; @@ -772,29 +768,6 @@ void lcAssetLoader::WaitForCompletionOrRetry() } } -bool lcAssetLoader::WaitForPieceRequest(const std::shared_ptr& RequestedPiece) -{ - Q_ASSERT(QThread::currentThread() == thread()); - - QOpenGLContext* const PreviousContext = QOpenGLContext::currentContext(); - QSurface* const PreviousSurface = PreviousContext ? PreviousContext->surface() : nullptr; - - for (;;) - { - ProcessCompletions(); - - if (mStopping || RequestedPiece->IsFinished()) - break; - - WaitForCompletionOrRetry(); - } - - if (PreviousContext && PreviousSurface && (QOpenGLContext::currentContext() != PreviousContext || PreviousContext->surface() != PreviousSurface)) - PreviousContext->makeCurrent(PreviousSurface); - - return RequestedPiece->State == RequestState::Succeeded; -} - bool lcAssetLoader::WaitForPieces(const std::vector& Pieces) { Q_ASSERT(QThread::currentThread() == thread()); @@ -819,10 +792,28 @@ bool lcAssetLoader::WaitForPieces(const std::vector& Pieces) DispatchJobs(); + QOpenGLContext* const PreviousContext = QOpenGLContext::currentContext(); + QSurface* const PreviousSurface = PreviousContext ? PreviousContext->surface() : nullptr; + bool Ready = true; for (const std::shared_ptr& RequestedPiece : Requests) - Ready = WaitForPieceRequest(RequestedPiece) && Ready; + { + for (;;) + { + ProcessCompletions(); + + if (mStopping || RequestedPiece->IsFinished()) + break; + + WaitForCompletionOrRetry(); + } + + Ready = RequestedPiece->State == RequestState::Succeeded && Ready; + } + + if (PreviousContext && PreviousSurface && (QOpenGLContext::currentContext() != PreviousContext || PreviousContext->surface() != PreviousSurface)) + PreviousContext->makeCurrent(PreviousSurface); return Ready; } @@ -939,9 +930,9 @@ void lcAssetLoader::InvalidatePieceLoad(lcPieceData* PieceData) { Q_ASSERT(QThread::currentThread() == thread()); - for (PieceNotification& Pending : mPieceNotifications) - if (Pending.RequestedPiece->PieceData == PieceData) - Pending.RequestedPiece->State = RequestState::Cancelled; + for (const std::shared_ptr& Pending : mPieceNotifications) + if (Pending->PieceData == PieceData) + Pending->State = RequestState::Cancelled; const auto It = mPieceRequests.find(PieceData); @@ -1166,11 +1157,11 @@ void lcAssetLoader::Shutdown() while (!mPieceNotifications.empty()) { - PieceNotification Notification = std::move(mPieceNotifications.front()); + const std::shared_ptr RequestedPiece = std::move(mPieceNotifications.front()); mPieceNotifications.pop_front(); - Notification.RequestedPiece->PieceData.Reset(); + RequestedPiece->PieceData.Reset(); } while (!mSynthNotifications.empty()) diff --git a/common/lc_assetloader.h b/common/lc_assetloader.h index 24968b0a..d634af2d 100644 --- a/common/lc_assetloader.h +++ b/common/lc_assetloader.h @@ -135,13 +135,6 @@ private: std::unique_ptr SynthMesh; }; - // UI-thread delivery retains the load hold after publication, until callbacks finish. - struct PieceNotification - { - std::shared_ptr RequestedPiece; - QString Error; - }; - // Retains the base identity; cancellation makes delivery safe after instance deletion. struct SynthNotification { @@ -156,8 +149,8 @@ private: void DispatchJobs(); // Process a bounded batch of results and upload retries, then start more jobs. - // Returns whether any result or retry was processed; consumer signals remain deferred. - bool ProcessCompletions(); + // Consumer signals remain deferred. + void ProcessCompletions(); // Handle worker results on the UI thread. void ProcessPieceCompletion(Completion& Completed); @@ -178,9 +171,6 @@ private: void CompletePieceRequest(const std::shared_ptr& RequestedPiece); void CompleteSynthRequest(const std::shared_ptr& RequestedSynth); - // Wait for a captured request to settle without delivering consumer signals. - bool WaitForPieceRequest(const std::shared_ptr& RequestedPiece); - // Sleep until a worker completion arrives or the next texture upload retry is due. void WaitForCompletionOrRetry(); @@ -224,7 +214,8 @@ private: std::deque mCompletions; // UI-thread only; public signals are deferred until outside synchronous waits. - std::deque mPieceNotifications; + // Retains each request and its definition reference through notification delivery. + std::deque> mPieceNotifications; std::deque mSynthNotifications; Q_DECLARE_TR_FUNCTIONS(lcAssetLoader)