EffectComposer: Simplify temporary file handling

QTemporaryFile was used just to generate file names for shader files,
and the actual files didn't get removed at shutdown. There was also
a possibility of reloading effect quickly, which left additional
dangling compiled qsb files because they are generated asynchronously
with a separate process.

Now temporary shader files are gathered into one temporary directory,
which removes itself at shutdown, so dangling files won't be an issue.

Fixes: QDS-12711
Change-Id: I7ce682acb1a0d8d58b84129c4e0442c8fb63ac2f
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>

src/plugins/effectcomposer/effectcomposermodel.cpp

@@ -21,6 +21,7 @@
 
 #include <QByteArrayView>
 #include <QLibraryInfo>
+#include <QTemporaryDir>
 #include <QVector2D>
 
 namespace EffectComposer {
@@ -48,6 +49,7 @@ static bool writeToFile(const QByteArray &buf, const QString &filename, FileType
 
 EffectComposerModel::EffectComposerModel(QObject *parent)
     : QAbstractListModel{parent}
+    , m_shaderDir(QDir::tempPath() + "/qds_ec_XXXXXX")
 {
     m_rebakeTimer.setSingleShot(true);
     connect(&m_rebakeTimer, &QTimer::timeout, this, &EffectComposerModel::bakeShaders);
@@ -1713,37 +1715,33 @@ void EffectComposerModel::updateCustomUniforms()
     m_exportedEffectPropertiesString = exportedEffectPropertiesString;
 }
 
-void EffectComposerModel::createFiles()
+void EffectComposerModel::initShaderDir()
 {
-    if (QFileInfo::exists(m_vertexShaderFilename))
+    static int count = 0;
-        QFile(m_vertexShaderFilename).remove();
+    static const QString fileNameTemplate = "%1_%2.%3";
-    if (QFileInfo::exists(m_fragmentShaderFilename))
+    const QString countStr = QString::number(count);
-        QFile(m_fragmentShaderFilename).remove();
-    if (QFileInfo::exists(m_vertexShaderPreviewFilename))
-        QFile(m_vertexShaderPreviewFilename).remove();
-    if (QFileInfo::exists(m_fragmentShaderPreviewFilename))
-        QFile(m_fragmentShaderPreviewFilename).remove();
 
-    auto vertexShaderFile = QTemporaryFile(QDir::tempPath() + "/dsem_XXXXXX.vert.qsb");
+    auto resetFile = [&countStr, this](QString &fileName, const QString prefix, const QString suffix) {
-    auto fragmentShaderFile = QTemporaryFile(QDir::tempPath() + "/dsem_XXXXXX.frag.qsb");
+        // qsb generation is done in separate process, so it is not guaranteed all of the old files
-    auto vertexShaderPreviewFile = QTemporaryFile(QDir::tempPath() + "/dsem_prev_XXXXXX.vert.qsb");
+        // get deleted here, as they may not exist yet. Any dangling files will be deleted at
-    auto fragmentShaderPreviewFile = QTemporaryFile(QDir::tempPath() + "/dsem_prev_XXXXXX.frag.qsb");
+        // application shutdown, when the temporary directory is destroyed.
+        if (!fileName.isEmpty()) {
+            QFile file(fileName);
+            if (file.exists())
+                file.remove();
+        }
 
-    m_vertexSourceFile.setFileTemplate(QDir::tempPath() + "/dsem_XXXXXX.vert");
+        fileName = m_shaderDir.filePath(fileNameTemplate.arg(prefix, countStr, suffix));
-    m_fragmentSourceFile.setFileTemplate(QDir::tempPath() + "/dsem_XXXXXX.frag");
+    };
 
-    if (!m_vertexSourceFile.open() || !m_fragmentSourceFile.open()
+    resetFile(m_vertexSourceFilename, "source", "vert");
-        || !vertexShaderFile.open() || !fragmentShaderFile.open()
+    resetFile(m_fragmentSourceFilename, "source", "frag");
-        || !vertexShaderPreviewFile.open() || !fragmentShaderPreviewFile.open()) {
+    resetFile(m_vertexShaderFilename, "compiled", "vert.qsb");
-        qWarning() << "Unable to open temporary files";
+    resetFile(m_fragmentShaderFilename, "compiled", "frag.qsb");
-    } else {
+    resetFile(m_vertexShaderPreviewFilename, "compiled_prev", "vert.qsb");
-        m_vertexSourceFilename = m_vertexSourceFile.fileName();
+    resetFile(m_fragmentShaderPreviewFilename, "compiled_prev", "frag.qsb");
-        m_fragmentSourceFilename = m_fragmentSourceFile.fileName();
+
-        m_vertexShaderFilename = vertexShaderFile.fileName();
+    ++count;
-        m_fragmentShaderFilename = fragmentShaderFile.fileName();
-        m_vertexShaderPreviewFilename = vertexShaderPreviewFile.fileName();
-        m_fragmentShaderPreviewFilename = fragmentShaderPreviewFile.fileName();
-    }
 }
 
 void EffectComposerModel::bakeShaders()
@@ -1756,7 +1754,7 @@ void EffectComposerModel::bakeShaders()
         return;
     }
 
-    createFiles();
+    initShaderDir();
 
     resetEffectError(ErrorPreprocessor);
     if (m_vertexShader == generateVertexShader() && m_fragmentShader == generateFragmentShader()) {
@@ -1775,11 +1773,11 @@ void EffectComposerModel::bakeShaders()
 
     setVertexShader(generateVertexShader());
     QString vs = m_vertexShader;
-    writeToFile(vs.toUtf8(), m_vertexSourceFile.fileName(), FileType::Text);
+    writeToFile(vs.toUtf8(), m_vertexSourceFilename, FileType::Text);
 
     setFragmentShader(generateFragmentShader());
     QString fs = m_fragmentShader;
-    writeToFile(fs.toUtf8(), m_fragmentSourceFile.fileName(), FileType::Text);
+    writeToFile(fs.toUtf8(), m_fragmentSourceFilename, FileType::Text);
 
     QtSupport::QtVersion *qtVer = QtSupport::QtKitAspect::qtVersion(target->kit());
     if (!qtVer) {

src/plugins/effectcomposer/effectcomposermodel.h

@@ -12,7 +12,7 @@
 #include <QMap>
 #include <QRegularExpression>
 #include <QSet>
-#include <QTemporaryFile>
+#include <QTemporaryDir>
 #include <QTimer>
 
 namespace ProjectExplorer {
@@ -176,7 +176,7 @@ private:
     QString getQmlEffectString();
 
     void updateCustomUniforms();
-    void createFiles();
+    void initShaderDir();
     void bakeShaders();
     void saveResources(const QString &name);
 
@@ -205,8 +205,7 @@ private:
     QStringList m_defaultRootVertexShader;
     QStringList m_defaultRootFragmentShader;
     // Temp files to store shaders sources and binary data
-    QTemporaryFile m_fragmentSourceFile;
+    QTemporaryDir m_shaderDir;
-    QTemporaryFile m_vertexSourceFile;
     QString m_fragmentSourceFilename;
     QString m_vertexSourceFilename;
     QString m_fragmentShaderFilename;