Merge pull request #13181 from zfox23/MS15103_snapFolderCrash

Fix MS15103: Interface crashes on snap if snap dir is deleted

interface/src/Application.cpp

@@ -7605,8 +7605,10 @@ void Application::takeSnapshot(bool notify, bool includeAnimated, float aspectRa
 
         // If we're not doing an animated snapshot as well...
         if (!includeAnimated) {
-            // Tell the dependency manager that the capture of the still snapshot has taken place.
-            emit DependencyManager::get<WindowScriptingInterface>()->stillSnapshotTaken(path, notify);
+            if (!path.isEmpty()) {
+                // Tell the dependency manager that the capture of the still snapshot has taken place.
+                emit DependencyManager::get<WindowScriptingInterface>()->stillSnapshotTaken(path, notify);
+            }
         } else if (!SnapshotAnimated::isAlreadyTakingSnapshotAnimated()) {
             // Get an animated GIF snapshot and save it
             SnapshotAnimated::saveSnapshotAnimated(path, aspectRatio, qApp, DependencyManager::get<WindowScriptingInterface>());

interface/src/ui/Snapshot.cpp

@@ -36,6 +36,7 @@
 #include <plugins/DisplayPlugin.h>
 
 #include "Application.h"
+#include "display-plugins/CompositorHelper.h"
 #include "scripting/WindowScriptingInterface.h"
 #include "MainWindow.h"
 #include "Snapshot.h"
@@ -97,14 +98,18 @@ QString Snapshot::saveSnapshot(QImage image, const QString& filename, const QStr
 
     QFile* snapshotFile = savedFileForSnapshot(image, false, filename, pathname);
 
-    // we don't need the snapshot file, so close it, grab its filename and delete it
-    snapshotFile->close();
+    if (snapshotFile) {
+        // we don't need the snapshot file, so close it, grab its filename and delete it
+        snapshotFile->close();
 
-    QString snapshotPath = QFileInfo(*snapshotFile).absoluteFilePath();
+        QString snapshotPath = QFileInfo(*snapshotFile).absoluteFilePath();
 
-    delete snapshotFile;
+        delete snapshotFile;
 
-    return snapshotPath;
+        return snapshotPath;
+    }
+
+    return "";
 }
 
 static const float CUBEMAP_SIDE_PIXEL_DIMENSION = 2048.0f;
@@ -377,7 +382,27 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QSt
             snapshotFullPath.append(filename);
 
             QFile* imageFile = new QFile(snapshotFullPath);
-            imageFile->open(QIODevice::WriteOnly);
+            while (!imageFile->open(QIODevice::WriteOnly)) {
+                // It'd be better for the directory chooser to restore the cursor to its previous state
+                // after choosing a directory, but if the user has entered this codepath,
+                // something terrible has happened. Let's just show the user their cursor so they can get
+                // out of this awful state.
+                qApp->getApplicationCompositor().getReticleInterface()->setVisible(true);
+                qApp->getApplicationCompositor().getReticleInterface()->setAllowMouseCapture(true);
+
+                snapshotFullPath = OffscreenUi::getExistingDirectory(nullptr, "Write Error - Choose New Snapshots Directory", QStandardPaths::writableLocation(QStandardPaths::DesktopLocation));
+                if (snapshotFullPath.isEmpty()) {
+                    return NULL;
+                }
+                _snapshotsLocation.set(snapshotFullPath);
+
+                if (!snapshotFullPath.endsWith(QDir::separator())) {
+                    snapshotFullPath.append(QDir::separator());
+                }
+                snapshotFullPath.append(filename);
+
+                imageFile = new QFile(snapshotFullPath);
+            }
 
             shot.save(imageFile, 0, IMAGE_QUALITY);
             imageFile->close();

interface/src/ui/SnapshotAnimated.cpp

@@ -103,18 +103,40 @@ void SnapshotAnimated::captureFrames() {
     }
 }
 
+void SnapshotAnimated::clearTempVariables() {
+    // Clear out the frame and frame delay vectors.
+    // Also release the memory not required to store the items.
+    SnapshotAnimated::snapshotAnimatedFrameVector.clear();
+    SnapshotAnimated::snapshotAnimatedFrameVector.squeeze();
+    SnapshotAnimated::snapshotAnimatedFrameDelayVector.clear();
+    SnapshotAnimated::snapshotAnimatedFrameDelayVector.squeeze();
+    // Reset the current frame timestamp
+    SnapshotAnimated::snapshotAnimatedTimestamp = 0;
+    SnapshotAnimated::snapshotAnimatedFirstFrameTimestamp = 0;
+}
+
 void SnapshotAnimated::processFrames() {
     uint32_t width = SnapshotAnimated::snapshotAnimatedFrameVector[0].width();
     uint32_t height = SnapshotAnimated::snapshotAnimatedFrameVector[0].height();
 
     // Create the GIF from the temporary files
     // Write out the header and beginning of the GIF file
-    GifBegin(
+    if (!GifBegin(
         &(SnapshotAnimated::snapshotAnimatedGifWriter),
         qPrintable(SnapshotAnimated::snapshotAnimatedPath),
         width,
         height,
-        1); // "1" means "yes there is a delay" with this GifCreator library.
+        1)) { // "1" means "yes there is a delay" with this GifCreator library.
+
+        // We should never, ever get here. If we do, that means that writing a still JPG to the filesystem
+        // has succeeded, but that writing the tiny header to a GIF file in the same directory failed.
+        // If that happens, we _could_ throw up the "Folder Chooser" dialog like we do for still JPG images,
+        // but I have no way of testing whether or not that'll work or get properly exercised,
+        // so I'm not going to bother for now.
+        SnapshotAnimated::clearTempVariables();
+        qDebug() << "Animated snapshot header failed to write - aborting GIF processing.";
+        return;
+    }
     for (int itr = 0; itr < SnapshotAnimated::snapshotAnimatedFrameVector.size(); itr++) {
         // Write each frame to the GIF
         GifWriteFrame(&(SnapshotAnimated::snapshotAnimatedGifWriter),
@@ -126,15 +148,7 @@ void SnapshotAnimated::processFrames() {
     // Write out the end of the GIF
     GifEnd(&(SnapshotAnimated::snapshotAnimatedGifWriter));
 
-    // Clear out the frame and frame delay vectors.
-    // Also release the memory not required to store the items.
-    SnapshotAnimated::snapshotAnimatedFrameVector.clear();
-    SnapshotAnimated::snapshotAnimatedFrameVector.squeeze();
-    SnapshotAnimated::snapshotAnimatedFrameDelayVector.clear();
-    SnapshotAnimated::snapshotAnimatedFrameDelayVector.squeeze();
-    // Reset the current frame timestamp
-    SnapshotAnimated::snapshotAnimatedTimestamp = 0;
-    SnapshotAnimated::snapshotAnimatedFirstFrameTimestamp = 0;
+    SnapshotAnimated::clearTempVariables();
     
     // Update the "Share" dialog with the processed GIF.
     emit SnapshotAnimated::snapshotAnimatedDM->processingGifCompleted(SnapshotAnimated::snapshotAnimatedPath);

interface/src/ui/SnapshotAnimated.h

@@ -49,6 +49,7 @@ private:
 
     static void captureFrames();
     static void processFrames();
+    static void clearTempVariables();
 public:
     static void saveSnapshotAnimated(QString pathStill, float aspectRatio, Application* app, QSharedPointer<WindowScriptingInterface> dm);
     static bool isAlreadyTakingSnapshotAnimated() { return snapshotAnimatedFirstFrameTimestamp != 0; };