From 9ac1443b8329e5ed85bfe20dda3b7e07685da00f Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Tue, 5 Jun 2018 11:17:17 -0700 Subject: [PATCH 01/21] Use optimized Bullet raytest function in CharacterGhostObject::rayTest() --- libraries/physics/src/CharacterGhostObject.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/physics/src/CharacterGhostObject.cpp b/libraries/physics/src/CharacterGhostObject.cpp index a771a52384..0e1ea22375 100755 --- a/libraries/physics/src/CharacterGhostObject.cpp +++ b/libraries/physics/src/CharacterGhostObject.cpp @@ -69,7 +69,7 @@ bool CharacterGhostObject::rayTest(const btVector3& start, const btVector3& end, CharacterRayResult& result) const { if (_world && _inWorld) { - _world->rayTest(start, end, result); + this->btGhostObject::rayTest(start, end, result); } return result.hasHit(); } From 2b0ac6a9bd7ea4e69fb28a7fde7df2410c6ef6e0 Mon Sep 17 00:00:00 2001 From: Cristian Luis Duarte Date: Thu, 7 Jun 2018 18:22:58 -0300 Subject: [PATCH 02/21] Android - Make sure we call Interface enterBackground when opening the app --- .../hifiinterface/InterfaceActivity.java | 14 +++++++++++++- interface/src/Application.cpp | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java b/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java index 28acc77609..20c216d5a9 100644 --- a/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java @@ -56,6 +56,8 @@ public class InterfaceActivity extends QtActivity { private AssetManager assetManager; private static boolean inVrMode; + + private boolean nativeEnterBackgroundCallEnqueued = false; // private GvrApi gvrApi; // Opaque native pointer to the Application C++ object. // This object is owned by the InterfaceActivity instance and passed to the native methods. @@ -121,13 +123,19 @@ public class InterfaceActivity extends QtActivity { @Override protected void onPause() { super.onPause(); - nativeEnterBackground(); + if (super.isLoading) { + nativeEnterBackgroundCallEnqueued = true; + } else { + Log.d("[ENTERBACKGROUND]","onPause calling nativeEnterBackground"); + nativeEnterBackground(); + } //gvrApi.pauseTracking(); } @Override protected void onStart() { super.onStart(); + nativeEnterBackgroundCallEnqueued = false; setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE); } @@ -256,6 +264,10 @@ public class InterfaceActivity extends QtActivity { public void onAppLoadedComplete() { super.isLoading = false; + if (nativeEnterBackgroundCallEnqueued) { + Log.d("[ENTERBACKGROUND]","onAppLoadedComplete calling nativeEnterBackground"); + nativeEnterBackground(); + } } public void performHapticFeedback(int duration) { diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index cbe713127d..6111406291 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -8248,10 +8248,12 @@ void Application::saveNextPhysicsStats(QString filename) { #if defined(Q_OS_ANDROID) void Application::enterBackground() { + qDebug() << "[ENTERBACKGROUND] Application::enterBackground()"; QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); if (getActiveDisplayPlugin()->isActive()) { getActiveDisplayPlugin()->deactivate(); + qDebug() << "[ENTERBACKGROUND] Application::enterBackground() getActiveDisplayPlugin()->deactivated"; } } From 8869af069e37e99e54cadebc085277f327e307c6 Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Thu, 7 Jun 2018 18:59:43 -0300 Subject: [PATCH 03/21] Integrate Breakpad in android Interface --- android/app/build.gradle | 9 +- .../hifiinterface/PermissionChecker.java | 125 +++++++++++++++++- cmake/macros/TargetBreakpad.cmake | 22 +++ interface/CMakeLists.txt | 1 + interface/src/Breakpad.cpp | 72 ++++++++++ interface/src/Breakpad.h | 20 +++ interface/src/Crashpad.cpp | 2 +- 7 files changed, 242 insertions(+), 9 deletions(-) create mode 100644 cmake/macros/TargetBreakpad.cmake create mode 100644 interface/src/Breakpad.cpp create mode 100644 interface/src/Breakpad.h diff --git a/android/app/build.gradle b/android/app/build.gradle index 46de9642d9..44ce64bf9b 100644 --- a/android/app/build.gradle +++ b/android/app/build.gradle @@ -24,7 +24,8 @@ android { '-DRELEASE_TYPE=' + RELEASE_TYPE, '-DBUILD_BRANCH=' + BUILD_BRANCH, '-DDISABLE_QML=OFF', - '-DDISABLE_KTX_CACHE=OFF' + '-DDISABLE_KTX_CACHE=OFF', + '-DUSE_BREAKPAD=' + (project.hasProperty("BACKTRACE_URL") && project.hasProperty("BACKTRACE_TOKEN") ? 'ON' : 'OFF'); } } signingConfigs { @@ -43,6 +44,10 @@ android { } buildTypes { + debug { + buildConfigField "String", "BACKTRACE_URL", "\"" + (project.hasProperty("BACKTRACE_URL") ? BACKTRACE_URL : '') + "\"" + buildConfigField "String", "BACKTRACE_TOKEN", "\"" + (project.hasProperty("BACKTRACE_TOKEN") ? BACKTRACE_TOKEN : '') + "\"" + } release { minifyEnabled false proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' @@ -50,6 +55,8 @@ android { project.hasProperty("HIFI_ANDROID_KEYSTORE_PASSWORD") && project.hasProperty("HIFI_ANDROID_KEY_ALIAS") && project.hasProperty("HIFI_ANDROID_KEY_PASSWORD")? signingConfigs.release : null + buildConfigField "String", "BACKTRACE_URL", "\"" + (project.hasProperty("BACKTRACE_URL") ? BACKTRACE_URL : '') + "\"" + buildConfigField "String", "BACKTRACE_TOKEN", "\"" + (project.hasProperty("BACKTRACE_TOKEN") ? BACKTRACE_TOKEN : '') + "\"" } } diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java b/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java index 45060d6d0c..0d2d39db7d 100644 --- a/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java @@ -8,22 +8,43 @@ import android.app.Activity; import android.content.DialogInterface; import android.app.AlertDialog; +import android.util.Log; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import org.json.JSONException; import org.json.JSONObject; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; import java.io.BufferedWriter; import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.UnsupportedEncodingException; import java.io.Writer; +import java.net.URL; +import java.net.URLEncoder; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; + +import javax.net.ssl.HttpsURLConnection; public class PermissionChecker extends Activity { private static final int REQUEST_PERMISSIONS = 20; private static final boolean CHOOSE_AVATAR_ON_STARTUP = false; + private static final String TAG = "Interface"; + private static final String ANNOTATIONS_JSON = "annotations.json"; @Override protected void onCreate(Bundle savedInstanceState) { @@ -31,13 +52,20 @@ public class PermissionChecker extends Activity { if (CHOOSE_AVATAR_ON_STARTUP) { showMenu(); } - this.requestAppPermissions(new - String[]{ - Manifest.permission.READ_EXTERNAL_STORAGE, - Manifest.permission.WRITE_EXTERNAL_STORAGE, - Manifest.permission.RECORD_AUDIO, - Manifest.permission.CAMERA} - ,2,REQUEST_PERMISSIONS); + + Thread networkThread = new Thread(new Runnable() { + public void run() { + UploadCrashReports(); + runOnUiThread(() -> requestAppPermissions(new + String[]{ + Manifest.permission.READ_EXTERNAL_STORAGE, + Manifest.permission.WRITE_EXTERNAL_STORAGE, + Manifest.permission.RECORD_AUDIO, + Manifest.permission.CAMERA} + ,2,REQUEST_PERMISSIONS)); + } + }); + networkThread.start(); } @@ -125,5 +153,88 @@ public class PermissionChecker extends Activity { } } + public void UploadCrashReports() + { + try + { + String parameters = getAnnotationsAsUrlEncodedParameters(); + URL url = new URL(BuildConfig.BACKTRACE_URL+ "/post?format=minidump&token=" + BuildConfig.BACKTRACE_TOKEN + (parameters.isEmpty() ? "" : ("&" + parameters))); + // Check if a crash .dmp exists + File[] matchingFiles = getFilesByExtension(getObbDir(), "dmp"); + for (File file : matchingFiles) + { + int size = (int) file.length(); + byte[] bytes = new byte[size]; + BufferedInputStream buf = new BufferedInputStream(new FileInputStream(file)); + buf.read(bytes, 0, bytes.length); + buf.close(); + HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection(); + urlConnection.setRequestMethod("POST"); + urlConnection.setDoOutput(true); + urlConnection.setChunkedStreamingMode(0); + OutputStream ostream = urlConnection.getOutputStream(); + + OutputStream out = new BufferedOutputStream(ostream); + out.write(bytes, 0, size); + + InputStream in = new BufferedInputStream(urlConnection.getInputStream()); + in.read(); + if (urlConnection.getResponseCode() == 200) { + file.delete(); + } + urlConnection.disconnect(); + } + } + catch (Exception e) + { + Log.e(TAG, "Error uploading breakpad dumps", e); + } + } + + private File[] getFilesByExtension(File dir, final String extension) + { + return dir.listFiles(pathName -> getExtension(pathName.getName()).equals(extension)); + } + + private String getExtension(String fileName) + { + String extension = ""; + + int i = fileName.lastIndexOf('.'); + int p = Math.max(fileName.lastIndexOf('/'), fileName.lastIndexOf('\\')); + + if (i > p) + { + extension = fileName.substring(i+1); + } + + return extension; + } + + + public String getAnnotationsAsUrlEncodedParameters() { + String parameters = ""; + File annotationsFile = new File(getObbDir(), ANNOTATIONS_JSON); + if (annotationsFile.exists()) { + JsonParser parser = new JsonParser(); + try { + JsonObject json = (JsonObject) parser.parse(new FileReader(annotationsFile)); + for (String k: json.keySet()) { + if (!json.get(k).getAsString().isEmpty()) { + String key = k.contains("/") ? k.substring(k.indexOf("/") + 1) : k; + if (!parameters.isEmpty()) { + parameters += "&"; + } + parameters += URLEncoder.encode(key, "UTF-8") + "=" + URLEncoder.encode(json.get(k).getAsString(), "UTF-8"); + } + } + } catch (FileNotFoundException e) { + Log.e(TAG, "Error reading annotations file", e); + } catch (UnsupportedEncodingException e) { + Log.e(TAG, "Error reading annotations file", e); + } + } + return parameters; + } } diff --git a/cmake/macros/TargetBreakpad.cmake b/cmake/macros/TargetBreakpad.cmake new file mode 100644 index 0000000000..dac581d6c7 --- /dev/null +++ b/cmake/macros/TargetBreakpad.cmake @@ -0,0 +1,22 @@ +# +# Copyright 2018 High Fidelity, Inc. +# Created by Gabriel Calero & Cristian Duarte on 2018/03/13 +# +# Distributed under the Apache License, Version 2.0. +# See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +# +macro(TARGET_BREAKPAD) + if (ANDROID) + set(INSTALL_DIR ${HIFI_ANDROID_PRECOMPILED}/breakpad) + set(BREAKPAD_INCLUDE_DIRS "${INSTALL_DIR}/include" CACHE TYPE INTERNAL) + set(LIB_DIR ${INSTALL_DIR}/lib) + list(APPEND BREAKPAD_LIBRARIES ${LIB_DIR}/libbreakpad_client.a) + target_include_directories(${TARGET_NAME} SYSTEM PRIVATE ${BREAKPAD_INCLUDE_DIRS}) + if (USE_BREAKPAD) + add_definitions(-DHAS_BREAKPAD) + endif() + endif() + target_link_libraries(${TARGET_NAME} ${BREAKPAD_LIBRARIES}) +endmacro() + + diff --git a/interface/CMakeLists.txt b/interface/CMakeLists.txt index ac9441319b..ee77369548 100644 --- a/interface/CMakeLists.txt +++ b/interface/CMakeLists.txt @@ -232,6 +232,7 @@ target_openssl() target_bullet() target_opengl() add_crashpad() +target_breakpad() # perform standard include and linking for found externals foreach(EXTERNAL ${OPTIONAL_EXTERNALS}) diff --git a/interface/src/Breakpad.cpp b/interface/src/Breakpad.cpp new file mode 100644 index 0000000000..3d27eee29c --- /dev/null +++ b/interface/src/Breakpad.cpp @@ -0,0 +1,72 @@ +// +// Breakpad.cpp +// interface/src +// +// Created by Gabriel Calero & Cristian Duarte on 06/06/18 +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "Crashpad.h" + +#if defined(HAS_BREAKPAD) +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +google_breakpad::ExceptionHandler *gBreakpadHandler; + +std::mutex annotationMutex; +QMap annotations; + +static bool breakpad_dumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, void* context, bool succeeded) { + return succeeded; +} + +QString obbDir() { + QAndroidJniObject mediaDir = QAndroidJniObject::callStaticObjectMethod("android/os/Environment", "getExternalStorageDirectory", "()Ljava/io/File;"); + QAndroidJniObject mediaPath = mediaDir.callObjectMethod( "getAbsolutePath", "()Ljava/lang/String;" ); + QAndroidJniObject activity = QAndroidJniObject::callStaticObjectMethod("org/qtproject/qt5/android/QtNative", "activity", "()Landroid/app/Activity;"); + QAndroidJniObject package = activity.callObjectMethod("getPackageName", "()Ljava/lang/String;"); + QString dataAbsPath = mediaPath.toString()+"/Android/obb/" + package.toString(); + return dataAbsPath; +} + +bool startCrashHandler() { + + gBreakpadHandler = new google_breakpad::ExceptionHandler( + google_breakpad::MinidumpDescriptor(obbDir().toStdString()), + nullptr, breakpad_dumpCallback, nullptr, true, -1); + return true; +} + +void setCrashAnnotation(std::string name, std::string value) { + std::lock_guard guard(annotationMutex); + QString qName = QString::fromStdString(name); + QString qValue = QString::fromStdString(value); + if(!annotations.contains(qName)) { + annotations.insert(qName, qValue); + } else { + annotations[qName] = qValue; + } + + QSettings settings(obbDir() + "/annotations.json", JSON_FORMAT); + settings.clear(); + settings.beginGroup("Annotations"); + for(auto k : annotations.keys()) { + settings.setValue(k, annotations.value(k)); + } + settings.endGroup(); + settings.sync(); +} + +#endif diff --git a/interface/src/Breakpad.h b/interface/src/Breakpad.h new file mode 100644 index 0000000000..3d4c46025b --- /dev/null +++ b/interface/src/Breakpad.h @@ -0,0 +1,20 @@ +// +// Breakpad.h +// interface/src +// +// Created by Gabriel Calero & Cristian Duarte on 06/06/18 +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_Breakpad_h +#define hifi_Breakpad_h + +#include + +bool startCrashHandler(); +void setCrashAnnotation(std::string name, std::string value); + +#endif // hifi_Crashpad_h diff --git a/interface/src/Crashpad.cpp b/interface/src/Crashpad.cpp index 45f1d0778f..6f29d9dd23 100644 --- a/interface/src/Crashpad.cpp +++ b/interface/src/Crashpad.cpp @@ -114,7 +114,7 @@ void setCrashAnnotation(std::string name, std::string value) { crashpadAnnotations->SetKeyValue(name, value); } -#else +#elif !defined(HAS_BREAKPAD) bool startCrashHandler() { qDebug() << "No crash handler available."; From cfc98afaf0bd56de5e50a5ff8628cff8967830c9 Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 8 Jun 2018 12:11:01 -0300 Subject: [PATCH 04/21] Add breakpad as external dependency --- android/build.gradle | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/android/build.gradle b/android/build.gradle index f09dc97850..84343e195b 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -149,6 +149,12 @@ def packages = [ file: 'etc2comp-patched-armv8-libcpp.tgz', versionId: 'bHhGECRAQR1vkpshBcK6ByNc1BQIM8gU', checksum: '14b02795d774457a33bbc60e00a786bc' + ], + breakpad: [ + file: 'breakpad.zip', + sharedLibFolder: 'breakpad/lib', + versionId: '8r5Dsw9pZqKKZquiM1zTWc7Rd8uOSQ1W', + checksum: 'f96b02a6733871e6df2af40d5480afc1' ] ] @@ -367,6 +373,7 @@ task verifyPolyvox(type: Verify) { def p = packages['polyvox']; src new File(bas task verifyTBB(type: Verify) { def p = packages['tbb']; src new File(baseFolder, p['file']); checksum p['checksum'] } task verifyHifiAC(type: Verify) { def p = packages['hifiAC']; src new File(baseFolder, p['file']); checksum p['checksum'] } task verifyEtc2Comp(type: Verify) { def p = packages['etc2comp']; src new File(baseFolder, p['file']); checksum p['checksum'] } +task verifyBreakpad(type: Verify) { def p = packages['breakpad']; src new File(baseFolder, p['file']); checksum p['checksum'] } task verifyDependencyDownloads(dependsOn: downloadDependencies) { } verifyDependencyDownloads.dependsOn verifyQt @@ -378,6 +385,7 @@ verifyDependencyDownloads.dependsOn verifyPolyvox verifyDependencyDownloads.dependsOn verifyTBB verifyDependencyDownloads.dependsOn verifyHifiAC verifyDependencyDownloads.dependsOn verifyEtc2Comp +verifyDependencyDownloads.dependsOn verifyBreakpad task extractDependencies(dependsOn: verifyDependencyDownloads) { doLast { From 1a039ca504c58e9597e290b1115d4658c207af9e Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 8 Jun 2018 12:38:24 -0300 Subject: [PATCH 05/21] Trying to fix build script --- android/build.gradle | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/android/build.gradle b/android/build.gradle index 84343e195b..7e5a0dc56e 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -152,9 +152,10 @@ def packages = [ ], breakpad: [ file: 'breakpad.zip', - sharedLibFolder: 'breakpad/lib', versionId: '8r5Dsw9pZqKKZquiM1zTWc7Rd8uOSQ1W', - checksum: 'f96b02a6733871e6df2af40d5480afc1' + checksum: 'f96b02a6733871e6df2af40d5480afc1', + sharedLibFolder: 'breakpad/lib', + includeLibs: ['libbreakpad_client.a'] ] ] From 1ec03475dd225284eb5db8e6b1641455b07b0ca7 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 8 Jun 2018 18:31:10 -0300 Subject: [PATCH 06/21] Trying to fix android build Trying a new breakpad.zip version --- android/build.gradle | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/android/build.gradle b/android/build.gradle index 7e5a0dc56e..1d451814fd 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -152,10 +152,10 @@ def packages = [ ], breakpad: [ file: 'breakpad.zip', - versionId: '8r5Dsw9pZqKKZquiM1zTWc7Rd8uOSQ1W', - checksum: 'f96b02a6733871e6df2af40d5480afc1', - sharedLibFolder: 'breakpad/lib', - includeLibs: ['libbreakpad_client.a'] + versionId: '2OwvCCZrF171wnte5T44AnjTYFhhJsGJ', + checksum: 'a46062a3167dfedd4fb4916136e204d2', + sharedLibFolder: 'lib', + includeLibs: ['libbreakpad_client.a','libbreakpad.a'] ] ] @@ -624,4 +624,4 @@ task testElf (dependsOn: 'externalNativeBuildDebug') { } } } -*/ \ No newline at end of file +*/ From 26a9364efd283ff3778c40f3bb62ec1b2f93aebb Mon Sep 17 00:00:00 2001 From: Cristian Luis Duarte Date: Fri, 8 Jun 2018 20:59:38 -0300 Subject: [PATCH 07/21] Android - Clean up enterBackground logs --- .../java/io/highfidelity/hifiinterface/InterfaceActivity.java | 2 -- interface/src/Application.cpp | 2 -- 2 files changed, 4 deletions(-) diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java b/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java index 20c216d5a9..84fd2678d8 100644 --- a/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/InterfaceActivity.java @@ -126,7 +126,6 @@ public class InterfaceActivity extends QtActivity { if (super.isLoading) { nativeEnterBackgroundCallEnqueued = true; } else { - Log.d("[ENTERBACKGROUND]","onPause calling nativeEnterBackground"); nativeEnterBackground(); } //gvrApi.pauseTracking(); @@ -265,7 +264,6 @@ public class InterfaceActivity extends QtActivity { public void onAppLoadedComplete() { super.isLoading = false; if (nativeEnterBackgroundCallEnqueued) { - Log.d("[ENTERBACKGROUND]","onAppLoadedComplete calling nativeEnterBackground"); nativeEnterBackground(); } } diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 6111406291..cbe713127d 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -8248,12 +8248,10 @@ void Application::saveNextPhysicsStats(QString filename) { #if defined(Q_OS_ANDROID) void Application::enterBackground() { - qDebug() << "[ENTERBACKGROUND] Application::enterBackground()"; QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); if (getActiveDisplayPlugin()->isActive()) { getActiveDisplayPlugin()->deactivate(); - qDebug() << "[ENTERBACKGROUND] Application::enterBackground() getActiveDisplayPlugin()->deactivated"; } } From d4d8f36e442230b5bada305e6bb35fcd9a860431 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Mon, 11 Jun 2018 15:40:39 -0700 Subject: [PATCH 08/21] Change CharacterGhostObject to use base btGhostObject class instead of btPairCachingGhostObject --- libraries/physics/src/CharacterGhostObject.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/physics/src/CharacterGhostObject.h b/libraries/physics/src/CharacterGhostObject.h index 44ab5c938a..a6efea8752 100755 --- a/libraries/physics/src/CharacterGhostObject.h +++ b/libraries/physics/src/CharacterGhostObject.h @@ -23,7 +23,7 @@ class CharacterGhostShape; -class CharacterGhostObject : public btPairCachingGhostObject { +class CharacterGhostObject : public btGhostObject { public: CharacterGhostObject() { } ~CharacterGhostObject(); From baf0df05cd1e13376cc6fac83a4fe5eb2a0245e1 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Thu, 14 Jun 2018 13:16:46 -0700 Subject: [PATCH 09/21] Remove unnecessary 'this->' in rayTest parent function call --- libraries/physics/src/CharacterGhostObject.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/physics/src/CharacterGhostObject.cpp b/libraries/physics/src/CharacterGhostObject.cpp index 0e1ea22375..78b1d9c08f 100755 --- a/libraries/physics/src/CharacterGhostObject.cpp +++ b/libraries/physics/src/CharacterGhostObject.cpp @@ -69,7 +69,7 @@ bool CharacterGhostObject::rayTest(const btVector3& start, const btVector3& end, CharacterRayResult& result) const { if (_world && _inWorld) { - this->btGhostObject::rayTest(start, end, result); + btGhostObject::rayTest(start, end, result); } return result.hasHit(); } From c02750edd10299c04fc2afd69862781dd8dcfce8 Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Thu, 14 Jun 2018 19:43:03 -0300 Subject: [PATCH 10/21] Upload breakpad minidumps from an android service --- android/app/src/main/AndroidManifest.xml | 6 + .../BreakpadUploaderService.java | 169 ++++++++++++++++++ .../hifiinterface/PermissionChecker.java | 138 ++------------ interface/src/Breakpad.cpp | 1 - 4 files changed, 193 insertions(+), 121 deletions(-) create mode 100644 android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 0b52046057..e763d471cb 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -67,6 +67,12 @@ android:name=".SplashActivity" android:screenOrientation="portrait" android:theme="@style/Theme.AppCompat.Translucent.NoActionBar" /> + + diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java b/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java new file mode 100644 index 0000000000..c8b337fb7e --- /dev/null +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java @@ -0,0 +1,169 @@ +package io.highfidelity.hifiinterface; + +import android.app.Service; +import android.content.Intent; +import android.os.FileObserver; +import android.os.IBinder; +import android.support.annotation.Nullable; +import android.util.Log; + +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.UnsupportedEncodingException; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLEncoder; + +import javax.net.ssl.HttpsURLConnection; + +public class BreakpadUploaderService extends Service { + + private static final String ANNOTATIONS_JSON = "annotations.json"; + private static final String TAG = "Interface"; + public static final String EXT_DMP = "dmp"; + + private FileObserver fileObserver; + + public BreakpadUploaderService() { + super(); + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + URL baseUrl; + + baseUrl = getUrl(); + + if (baseUrl == null) { + stopSelf(); + return START_NOT_STICKY; + } + + new Thread(() -> { + File[] matchingFiles = getFilesByExtension(getObbDir(), EXT_DMP); + for (File file : matchingFiles) + { + uploadDumpAndDelete(file, baseUrl); + } + }).start(); + + fileObserver = new FileObserver(getObbDir().getPath()) { + @Override + public void onEvent(int event, String path) { + if (path == null) { + return; + } + if (FileObserver.CREATE == event && EXT_DMP.equals(getExtension(path))) { + URL baseUrl = getUrl(); + if (baseUrl != null) { + new Thread(() -> uploadDumpAndDelete(new File(path), baseUrl)).start(); + } + } + } + }; + + fileObserver.startWatching(); + return START_STICKY; + } + + private URL getUrl() { + String parameters = getAnnotationsAsUrlEncodedParameters(); + try { + return new URL(BuildConfig.BACKTRACE_URL+ "/post?format=minidump&token=" + BuildConfig.BACKTRACE_TOKEN + (parameters.isEmpty() ? "" : ("&" + parameters))); + } catch (MalformedURLException e) { + Log.e(TAG, "Could not initialize Breakpad URL", e); + } + return null; + } + + private void uploadDumpAndDelete(File file, URL url) { + int size = (int) file.length(); + byte[] bytes = new byte[size]; + try { + BufferedInputStream buf = new BufferedInputStream(new FileInputStream(file)); + buf.read(bytes, 0, bytes.length); + buf.close(); + HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection(); + urlConnection.setRequestMethod("POST"); + urlConnection.setDoOutput(true); + urlConnection.setChunkedStreamingMode(0); + + OutputStream ostream = urlConnection.getOutputStream(); + + OutputStream out = new BufferedOutputStream(ostream); + out.write(bytes, 0, size); + + InputStream in = new BufferedInputStream(urlConnection.getInputStream()); + in.read(); + if (urlConnection.getResponseCode() == 200) { + file.delete(); + } + urlConnection.disconnect(); + } catch (IOException e) { + Log.e(TAG, "Error uploading file " + file.getAbsolutePath(), e); + } + } + + @Nullable + @Override + public IBinder onBind(Intent intent) { + return null; + } + + private File[] getFilesByExtension(File dir, final String extension) + { + return dir.listFiles(pathName -> getExtension(pathName.getName()).equals(extension)); + } + + private String getExtension(String fileName) + { + String extension = ""; + + int i = fileName.lastIndexOf('.'); + int p = Math.max(fileName.lastIndexOf('/'), fileName.lastIndexOf('\\')); + + if (i > p) + { + extension = fileName.substring(i+1); + } + + return extension; + } + + + public String getAnnotationsAsUrlEncodedParameters() { + String parameters = ""; + File annotationsFile = new File(getObbDir(), ANNOTATIONS_JSON); + if (annotationsFile.exists()) { + JsonParser parser = new JsonParser(); + try { + JsonObject json = (JsonObject) parser.parse(new FileReader(annotationsFile)); + for (String k: json.keySet()) { + if (!json.get(k).getAsString().isEmpty()) { + String key = k.contains("/") ? k.substring(k.indexOf("/") + 1) : k; + if (!parameters.isEmpty()) { + parameters += "&"; + } + parameters += URLEncoder.encode(key, "UTF-8") + "=" + URLEncoder.encode(json.get(k).getAsString(), "UTF-8"); + } + } + } catch (FileNotFoundException e) { + Log.e(TAG, "Error reading annotations file", e); + } catch (UnsupportedEncodingException e) { + Log.e(TAG, "Error reading annotations file", e); + } + } + return parameters; + } + +} diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java b/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java index 0d2d39db7d..10cfd85b50 100644 --- a/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java @@ -1,71 +1,54 @@ package io.highfidelity.hifiinterface; import android.Manifest; +import android.app.Activity; +import android.app.AlertDialog; +import android.content.DialogInterface; import android.content.Intent; import android.content.pm.PackageManager; import android.os.Bundle; -import android.app.Activity; - -import android.content.DialogInterface; -import android.app.AlertDialog; import android.util.Log; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; -import com.google.gson.JsonParser; - import org.json.JSONException; import org.json.JSONObject; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; import java.io.BufferedWriter; import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.UnsupportedEncodingException; import java.io.Writer; -import java.net.URL; -import java.net.URLEncoder; import java.util.Arrays; -import java.util.HashMap; import java.util.List; -import java.util.Map; - -import javax.net.ssl.HttpsURLConnection; public class PermissionChecker extends Activity { private static final int REQUEST_PERMISSIONS = 20; private static final boolean CHOOSE_AVATAR_ON_STARTUP = false; private static final String TAG = "Interface"; - private static final String ANNOTATIONS_JSON = "annotations.json"; @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + Intent myIntent = new Intent(this, BreakpadUploaderService.class); + startService(myIntent); if (CHOOSE_AVATAR_ON_STARTUP) { showMenu(); } - Thread networkThread = new Thread(new Runnable() { - public void run() { - UploadCrashReports(); - runOnUiThread(() -> requestAppPermissions(new - String[]{ - Manifest.permission.READ_EXTERNAL_STORAGE, - Manifest.permission.WRITE_EXTERNAL_STORAGE, - Manifest.permission.RECORD_AUDIO, - Manifest.permission.CAMERA} - ,2,REQUEST_PERMISSIONS)); + File obbDir = getObbDir(); + if (!obbDir.exists()) { + if (obbDir.mkdirs()) { + Log.d(TAG, "Obb dir created"); } - }); - networkThread.start(); + } + + requestAppPermissions(new + String[]{ + Manifest.permission.READ_EXTERNAL_STORAGE, + Manifest.permission.WRITE_EXTERNAL_STORAGE, + Manifest.permission.RECORD_AUDIO, + Manifest.permission.CAMERA} + ,2,REQUEST_PERMISSIONS); } @@ -152,89 +135,4 @@ public class PermissionChecker extends Activity { launchActivityWithPermissions(); } } - - public void UploadCrashReports() - { - try - { - String parameters = getAnnotationsAsUrlEncodedParameters(); - URL url = new URL(BuildConfig.BACKTRACE_URL+ "/post?format=minidump&token=" + BuildConfig.BACKTRACE_TOKEN + (parameters.isEmpty() ? "" : ("&" + parameters))); - // Check if a crash .dmp exists - File[] matchingFiles = getFilesByExtension(getObbDir(), "dmp"); - for (File file : matchingFiles) - { - int size = (int) file.length(); - byte[] bytes = new byte[size]; - BufferedInputStream buf = new BufferedInputStream(new FileInputStream(file)); - buf.read(bytes, 0, bytes.length); - buf.close(); - HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection(); - urlConnection.setRequestMethod("POST"); - urlConnection.setDoOutput(true); - urlConnection.setChunkedStreamingMode(0); - - OutputStream ostream = urlConnection.getOutputStream(); - - OutputStream out = new BufferedOutputStream(ostream); - out.write(bytes, 0, size); - - InputStream in = new BufferedInputStream(urlConnection.getInputStream()); - in.read(); - if (urlConnection.getResponseCode() == 200) { - file.delete(); - } - urlConnection.disconnect(); - } - } - catch (Exception e) - { - Log.e(TAG, "Error uploading breakpad dumps", e); - } - } - - private File[] getFilesByExtension(File dir, final String extension) - { - return dir.listFiles(pathName -> getExtension(pathName.getName()).equals(extension)); - } - - private String getExtension(String fileName) - { - String extension = ""; - - int i = fileName.lastIndexOf('.'); - int p = Math.max(fileName.lastIndexOf('/'), fileName.lastIndexOf('\\')); - - if (i > p) - { - extension = fileName.substring(i+1); - } - - return extension; - } - - - public String getAnnotationsAsUrlEncodedParameters() { - String parameters = ""; - File annotationsFile = new File(getObbDir(), ANNOTATIONS_JSON); - if (annotationsFile.exists()) { - JsonParser parser = new JsonParser(); - try { - JsonObject json = (JsonObject) parser.parse(new FileReader(annotationsFile)); - for (String k: json.keySet()) { - if (!json.get(k).getAsString().isEmpty()) { - String key = k.contains("/") ? k.substring(k.indexOf("/") + 1) : k; - if (!parameters.isEmpty()) { - parameters += "&"; - } - parameters += URLEncoder.encode(key, "UTF-8") + "=" + URLEncoder.encode(json.get(k).getAsString(), "UTF-8"); - } - } - } catch (FileNotFoundException e) { - Log.e(TAG, "Error reading annotations file", e); - } catch (UnsupportedEncodingException e) { - Log.e(TAG, "Error reading annotations file", e); - } - } - return parameters; - } } diff --git a/interface/src/Breakpad.cpp b/interface/src/Breakpad.cpp index 3d27eee29c..cb4785305a 100644 --- a/interface/src/Breakpad.cpp +++ b/interface/src/Breakpad.cpp @@ -14,7 +14,6 @@ #if defined(HAS_BREAKPAD) #include -#include #include #include #include From 2a20ffcca4eb0cbf992bf9bf4604f1b3d7ba39d4 Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 15 Jun 2018 17:42:42 -0300 Subject: [PATCH 11/21] Add delay before uploading minidump --- .../hifiinterface/BreakpadUploaderService.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java b/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java index c8b337fb7e..a90a12b86b 100644 --- a/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java @@ -23,6 +23,8 @@ import java.io.UnsupportedEncodingException; import java.net.MalformedURLException; import java.net.URL; import java.net.URLEncoder; +import java.util.Timer; +import java.util.TimerTask; import javax.net.ssl.HttpsURLConnection; @@ -31,6 +33,7 @@ public class BreakpadUploaderService extends Service { private static final String ANNOTATIONS_JSON = "annotations.json"; private static final String TAG = "Interface"; public static final String EXT_DMP = "dmp"; + private static final long DUMP_DELAY = 5000; private FileObserver fileObserver; @@ -66,7 +69,12 @@ public class BreakpadUploaderService extends Service { if (FileObserver.CREATE == event && EXT_DMP.equals(getExtension(path))) { URL baseUrl = getUrl(); if (baseUrl != null) { - new Thread(() -> uploadDumpAndDelete(new File(path), baseUrl)).start(); + new Timer().schedule(new TimerTask() { + @Override + public void run() { + uploadDumpAndDelete(new File(getObbDir(), path), baseUrl); + } + }, DUMP_DELAY); } } } From 4cd94a32b874759e9dc0bf542b1109d3cf6a5343 Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 15 Jun 2018 17:44:28 -0300 Subject: [PATCH 12/21] Crash on audio button press with testing purpose --- scripts/system/+android/audio.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/system/+android/audio.js b/scripts/system/+android/audio.js index 34dd52604a..50919ed5d1 100644 --- a/scripts/system/+android/audio.js +++ b/scripts/system/+android/audio.js @@ -46,6 +46,7 @@ function init() { function onMuteClicked() { Audio.muted = !Audio.muted; + Menu.triggerOption("Out of Bounds Vector Access"); } function onMutePressed() { From 20727378e21da124d85bdcd99309b8fd2f30be3a Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 15 Jun 2018 20:32:10 -0300 Subject: [PATCH 13/21] Rename crash handler files --- interface/src/Application.cpp | 4 +-- interface/src/Breakpad.h | 20 ----------- interface/src/CrashHandler.h | 31 +++++++++-------- ...Breakpad.cpp => CrashHandler_Breakpad.cpp} | 27 ++++++++------- ...Crashpad.cpp => CrashHandler_Crashpad.cpp} | 12 +------ interface/src/CrashHandler_None.cpp | 27 +++++++++++++++ ...shHandler.cpp => CrashRecoveryHandler.cpp} | 24 +++++++------- interface/src/CrashRecoveryHandler.h | 33 +++++++++++++++++++ interface/src/Crashpad.h | 20 ----------- interface/src/DiscoverabilityManager.cpp | 2 +- 10 files changed, 104 insertions(+), 96 deletions(-) delete mode 100644 interface/src/Breakpad.h rename interface/src/{Breakpad.cpp => CrashHandler_Breakpad.cpp} (86%) rename interface/src/{Crashpad.cpp => CrashHandler_Crashpad.cpp} (94%) create mode 100644 interface/src/CrashHandler_None.cpp rename interface/src/{CrashHandler.cpp => CrashRecoveryHandler.cpp} (85%) create mode 100644 interface/src/CrashRecoveryHandler.h delete mode 100644 interface/src/Crashpad.h diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 62b2486555..5efce2ea00 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -150,8 +150,8 @@ #include "audio/AudioScope.h" #include "avatar/AvatarManager.h" #include "avatar/MyHead.h" +#include "CrashRecoveryHandler.h" #include "CrashHandler.h" -#include "Crashpad.h" #include "devices/DdeFaceTracker.h" #include "DiscoverabilityManager.h" #include "GLCanvas.h" @@ -792,7 +792,7 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) { bool previousSessionCrashed { false }; if (!inTestMode) { - previousSessionCrashed = CrashHandler::checkForResetSettings(runningMarkerExisted, suppressPrompt); + previousSessionCrashed = CrashRecoveryHandler::checkForResetSettings(runningMarkerExisted, suppressPrompt); } // get dir to use for cache diff --git a/interface/src/Breakpad.h b/interface/src/Breakpad.h deleted file mode 100644 index 3d4c46025b..0000000000 --- a/interface/src/Breakpad.h +++ /dev/null @@ -1,20 +0,0 @@ -// -// Breakpad.h -// interface/src -// -// Created by Gabriel Calero & Cristian Duarte on 06/06/18 -// Copyright 2018 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_Breakpad_h -#define hifi_Breakpad_h - -#include - -bool startCrashHandler(); -void setCrashAnnotation(std::string name, std::string value); - -#endif // hifi_Crashpad_h diff --git a/interface/src/CrashHandler.h b/interface/src/CrashHandler.h index bff8bba6dd..d96ecf0312 100644 --- a/interface/src/CrashHandler.h +++ b/interface/src/CrashHandler.h @@ -2,8 +2,8 @@ // CrashHandler.h // interface/src // -// Created by David Rowe on 24 Aug 2015. -// Copyright 2015 High Fidelity, Inc. +// Created by Clement Brisset on 01/19/18. +// Copyright 2018 High Fidelity, Inc. // // Distributed under the Apache License, Version 2.0. // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html @@ -12,22 +12,21 @@ #ifndef hifi_CrashHandler_h #define hifi_CrashHandler_h -#include +#include -class CrashHandler { +#if HAS_CRASHPAD -public: - static bool checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt = false); +bool startCrashHandler(); +void setCrashAnnotation(std::string name, std::string value); -private: - enum Action { - DELETE_INTERFACE_INI, - RETAIN_IMPORTANT_INFO, - DO_NOTHING - }; +#elif HAS_BREAKPAD - static Action promptUserForAction(bool showCrashMessage); - static void handleCrash(Action action); -}; +bool startCrashHandler(); +void setCrashAnnotation(std::string name, std::string value); -#endif // hifi_CrashHandler_h +#else + +bool startCrashHandler(); +void setCrashAnnotation(std::string name, std::string value); + +#endif // hifi_Crashpad_h diff --git a/interface/src/Breakpad.cpp b/interface/src/CrashHandler_Breakpad.cpp similarity index 86% rename from interface/src/Breakpad.cpp rename to interface/src/CrashHandler_Breakpad.cpp index cb4785305a..14b5c7c5f2 100644 --- a/interface/src/Breakpad.cpp +++ b/interface/src/CrashHandler_Breakpad.cpp @@ -1,28 +1,31 @@ // -// Breakpad.cpp +// Crashpad.cpp // interface/src // -// Created by Gabriel Calero & Cristian Duarte on 06/06/18 +// Created by Clement Brisset on 01/19/18. // Copyright 2018 High Fidelity, Inc. // // Distributed under the Apache License, Version 2.0. // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include "Crashpad.h" +#include "CrashHandler.h" -#if defined(HAS_BREAKPAD) -#include +#if HAS_BREAKPAD + +#include #include #include + +#include +#include #include #include -#include -#include -#include -google_breakpad::ExceptionHandler *gBreakpadHandler; +#include + +google_breakpad::ExceptionHandler* gBreakpadHandler; std::mutex annotationMutex; QMap annotations; @@ -52,11 +55,7 @@ void setCrashAnnotation(std::string name, std::string value) { std::lock_guard guard(annotationMutex); QString qName = QString::fromStdString(name); QString qValue = QString::fromStdString(value); - if(!annotations.contains(qName)) { - annotations.insert(qName, qValue); - } else { - annotations[qName] = qValue; - } + annotations[qName] = qValue; QSettings settings(obbDir() + "/annotations.json", JSON_FORMAT); settings.clear(); diff --git a/interface/src/Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp similarity index 94% rename from interface/src/Crashpad.cpp rename to interface/src/CrashHandler_Crashpad.cpp index 6f29d9dd23..96c7d5d538 100644 --- a/interface/src/Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -9,7 +9,7 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include "Crashpad.h" +#include "CrashHandler.h" #include @@ -114,14 +114,4 @@ void setCrashAnnotation(std::string name, std::string value) { crashpadAnnotations->SetKeyValue(name, value); } -#elif !defined(HAS_BREAKPAD) - -bool startCrashHandler() { - qDebug() << "No crash handler available."; - return false; -} - -void setCrashAnnotation(std::string name, std::string value) { -} - #endif diff --git a/interface/src/CrashHandler_None.cpp b/interface/src/CrashHandler_None.cpp new file mode 100644 index 0000000000..0d92dc2005 --- /dev/null +++ b/interface/src/CrashHandler_None.cpp @@ -0,0 +1,27 @@ +// +// Crashpad.cpp +// interface/src +// +// Created by Clement Brisset on 01/19/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "CrashHandler.h" + +#include +#include + +#if !defined(HAS_CRASHPAD) && !defined(HAS_BREAKPAD) + +bool startCrashHandler() { + qDebug() << "No crash handler available."; + return false; +} + +void setCrashAnnotation(std::string name, std::string value) { +} + +#endif diff --git a/interface/src/CrashHandler.cpp b/interface/src/CrashRecoveryHandler.cpp similarity index 85% rename from interface/src/CrashHandler.cpp rename to interface/src/CrashRecoveryHandler.cpp index d3079b9bf4..4bf4d04180 100644 --- a/interface/src/CrashHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -1,5 +1,5 @@ // -// CrashHandler.cpp +// CrashRecoveryHandler.cpp // interface/src // // Created by David Rowe on 24 Aug 2015. @@ -9,7 +9,7 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include "CrashHandler.h" +#include "CrashRecoveryHandler.h" #include #include @@ -30,7 +30,7 @@ #include -bool CrashHandler::checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt) { +bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt) { QSettings::setDefaultFormat(JSON_FORMAT); QSettings settings; settings.beginGroup("Developer"); @@ -59,7 +59,7 @@ bool CrashHandler::checkForResetSettings(bool wasLikelyCrash, bool suppressPromp return wasLikelyCrash; } -CrashHandler::Action CrashHandler::promptUserForAction(bool showCrashMessage) { +CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool showCrashMessage) { QDialog crashDialog; QLabel* label; if (showCrashMessage) { @@ -94,20 +94,20 @@ CrashHandler::Action CrashHandler::promptUserForAction(bool showCrashMessage) { if (result == QDialog::Accepted) { if (option1->isChecked()) { - return CrashHandler::DELETE_INTERFACE_INI; + return CrashRecoveryHandler::DELETE_INTERFACE_INI; } if (option2->isChecked()) { - return CrashHandler::RETAIN_IMPORTANT_INFO; + return CrashRecoveryHandler::RETAIN_IMPORTANT_INFO; } } // Dialog cancelled or "do nothing" option chosen - return CrashHandler::DO_NOTHING; + return CrashRecoveryHandler::DO_NOTHING; } -void CrashHandler::handleCrash(CrashHandler::Action action) { - if (action != CrashHandler::DELETE_INTERFACE_INI && action != CrashHandler::RETAIN_IMPORTANT_INFO) { - // CrashHandler::DO_NOTHING or unexpected value +void CrashRecoveryHandler::handleCrash(CrashRecoveryHandler::Action action) { + if (action != CrashRecoveryHandler::DELETE_INTERFACE_INI && action != CrashRecoveryHandler::RETAIN_IMPORTANT_INFO) { + // CrashRecoveryHandler::DO_NOTHING or unexpected value return; } @@ -126,7 +126,7 @@ void CrashHandler::handleCrash(CrashHandler::Action action) { QUrl address; bool tutorialComplete = false; - if (action == CrashHandler::RETAIN_IMPORTANT_INFO) { + if (action == CrashRecoveryHandler::RETAIN_IMPORTANT_INFO) { // Read avatar info // Location and orientation @@ -151,7 +151,7 @@ void CrashHandler::handleCrash(CrashHandler::Action action) { settingsFile.remove(); } - if (action == CrashHandler::RETAIN_IMPORTANT_INFO) { + if (action == CrashRecoveryHandler::RETAIN_IMPORTANT_INFO) { // Write avatar info // Location and orientation diff --git a/interface/src/CrashRecoveryHandler.h b/interface/src/CrashRecoveryHandler.h new file mode 100644 index 0000000000..64aa8ea0e4 --- /dev/null +++ b/interface/src/CrashRecoveryHandler.h @@ -0,0 +1,33 @@ +// +// CrashRecoveryHandler.h +// interface/src +// +// Created by David Rowe on 24 Aug 2015. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_CrashHandler_h +#define hifi_CrashHandler_h + +#include + +class CrashRecoveryHandler { + +public: + static bool checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt = false); + +private: + enum Action { + DELETE_INTERFACE_INI, + RETAIN_IMPORTANT_INFO, + DO_NOTHING + }; + + static Action promptUserForAction(bool showCrashMessage); + static void handleCrash(Action action); +}; + +#endif // hifi_CrashHandler_h diff --git a/interface/src/Crashpad.h b/interface/src/Crashpad.h deleted file mode 100644 index a815ed701a..0000000000 --- a/interface/src/Crashpad.h +++ /dev/null @@ -1,20 +0,0 @@ -// -// Crashpad.h -// interface/src -// -// Created by Clement Brisset on 01/19/18. -// Copyright 2018 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_Crashpad_h -#define hifi_Crashpad_h - -#include - -bool startCrashHandler(); -void setCrashAnnotation(std::string name, std::string value); - -#endif // hifi_Crashpad_h diff --git a/interface/src/DiscoverabilityManager.cpp b/interface/src/DiscoverabilityManager.cpp index 684539145e..d39aaa4f1a 100644 --- a/interface/src/DiscoverabilityManager.cpp +++ b/interface/src/DiscoverabilityManager.cpp @@ -23,7 +23,7 @@ #include #include -#include "Crashpad.h" +#include "CrashHandler.h" #include "Menu.h" const Discoverability::Mode DEFAULT_DISCOVERABILITY_MODE = Discoverability::Connections; From 1cdd8c2f3cc8c4f78b986f504ac9efebf0b440fa Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 15 Jun 2018 20:39:24 -0300 Subject: [PATCH 14/21] Fix compilation error --- interface/src/CrashHandler.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/CrashHandler.h b/interface/src/CrashHandler.h index d96ecf0312..2f51f6f158 100644 --- a/interface/src/CrashHandler.h +++ b/interface/src/CrashHandler.h @@ -30,3 +30,5 @@ bool startCrashHandler(); void setCrashAnnotation(std::string name, std::string value); #endif // hifi_Crashpad_h + +#endif \ No newline at end of file From 97fa22b69ba2a1dd723f369d25bbe5809c4ce429 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Fri, 15 Jun 2018 16:57:21 -0700 Subject: [PATCH 15/21] fix concurrency drillDown --- interface/resources/qml/hifi/Feed.qml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/resources/qml/hifi/Feed.qml b/interface/resources/qml/hifi/Feed.qml index 4cd52a582d..5f2dfea8c7 100644 --- a/interface/resources/qml/hifi/Feed.qml +++ b/interface/resources/qml/hifi/Feed.qml @@ -87,8 +87,8 @@ Column { description: description, online_users: data.details.connections || data.details.concurrency || 0, // Server currently doesn't give isStacked (undefined). Could give bool. - drillDownToPlace: (data.isStacked === undefined) ? (data.action !== 'concurrency') : data.isStacked, - isStacked: !!data.isStacked + drillDownToPlace: data.is_stacked || (data.action === 'concurrency'), + isStacked: !!data.is_stacked }; } From 20c147ef1176bb85a8e980956f0f9324c964df9b Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Fri, 15 Jun 2018 21:12:38 -0300 Subject: [PATCH 16/21] Fix compilation error --- interface/src/CrashHandler.h | 13 ------------- interface/src/CrashHandler_Breakpad.cpp | 2 +- interface/src/CrashHandler_Crashpad.cpp | 2 +- interface/src/CrashHandler_None.cpp | 2 +- interface/src/CrashRecoveryHandler.h | 6 +++--- interface/src/main.cpp | 2 +- 6 files changed, 7 insertions(+), 20 deletions(-) diff --git a/interface/src/CrashHandler.h b/interface/src/CrashHandler.h index 2f51f6f158..4a6483c700 100644 --- a/interface/src/CrashHandler.h +++ b/interface/src/CrashHandler.h @@ -14,21 +14,8 @@ #include -#if HAS_CRASHPAD - bool startCrashHandler(); void setCrashAnnotation(std::string name, std::string value); -#elif HAS_BREAKPAD - -bool startCrashHandler(); -void setCrashAnnotation(std::string name, std::string value); - -#else - -bool startCrashHandler(); -void setCrashAnnotation(std::string name, std::string value); - -#endif // hifi_Crashpad_h #endif \ No newline at end of file diff --git a/interface/src/CrashHandler_Breakpad.cpp b/interface/src/CrashHandler_Breakpad.cpp index 14b5c7c5f2..ffc49c4ec4 100644 --- a/interface/src/CrashHandler_Breakpad.cpp +++ b/interface/src/CrashHandler_Breakpad.cpp @@ -1,5 +1,5 @@ // -// Crashpad.cpp +// CrashHandler_Breakpad.cpp // interface/src // // Created by Clement Brisset on 01/19/18. diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index 96c7d5d538..2363e2a643 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -1,5 +1,5 @@ // -// Crashpad.cpp +// CrashHandler_Crashpad.cpp // interface/src // // Created by Clement Brisset on 01/19/18. diff --git a/interface/src/CrashHandler_None.cpp b/interface/src/CrashHandler_None.cpp index 0d92dc2005..cba585f7b7 100644 --- a/interface/src/CrashHandler_None.cpp +++ b/interface/src/CrashHandler_None.cpp @@ -1,5 +1,5 @@ // -// Crashpad.cpp +// CrashHandler_None.cpp // interface/src // // Created by Clement Brisset on 01/19/18. diff --git a/interface/src/CrashRecoveryHandler.h b/interface/src/CrashRecoveryHandler.h index 64aa8ea0e4..8df72eeb75 100644 --- a/interface/src/CrashRecoveryHandler.h +++ b/interface/src/CrashRecoveryHandler.h @@ -9,8 +9,8 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#ifndef hifi_CrashHandler_h -#define hifi_CrashHandler_h +#ifndef hifi_CrashRecoveryHandler_h +#define hifi_CrashRecoveryHandler_h #include @@ -30,4 +30,4 @@ private: static void handleCrash(Action action); }; -#endif // hifi_CrashHandler_h +#endif // hifi_CrashRecoveryHandler_h diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 22db128f7e..e5a436e75a 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -26,7 +26,7 @@ #include "AddressManager.h" #include "Application.h" -#include "Crashpad.h" +#include "CrashHandler.h" #include "InterfaceLogging.h" #include "UserActivityLogger.h" #include "MainWindow.h" From d8ea47b78f42f17b0212347a26d942eece689654 Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Sun, 17 Jun 2018 14:16:25 +0300 Subject: [PATCH 17/21] Don't call parseSnapshotData() in a static manner. Shouldn't call non-static methods in a static manner. (Some compilers don't even allow this, although apparently Visual Studio 2017 does.) --- interface/src/ui/Snapshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index 2b306ace91..253490915a 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -434,7 +434,7 @@ void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { const QString SNAPSHOT_UPLOAD_URL = "/api/v1/snapshots"; QUrl url = href; if (url.isEmpty()) { - SnapshotMetaData* snapshotData = Snapshot::parseSnapshotData(filename); + SnapshotMetaData* snapshotData = parseSnapshotData(filename); if (snapshotData) { url = snapshotData->getURL(); } From 3eecb2f3796c160f6cf050a5d1589fa9df8c7343 Mon Sep 17 00:00:00 2001 From: Gabriel Calero Date: Mon, 18 Jun 2018 14:40:14 -0300 Subject: [PATCH 18/21] Apply code conventions --- .../BreakpadUploaderService.java | 44 +++++++++---------- interface/src/CrashHandler_Breakpad.cpp | 2 +- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java b/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java index a90a12b86b..89b7bf451e 100644 --- a/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java +++ b/android/app/src/main/java/io/highfidelity/hifiinterface/BreakpadUploaderService.java @@ -54,8 +54,7 @@ public class BreakpadUploaderService extends Service { new Thread(() -> { File[] matchingFiles = getFilesByExtension(getObbDir(), EXT_DMP); - for (File file : matchingFiles) - { + for (File file : matchingFiles) { uploadDumpAndDelete(file, baseUrl); } }).start(); @@ -98,25 +97,25 @@ public class BreakpadUploaderService extends Service { int size = (int) file.length(); byte[] bytes = new byte[size]; try { - BufferedInputStream buf = new BufferedInputStream(new FileInputStream(file)); - buf.read(bytes, 0, bytes.length); - buf.close(); - HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection(); - urlConnection.setRequestMethod("POST"); - urlConnection.setDoOutput(true); - urlConnection.setChunkedStreamingMode(0); + BufferedInputStream buf = new BufferedInputStream(new FileInputStream(file)); + buf.read(bytes, 0, bytes.length); + buf.close(); + HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection(); + urlConnection.setRequestMethod("POST"); + urlConnection.setDoOutput(true); + urlConnection.setChunkedStreamingMode(0); - OutputStream ostream = urlConnection.getOutputStream(); + OutputStream ostream = urlConnection.getOutputStream(); - OutputStream out = new BufferedOutputStream(ostream); - out.write(bytes, 0, size); + OutputStream out = new BufferedOutputStream(ostream); + out.write(bytes, 0, size); - InputStream in = new BufferedInputStream(urlConnection.getInputStream()); - in.read(); - if (urlConnection.getResponseCode() == 200) { - file.delete(); - } - urlConnection.disconnect(); + InputStream in = new BufferedInputStream(urlConnection.getInputStream()); + in.read(); + if (urlConnection.getResponseCode() == 200) { + file.delete(); + } + urlConnection.disconnect(); } catch (IOException e) { Log.e(TAG, "Error uploading file " + file.getAbsolutePath(), e); } @@ -128,20 +127,17 @@ public class BreakpadUploaderService extends Service { return null; } - private File[] getFilesByExtension(File dir, final String extension) - { + private File[] getFilesByExtension(File dir, final String extension) { return dir.listFiles(pathName -> getExtension(pathName.getName()).equals(extension)); } - private String getExtension(String fileName) - { + private String getExtension(String fileName) { String extension = ""; int i = fileName.lastIndexOf('.'); int p = Math.max(fileName.lastIndexOf('/'), fileName.lastIndexOf('\\')); - if (i > p) - { + if (i > p) { extension = fileName.substring(i+1); } diff --git a/interface/src/CrashHandler_Breakpad.cpp b/interface/src/CrashHandler_Breakpad.cpp index ffc49c4ec4..fe4979853e 100644 --- a/interface/src/CrashHandler_Breakpad.cpp +++ b/interface/src/CrashHandler_Breakpad.cpp @@ -60,7 +60,7 @@ void setCrashAnnotation(std::string name, std::string value) { QSettings settings(obbDir() + "/annotations.json", JSON_FORMAT); settings.clear(); settings.beginGroup("Annotations"); - for(auto k : annotations.keys()) { + for (auto k : annotations.keys()) { settings.setValue(k, annotations.value(k)); } settings.endGroup(); From dbf3ade04f077b118d1bbd36fe6d2cea3a5c7845 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Mon, 18 Jun 2018 11:22:05 -0700 Subject: [PATCH 19/21] Implement MS16007: Add support for PNG snapshots --- interface/src/ui/Snapshot.cpp | 76 ++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index 2b306ace91..f505d7d2d7 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -50,6 +50,7 @@ const QString DATETIME_FORMAT = "yyyy-MM-dd_hh-mm-ss"; const QString SNAPSHOTS_DIRECTORY = "Snapshots"; const QString URL = "highfidelity_url"; static const int SNAPSHOT_360_TIMER_INTERVAL = 350; +static const QList SUPPORTED_IMAGE_FORMATS = { "jpg", "jpeg", "png" }; Snapshot::Snapshot() { _snapshotTimer.setSingleShot(false); @@ -67,7 +68,6 @@ Snapshot::Snapshot() { } SnapshotMetaData* Snapshot::parseSnapshotData(QString snapshotPath) { - if (!QFile(snapshotPath).exists()) { return NULL; } @@ -95,7 +95,6 @@ SnapshotMetaData* Snapshot::parseSnapshotData(QString snapshotPath) { } QString Snapshot::saveSnapshot(QImage image, const QString& filename, const QString& pathname) { - QFile* snapshotFile = savedFileForSnapshot(image, false, filename, pathname); if (snapshotFile) { @@ -122,11 +121,15 @@ static const glm::quat CAMERA_ORIENTATION_LEFT(glm::quat(glm::radians(glm::vec3( static const glm::quat CAMERA_ORIENTATION_BACK(glm::quat(glm::radians(glm::vec3(0.0f, 180.0f, 0.0f)))); static const glm::quat CAMERA_ORIENTATION_RIGHT(glm::quat(glm::radians(glm::vec3(0.0f, 270.0f, 0.0f)))); static const glm::quat CAMERA_ORIENTATION_UP(glm::quat(glm::radians(glm::vec3(90.0f, 0.0f, 0.0f)))); -void Snapshot::save360Snapshot(const glm::vec3& cameraPosition, const bool& cubemapOutputFormat, const bool& notify, const QString& filename) { +void Snapshot::save360Snapshot(const glm::vec3& cameraPosition, + const bool& cubemapOutputFormat, + const bool& notify, + const QString& filename) { _snapshotFilename = filename; _notify360 = notify; _cubemapOutputFormat = cubemapOutputFormat; - SecondaryCameraJobConfig* secondaryCameraRenderConfig = static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCamera")); + SecondaryCameraJobConfig* secondaryCameraRenderConfig = + static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCamera")); // Save initial values of secondary camera render config _oldEnabled = secondaryCameraRenderConfig->isEnabled(); @@ -141,9 +144,11 @@ void Snapshot::save360Snapshot(const glm::vec3& cameraPosition, const bool& cube } // Initialize some secondary camera render config options for 360 snapshot capture - static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCameraJob.ToneMapping"))->setCurve(0); + static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCameraJob.ToneMapping")) + ->setCurve(0); - secondaryCameraRenderConfig->resetSizeSpectatorCamera(static_cast(CUBEMAP_SIDE_PIXEL_DIMENSION), static_cast(CUBEMAP_SIDE_PIXEL_DIMENSION)); + secondaryCameraRenderConfig->resetSizeSpectatorCamera(static_cast(CUBEMAP_SIDE_PIXEL_DIMENSION), + static_cast(CUBEMAP_SIDE_PIXEL_DIMENSION)); secondaryCameraRenderConfig->setProperty("attachedEntityId", ""); secondaryCameraRenderConfig->setPosition(cameraPosition); secondaryCameraRenderConfig->setProperty("vFoV", SNAPSHOT_360_FOV); @@ -159,7 +164,8 @@ void Snapshot::save360Snapshot(const glm::vec3& cameraPosition, const bool& cube } void Snapshot::takeNextSnapshot() { - SecondaryCameraJobConfig* config = static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCamera")); + SecondaryCameraJobConfig* config = + static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCamera")); // Order is: // 0. Down @@ -191,7 +197,9 @@ void Snapshot::takeNextSnapshot() { _snapshotTimer.stop(); // Reset secondary camera render config - static_cast(qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCameraJob.ToneMapping"))->setCurve(1); + static_cast( + qApp->getRenderEngine()->getConfiguration()->getConfig("SecondaryCameraJob.ToneMapping")) + ->setCurve(1); config->resetSizeSpectatorCamera(qApp->getWindow()->geometry().width(), qApp->getWindow()->geometry().height()); config->setProperty("attachedEntityId", _oldAttachedEntityId); config->setProperty("vFoV", _oldvFoV); @@ -338,8 +346,10 @@ QTemporaryFile* Snapshot::saveTempSnapshot(QImage image) { return static_cast(savedFileForSnapshot(image, true)); } -QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QString& userSelectedFilename, const QString& userSelectedPathname) { - +QFile* Snapshot::savedFileForSnapshot(QImage& shot, + bool isTemporary, + const QString& userSelectedFilename, + const QString& userSelectedPathname) { // adding URL to snapshot QUrl currentURL = DependencyManager::get()->currentPublicAddress(); shot.setText(URL, currentURL.toString()); @@ -350,12 +360,22 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QSt QDateTime now = QDateTime::currentDateTime(); - // If user has requested specific filename then use it, else create the filename - // 'jpg" is appended, as the image is saved in jpg format. This is the case for all snapshots - // (see definition of FILENAME_PATH_FORMAT) + // If user has supplied a specific filename for the snapshot: + // If the user's requested filename has a suffix that's contained within SUPPORTED_IMAGE_FORMATS, + // DON'T append ".jpg" to the filename. QT will save the image in the format associated with the + // filename's suffix. + // Otherwise, ".jpg" is appended to the user's requested filename so that the image is saved in JPG format. + // If the user hasn't supplied a specific filename for the snapshot: + // Save the snapshot in JPG format according to FILENAME_PATH_FORMAT QString filename; if (!userSelectedFilename.isNull()) { - filename = userSelectedFilename + ".jpg"; + QFileInfo snapshotFileInfo(userSelectedFilename); + QString userSelectedFilenameSuffix = snapshotFileInfo.suffix(); + if (SUPPORTED_IMAGE_FORMATS.contains(userSelectedFilenameSuffix)) { + filename = userSelectedFilename; + } else { + filename = userSelectedFilename + ".jpg"; + } } else { filename = FILENAME_PATH_FORMAT.arg(username, now.toString(DATETIME_FORMAT)); } @@ -372,11 +392,13 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QSt } if (snapshotFullPath.isEmpty()) { - snapshotFullPath = OffscreenUi::getExistingDirectory(nullptr, "Choose Snapshots Directory", QStandardPaths::writableLocation(QStandardPaths::DesktopLocation)); + snapshotFullPath = + OffscreenUi::getExistingDirectory(nullptr, "Choose Snapshots Directory", + QStandardPaths::writableLocation(QStandardPaths::DesktopLocation)); _snapshotsLocation.set(snapshotFullPath); } - if (!snapshotFullPath.isEmpty()) { // not cancelled + if (!snapshotFullPath.isEmpty()) { // not cancelled if (!snapshotFullPath.endsWith(QDir::separator())) { snapshotFullPath.append(QDir::separator()); @@ -393,7 +415,9 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QSt qApp->getApplicationCompositor().getReticleInterface()->setVisible(true); qApp->getApplicationCompositor().getReticleInterface()->setAllowMouseCapture(true); - snapshotFullPath = OffscreenUi::getExistingDirectory(nullptr, "Write Error - Choose New Snapshots Directory", QStandardPaths::writableLocation(QStandardPaths::DesktopLocation)); + snapshotFullPath = + OffscreenUi::getExistingDirectory(nullptr, "Write Error - Choose New Snapshots Directory", + QStandardPaths::writableLocation(QStandardPaths::DesktopLocation)); if (snapshotFullPath.isEmpty()) { return NULL; } @@ -412,7 +436,6 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QSt return imageFile; } - } // Either we were asked for a tempororary, or the user didn't set a directory. QTemporaryFile* imageTempFile = new QTemporaryFile(QDir::tempPath() + "/XXXXXX-" + filename); @@ -430,7 +453,6 @@ QFile* Snapshot::savedFileForSnapshot(QImage & shot, bool isTemporary, const QSt } void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { - const QString SNAPSHOT_UPLOAD_URL = "/api/v1/snapshots"; QUrl url = href; if (url.isEmpty()) { @@ -444,7 +466,7 @@ void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { url = QUrl(DependencyManager::get()->currentShareableAddress()); } SnapshotUploader* uploader = new SnapshotUploader(url, filename); - + QFile* file = new QFile(filename); Q_ASSERT(file->exists()); file->open(QIODevice::ReadOnly); @@ -458,20 +480,16 @@ void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { imagePart.setHeader(QNetworkRequest::ContentDispositionHeader, QVariant("form-data; name=\"image\"; filename=\"" + file->fileName() + "\"")); imagePart.setBodyDevice(file); - + QHttpMultiPart* multiPart = new QHttpMultiPart(QHttpMultiPart::FormDataType); - file->setParent(multiPart); // we cannot delete the file now, so delete it with the multiPart + file->setParent(multiPart); // we cannot delete the file now, so delete it with the multiPart multiPart->append(imagePart); - + auto accountManager = DependencyManager::get(); JSONCallbackParameters callbackParams(uploader, "uploadSuccess", uploader, "uploadFailure"); - accountManager->sendRequest(SNAPSHOT_UPLOAD_URL, - AccountManagerAuth::Required, - QNetworkAccessManager::PostOperation, - callbackParams, - nullptr, - multiPart); + accountManager->sendRequest(SNAPSHOT_UPLOAD_URL, AccountManagerAuth::Required, QNetworkAccessManager::PostOperation, + callbackParams, nullptr, multiPart); } QString Snapshot::getSnapshotsLocation() { From 614b51241130f1cb7a3eefb29d62b4b12befc02d Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Mon, 18 Jun 2018 11:43:31 -0700 Subject: [PATCH 20/21] Add docs --- interface/src/scripting/WindowScriptingInterface.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/interface/src/scripting/WindowScriptingInterface.h b/interface/src/scripting/WindowScriptingInterface.h index 29eb6421e1..c1a4aa8635 100644 --- a/interface/src/scripting/WindowScriptingInterface.h +++ b/interface/src/scripting/WindowScriptingInterface.h @@ -319,6 +319,14 @@ public slots: * {@link Window.processingGifStarted|processingGifStarted} and {@link Window.processingGifCompleted|processingGifCompleted} * are emitted. The path to store the snapshots and the length of the animated GIF to capture are specified in Settings > * General > Snapshots. + * + * If user has supplied a specific filename for the snapshot: + * If the user's requested filename has a suffix that's contained within SUPPORTED_IMAGE_FORMATS, + * DON'T append ".jpg" to the filename. QT will save the image in the format associated with the + * filename's suffix. + * Otherwise, ".jpg" is appended to the user's requested filename so that the image is saved in JPG format. + * If the user hasn't supplied a specific filename for the snapshot: + * Save the snapshot in JPG format according to FILENAME_PATH_FORMAT * @function Window.takeSnapshot * @param {boolean} [notify=true] - This value is passed on through the {@link Window.stillSnapshotTaken|stillSnapshotTaken} * signal. From e83b76781b84abc5d7c3562b2996f0c4b1476e4c Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Mon, 18 Jun 2018 11:47:26 -0700 Subject: [PATCH 21/21] Be a bit more specific in the docs --- interface/src/scripting/WindowScriptingInterface.h | 1 + interface/src/ui/Snapshot.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/interface/src/scripting/WindowScriptingInterface.h b/interface/src/scripting/WindowScriptingInterface.h index c1a4aa8635..dc868e6fcd 100644 --- a/interface/src/scripting/WindowScriptingInterface.h +++ b/interface/src/scripting/WindowScriptingInterface.h @@ -324,6 +324,7 @@ public slots: * If the user's requested filename has a suffix that's contained within SUPPORTED_IMAGE_FORMATS, * DON'T append ".jpg" to the filename. QT will save the image in the format associated with the * filename's suffix. + * If you want lossless Snapshots, supply a `.png` filename. Otherwise, use `.jpeg` or `.jpg`. * Otherwise, ".jpg" is appended to the user's requested filename so that the image is saved in JPG format. * If the user hasn't supplied a specific filename for the snapshot: * Save the snapshot in JPG format according to FILENAME_PATH_FORMAT diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index f505d7d2d7..eba3f25ded 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -364,6 +364,7 @@ QFile* Snapshot::savedFileForSnapshot(QImage& shot, // If the user's requested filename has a suffix that's contained within SUPPORTED_IMAGE_FORMATS, // DON'T append ".jpg" to the filename. QT will save the image in the format associated with the // filename's suffix. + // If you want lossless Snapshots, supply a `.png` filename. Otherwise, use `.jpeg` or `.jpg`. // Otherwise, ".jpg" is appended to the user's requested filename so that the image is saved in JPG format. // If the user hasn't supplied a specific filename for the snapshot: // Save the snapshot in JPG format according to FILENAME_PATH_FORMAT