From 5188de924f3418b757f4350f66c4077f0ab3df95 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 22 Nov 2016 11:09:43 -0800 Subject: [PATCH 1/5] allow for SnapshotReview dialog to be up during a change of href for all media capture --- interface/src/Application.cpp | 6 +++--- interface/src/Application.h | 2 +- .../src/scripting/WindowScriptingInterface.cpp | 4 ++-- interface/src/scripting/WindowScriptingInterface.h | 2 +- interface/src/ui/Snapshot.cpp | 14 +++++++++----- interface/src/ui/Snapshot.h | 2 +- scripts/system/snapshot.js | 10 +++++++--- 7 files changed, 24 insertions(+), 16 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 82007c4f06..a072615c03 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5450,10 +5450,10 @@ void Application::takeSnapshot(bool notify, bool includeAnimated, float aspectRa } }); } -void Application::shareSnapshot(const QString& path) { - postLambdaEvent([path] { +void Application::shareSnapshot(const QString& path, const QUrl& href) { + postLambdaEvent([path, href] { // not much to do here, everything is done in snapshot code... - Snapshot::uploadSnapshot(path); + Snapshot::uploadSnapshot(path, href); }); } diff --git a/interface/src/Application.h b/interface/src/Application.h index 8f8b42d66a..5ab94465cc 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -267,7 +267,7 @@ public: float getAverageSimsPerSecond() const { return _simCounter.rate(); } void takeSnapshot(bool notify, bool includeAnimated = false, float aspectRatio = 0.0f); - void shareSnapshot(const QString& filename); + void shareSnapshot(const QString& filename, const QUrl& href = QUrl("")); model::SkyboxPointer getDefaultSkybox() const { return _defaultSkybox; } gpu::TexturePointer getDefaultSkyboxTexture() const { return _defaultSkyboxTexture; } diff --git a/interface/src/scripting/WindowScriptingInterface.cpp b/interface/src/scripting/WindowScriptingInterface.cpp index 0cb574c1f6..765c9a8499 100644 --- a/interface/src/scripting/WindowScriptingInterface.cpp +++ b/interface/src/scripting/WindowScriptingInterface.cpp @@ -203,8 +203,8 @@ void WindowScriptingInterface::takeSnapshot(bool notify, bool includeAnimated, f qApp->takeSnapshot(notify, includeAnimated, aspectRatio); } -void WindowScriptingInterface::shareSnapshot(const QString& path) { - qApp->shareSnapshot(path); +void WindowScriptingInterface::shareSnapshot(const QString& path, const QUrl& href) { + qApp->shareSnapshot(path, href); } bool WindowScriptingInterface::isPhysicsEnabled() { diff --git a/interface/src/scripting/WindowScriptingInterface.h b/interface/src/scripting/WindowScriptingInterface.h index 7246dc0927..2e552d44d1 100644 --- a/interface/src/scripting/WindowScriptingInterface.h +++ b/interface/src/scripting/WindowScriptingInterface.h @@ -53,7 +53,7 @@ public slots: void showAssetServer(const QString& upload = ""); void copyToClipboard(const QString& text); void takeSnapshot(bool notify = true, bool includeAnimated = false, float aspectRatio = 0.0f); - void shareSnapshot(const QString& path); + void shareSnapshot(const QString& path, const QUrl& href = QUrl("")); bool isPhysicsEnabled(); signals: diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index 5df0d4575b..45e2680032 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -151,13 +151,17 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary) { return imageTempFile; } -void Snapshot::uploadSnapshot(const QString& filename) { +void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { const QString SNAPSHOT_UPLOAD_URL = "/api/v1/snapshots"; - // Alternatively to parseSnapshotData, we could pass the inWorldLocation through the call chain. This way is less disruptive to existing code. - SnapshotMetaData* snapshotData = Snapshot::parseSnapshotData(filename); - SnapshotUploader* uploader = new SnapshotUploader(snapshotData->getURL(), filename); - delete snapshotData; + SnapshotUploader* uploader; + if (href.isEmpty()) { + SnapshotMetaData* snapshotData = Snapshot::parseSnapshotData(filename); + uploader = new SnapshotUploader(snapshotData->getURL(), filename); + delete snapshotData; + } else { + uploader = new SnapshotUploader(href, filename); + } QFile* file = new QFile(filename); Q_ASSERT(file->exists()); diff --git a/interface/src/ui/Snapshot.h b/interface/src/ui/Snapshot.h index 2daed0e860..14e1bc2e9f 100644 --- a/interface/src/ui/Snapshot.h +++ b/interface/src/ui/Snapshot.h @@ -39,7 +39,7 @@ public: static SnapshotMetaData* parseSnapshotData(QString snapshotPath); static Setting::Handle snapshotsLocation; - static void uploadSnapshot(const QString& filename); + static void uploadSnapshot(const QString& filename, const QUrl& href = QUrl("")); private: static QFile* savedFileForSnapshot(QImage & image, bool isTemporary); }; diff --git a/scripts/system/snapshot.js b/scripts/system/snapshot.js index f6ae6d153d..18015a479d 100644 --- a/scripts/system/snapshot.js +++ b/scripts/system/snapshot.js @@ -71,7 +71,7 @@ function confirmShare(data) { if (submessage.share) { print('sharing', submessage.localPath); outstanding++; - Window.shareSnapshot(submessage.localPath); + Window.shareSnapshot(submessage.localPath, submessage.href); } else { print('not sharing', submessage.localPath); } @@ -159,10 +159,14 @@ function resetButtons(pathStillSnapshot, pathAnimatedSnapshot, notify) { button.writeProperty("hoverState", 3); Window.snapshotTaken.disconnect(resetButtons); + // A Snapshot Review dialog might be left open indefinitely after taking the picture, + // during which time the user may have moved. So stash that info in the dialog so that + // it records the correct href. (We can also stash in .jpegs, but not .gifs.) + var href = location.href; // last element in data array tells dialog whether we can share or not confirmShare([ - { localPath: pathAnimatedSnapshot }, - { localPath: pathStillSnapshot }, + { localPath: pathAnimatedSnapshot, href: href }, + { localPath: pathStillSnapshot, href: href }, { canShare: !!isDomainOpen(location.domainId), openFeedAfterShare: shouldOpenFeedAfterShare() From 2c9046742eed14296600c60d661e1413f6e3ae05 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 22 Nov 2016 16:02:01 -0800 Subject: [PATCH 2/5] allow movement during .gif recording --- scripts/system/snapshot.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/scripts/system/snapshot.js b/scripts/system/snapshot.js index 18015a479d..439cae22f8 100644 --- a/scripts/system/snapshot.js +++ b/scripts/system/snapshot.js @@ -14,6 +14,7 @@ var SNAPSHOT_DELAY = 500; // 500ms var toolBar = Toolbars.getToolbar("com.highfidelity.interface.toolbar.system"); var resetOverlays; var reticleVisible; +var clearOverlayWhenMoving; var button = toolBar.addButton({ objectName: "snapshot", imageURL: Script.resolvePath("assets/images/tools/snap.svg"), @@ -100,8 +101,13 @@ function snapshotShared(errorMessage) { } function onClicked() { + // Raising the desktop for the share dialog at end will interact badly with clearOverlayWhenMoving. + // Turn it off now, before we start futzing with things (and possibly moving). + clearOverlayWhenMoving = MyAvatar.getClearOverlayWhenMoving(); // Do not use Settings. MyAvatar keeps a separate copy. + MyAvatar.setClearOverlayWhenMoving(false); + // update button states - resetOverlays = Menu.isOptionChecked("Overlays"); + resetOverlays = Menu.isOptionChecked("Overlays"); // For completness. Certainly true if the button is visible to be clicke. reticleVisible = Reticle.visible; Reticle.visible = false; Window.snapshotTaken.connect(resetButtons); @@ -109,7 +115,7 @@ function onClicked() { button.writeProperty("buttonState", 0); button.writeProperty("defaultState", 0); button.writeProperty("hoverState", 2); - + // hide overlays if they are on if (resetOverlays) { Menu.setIsOptionChecked("Overlays", false); @@ -145,10 +151,6 @@ function isDomainOpen(id) { } function resetButtons(pathStillSnapshot, pathAnimatedSnapshot, notify) { - // show overlays if they were on - if (resetOverlays) { - Menu.setIsOptionChecked("Overlays", true); - } // show hud toolBar.writeProperty("visible", true); Reticle.visible = reticleVisible; @@ -158,6 +160,10 @@ function resetButtons(pathStillSnapshot, pathAnimatedSnapshot, notify) { button.writeProperty("defaultState", 1); button.writeProperty("hoverState", 3); Window.snapshotTaken.disconnect(resetButtons); + // show overlays if they were on + if (resetOverlays) { + Menu.setIsOptionChecked("Overlays", true); + } // A Snapshot Review dialog might be left open indefinitely after taking the picture, // during which time the user may have moved. So stash that info in the dialog so that @@ -172,7 +178,10 @@ function resetButtons(pathStillSnapshot, pathAnimatedSnapshot, notify) { openFeedAfterShare: shouldOpenFeedAfterShare() } ]); - } + if (clearOverlayWhenMoving) { + MyAvatar.setClearOverlayWhenMoving(true); // not until after the share dialog + } +} button.clicked.connect(onClicked); Window.snapshotShared.connect(snapshotShared); From 414371d1fc5b3b53e3742b14a5181f9c9807b55c Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 22 Nov 2016 16:13:08 -0800 Subject: [PATCH 3/5] record starting href/domainId, not ending --- scripts/system/snapshot.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/scripts/system/snapshot.js b/scripts/system/snapshot.js index 439cae22f8..f78b7ec985 100644 --- a/scripts/system/snapshot.js +++ b/scripts/system/snapshot.js @@ -99,13 +99,18 @@ function snapshotShared(errorMessage) { showFeedWindow(); } } - +var href, domainId; function onClicked() { // Raising the desktop for the share dialog at end will interact badly with clearOverlayWhenMoving. // Turn it off now, before we start futzing with things (and possibly moving). clearOverlayWhenMoving = MyAvatar.getClearOverlayWhenMoving(); // Do not use Settings. MyAvatar keeps a separate copy. MyAvatar.setClearOverlayWhenMoving(false); + // We will record snapshots based on the starting location. That could change, e.g., when recording a .gif. + // Even the domainId could change (e.g., if the user falls into a teleporter while recording). + href = location.href; + domainId = location.domainId; + // update button states resetOverlays = Menu.isOptionChecked("Overlays"); // For completness. Certainly true if the button is visible to be clicke. reticleVisible = Reticle.visible; @@ -168,13 +173,12 @@ function resetButtons(pathStillSnapshot, pathAnimatedSnapshot, notify) { // A Snapshot Review dialog might be left open indefinitely after taking the picture, // during which time the user may have moved. So stash that info in the dialog so that // it records the correct href. (We can also stash in .jpegs, but not .gifs.) - var href = location.href; // last element in data array tells dialog whether we can share or not confirmShare([ { localPath: pathAnimatedSnapshot, href: href }, { localPath: pathStillSnapshot, href: href }, { - canShare: !!isDomainOpen(location.domainId), + canShare: !!isDomainOpen(domainId), openFeedAfterShare: shouldOpenFeedAfterShare() } ]); From 4e0c6e5d0f793c31cb242b4099e5da073bac5e20 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 23 Nov 2016 11:00:31 -0800 Subject: [PATCH 4/5] handle empty snapshot text data more broadly (and at the same time, don't lie from parseSnapshotData). --- interface/src/ui/Snapshot.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index 45e2680032..06aa90386b 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -63,8 +63,6 @@ SnapshotMetaData* Snapshot::parseSnapshotData(QString snapshotPath) { // parsing URL url = QUrl(shot.text(URL), QUrl::ParsingMode::StrictMode); - } else if (snapshotPath.right(3) == "gif") { - url = QUrl(DependencyManager::get()->currentShareableAddress()); } else { return NULL; } @@ -154,14 +152,16 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary) { void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { const QString SNAPSHOT_UPLOAD_URL = "/api/v1/snapshots"; - SnapshotUploader* uploader; - if (href.isEmpty()) { + QUrl url = href; + if (url.isEmpty()) { SnapshotMetaData* snapshotData = Snapshot::parseSnapshotData(filename); - uploader = new SnapshotUploader(snapshotData->getURL(), filename); + url = snapshotData->getURL(); delete snapshotData; - } else { - uploader = new SnapshotUploader(href, filename); } + if (url.isEmpty()) { + url = QUrl(DependencyManager::get()->currentShareableAddress()); + } + SnapshotUploader* uploader = new SnapshotUploader(url, filename); QFile* file = new QFile(filename); Q_ASSERT(file->exists()); From bf0f56c138e2577c0e6cd3ba50197e9e14a4d64d Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 23 Nov 2016 15:27:24 -0800 Subject: [PATCH 5/5] null guard --- interface/src/ui/Snapshot.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index 06aa90386b..f75190530c 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -155,7 +155,9 @@ void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { QUrl url = href; if (url.isEmpty()) { SnapshotMetaData* snapshotData = Snapshot::parseSnapshotData(filename); - url = snapshotData->getURL(); + if (snapshotData) { + url = snapshotData->getURL(); + } delete snapshotData; } if (url.isEmpty()) {