Feature/serve screenshots (#1003)

* Take screnshots using lua function, returning screenshot number. Add screenshot endpoint.
* Change capitalization from screenShot to screenshot
* Fix screenshot filename collision bug when using multiple windows.

apps/OpenSpace/main.cpp

@@ -383,7 +383,10 @@ void mainInitFunc() {
 
     for (size_t i = 0; i < nWindows; ++i) {
         sgct::SGCTWindow* w = SgctEngine->getWindowPtr(i);
-        constexpr const char* screenshotNames = "OpenSpace";
+        const std::string screenshotNames = nWindows > 1 ?
+            fmt::format("OpenSpace_{}", i) :
+            "OpenSpace";
+
         sgct_core::ScreenCapture* cpt0 = w->getScreenCapturePointer(0);
         sgct_core::ScreenCapture* cpt1 = w->getScreenCapturePointer(1);
 
@@ -1014,6 +1017,7 @@ void setSgctDelegateFunctions() {
     sgctDelegate.takeScreenshot = [](bool applyWarping) {
         sgct::SGCTSettings::instance()->setCaptureFromBackBuffer(applyWarping);
         sgct::Engine::instance()->takeScreenshot();
+        return sgct::Engine::instance()->getScreenShotNumber();
     };
     sgctDelegate.swapBuffer = []() {
         GLFWwindow* w = glfwGetCurrentContext();

data/assets/util/default_keybindings.asset

@@ -20,16 +20,16 @@ local Keybindings = {
     {
         Key = "PRINT_SCREEN",
         Name = "Take Screenshot",
-        Command = "openspace.setPropertyValueSingle('RenderEngine.TakeScreenshot', nil)",
-        Documentation = "Saves the contents of the screen to a file in the working directory.",
+        Command = "openspace.takeScreenshot()",
+        Documentation = "Saves the contents of the screen to a file in the ${SCREENSHOTS} directory.",
         GuiPath = "/Rendering",
         Local = true
     },
     {
         Key = "F12",
         Name = "Take Screenshot",
-        Command = "openspace.setPropertyValueSingle('RenderEngine.TakeScreenshot', nil)",
-        Documentation = "Saves the contents of the screen to a file in the working directory.",
+        Command = "openspace.takeScreenshot()",
+        Documentation = "Saves the contents of the screen to a file in the ${SCREENSHOTS} directory.",
         GuiPath = "/Rendering",
         Local = true
     },

data/assets/util/screenshots_endpoint.asset

@@ -0,0 +1,21 @@
+asset.onInitialize(function ()
+    -- Disable the server, add production gui endpoint, and restart server.
+    -- The temporary disabling avoids restarting the server on each property change.
+    -- TODO: Add a trigger property to the module to restart the server "manually"
+    -- and remove automatic restart on each property change,
+    -- since frequent restarting seems to be unstable on mac.
+
+    local enabled = openspace.getPropertyValue("Modules.WebGui.ServerProcessEnabled")
+    openspace.setPropertyValueSingle("Modules.WebGui.ServerProcessEnabled", false)
+
+    local directories = openspace.getPropertyValue("Modules.WebGui.Directories")
+    directories[#directories + 1] = "screenshots"
+    directories[#directories + 1] = "${SCREENSHOTS}"
+    openspace.setPropertyValueSingle("Modules.WebGui.Directories", directories)
+    openspace.setPropertyValueSingle("Modules.WebGui.ServerProcessEnabled", enabled)
+end)
+
+asset.onDeinitialize(function ()
+    -- TODO: Remove endpoints. As of 2019-10-29, OpenSpace sometimes
+    -- crashes when endpoints are removed while the application is closing.
+end)

data/assets/util/testing_keybindings.asset

@@ -7,8 +7,8 @@ local Keybindings = {
     {
         Key = "F7",
         Name = "Take Screenshot",
-        Command = "openspace.setPropertyValueSingle('RenderEngine.TakeScreenshot', nil)",
-        Documentation = "Saves the contents of the screen to a file in the working directory.",
+        Command = "openspace.takeScreenshot()",
+        Documentation = "Saves the contents of the screen to a file in the ${SCREENSHOTS} directory.",
         GuiPath = "/Rendering",
         Local = true
     }

include/openspace/engine/windowdelegate.h

@@ -107,7 +107,7 @@ struct WindowDelegate {
 
     bool (*isFisheyeRendering)() = []() { return false; };
 
-    void (*takeScreenshot)(bool applyWarping) = [](bool) { };
+    unsigned int(*takeScreenshot)(bool applyWarping) = [](bool) { return 0u; };
 
     void (*swapBuffer)() = []() {};
 

include/openspace/rendering/renderengine.h

@@ -142,9 +142,14 @@ public:
     void setResolveData(ghoul::Dictionary resolveData);
 
     /**
-     * Mark that one screenshot should be taken
+     * Take a screenshot and store in the ${SCREENSHOTS} directory
      */
-    void takeScreenShot();
+    void takeScreenshot();
+
+    /**
+     * Get the filename of the latest screenshot
+     */
+    unsigned int latestScreenshotNumber() const;
 
     /**
      * Returns the Lua library that contains all Lua functions available to affect the
@@ -187,8 +192,6 @@ private:
     properties::BoolProperty _showVersionInfo;
     properties::BoolProperty _showCameraInfo;
 
-    properties::TriggerProperty _takeScreenshot;
-    bool _shouldTakeScreenshot = false;
     properties::BoolProperty _applyWarping;
     properties::BoolProperty _showFrameInformation;
 #ifdef OPENSPACE_WITH_INSTRUMENTATION
@@ -226,6 +229,7 @@ private:
     properties::Vec3Property _masterRotation;
     
     uint64_t _frameNumber = 0;
+    unsigned int _latestScreenshotNumber = 0;
 
     std::vector<ghoul::opengl::ProgramObject*> _programs;
 

modules/webgui/webguimodule.cpp

@@ -246,16 +246,15 @@ void WebGuiModule::startProcess() {
     std::string formattedDirectories = "[";
 
     std::vector<std::string> directories = _directories.value();
-    bool first = true;
-
-    for (const std::string& str : directories) {
-        if (!first) {
+    for (size_t i = 0; i < directories.size(); ++i) {
+        std::string arg = directories[i];
+        if (i & 1) {
+            arg = absPath(arg);
+        }
+        formattedDirectories += "\\\"" + escapedJson(escapedJson(arg)) + "\\\"";
+        if (i != directories.size() - 1) {
             formattedDirectories += ",";
         }
-        first = false;
-
-        // Escape twice: First json, and then bash (which needs same escape sequences)
-        formattedDirectories += "\\\"" + escapedJson(escapedJson(str)) + "\\\"";
     }
     formattedDirectories += "]";
 

src/interaction/sessionrecording.cpp

@@ -1033,7 +1033,7 @@ void SessionRecording::moveAheadInTime() {
         const Renderable* focusRenderable = focusNode->renderable();
         if (!focusRenderable || focusRenderable->renderedWithDesiredData()) {
             _saveRenderingCurrentRecordedTime += _saveRenderingDeltaTime;
-            global::renderEngine.takeScreenShot();
+            global::renderEngine.takeScreenshot();
         }
     }
 }

src/rendering/renderengine.cpp

@@ -124,16 +124,6 @@ namespace {
         "shown on the screen"
     };
 
-    constexpr openspace::properties::Property::PropertyInfo TakeScreenshotInfo = {
-        "TakeScreenshot",
-        "Take Screenshot",
-        "If this property is triggered, a screenshot is taken and stored in the current "
-        "working directory (which is the same directory where the OpenSpace.exe) is "
-        "located in most cases. The images are prefixed with 'SGCT' and postfixed with "
-        "the number of frames. This function will silently overwrite images that are "
-        "already present in the folder."
-    };
-
     constexpr openspace::properties::Property::PropertyInfo ApplyWarpingInfo = {
         "ApplyWarpingScreenshot",
         "Apply Warping to Screenshots",
@@ -264,7 +254,6 @@ RenderEngine::RenderEngine()
     , _showLog(ShowLogInfo, true)
     , _showVersionInfo(ShowVersionInfo, true)
     , _showCameraInfo(ShowCameraInfo, true)
-    , _takeScreenshot(TakeScreenshotInfo)
     , _applyWarping(ApplyWarpingInfo, false)
     , _showFrameInformation(ShowFrameNumberInfo, false)
 #ifdef OPENSPACE_WITH_INSTRUMENTATION
@@ -379,9 +368,6 @@ RenderEngine::RenderEngine()
     });
     addProperty(_horizFieldOfView);
 
-    _takeScreenshot.onChange([this](){ _shouldTakeScreenshot = true; });
-    addProperty(_takeScreenshot);
-
     addProperty(_showFrameInformation);
 #ifdef OPENSPACE_WITH_INSTRUMENTATION
     _saveFrameInformation.onChange([&]() {
@@ -864,21 +850,6 @@ void RenderEngine::renderDashboard() {
 void RenderEngine::postDraw() {
     ++_frameNumber;
 
-    if (_shouldTakeScreenshot) {
-        // We only create the directory here, as we don't want to spam the users
-        // screenshot folder everytime we start OpenSpace even when we are not taking any
-        // screenshots. So the first time we actually take one, we create the folder:
-        if (!FileSys.directoryExists(absPath("${SCREENSHOTS}"))) {
-            FileSys.createDirectory(
-                absPath("${SCREENSHOTS}"),
-                ghoul::filesystem::FileSystem::Recursive::Yes
-            );
-        }
-
-        global::windowDelegate.takeScreenshot(_applyWarping);
-        _shouldTakeScreenshot = false;
-    }
-
     if (global::performanceManager.isEnabled()) {
         global::performanceManager.storeScenePerformanceMeasurements(
             scene()->allSceneGraphNodes()
@@ -1061,10 +1032,28 @@ void RenderEngine::setResolveData(ghoul::Dictionary resolveData) {
 }
 
 /**
-  * Mark that one screenshot should be taken
-*/
-void RenderEngine::takeScreenShot() {
-    _shouldTakeScreenshot = true;
+ * Take a screenshot and store it in the ${SCREENSHOTS} directory
+ */
+void RenderEngine::takeScreenshot() {
+    // We only create the directory here, as we don't want to spam the users
+    // screenshot folder everytime we start OpenSpace even when we are not taking any
+    // screenshots. So the first time we actually take one, we create the folder:
+
+    if (!FileSys.directoryExists(absPath("${SCREENSHOTS}"))) {
+        FileSys.createDirectory(
+            absPath("${SCREENSHOTS}"),
+            ghoul::filesystem::FileSystem::Recursive::Yes
+        );
+    }
+
+    _latestScreenshotNumber = global::windowDelegate.takeScreenshot(_applyWarping);
+}
+
+/**
+ * Get the latest screenshot filename
+ */
+unsigned int RenderEngine::latestScreenshotNumber() const {
+    return _latestScreenshotNumber;
 }
 
 /**
@@ -1123,6 +1112,14 @@ scripting::LuaLibrary RenderEngine::luaLibrary() {
                 "Given a ScreenSpaceRenderable name this script will remove it from the "
                 "renderengine"
             },
+            {
+                "takeScreenshot",
+                &luascriptfunctions::takeScreenshot,
+                {},
+                "",
+                "Take a screenshot and return the screenshot number. The screenshot will "
+                "be stored in the ${SCREENSHOTS} folder. "
+            }
         },
     };
 }

src/rendering/renderengine_lua.inl

@@ -89,4 +89,14 @@ int removeScreenSpaceRenderable(lua_State* L) {
     return 0;
 }
 
+int takeScreenshot(lua_State* L) {
+    ghoul::lua::checkArgumentsAndThrow(L, 0, "lua::takeScreenshot");
+
+    global::renderEngine.takeScreenshot();
+    const unsigned int screenshotNumber = global::renderEngine.latestScreenshotNumber();
+
+    lua_pushinteger(L, screenshotNumber);
+    return 1;
+}
+
 }// namespace openspace::luascriptfunctions