From 24f7eeb210f91472c70fcd1ff29ee1fb473cc7e8 Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Wed, 22 Jan 2020 17:11:17 +1100 Subject: [PATCH] renderer: implemented function to wait for workers in the main thread The workers run in separate threads meaning if any significant change is made (e.g. parameters changing, or even closing the program), these workers may still be mid-render. This is particularly problematic when closing since the nodes a worker is rendering may be deleted mid-render. Render backends now have a function that pauses the main thread (but starts a second event loop so the UI isn't frozen) until the worker threads are all finished. This way, massive changes can be made safely without race conditions. --- app/dialog/CMakeLists.txt | 1 + .../preferences/tabs/preferencesaudiotab.cpp | 13 +++-- app/dialog/rendercancel/CMakeLists.txt | 22 ++++++++ app/dialog/rendercancel/rendercancel.cpp | 56 +++++++++++++++++++ app/dialog/rendercancel/rendercancel.h | 35 ++++++++++++ app/panel/panelmanager.h | 26 ++++++++- app/render/backend/opengl/openglbackend.cpp | 15 +++-- app/render/backend/renderbackend.cpp | 29 ++++++++++ app/render/backend/renderbackend.h | 7 ++- app/render/backend/videorenderbackend.cpp | 5 +- app/window/mainwindow/mainwindow.cpp | 8 ++- 11 files changed, 197 insertions(+), 20 deletions(-) create mode 100644 app/dialog/rendercancel/CMakeLists.txt create mode 100644 app/dialog/rendercancel/rendercancel.cpp create mode 100644 app/dialog/rendercancel/rendercancel.h diff --git a/app/dialog/CMakeLists.txt b/app/dialog/CMakeLists.txt index 39a7c201a..5ce244a46 100644 --- a/app/dialog/CMakeLists.txt +++ b/app/dialog/CMakeLists.txt @@ -22,6 +22,7 @@ add_subdirectory(keyframeproperties) add_subdirectory(loadsave) add_subdirectory(preferences) add_subdirectory(projectproperties) +add_subdirectory(rendercancel) add_subdirectory(sequence) add_subdirectory(speedduration) diff --git a/app/dialog/preferences/tabs/preferencesaudiotab.cpp b/app/dialog/preferences/tabs/preferencesaudiotab.cpp index 160feede8..a450cf2bd 100644 --- a/app/dialog/preferences/tabs/preferencesaudiotab.cpp +++ b/app/dialog/preferences/tabs/preferencesaudiotab.cpp @@ -105,12 +105,15 @@ void PreferencesAudioTab::Accept() // FIXME: Qt documentation states that QAudioDeviceInfo::deviceName() is a "unique identifiers", which would make them // ideal for saving in preferences, but in practice they don't actually appear to be unique. // See: https://bugreports.qt.io/browse/QTBUG-16841 - Config::Current()["AudioOutput"] = selected_output_name; - Config::Current()["AudioInput"] = selected_input_name; + if (Config::Current()["AudioOutput"] != selected_output_name) { + Config::Current()["AudioOutput"] = selected_output_name; + AudioManager::instance()->SetOutputDevice(selected_output); + } - // Finally, set these as the current device - AudioManager::instance()->SetOutputDevice(selected_output); - AudioManager::instance()->SetInputDevice(selected_input); + if (Config::Current()["AudioInput"] != selected_input_name) { + Config::Current()["AudioInput"] = selected_input_name; + AudioManager::instance()->SetInputDevice(selected_input); + } } void PreferencesAudioTab::RefreshDevices() diff --git a/app/dialog/rendercancel/CMakeLists.txt b/app/dialog/rendercancel/CMakeLists.txt new file mode 100644 index 000000000..a9ca563e9 --- /dev/null +++ b/app/dialog/rendercancel/CMakeLists.txt @@ -0,0 +1,22 @@ +# Olive - Non-Linear Video Editor +# Copyright (C) 2019 Olive Team +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +set(OLIVE_SOURCES + ${OLIVE_SOURCES} + dialog/rendercancel/rendercancel.h + dialog/rendercancel/rendercancel.cpp + PARENT_SCOPE +) diff --git a/app/dialog/rendercancel/rendercancel.cpp b/app/dialog/rendercancel/rendercancel.cpp new file mode 100644 index 000000000..3b78e00e0 --- /dev/null +++ b/app/dialog/rendercancel/rendercancel.cpp @@ -0,0 +1,56 @@ +#include "rendercancel.h" + +RenderCancelDialog::RenderCancelDialog(QWidget *parent) : + LoadSaveDialog(tr("Waiting for workers to finish..."), tr("Renderer"), parent), + busy_workers_(0), + total_workers_(0) +{ +} + +void RenderCancelDialog::RunIfWorkersAreBusy() +{ + if (busy_workers_ > 0) { + waiting_workers_ = busy_workers_; + + exec(); + } +} + +void RenderCancelDialog::SetWorkerCount(int count) +{ + total_workers_ = count; + + UpdateProgress(); +} + +void RenderCancelDialog::WorkerStarted() +{ + busy_workers_++; + + UpdateProgress(); +} + +void RenderCancelDialog::WorkerDone() +{ + busy_workers_--; + + UpdateProgress(); +} + +void RenderCancelDialog::showEvent(QShowEvent *event) +{ + UpdateProgress(); +} + +void RenderCancelDialog::UpdateProgress() +{ + if (!total_workers_ || !isVisible()) { + return; + } + + SetProgress(qRound(100.0 * static_cast(waiting_workers_ - busy_workers_) / static_cast(waiting_workers_))); + + if (busy_workers_ == 0) { + accept(); + } +} diff --git a/app/dialog/rendercancel/rendercancel.h b/app/dialog/rendercancel/rendercancel.h new file mode 100644 index 000000000..f71050fd9 --- /dev/null +++ b/app/dialog/rendercancel/rendercancel.h @@ -0,0 +1,35 @@ +#ifndef RENDERCANCELDIALOG_H +#define RENDERCANCELDIALOG_H + +#include "dialog/loadsave/loadsave.h" + +class RenderCancelDialog : public LoadSaveDialog +{ + Q_OBJECT +public: + RenderCancelDialog(QWidget* parent = nullptr); + + void RunIfWorkersAreBusy(); + + void SetWorkerCount(int count); + + void WorkerStarted(); + +public slots: + void WorkerDone(); + +protected: + virtual void showEvent(QShowEvent* event) override; + +private: + void UpdateProgress(); + + int busy_workers_; + + int total_workers_; + + int waiting_workers_; + +}; + +#endif // RENDERCANCELDIALOG_H diff --git a/app/panel/panelmanager.h b/app/panel/panelmanager.h index 6e21bfcb5..dda92eae2 100644 --- a/app/panel/panelmanager.h +++ b/app/panel/panelmanager.h @@ -87,8 +87,6 @@ public: template /** * @brief Create a panel - * @param parent - * @return */ T* CreatePanel(QWidget* parent); @@ -114,6 +112,12 @@ public: */ static PanelManager* instance(); + template + /** + * @brief Get a list of panels of a certain type + */ + QList GetPanelsOfType(); + public slots: /** * @brief Connect this to a QApplication's SIGNAL(focusChanged()) @@ -173,4 +177,22 @@ T* PanelManager::MostRecentlyFocused() return nullptr; } +template +QList PanelManager::GetPanelsOfType() +{ + QList panels; + + T* cast_test; + + foreach (PanelWidget* panel, focus_history_) { + cast_test = dynamic_cast(panel); + + if (cast_test) { + panels.append(cast_test); + } + } + + return panels; +} + #endif // PANELFOCUSMANAGER_H diff --git a/app/render/backend/opengl/openglbackend.cpp b/app/render/backend/opengl/openglbackend.cpp index 4a529b0ec..bab0e1a28 100644 --- a/app/render/backend/opengl/openglbackend.cpp +++ b/app/render/backend/opengl/openglbackend.cpp @@ -39,7 +39,10 @@ bool OpenGLBackend::InitInternal() // Create master texture (the one sent to the viewer) master_texture_ = std::make_shared(); - master_texture_->Create(share_ctx, params().effective_width(), params().effective_height(), params().format()); + master_texture_->Create(share_ctx, + params().effective_width(), + params().effective_height(), + params().format()); // Create copy buffer/pipeline copy_buffer_.Create(share_ctx); @@ -156,11 +159,13 @@ void OpenGLBackend::EmitCachedFrameReady(const rational &time, const QVariant &v void OpenGLBackend::ParamsChangedEvent() { - // Assume if the texture is allocated, it needs to be changed. Otherwise the texture should be created through - // InitInternal() with the correct parameters - if (master_texture_) { + // If we're initiated, we need to recreate the texture. Otherwise this backend isn't active so it doesn't matter. + if (IsInitiated()) { master_texture_->Destroy(); - master_texture_->Create(QOpenGLContext::currentContext(), params().effective_width(), params().effective_height(), params().format()); + master_texture_->Create(QOpenGLContext::currentContext(), + params().effective_width(), + params().effective_height(), + params().format()); } } diff --git a/app/render/backend/renderbackend.cpp b/app/render/backend/renderbackend.cpp index 9c140444f..f1ff1d99e 100644 --- a/app/render/backend/renderbackend.cpp +++ b/app/render/backend/renderbackend.cpp @@ -3,6 +3,9 @@ #include #include +#include "core.h" +#include "window/mainwindow/mainwindow.h" + RenderBackend::RenderBackend(QObject *parent) : QObject(parent), compiled_(false), @@ -12,6 +15,8 @@ RenderBackend::RenderBackend(QObject *parent) : recompile_queued_(false), input_update_queued_(false) { + // FIXME: Don't create in CLI mode + cancel_dialog_ = new RenderCancelDialog(Core::instance()->main_window()); } bool RenderBackend::Init() @@ -30,6 +35,8 @@ bool RenderBackend::Init() thread->start(QThread::LowPriority); } + cancel_dialog_->SetWorkerCount(threads_.size()); + started_ = InitInternal(); // Connects workers and moves them to their respective threads @@ -50,6 +57,8 @@ void RenderBackend::Close() started_ = false; + CancelQueue(); + Decompile(); CloseInternal(); @@ -81,6 +90,8 @@ const QString &RenderBackend::GetError() const void RenderBackend::SetViewerNode(ViewerOutput *viewer_node) { if (viewer_node_ != nullptr) { + CancelQueue(); + DisconnectViewer(viewer_node_); Decompile(); @@ -297,6 +308,7 @@ void RenderBackend::CacheNext() render_job_info_.insert(cache_frame, job_time); SetWorkerBusyState(worker, true); + cancel_dialog_->WorkerStarted(); QMetaObject::invokeMethod(worker, "Render", @@ -312,6 +324,20 @@ ViewerOutput *RenderBackend::viewer_node() const return copied_viewer_node_; } +void RenderBackend::CancelQueue() +{ + cache_queue_.clear(); + + int busy = 0; + for (int i=0;iRunIfWorkersAreBusy(); +} + bool RenderBackend::ViewerIsConnected() const { return viewer_node_ != nullptr; @@ -375,6 +401,9 @@ void RenderBackend::InitWorkers() // Connect to it ConnectWorkerToThis(processor); + // Connect cancel dialog to it + connect(processor, &RenderWorker::CompletedCache, cancel_dialog_, &RenderCancelDialog::WorkerDone, Qt::QueuedConnection); + // Finally, we can move it to its own thread processor->moveToThread(thread); diff --git a/app/render/backend/renderbackend.h b/app/render/backend/renderbackend.h index 01cd5d5a7..db58f1cf9 100644 --- a/app/render/backend/renderbackend.h +++ b/app/render/backend/renderbackend.h @@ -4,6 +4,7 @@ #include #include "common/constructors.h" +#include "dialog/rendercancel/rendercancel.h" #include "decodercache.h" #include "node/graph.h" #include "node/output/viewer/viewer.h" @@ -15,8 +16,6 @@ class RenderBackend : public QObject public: RenderBackend(QObject* parent = nullptr); - DISABLE_COPY_MOVE(RenderBackend) - bool Init(); void Close(); @@ -29,6 +28,8 @@ public: ViewerOutput* viewer_node() const; + void CancelQueue(); + public slots: void InvalidateCache(const rational &start_range, const rational &end_range); @@ -142,6 +143,8 @@ private: QVector processor_busy_state_; + RenderCancelDialog* cancel_dialog_; + }; #endif // RENDERBACKEND_H diff --git a/app/render/backend/videorenderbackend.cpp b/app/render/backend/videorenderbackend.cpp index cd4ef5c1e..985857f46 100644 --- a/app/render/backend/videorenderbackend.cpp +++ b/app/render/backend/videorenderbackend.cpp @@ -76,10 +76,7 @@ const VideoRenderingParams &VideoRenderBackend::params() const void VideoRenderBackend::SetParameters(const VideoRenderingParams& params) { - if (!AllProcessorsAreAvailable()) { - qCritical() << "Attempted to set parameters on a backend whose workers are still busy"; - return; - } + CancelQueue(); // Set new parameters params_ = params; diff --git a/app/window/mainwindow/mainwindow.cpp b/app/window/mainwindow/mainwindow.cpp index 04e49f26f..81ac2cbe8 100644 --- a/app/window/mainwindow/mainwindow.cpp +++ b/app/window/mainwindow/mainwindow.cpp @@ -145,10 +145,14 @@ void MainWindow::ProjectOpen(Project* p) void MainWindow::closeEvent(QCloseEvent *e) { + // Close viewers first since we don't want to delete any nodes while they might be mid-render + QList viewers = PanelManager::instance()->GetPanelsOfType(); + foreach (ViewerPanel* viewer, viewers) { + viewer->ConnectViewerNode(nullptr); + } + PanelManager::instance()->DeleteAllPanels(); - - QMainWindow::closeEvent(e); }