ExternalWindow mostly working now, update old hacky example too

Signed-off-by: falkTX <falktx@falktx.com>

distrho/extra/ExternalWindow.hpp

@@ -43,26 +43,7 @@ START_NAMESPACE_DISTRHO
  */
 class ExternalWindow
 {
-    struct PrivateData {
-        uintptr_t parentWindowHandle;
-        uintptr_t transientWinId;
-        uint width;
-        uint height;
-        double scaleFactor;
-        String title;
-        bool visible;
-        pid_t pid;
-
-        PrivateData()
-            : parentWindowHandle(0),
-              transientWinId(0),
-              width(1),
-              height(1),
-              scaleFactor(1.0),
-              title(),
-              visible(false),
-              pid(0) {}
-    } pData;
+    struct PrivateData;
 
 public:
    /**
@@ -82,9 +63,7 @@ public:
     */
     virtual ~ExternalWindow()
     {
-        /*
-        terminateAndWaitForProcess();
-        */
+        DISTRHO_SAFE_ASSERT(!pData.visible);
     }
 
    /* --------------------------------------------------------------------------------------------------------
@@ -92,11 +71,17 @@ public:
 
     virtual bool isRunning() const
     {
+        if (ext.inUse)
+            return ext.isRunning();
+
         return isVisible();
     }
 
     virtual bool isQuiting() const
     {
+        if (ext.inUse)
+            return ext.isQuiting;
+
         return !isVisible();
     }
 
@@ -113,6 +98,14 @@ public:
         transientWindowChanged(winId);
     }
 
+    void close()
+    {
+        hide();
+
+        if (ext.inUse)
+            terminateAndWaitForExternalProcess();
+    }
+
 #if DISTRHO_PLUGIN_HAS_EMBED_UI
    /**
       Whether this Window is embed into another (usually not DGL-controlled) Window.
@@ -280,6 +273,25 @@ public:
     virtual void focus() {}
 
 protected:
+   /* --------------------------------------------------------------------------------------------------------
+    * ExternalWindow special calls for running externals tools */
+
+    bool startExternalProcess(const char* args[])
+    {
+        ext.inUse = true;
+
+        return ext.start(args);
+    }
+
+    void terminateAndWaitForExternalProcess()
+    {
+        ext.isQuiting = true;
+        ext.terminateAndWait();
+    }
+
+   /* --------------------------------------------------------------------------------------------------------
+    * ExternalWindow specific callbacks */
+
    /**
       A function called when the window is resized.
     */
@@ -309,101 +321,125 @@ protected:
         return; (void)winId;
     }
 
-    /*
-    bool isRunning() noexcept
-    {
-        if (pid <= 0)
-            return false;
+private:
+    friend class PluginWindow;
+    friend class UI;
 
-        const pid_t p = ::waitpid(pid, nullptr, WNOHANG);
+    struct ExternalProcess {
+        bool inUse;
+        bool isQuiting;
+        mutable pid_t pid;
 
-        if (p == pid || (p == -1 && errno == ECHILD))
-        {
-            printf("NOTICE: Child process exited while idle\n");
-            pid = 0;
-            return false;
-        }
+        ExternalProcess()
+            : inUse(false),
+              isQuiting(false),
+              pid(0) {}
 
-        return true;
-    }
+        bool isRunning() const noexcept
+        {
+            if (pid <= 0)
+                return false;
 
-    */
+            const pid_t p = ::waitpid(pid, nullptr, WNOHANG);
 
-protected:
-    /*
-    bool startExternalProcess(const char* args[])
-    {
-        terminateAndWaitForProcess();
+            if (p == pid || (p == -1 && errno == ECHILD))
+            {
+                d_stdout("NOTICE: Child process exited while idle");
+                pid = 0;
+                return false;
+            }
 
-        pid = vfork();
+            return true;
+        }
 
-        switch (pid)
+        bool start(const char* args[])
         {
-        case 0:
-            execvp(args[0], (char**)args);
-            _exit(1);
-            return false;
-
-        case -1:
-            printf("Could not start external ui\n");
-            return false;
+            terminateAndWait();
 
-        default:
-            return true;
-        }
-    }
+            pid = vfork();
 
-    void terminateAndWaitForProcess()
-    {
-        if (pid <= 0)
-            return;
+            switch (pid)
+            {
+            case 0:
+                execvp(args[0], (char**)args);
+                _exit(1);
+                return false;
 
-        printf("Waiting for previous process to stop,,,\n");
+            case -1:
+                d_stderr("Could not start external ui");
+                return false;
 
-        bool sendTerm = true;
+            default:
+                return true;
+            }
+        }
 
-        for (pid_t p;;)
+        void terminateAndWait()
         {
-            p = ::waitpid(pid, nullptr, WNOHANG);
+            if (pid <= 0)
+                return;
+
+            d_stdout("Waiting for external process to stop,,,");
+
+            bool sendTerm = true;
 
-            switch (p)
+            for (pid_t p;;)
             {
-            case 0:
-                if (sendTerm)
-                {
-                    sendTerm = false;
-                    ::kill(pid, SIGTERM);
-                }
-                break;
+                p = ::waitpid(pid, nullptr, WNOHANG);
 
-            case -1:
-                if (errno == ECHILD)
+                switch (p)
                 {
-                    printf("Done! (no such process)\n");
-                    pid = 0;
-                    return;
+                case 0:
+                    if (sendTerm)
+                    {
+                        sendTerm = false;
+                        ::kill(pid, SIGTERM);
+                    }
+                    break;
+
+                case -1:
+                    if (errno == ECHILD)
+                    {
+                        d_stdout("Done! (no such process)");
+                        pid = 0;
+                        return;
+                    }
+                    break;
+
+                default:
+                    if (p == pid)
+                    {
+                        d_stdout("Done! (clean wait)");
+                        pid = 0;
+                        return;
+                    }
+                    break;
                 }
-                break;
 
-            default:
-                if (p == pid)
-                {
-                    printf("Done! (clean wait)\n");
-                    pid = 0;
-                    return;
-                }
-                break;
+                // 5 msec
+                usleep(5*1000);
             }
-
-            // 5 msec
-            usleep(5*1000);
         }
-    }
-    */
+    } ext;
 
-private:
-    friend class PluginWindow;
-    friend class UI;
+    struct PrivateData {
+        uintptr_t parentWindowHandle;
+        uintptr_t transientWinId;
+        uint width;
+        uint height;
+        double scaleFactor;
+        String title;
+        bool visible;
+
+        PrivateData()
+            : parentWindowHandle(0),
+              transientWinId(0),
+              width(1),
+              height(1),
+              scaleFactor(1.0),
+              title(),
+              visible(false) {}
+    } pData;
 
     DISTRHO_DECLARE_NON_COPYABLE(ExternalWindow)
 };

distrho/src/DistrhoUIPrivateData.hpp

@@ -84,6 +84,9 @@ struct PluginApplication
             d_msleep(30);
             idleCallback->idleCallback();
         }
+
+        if (! ui->isQuiting())
+            ui->close();
     }
 
     // these are not needed
@@ -136,18 +139,13 @@ public:
     uintptr_t getNativeWindowHandle() const noexcept { return ui->pData.parentWindowHandle; }
 
     // direct mappings
-    bool isVisible() const noexcept { return ui->isVisible(); }
+    void close() { ui->close(); }
     void focus() { ui->focus(); }
     void show() { ui->show(); }
+    bool isVisible() const noexcept { return ui->isVisible(); }
     void setTitle(const char* const title) { ui->setTitle(title); }
     void setVisible(const bool visible) { ui->setVisible(visible); }
 
-    // custom
-    void close()
-    {
-        ui->hide();
-    }
-
     DISTRHO_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(PluginWindow)
 };
 #else // DISTRHO_PLUGIN_HAS_EXTERNAL_UI

examples/ExternalUI/ExternalExampleUI.cpp

@@ -1,6 +1,6 @@
 /*
  * DISTRHO Plugin Framework (DPF)
- * Copyright (C) 2012-2019 Filipe Coelho <falktx@falktx.com>
+ * Copyright (C) 2012-2021 Filipe Coelho <falktx@falktx.com>
  *
  * Permission to use, copy, modify, and/or distribute this software for any purpose with
  * or without fee is hereby granted, provided that the above copyright notice and this
@@ -14,6 +14,9 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+// needed for IDE
+#include "DistrhoPluginInfo.h"
+
 #include "DistrhoUI.hpp"
 
 // Extra includes for current path and fifo stuff
@@ -46,13 +49,14 @@ static bool fileExists(const char* const filename)
 static ssize_t
 writeRetry(int fd, const void* src, size_t size)
 {
-	ssize_t error;
+    ssize_t error;
+    int attempts = 0;
 
-	do {
-		error = write(fd, src, size);
-	} while (error == -1 && (errno == EINTR || errno == EPIPE));
+    do {
+        error = write(fd, src, size);
+    } while (error == -1 && (errno == EINTR || errno == EPIPE) && ++attempts < 5);
 
-	return error;
+    return error;
 }
 
 // -----------------------------------------------------------------------------------------------------------
@@ -72,7 +76,7 @@ public:
             fExternalScript.truncate(fExternalScript.rfind('/'));
         }
 
-        fExternalScript += "/d_extui.sh";
+        fExternalScript += "/ExternalLauncher.sh";
         d_stdout("External script = %s", fExternalScript.buffer());
     }
 
@@ -105,6 +109,17 @@ protected:
    /* --------------------------------------------------------------------------------------------------------
     * External Window overrides */
 
+   /**
+      Keep-alive.
+    */
+    void uiIdle() override
+    {
+        if (fFifo == -1)
+            return;
+
+        writeRetry(fFifo, "idle\n", 5);
+    }
+
    /**
       Manage external process and IPC when UI is requested to be visible.
     */
@@ -119,7 +134,7 @@ protected:
 
             char winIdStr[24];
             std::memset(winIdStr, 0, sizeof(winIdStr));
-            std::snprintf(winIdStr, 23, "%lu", getTransientWinId());
+            std::snprintf(winIdStr, 23, "%lu", getTransientWindowId());
 
             const char* args[] = {
                 fExternalScript.buffer(),
@@ -145,12 +160,12 @@ protected:
                     DISTRHO_SAFE_ASSERT(writeRetry(fFifo, "quit\n", 5) == 5);
                     fsync(fFifo);
                 }
-                close(fFifo);
+                ::close(fFifo);
                 fFifo = -1;
             }
 
             unlink(kFifoFilename);
-            terminateAndWaitForProcess();
+            terminateAndWaitForExternalProcess();
         }
 
         UI::setVisible(yesNo);

examples/ExternalUI/ExternalLauncher.sh

@@ -20,13 +20,21 @@ fi
 # Setup cancellation point for this script
 quitfn() {
     qdbus ${dbusRef} close 2>/dev/null
+    exit 0
 }
 
 trap quitfn SIGINT
 trap quitfn SIGTERM
 
 # Read Fifo for new values or a quit message
-while read line <"${FIFO}"; do
+while read -t 5 line < "${FIFO}"; do
+  if [ $? != 0 ]; then
+    echo "Timed out, closing"
+    break
+  fi
+  if echo "${line}" | grep -q "idle"; then
+    continue
+  fi
   if echo "${line}" | grep -q "quit"; then
     break
   fi