From dee3c37d90f29dd225e89eef2249834ceb526330 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 18 Apr 2022 16:39:12 +0100 Subject: [PATCH] Fix some edge cases when handling unsaved captures. Closes #2554 * The 'no to all' option should always be present if there are multiple unsaved captures across the connections, even if the current connection only has one capture and so otherwise wouldn't have a 'no to all' option. * The 'no to all' option will always discard all unsaved captures in all connections when closing the window, if the user clicks no on the confirmation of this, abort the close operation entirely and let them decide how to handle it (e.g. discarding/saving captures in connections individually). --- qrenderdoc/Windows/Dialogs/LiveCapture.cpp | 45 +++++++++++--- qrenderdoc/Windows/Dialogs/LiveCapture.h | 3 +- qrenderdoc/Windows/MainWindow.cpp | 72 +++++++++++----------- 3 files changed, 74 insertions(+), 46 deletions(-) diff --git a/qrenderdoc/Windows/Dialogs/LiveCapture.cpp b/qrenderdoc/Windows/Dialogs/LiveCapture.cpp index fad35931d..5132b1e94 100644 --- a/qrenderdoc/Windows/Dialogs/LiveCapture.cpp +++ b/qrenderdoc/Windows/Dialogs/LiveCapture.cpp @@ -690,7 +690,7 @@ QString LiveCapture::MakeText(Capture *cap) return text; } -bool LiveCapture::checkAllowClose(bool multipleClosures, bool &noToAll) +bool LiveCapture::checkAllowClose(int totalUnsavedCaptures, bool &noToAll) { m_IgnoreThreadClosed = true; @@ -698,7 +698,10 @@ bool LiveCapture::checkAllowClose(bool multipleClosures, bool &noToAll) QMessageBox::StandardButtons msgFlags = RDDialog::YesNoCancel; - if(ui->captures->count() > 1) + const int unsavedCaptures = unsavedCaptureCount(); + const bool multipleClosures = totalUnsavedCaptures > unsavedCaptures; + + if(unsavedCaptures > 1 || multipleClosures) msgFlags |= QMessageBox::NoToAll; for(int i = 0; i < ui->captures->count(); i++) @@ -734,16 +737,25 @@ bool LiveCapture::checkAllowClose(bool multipleClosures, bool &noToAll) // if we're closing multiple connections make sure the user is sure of what they're doing if(multipleClosures) { - QMessageBox::StandardButton res2 = - RDDialog::question(this, tr("Discarding all captures"), - tr("Multiple connections open have potentially unsaved captures, " - "are you sure you wish to discard them all?")); + QMessageBox::StandardButton res2 = RDDialog::question( + this, tr("Discarding all captures"), + tr("Multiple connections open have potentially unsaved captures, " + "this will discard all captures in all connections, are you sure?")); // if the user is sure, apply the no to all if(res2 == QMessageBox::Yes) + { noToAll = true; - - // otherwise we'll treat this as a simple 'no' in case they changed their mind. + } + else + { + // otherwise if the user changed their mind at this stage, cancel everything rather than + // trying to continue, to keep the flow simple and ensure the user is clear what is + // happening at all points. We do not support discarding all captures in one connection + // then individually filtering another. + m_IgnoreThreadClosed = false; + return false; + } } else { @@ -808,7 +820,7 @@ bool LiveCapture::checkAllowClose(bool multipleClosures, bool &noToAll) bool LiveCapture::checkAllowClose() { bool dummy = false; - return checkAllowClose(false, dummy); + return checkAllowClose(unsavedCaptureCount(), dummy); } void LiveCapture::openCapture(Capture *cap) @@ -980,6 +992,21 @@ void LiveCapture::fileSaved(QString from, QString to) } } +int LiveCapture::unsavedCaptureCount() +{ + int ret = 0; + + for(int i = 0; i < ui->captures->count(); i++) + { + Capture *cap = GetCapture(ui->captures->item(i)); + + if(!cap->saved) + ret++; + } + + return ret; +} + void LiveCapture::previewToggle_toggled(bool checked) { if(m_IgnorePreviewToggle) diff --git a/qrenderdoc/Windows/Dialogs/LiveCapture.h b/qrenderdoc/Windows/Dialogs/LiveCapture.h index 932e0503e..fa10c8030 100644 --- a/qrenderdoc/Windows/Dialogs/LiveCapture.h +++ b/qrenderdoc/Windows/Dialogs/LiveCapture.h @@ -61,7 +61,8 @@ public: const QString &hostname() { return m_Hostname; } void cleanItems(); void fileSaved(QString from, QString to); - bool checkAllowClose(bool multipleClosures, bool &noToAll); + int unsavedCaptureCount(); + bool checkAllowClose(int totalUnsavedCaptures, bool &noToAll); public slots: bool checkAllowClose(); diff --git a/qrenderdoc/Windows/MainWindow.cpp b/qrenderdoc/Windows/MainWindow.cpp index b9f4c15ce..7e8476f7e 100644 --- a/qrenderdoc/Windows/MainWindow.cpp +++ b/qrenderdoc/Windows/MainWindow.cpp @@ -1954,6 +1954,9 @@ void MainWindow::FillRemotesMenu(QMenu *menu, bool includeLocalhost) void MainWindow::setRemoteHost(int hostIdx) { + if(!PromptCloseCapture()) + return; + rdcarray hosts = m_Ctx.Config().GetRemoteHosts(); RemoteHost host; @@ -1962,7 +1965,13 @@ void MainWindow::setRemoteHost(int hostIdx) bool noToAll = false; - for(LiveCapture *live : m_LiveCaptures) + QList liveCaptures = m_LiveCaptures; + + int unsavedCaps = 0; + for(LiveCapture *live : liveCaptures) + unsavedCaps += live->unsavedCaptureCount(); + + for(LiveCapture *live : liveCaptures) { // allow live captures to this host to stay open, that way // we can connect to a live capture, then switch into that @@ -1971,24 +1980,12 @@ void MainWindow::setRemoteHost(int hostIdx) continue; // if the user previously selected 'no to all' in the save prompts below, apply that to all - // subsequent live captures - if(noToAll) - continue; - - if(!live->checkAllowClose(m_LiveCaptures.count() > 1, noToAll)) - return; - } - - if(!PromptCloseCapture()) - return; - - for(LiveCapture *live : m_LiveCaptures) - { - // allow live captures to this host to stay open, that way - // we can connect to a live capture, then switch into that - // context - if(host.IsValid() && live->hostname() == host.Hostname()) - continue; + // subsequent live captures by skipping the check and unconditionally cleaning all captures + if(!noToAll) + { + if(!live->checkAllowClose(unsavedCaps, noToAll)) + return; + } live->cleanItems(); live->close(); @@ -2953,22 +2950,6 @@ void MainWindow::loadLayout_triggered() void MainWindow::closeEvent(QCloseEvent *event) { - bool noToAll = false; - - for(LiveCapture *live : m_LiveCaptures) - { - // if the user previously selected 'no to all' in the save prompts below, apply that to all - // subsequent live captures - if(noToAll) - continue; - - if(!live->checkAllowClose(m_LiveCaptures.count() > 1, noToAll)) - { - event->ignore(); - return; - } - } - if(RENDERDOC_IsGlobalHookActive()) { RDDialog::critical(this, tr("Global hook active"), @@ -2983,8 +2964,27 @@ void MainWindow::closeEvent(QCloseEvent *event) return; } - for(LiveCapture *live : m_LiveCaptures) + bool noToAll = false; + + QList liveCaptures = m_LiveCaptures; + + int unsavedCaps = 0; + for(LiveCapture *live : liveCaptures) + unsavedCaps += live->unsavedCaptureCount(); + + for(LiveCapture *live : liveCaptures) { + // if the user previously selected 'no to all' in the save prompts below, apply that to all + // subsequent live captures by skipping the check and unconditionally cleaning all captures + if(!noToAll) + { + if(!live->checkAllowClose(unsavedCaps, noToAll)) + { + event->ignore(); + return; + } + } + live->cleanItems(); delete live; }