From 24dd06b65e9068ce4b3b8913266a703983e88d7a Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 09:09:54 -0500 Subject: [PATCH 01/20] * address FIXME regarding jasmine.done() * support disabling tests / reporting as pending --- .../developer/libraries/jasmine/hifi-boot.js | 12 +++++++++-- .../tests/unit_tests/avatarUnitTests.js | 11 +++++++++- .../tests/unit_tests/entityUnitTests.js | 8 +++++++ .../developer/tests/unit_tests/testRunner.js | 21 ++++++++++++++++--- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/scripts/developer/libraries/jasmine/hifi-boot.js b/scripts/developer/libraries/jasmine/hifi-boot.js index f490a3618f..49d7fadd29 100644 --- a/scripts/developer/libraries/jasmine/hifi-boot.js +++ b/scripts/developer/libraries/jasmine/hifi-boot.js @@ -6,7 +6,7 @@ var lastSpecStartTime; function ConsoleReporter(options) { var startTime = new Date().getTime(); - var errorCount = 0; + var errorCount = 0, pending = []; this.jasmineStarted = function (obj) { print('Jasmine started with ' + obj.totalSpecsDefined + ' tests.'); }; @@ -15,11 +15,15 @@ var endTime = new Date().getTime(); print('
'); if (errorCount === 0) { - print ('All tests passed!'); + print ('All enabled tests passed!'); } else { print('Tests completed with ' + errorCount + ' ' + ERROR + '.'); } + if (pending.length) { + print ('disabled:
   '+ + pending.join('
   ')+''); + } print('Tests completed in ' + (endTime - startTime) + 'ms.'); }; this.suiteStarted = function(obj) { @@ -32,6 +36,10 @@ lastSpecStartTime = new Date().getTime(); }; this.specDone = function(obj) { + if (obj.status === 'pending') { + pending.push(obj.fullName); + return print('...(pending ' + obj.fullName +')'); + } var specEndTime = new Date().getTime(); var symbol = obj.status === PASSED ? '' + CHECKMARK + '' : diff --git a/scripts/developer/tests/unit_tests/avatarUnitTests.js b/scripts/developer/tests/unit_tests/avatarUnitTests.js index 7032b5f5e6..fc2801f83e 100644 --- a/scripts/developer/tests/unit_tests/avatarUnitTests.js +++ b/scripts/developer/tests/unit_tests/avatarUnitTests.js @@ -8,6 +8,15 @@ var ROT_IDENT = {x: 0, y: 0, z: 0, w: 1}; describe("MyAvatar", function () { + // backup/restore current skeletonModelURL + beforeAll(function() { + this.oldURL = MyAvatar.skeletonModelURL; + }); + + afterAll(function() { + MyAvatar.skeletonModelURL = this.oldURL; + }); + // reload the avatar from scratch before each test. beforeEach(function (done) { MyAvatar.skeletonModelURL = DEFAULT_AVATAR_URL; @@ -25,7 +34,7 @@ describe("MyAvatar", function () { }, 500); } }, 500); - }); + }, 10000 /*timeout -- allow time to download avatar*/); // makes the assumption that there is solid ground somewhat underneath the avatar. it("position and orientation getters", function () { diff --git a/scripts/developer/tests/unit_tests/entityUnitTests.js b/scripts/developer/tests/unit_tests/entityUnitTests.js index 033a484663..612bf6cdcd 100644 --- a/scripts/developer/tests/unit_tests/entityUnitTests.js +++ b/scripts/developer/tests/unit_tests/entityUnitTests.js @@ -19,6 +19,14 @@ describe('Entity', function() { }, }; + it('serversExist', function() { + expect(Entities.serversExist()).toBe(true); + }); + + it('canRezTmp', function() { + expect(Entities.canRezTmp()).toBe(true); + }); + beforeEach(function() { boxEntity = Entities.addEntity(boxProps); }); diff --git a/scripts/developer/tests/unit_tests/testRunner.js b/scripts/developer/tests/unit_tests/testRunner.js index 31d83cd986..545bb89600 100644 --- a/scripts/developer/tests/unit_tests/testRunner.js +++ b/scripts/developer/tests/unit_tests/testRunner.js @@ -3,11 +3,26 @@ Script.include('../../libraries/jasmine/jasmine.js'); Script.include('../../libraries/jasmine/hifi-boot.js') // Include unit tests -// FIXME: Figure out why jasmine done() is not working. -// Script.include('avatarUnitTests.js'); +Script.include('avatarUnitTests.js'); Script.include('bindUnitTest.js'); Script.include('entityUnitTests.js'); +describe("jasmine internal tests", function() { + it('should support async .done()', function(done) { + var start = new Date; + Script.setTimeout(function() { + expect((new Date - start)/1000).toBeCloseTo(0.5, 1); + done(); + }, 500); + }); + // jasmine pending test + xit('disabled test', function() { + expect(false).toBe(true); + }); +}); + +// invoke Script.stop (after any async tests complete) +jasmine.getEnv().addReporter({ jasmineDone: Script.stop }); + // Run the tests jasmine.getEnv().execute(); -Script.stop(); From 524806b2d99568f68b12b790d169ccee5fce05f4 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 11:22:13 -0500 Subject: [PATCH 02/20] Add initial eslint config comments --- scripts/developer/tests/unit_tests/avatarUnitTests.js | 6 ++++-- scripts/developer/tests/unit_tests/bindUnitTest.js | 2 ++ scripts/developer/tests/unit_tests/entityUnitTests.js | 4 +++- scripts/developer/tests/unit_tests/testRunner.js | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/developer/tests/unit_tests/avatarUnitTests.js b/scripts/developer/tests/unit_tests/avatarUnitTests.js index fc2801f83e..4ab8556ab7 100644 --- a/scripts/developer/tests/unit_tests/avatarUnitTests.js +++ b/scripts/developer/tests/unit_tests/avatarUnitTests.js @@ -1,5 +1,7 @@ +/* eslint-env jasmine */ // Art3mis +// eslint-disable-next-line max-len var DEFAULT_AVATAR_URL = "https://hifi-metaverse.s3-us-west-1.amazonaws.com/marketplace/contents/e76946cc-c272-4adf-9bb6-02cde0a4b57d/8fd984ea6fe1495147a3303f87fa6e23.fst?1460131758"; var ORIGIN = {x: 0, y: 0, z: 0}; @@ -29,12 +31,12 @@ describe("MyAvatar", function () { MyAvatar.position = ORIGIN; MyAvatar.orientation = ROT_IDENT; // give the avatar 1/2 a second to settle on the ground in the idle pose. - Script.setTimeout(function () { + Script.setTimeout(function () { done(); }, 500); } }, 500); - }, 10000 /*timeout -- allow time to download avatar*/); + }, 10000 /* timeout -- allow time to download avatar*/); // makes the assumption that there is solid ground somewhat underneath the avatar. it("position and orientation getters", function () { diff --git a/scripts/developer/tests/unit_tests/bindUnitTest.js b/scripts/developer/tests/unit_tests/bindUnitTest.js index 609487a30b..3407490a04 100644 --- a/scripts/developer/tests/unit_tests/bindUnitTest.js +++ b/scripts/developer/tests/unit_tests/bindUnitTest.js @@ -1,3 +1,5 @@ +/* eslint-env jasmine */ + Script.include('../../../system/libraries/utils.js'); describe('Bind', function() { diff --git a/scripts/developer/tests/unit_tests/entityUnitTests.js b/scripts/developer/tests/unit_tests/entityUnitTests.js index 612bf6cdcd..730fceb144 100644 --- a/scripts/developer/tests/unit_tests/entityUnitTests.js +++ b/scripts/developer/tests/unit_tests/entityUnitTests.js @@ -1,3 +1,5 @@ +/* eslint-env jasmine */ + describe('Entity', function() { var center = Vec3.sum( MyAvatar.position, @@ -70,4 +72,4 @@ describe('Entity', function() { props = Entities.getEntityProperties(boxEntity); expect(props.lastEdited).toBeGreaterThan(prevLastEdited); }); -}); \ No newline at end of file +}); diff --git a/scripts/developer/tests/unit_tests/testRunner.js b/scripts/developer/tests/unit_tests/testRunner.js index 545bb89600..7ce55b1874 100644 --- a/scripts/developer/tests/unit_tests/testRunner.js +++ b/scripts/developer/tests/unit_tests/testRunner.js @@ -1,6 +1,8 @@ +/* eslint-env jasmine */ + // Include testing library Script.include('../../libraries/jasmine/jasmine.js'); -Script.include('../../libraries/jasmine/hifi-boot.js') +Script.include('../../libraries/jasmine/hifi-boot.js'); // Include unit tests Script.include('avatarUnitTests.js'); From 3d6778e9cdd722ed1bef438179abb0f9014706a3 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 1 May 2017 15:02:23 -0700 Subject: [PATCH 03/20] Add tutorial Changelog.md --- tutorial/Changelog.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 tutorial/Changelog.md diff --git a/tutorial/Changelog.md b/tutorial/Changelog.md new file mode 100644 index 0000000000..bd923b6841 --- /dev/null +++ b/tutorial/Changelog.md @@ -0,0 +1,3 @@ + * home-tutorial-34 + * Update tutorial to only start if `HMD.active` + * Update builder's grid to use "Good - Sub-meshes" for collision shape type From fe36e6a793c2a7dcffd0777d97466d5ab9ea058b Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 16:45:12 -0400 Subject: [PATCH 04/20] fix JSConsole script message handlers and about: filename --- interface/src/ui/JSConsole.cpp | 13 +++++++------ interface/src/ui/JSConsole.h | 5 +++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/interface/src/ui/JSConsole.cpp b/interface/src/ui/JSConsole.cpp index 7700874d9a..2d21d1d7ec 100644 --- a/interface/src/ui/JSConsole.cpp +++ b/interface/src/ui/JSConsole.cpp @@ -33,6 +33,8 @@ const QString RESULT_ERROR_STYLE = "color: #d13b22;"; const QString GUTTER_PREVIOUS_COMMAND = "<"; const QString GUTTER_ERROR = "X"; +const QString JSConsole::_consoleFileName { "about:console" }; + JSConsole::JSConsole(QWidget* parent, ScriptEngine* scriptEngine) : QWidget(parent), _ui(new Ui::Console), @@ -84,8 +86,8 @@ void JSConsole::setScriptEngine(ScriptEngine* scriptEngine) { } // if scriptEngine is NULL then create one and keep track of it using _ownScriptEngine - _ownScriptEngine = scriptEngine == NULL; - _scriptEngine = _ownScriptEngine ? DependencyManager::get()->loadScript(QString(), false) : scriptEngine; + _ownScriptEngine = (scriptEngine == NULL); + _scriptEngine = _ownScriptEngine ? DependencyManager::get()->loadScript(_consoleFileName, false) : scriptEngine; connect(_scriptEngine, &ScriptEngine::printedMessage, this, &JSConsole::handlePrint); connect(_scriptEngine, &ScriptEngine::errorMessage, this, &JSConsole::handleError); @@ -107,11 +109,10 @@ void JSConsole::executeCommand(const QString& command) { QScriptValue JSConsole::executeCommandInWatcher(const QString& command) { QScriptValue result; - static const QString filename = "JSConcole"; QMetaObject::invokeMethod(_scriptEngine, "evaluate", Qt::ConnectionType::BlockingQueuedConnection, Q_RETURN_ARG(QScriptValue, result), Q_ARG(const QString&, command), - Q_ARG(const QString&, filename)); + Q_ARG(const QString&, _consoleFileName)); return result; } @@ -134,12 +135,12 @@ void JSConsole::commandFinished() { resetCurrentCommandHistory(); } -void JSConsole::handleError(const QString& scriptName, const QString& message) { +void JSConsole::handleError(const QString& message, const QString& scriptName) { Q_UNUSED(scriptName); appendMessage(GUTTER_ERROR, "" + message.toHtmlEscaped() + ""); } -void JSConsole::handlePrint(const QString& scriptName, const QString& message) { +void JSConsole::handlePrint(const QString& message, const QString& scriptName) { Q_UNUSED(scriptName); appendMessage("", message); } diff --git a/interface/src/ui/JSConsole.h b/interface/src/ui/JSConsole.h index d5f5aff301..938b670e78 100644 --- a/interface/src/ui/JSConsole.h +++ b/interface/src/ui/JSConsole.h @@ -47,8 +47,8 @@ protected: protected slots: void scrollToBottom(); void resizeTextInput(); - void handlePrint(const QString& scriptName, const QString& message); - void handleError(const QString& scriptName, const QString& message); + void handlePrint(const QString& message, const QString& scriptName); + void handleError(const QString& message, const QString& scriptName); void commandFinished(); private: @@ -66,6 +66,7 @@ private: bool _ownScriptEngine; QString _rootCommand; ScriptEngine* _scriptEngine; + static const QString _consoleFileName; }; From 8cea7f6a62a8d0a7c006b344f1e703aae873146a Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 16:54:16 -0400 Subject: [PATCH 05/20] add test for print/Script.print behavior --- scripts/developer/tests/printTest.js | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 scripts/developer/tests/printTest.js diff --git a/scripts/developer/tests/printTest.js b/scripts/developer/tests/printTest.js new file mode 100644 index 0000000000..b94ff0c54b --- /dev/null +++ b/scripts/developer/tests/printTest.js @@ -0,0 +1,34 @@ +/* eslint-env jasmine */ + +// this test generates sample print, Script.print, etc. output + +main(); + +function main() { + // to match with historical behavior, Script.print(message) output only triggers + // the printedMessage signal (and therefore doesn't show up in the application log) + Script.print('[Script.print] hello world'); + + // the rest of these should show up in both the application log and signaled print handlers + print('[print]', 'hello', 'world'); + + // note: these trigger the equivalent of an emit + Script.printedMessage('[Script.printedMessage] hello world', '{filename}'); + Script.errorMessage('[Script.errorMessage] hello world', '{filename}'); + + { + // FIXME: while not directly related to Script.print, these currently don't + // show up at all in the "HMD-friendly script log" or "Console..." + + Script.infoMessage('[Script.infoMessage] hello world', '{filename}'); + Script.warningMessage('[Script.warningMessage] hello world', '{filename}'); + + Vec3.print('[Vec3.print]', Vec3.HALF); + + var q = Quat.fromPitchYawRollDegrees(45, 45, 45); + Quat.print('[Quat.print]', q); + + var m = Mat4.createFromRotAndTrans(q, Vec3.HALF); + Mat4.print('[Mat4.print (row major)]', m); + } +} From e765d8858ce874dbce989286ea54c635138ac8f4 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 16:57:41 -0400 Subject: [PATCH 06/20] update script engine to not treat about:* as a local filename; resolve debugPrint FIXME --- libraries/script-engine/src/ScriptEngine.cpp | 13 +++++++++---- libraries/script-engine/src/ScriptEngine.h | 1 + libraries/script-engine/src/ScriptEngines.cpp | 5 +++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index c904062507..8bbb9a3e2c 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -105,11 +105,11 @@ static QScriptValue debugPrint(QScriptContext* context, QScriptEngine* engine) { } message += context->argument(i).toString(); } - qCDebug(scriptengineScript).noquote() << "script:print()<<" << message; // noquote() so that \n is treated as newline + qCDebug(scriptengineScript).noquote() << message; // noquote() so that \n is treated as newline - // FIXME - this approach neeeds revisiting. print() comes here, which ends up calling Script.print? - engine->globalObject().property("Script").property("print") - .call(engine->nullValue(), QScriptValueList({ message })); + if (ScriptEngine *scriptEngine = qobject_cast(engine)) { + scriptEngine->print(message); + } return QScriptValue(); } @@ -472,6 +472,11 @@ void ScriptEngine::scriptInfoMessage(const QString& message) { emit infoMessage(message, getFilename()); } +void ScriptEngine::scriptPrintedMessage(const QString& message) { + qCDebug(scriptengine) << message; + emit printedMessage(message, getFilename()); +} + // Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of // callAnimationStateHandler requires that the type be registered. // These two are meaningful, if we ever do want to use them... diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 5ea8d052e9..6188f24d71 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -221,6 +221,7 @@ public: void scriptErrorMessage(const QString& message); void scriptWarningMessage(const QString& message); void scriptInfoMessage(const QString& message); + void scriptPrintedMessage(const QString& message); int getNumRunningEntityScripts() const; bool getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const; diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 88b0e0b7b5..2076657288 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -453,7 +453,8 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL (scriptFilename.scheme() != "http" && scriptFilename.scheme() != "https" && scriptFilename.scheme() != "atp" && - scriptFilename.scheme() != "file")) { + scriptFilename.scheme() != "file" && + scriptFilename.scheme() != "about")) { // deal with a "url" like c:/something scriptUrl = normalizeScriptURL(QUrl::fromLocalFile(scriptFilename.toString())); } else { @@ -472,7 +473,7 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL }, Qt::QueuedConnection); - if (scriptFilename.isEmpty()) { + if (scriptFilename.isEmpty() || !scriptUrl.isValid()) { launchScriptEngine(scriptEngine); } else { // connect to the appropriate signals of this script engine From 6cb95b239340f5d58eaf263598e7e3a952b1bf64 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 16:59:38 -0400 Subject: [PATCH 07/20] add and connect handlers for scriptWarningMessage and scriptInfoMessage signals --- interface/src/ui/JSConsole.cpp | 16 ++++++++++++++++ interface/src/ui/JSConsole.h | 2 ++ 2 files changed, 18 insertions(+) diff --git a/interface/src/ui/JSConsole.cpp b/interface/src/ui/JSConsole.cpp index 2d21d1d7ec..1abc5e8c2e 100644 --- a/interface/src/ui/JSConsole.cpp +++ b/interface/src/ui/JSConsole.cpp @@ -28,6 +28,8 @@ const int MAX_HISTORY_SIZE = 64; const QString COMMAND_STYLE = "color: #266a9b;"; const QString RESULT_SUCCESS_STYLE = "color: #677373;"; +const QString RESULT_INFO_STYLE = "color: #223bd1;"; +const QString RESULT_WARNING_STYLE = "color: #d1b322;"; const QString RESULT_ERROR_STYLE = "color: #d13b22;"; const QString GUTTER_PREVIOUS_COMMAND = "<"; @@ -79,6 +81,8 @@ void JSConsole::setScriptEngine(ScriptEngine* scriptEngine) { } if (_scriptEngine != NULL) { disconnect(_scriptEngine, &ScriptEngine::printedMessage, this, &JSConsole::handlePrint); + disconnect(_scriptEngine, &ScriptEngine::infoMessage, this, &JSConsole::handleInfo); + disconnect(_scriptEngine, &ScriptEngine::warningMessage, this, &JSConsole::handleWarning); disconnect(_scriptEngine, &ScriptEngine::errorMessage, this, &JSConsole::handleError); if (_ownScriptEngine) { _scriptEngine->deleteLater(); @@ -90,6 +94,8 @@ void JSConsole::setScriptEngine(ScriptEngine* scriptEngine) { _scriptEngine = _ownScriptEngine ? DependencyManager::get()->loadScript(_consoleFileName, false) : scriptEngine; connect(_scriptEngine, &ScriptEngine::printedMessage, this, &JSConsole::handlePrint); + connect(_scriptEngine, &ScriptEngine::infoMessage, this, &JSConsole::handleInfo); + connect(_scriptEngine, &ScriptEngine::warningMessage, this, &JSConsole::handleWarning); connect(_scriptEngine, &ScriptEngine::errorMessage, this, &JSConsole::handleError); } @@ -145,6 +151,16 @@ void JSConsole::handlePrint(const QString& message, const QString& scriptName) { appendMessage("", message); } +void JSConsole::handleInfo(const QString& message, const QString& scriptName) { + Q_UNUSED(scriptName); + appendMessage("", "" + message.toHtmlEscaped() + ""); +} + +void JSConsole::handleWarning(const QString& message, const QString& scriptName) { + Q_UNUSED(scriptName); + appendMessage("", "" + message.toHtmlEscaped() + ""); +} + void JSConsole::mouseReleaseEvent(QMouseEvent* event) { _ui->promptTextEdit->setFocus(); } diff --git a/interface/src/ui/JSConsole.h b/interface/src/ui/JSConsole.h index 938b670e78..864f847071 100644 --- a/interface/src/ui/JSConsole.h +++ b/interface/src/ui/JSConsole.h @@ -48,6 +48,8 @@ protected slots: void scrollToBottom(); void resizeTextInput(); void handlePrint(const QString& message, const QString& scriptName); + void handleInfo(const QString& message, const QString& scriptName); + void handleWarning(const QString& message, const QString& scriptName); void handleError(const QString& message, const QString& scriptName); void commandFinished(); From deeb9c367ad7cdf7dd991060716461a64c13659d Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 17:46:28 -0400 Subject: [PATCH 08/20] update test to reflect support for scriptWarningMessage and scriptInfoMessage --- scripts/developer/tests/printTest.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/scripts/developer/tests/printTest.js b/scripts/developer/tests/printTest.js index b94ff0c54b..d1069a6a8f 100644 --- a/scripts/developer/tests/printTest.js +++ b/scripts/developer/tests/printTest.js @@ -14,15 +14,12 @@ function main() { // note: these trigger the equivalent of an emit Script.printedMessage('[Script.printedMessage] hello world', '{filename}'); + Script.infoMessage('[Script.infoMessage] hello world', '{filename}'); + Script.warningMessage('[Script.warningMessage] hello world', '{filename}'); Script.errorMessage('[Script.errorMessage] hello world', '{filename}'); { - // FIXME: while not directly related to Script.print, these currently don't - // show up at all in the "HMD-friendly script log" or "Console..." - - Script.infoMessage('[Script.infoMessage] hello world', '{filename}'); - Script.warningMessage('[Script.warningMessage] hello world', '{filename}'); - + // FIXME: these only show up in the application debug log Vec3.print('[Vec3.print]', Vec3.HALF); var q = Quat.fromPitchYawRollDegrees(45, 45, 45); From f71552c648af5c10e32e764c0601555a87bc35be Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 17:04:13 -0400 Subject: [PATCH 09/20] * update Vec3.print, Quat.print, Mat4.print, ScriptUUID.print to work with JSConsole and HMD-friendly script log * add test output for Quat-euler and Mat4-row/col prints --- libraries/script-engine/src/Mat4.cpp | 16 ++++++++++------ libraries/script-engine/src/Mat4.h | 5 +++-- libraries/script-engine/src/Quat.cpp | 15 +++++++++++++-- libraries/script-engine/src/Quat.h | 5 +++-- libraries/script-engine/src/ScriptUUID.cpp | 10 ++++++++-- libraries/script-engine/src/ScriptUUID.h | 5 +++-- libraries/script-engine/src/Vec3.cpp | 12 +++++++++--- libraries/script-engine/src/Vec3.h | 3 ++- scripts/developer/tests/printTest.js | 14 +++++++++++--- 9 files changed, 62 insertions(+), 23 deletions(-) diff --git a/libraries/script-engine/src/Mat4.cpp b/libraries/script-engine/src/Mat4.cpp index 6676d0cde1..2d26a66823 100644 --- a/libraries/script-engine/src/Mat4.cpp +++ b/libraries/script-engine/src/Mat4.cpp @@ -11,7 +11,9 @@ #include #include +#include #include "ScriptEngineLogging.h" +#include "ScriptEngine.h" #include "Mat4.h" glm::mat4 Mat4::multiply(const glm::mat4& m1, const glm::mat4& m2) const { @@ -66,10 +68,12 @@ glm::vec3 Mat4::getUp(const glm::mat4& m) const { return glm::vec3(m[0][1], m[1][1], m[2][1]); } -void Mat4::print(const QString& label, const glm::mat4& m) const { - qCDebug(scriptengine) << qPrintable(label) << - "row0 =" << m[0][0] << "," << m[1][0] << "," << m[2][0] << "," << m[3][0] << - "row1 =" << m[0][1] << "," << m[1][1] << "," << m[2][1] << "," << m[3][1] << - "row2 =" << m[0][2] << "," << m[1][2] << "," << m[2][2] << "," << m[3][2] << - "row3 =" << m[0][3] << "," << m[1][3] << "," << m[2][3] << "," << m[3][3]; +void Mat4::print(const QString& label, const glm::mat4& m, bool transpose) const { + glm::mat4 out = transpose ? glm::transpose(m) : m; + QString message = QString("%1 %2").arg(qPrintable(label)); + message = message.arg(glm::to_string(out).c_str()); + qCDebug(scriptengine) << message; + if (ScriptEngine* scriptEngine = qobject_cast(engine())) { + scriptEngine->print(message); + } } diff --git a/libraries/script-engine/src/Mat4.h b/libraries/script-engine/src/Mat4.h index 19bbbe178a..8b942874ee 100644 --- a/libraries/script-engine/src/Mat4.h +++ b/libraries/script-engine/src/Mat4.h @@ -16,9 +16,10 @@ #include #include +#include /// Scriptable Mat4 object. Used exclusively in the JavaScript API -class Mat4 : public QObject { +class Mat4 : public QObject, protected QScriptable { Q_OBJECT public slots: @@ -43,7 +44,7 @@ public slots: glm::vec3 getRight(const glm::mat4& m) const; glm::vec3 getUp(const glm::mat4& m) const; - void print(const QString& label, const glm::mat4& m) const; + void print(const QString& label, const glm::mat4& m, bool transpose = false) const; }; #endif // hifi_Mat4_h diff --git a/libraries/script-engine/src/Quat.cpp b/libraries/script-engine/src/Quat.cpp index 05002dcf5d..8d84b45b90 100644 --- a/libraries/script-engine/src/Quat.cpp +++ b/libraries/script-engine/src/Quat.cpp @@ -15,7 +15,9 @@ #include #include +#include #include "ScriptEngineLogging.h" +#include "ScriptEngine.h" #include "Quat.h" quat Quat::normalize(const glm::quat& q) { @@ -114,8 +116,17 @@ float Quat::dot(const glm::quat& q1, const glm::quat& q2) { return glm::dot(q1, q2); } -void Quat::print(const QString& label, const glm::quat& q) { - qCDebug(scriptengine) << qPrintable(label) << q.x << "," << q.y << "," << q.z << "," << q.w; +void Quat::print(const QString& label, const glm::quat& q, bool asDegrees) { + QString message = QString("%1 %2").arg(qPrintable(label)); + if (asDegrees) { + message = message.arg(glm::to_string(safeEulerAngles(q)).c_str()); + } else { + message = message.arg(glm::to_string(q).c_str()); + } + qCDebug(scriptengine) << message; + if (ScriptEngine* scriptEngine = qobject_cast(engine())) { + scriptEngine->print(message); + } } bool Quat::equal(const glm::quat& q1, const glm::quat& q2) { diff --git a/libraries/script-engine/src/Quat.h b/libraries/script-engine/src/Quat.h index ee3ab9aa7c..3b3a6fde7c 100644 --- a/libraries/script-engine/src/Quat.h +++ b/libraries/script-engine/src/Quat.h @@ -18,6 +18,7 @@ #include #include +#include /**jsdoc * A Quaternion @@ -30,7 +31,7 @@ */ /// Scriptable interface a Quaternion helper class object. Used exclusively in the JavaScript API -class Quat : public QObject { +class Quat : public QObject, protected QScriptable { Q_OBJECT public slots: @@ -58,7 +59,7 @@ public slots: glm::quat slerp(const glm::quat& q1, const glm::quat& q2, float alpha); glm::quat squad(const glm::quat& q1, const glm::quat& q2, const glm::quat& s1, const glm::quat& s2, float h); float dot(const glm::quat& q1, const glm::quat& q2); - void print(const QString& label, const glm::quat& q); + void print(const QString& label, const glm::quat& q, bool asDegrees = false); bool equal(const glm::quat& q1, const glm::quat& q2); glm::quat cancelOutRollAndPitch(const glm::quat& q); glm::quat cancelOutRoll(const glm::quat& q); diff --git a/libraries/script-engine/src/ScriptUUID.cpp b/libraries/script-engine/src/ScriptUUID.cpp index 6a52f4f6ca..ee15f1a760 100644 --- a/libraries/script-engine/src/ScriptUUID.cpp +++ b/libraries/script-engine/src/ScriptUUID.cpp @@ -14,6 +14,7 @@ #include #include "ScriptEngineLogging.h" +#include "ScriptEngine.h" #include "ScriptUUID.h" QUuid ScriptUUID::fromString(const QString& s) { @@ -36,6 +37,11 @@ bool ScriptUUID::isNull(const QUuid& id) { return id.isNull(); } -void ScriptUUID::print(const QString& lable, const QUuid& id) { - qCDebug(scriptengine) << qPrintable(lable) << id.toString(); +void ScriptUUID::print(const QString& label, const QUuid& id) { + QString message = QString("%1 %2").arg(qPrintable(label)); + message = message.arg(id.toString()); + qCDebug(scriptengine) << message; + if (ScriptEngine* scriptEngine = qobject_cast(engine())) { + scriptEngine->print(message); + } } diff --git a/libraries/script-engine/src/ScriptUUID.h b/libraries/script-engine/src/ScriptUUID.h index db94b5082b..221f9c46f0 100644 --- a/libraries/script-engine/src/ScriptUUID.h +++ b/libraries/script-engine/src/ScriptUUID.h @@ -15,9 +15,10 @@ #define hifi_ScriptUUID_h #include +#include /// Scriptable interface for a UUID helper class object. Used exclusively in the JavaScript API -class ScriptUUID : public QObject { +class ScriptUUID : public QObject, protected QScriptable { Q_OBJECT public slots: @@ -26,7 +27,7 @@ public slots: QUuid generate(); bool isEqual(const QUuid& idA, const QUuid& idB); bool isNull(const QUuid& id); - void print(const QString& lable, const QUuid& id); + void print(const QString& label, const QUuid& id); }; #endif // hifi_ScriptUUID_h diff --git a/libraries/script-engine/src/Vec3.cpp b/libraries/script-engine/src/Vec3.cpp index 6c8f618500..f66b99dfa1 100644 --- a/libraries/script-engine/src/Vec3.cpp +++ b/libraries/script-engine/src/Vec3.cpp @@ -14,20 +14,26 @@ #include #include +#include #include "ScriptEngineLogging.h" #include "NumericalConstants.h" #include "Vec3.h" +#include "ScriptEngine.h" float Vec3::orientedAngle(const glm::vec3& v1, const glm::vec3& v2, const glm::vec3& v3) { float radians = glm::orientedAngle(glm::normalize(v1), glm::normalize(v2), glm::normalize(v3)); return glm::degrees(radians); } - -void Vec3::print(const QString& lable, const glm::vec3& v) { - qCDebug(scriptengine) << qPrintable(lable) << v.x << "," << v.y << "," << v.z; +void Vec3::print(const QString& label, const glm::vec3& v) { + QString message = QString("%1 %2").arg(qPrintable(label)); + message = message.arg(glm::to_string(v).c_str()); + qCDebug(scriptengine) << message; + if (ScriptEngine* scriptEngine = qobject_cast(engine())) { + scriptEngine->print(message); + } } bool Vec3::withinEpsilon(const glm::vec3& v1, const glm::vec3& v2, float epsilon) { diff --git a/libraries/script-engine/src/Vec3.h b/libraries/script-engine/src/Vec3.h index b3a3dc3035..c7179a80c0 100644 --- a/libraries/script-engine/src/Vec3.h +++ b/libraries/script-engine/src/Vec3.h @@ -17,6 +17,7 @@ #include #include +#include #include "GLMHelpers.h" @@ -48,7 +49,7 @@ */ /// Scriptable interface a Vec3ernion helper class object. Used exclusively in the JavaScript API -class Vec3 : public QObject { +class Vec3 : public QObject, protected QScriptable { Q_OBJECT Q_PROPERTY(glm::vec3 UNIT_X READ UNIT_X CONSTANT) Q_PROPERTY(glm::vec3 UNIT_Y READ UNIT_Y CONSTANT) diff --git a/scripts/developer/tests/printTest.js b/scripts/developer/tests/printTest.js index d1069a6a8f..5fcd8ecb74 100644 --- a/scripts/developer/tests/printTest.js +++ b/scripts/developer/tests/printTest.js @@ -19,13 +19,21 @@ function main() { Script.errorMessage('[Script.errorMessage] hello world', '{filename}'); { - // FIXME: these only show up in the application debug log Vec3.print('[Vec3.print]', Vec3.HALF); var q = Quat.fromPitchYawRollDegrees(45, 45, 45); Quat.print('[Quat.print]', q); + Quat.print('[Quat.print (euler)]', q, true); - var m = Mat4.createFromRotAndTrans(q, Vec3.HALF); - Mat4.print('[Mat4.print (row major)]', m); + function vec4(x,y,z,w) { + return { x: x, y: y, z: z, w: w }; + } + var m = Mat4.createFromColumns( + vec4(1,2,3,4), vec4(5,6,7,8), vec4(9,10,11,12), vec4(13,14,15,16) + ); + Mat4.print('[Mat4.print (col major)]', m); + Mat4.print('[Mat4.print (row major)]', m, true); + + Uuid.print('[Uuid.print]', Uuid.toString(0)); } } From f9d29256e0531c850ac9351ff80ad406ae0f9c6c Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 18:58:30 -0400 Subject: [PATCH 10/20] use an actual Uuid value for the Uuid.print test --- scripts/developer/tests/printTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/developer/tests/printTest.js b/scripts/developer/tests/printTest.js index 5fcd8ecb74..c1fe6ec745 100644 --- a/scripts/developer/tests/printTest.js +++ b/scripts/developer/tests/printTest.js @@ -34,6 +34,6 @@ function main() { Mat4.print('[Mat4.print (col major)]', m); Mat4.print('[Mat4.print (row major)]', m, true); - Uuid.print('[Uuid.print]', Uuid.toString(0)); + Uuid.print('[Uuid.print]', Uuid.fromString(Uuid.toString(0))); } } From f39411656a5f515bf69782d105b37bf823e51465 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 1 May 2017 16:46:03 -0700 Subject: [PATCH 11/20] use machine GUID from registry for machine ID --- libraries/networking/src/FingerprintUtils.cpp | 129 +++--------------- 1 file changed, 20 insertions(+), 109 deletions(-) diff --git a/libraries/networking/src/FingerprintUtils.cpp b/libraries/networking/src/FingerprintUtils.cpp index 1990d356b6..89f692e77a 100644 --- a/libraries/networking/src/FingerprintUtils.cpp +++ b/libraries/networking/src/FingerprintUtils.cpp @@ -19,8 +19,8 @@ #include #ifdef Q_OS_WIN -#include -#include +#include +#include #endif //Q_OS_WIN #ifdef Q_OS_MAC @@ -47,122 +47,32 @@ QString FingerprintUtils::getMachineFingerprintString() { #endif //Q_OS_MAC #ifdef Q_OS_WIN - HRESULT hres; - IWbemLocator *pLoc = NULL; - - // initialize com. Interface already does, but other - // users of this lib don't necessarily do so. - hres = CoInitializeEx(0, COINIT_MULTITHREADED); - if (FAILED(hres)) { - qCDebug(networking) << "Failed to initialize COM library!"; - return uuidString; - } + HKEY cryptoKey; - // initialize WbemLocator - hres = CoCreateInstance( - CLSID_WbemLocator, - 0, - CLSCTX_INPROC_SERVER, - IID_IWbemLocator, (LPVOID *) &pLoc); + // try and open the key that contains the machine GUID + if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, "SOFTWARE\\Microsoft\\Cryptography", 0, KEY_READ, &cryptoKey) == ERROR_SUCCESS) { + DWORD type; + DWORD guidSize; - if (FAILED(hres)) { - qCDebug(networking) << "Failed to initialize WbemLocator"; - return uuidString; - } - - // Connect to WMI through the IWbemLocator::ConnectServer method - IWbemServices *pSvc = NULL; + const char* MACHINE_GUID_KEY = "MachineGuid"; - // Connect to the root\cimv2 namespace with - // the current user and obtain pointer pSvc - // to make IWbemServices calls. - hres = pLoc->ConnectServer( - _bstr_t(L"ROOT\\CIMV2"), // Object path of WMI namespace - NULL, // User name. NULL = current user - NULL, // User password. NULL = current - 0, // Locale. NULL indicates current - NULL, // Security flags. - 0, // Authority (for example, Kerberos) - 0, // Context object - &pSvc // pointer to IWbemServices proxy - ); + // try and retrieve the size of the GUID value + if (RegQueryValueEx(cryptoKey, MACHINE_GUID_KEY, NULL, &type, NULL, &guidSize) == ERROR_SUCCESS) { + // make sure that the value is a string + if (type == REG_SZ) { + // retrieve the machine GUID and return that as our UUID string + std::string machineGUID(guidSize / sizeof(char), '\0'); - if (FAILED(hres)) { - pLoc->Release(); - qCDebug(networking) << "Failed to connect to WMI"; - return uuidString; - } - - // Set security levels on the proxy - hres = CoSetProxyBlanket( - pSvc, // Indicates the proxy to set - RPC_C_AUTHN_WINNT, // RPC_C_AUTHN_xxx - RPC_C_AUTHZ_NONE, // RPC_C_AUTHZ_xxx - NULL, // Server principal name - RPC_C_AUTHN_LEVEL_CALL, // RPC_C_AUTHN_LEVEL_xxx - RPC_C_IMP_LEVEL_IMPERSONATE, // RPC_C_IMP_LEVEL_xxx - NULL, // client identity - EOAC_NONE // proxy capabilities - ); - - if (FAILED(hres)) { - pSvc->Release(); - pLoc->Release(); - qCDebug(networking) << "Failed to set security on proxy blanket"; - return uuidString; - } - - // Use the IWbemServices pointer to grab the Win32_BIOS stuff - IEnumWbemClassObject* pEnumerator = NULL; - hres = pSvc->ExecQuery( - bstr_t("WQL"), - bstr_t("SELECT * FROM Win32_ComputerSystemProduct"), - WBEM_FLAG_FORWARD_ONLY | WBEM_FLAG_RETURN_IMMEDIATELY, - NULL, - &pEnumerator); - - if (FAILED(hres)) { - pSvc->Release(); - pLoc->Release(); - qCDebug(networking) << "query to get Win32_ComputerSystemProduct info"; - return uuidString; - } - - // Get the SerialNumber from the Win32_BIOS data - IWbemClassObject *pclsObj; - ULONG uReturn = 0; - - SHORT sRetStatus = -100; - - while (pEnumerator) { - HRESULT hr = pEnumerator->Next(WBEM_INFINITE, 1, &pclsObj, &uReturn); - - if(0 == uReturn){ - break; - } - - VARIANT vtProp; - - // Get the value of the Name property - hr = pclsObj->Get(L"UUID", 0, &vtProp, 0, 0); - if (!FAILED(hres)) { - switch (vtProp.vt) { - case VT_BSTR: - uuidString = QString::fromWCharArray(vtProp.bstrVal); - break; + if (RegQueryValueEx(cryptoKey, MACHINE_GUID_KEY, NULL, NULL, + reinterpret_cast(&machineGUID[0]), &guidSize) == ERROR_SUCCESS) { + uuidString = QString::fromStdString(machineGUID); + } } } - VariantClear(&vtProp); - pclsObj->Release(); + RegCloseKey(cryptoKey); } - pEnumerator->Release(); - // Cleanup - pSvc->Release(); - pLoc->Release(); - - qCDebug(networking) << "Windows BIOS UUID: " << uuidString; #endif //Q_OS_WIN return uuidString; @@ -177,6 +87,7 @@ QUuid FingerprintUtils::getMachineFingerprint() { // return QUuid() ("{00000...}"), which handles // any errors in getting the string QUuid uuid(uuidString); + if (uuid == QUuid()) { // if you cannot read a fallback key cuz we aren't saving them, just generate one for // this session and move on From 1857b297e091bf0441526c5420436e13f8bc9582 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 4 May 2017 15:52:04 -0700 Subject: [PATCH 12/20] don't re-grab the machine fingerprint every DS connection --- libraries/networking/src/FingerprintUtils.cpp | 51 +++++++++++-------- libraries/networking/src/FingerprintUtils.h | 1 + 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/libraries/networking/src/FingerprintUtils.cpp b/libraries/networking/src/FingerprintUtils.cpp index 89f692e77a..216e0f28dd 100644 --- a/libraries/networking/src/FingerprintUtils.cpp +++ b/libraries/networking/src/FingerprintUtils.cpp @@ -30,6 +30,9 @@ #endif //Q_OS_MAC static const QString FALLBACK_FINGERPRINT_KEY = "fallbackFingerprint"; + +QUuid FingerprintUtils::_machineFingerprint { QUuid() }; + QString FingerprintUtils::getMachineFingerprintString() { QString uuidString; #ifdef Q_OS_LINUX @@ -81,30 +84,36 @@ QString FingerprintUtils::getMachineFingerprintString() { QUuid FingerprintUtils::getMachineFingerprint() { - QString uuidString = getMachineFingerprintString(); + if (_machineFingerprint.isNull()) { + QString uuidString = getMachineFingerprintString(); + + // now, turn into uuid. A malformed string will + // return QUuid() ("{00000...}"), which handles + // any errors in getting the string + QUuid uuid(uuidString); - // now, turn into uuid. A malformed string will - // return QUuid() ("{00000...}"), which handles - // any errors in getting the string - QUuid uuid(uuidString); - - if (uuid == QUuid()) { - // if you cannot read a fallback key cuz we aren't saving them, just generate one for - // this session and move on - if (DependencyManager::get().isNull()) { - return QUuid::createUuid(); - } - // read fallback key (if any) - Settings settings; - uuid = QUuid(settings.value(FALLBACK_FINGERPRINT_KEY).toString()); - qCDebug(networking) << "read fallback maching fingerprint: " << uuid.toString(); if (uuid == QUuid()) { - // no fallback yet, set one - uuid = QUuid::createUuid(); - settings.setValue(FALLBACK_FINGERPRINT_KEY, uuid.toString()); - qCDebug(networking) << "no fallback machine fingerprint, setting it to: " << uuid.toString(); + // if you cannot read a fallback key cuz we aren't saving them, just generate one for + // this session and move on + if (DependencyManager::get().isNull()) { + return QUuid::createUuid(); + } + // read fallback key (if any) + Settings settings; + uuid = QUuid(settings.value(FALLBACK_FINGERPRINT_KEY).toString()); + qCDebug(networking) << "read fallback maching fingerprint: " << uuid.toString(); + + if (uuid == QUuid()) { + // no fallback yet, set one + uuid = QUuid::createUuid(); + settings.setValue(FALLBACK_FINGERPRINT_KEY, uuid.toString()); + qCDebug(networking) << "no fallback machine fingerprint, setting it to: " << uuid.toString(); + } } + + _machineFingerprint = uuid; } - return uuid; + + return _machineFingerprint; } diff --git a/libraries/networking/src/FingerprintUtils.h b/libraries/networking/src/FingerprintUtils.h index 572b150ec4..c4cb900a48 100644 --- a/libraries/networking/src/FingerprintUtils.h +++ b/libraries/networking/src/FingerprintUtils.h @@ -21,6 +21,7 @@ public: private: static QString getMachineFingerprintString(); + static QUuid _machineFingerprint; }; #endif // hifi_FingerprintUtils_h From c17d1a6d881333f196bd8272d2346cc062c98f71 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 4 May 2017 19:21:32 -0400 Subject: [PATCH 13/20] manually promote glm values to double when stringifying (to prevent -Wdouble-promotion "error") --- libraries/script-engine/src/Mat4.cpp | 2 +- libraries/script-engine/src/Quat.cpp | 4 ++-- libraries/script-engine/src/Vec3.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/script-engine/src/Mat4.cpp b/libraries/script-engine/src/Mat4.cpp index 2d26a66823..6965f43b32 100644 --- a/libraries/script-engine/src/Mat4.cpp +++ b/libraries/script-engine/src/Mat4.cpp @@ -69,7 +69,7 @@ glm::vec3 Mat4::getUp(const glm::mat4& m) const { } void Mat4::print(const QString& label, const glm::mat4& m, bool transpose) const { - glm::mat4 out = transpose ? glm::transpose(m) : m; + glm::dmat4 out = transpose ? glm::transpose(m) : m; QString message = QString("%1 %2").arg(qPrintable(label)); message = message.arg(glm::to_string(out).c_str()); qCDebug(scriptengine) << message; diff --git a/libraries/script-engine/src/Quat.cpp b/libraries/script-engine/src/Quat.cpp index 8d84b45b90..a6f7acffc8 100644 --- a/libraries/script-engine/src/Quat.cpp +++ b/libraries/script-engine/src/Quat.cpp @@ -119,9 +119,9 @@ float Quat::dot(const glm::quat& q1, const glm::quat& q2) { void Quat::print(const QString& label, const glm::quat& q, bool asDegrees) { QString message = QString("%1 %2").arg(qPrintable(label)); if (asDegrees) { - message = message.arg(glm::to_string(safeEulerAngles(q)).c_str()); + message = message.arg(glm::to_string(glm::dvec3(safeEulerAngles(q))).c_str()); } else { - message = message.arg(glm::to_string(q).c_str()); + message = message.arg(glm::to_string(glm::dquat(q)).c_str()); } qCDebug(scriptengine) << message; if (ScriptEngine* scriptEngine = qobject_cast(engine())) { diff --git a/libraries/script-engine/src/Vec3.cpp b/libraries/script-engine/src/Vec3.cpp index f66b99dfa1..a156f56d96 100644 --- a/libraries/script-engine/src/Vec3.cpp +++ b/libraries/script-engine/src/Vec3.cpp @@ -29,7 +29,7 @@ float Vec3::orientedAngle(const glm::vec3& v1, const glm::vec3& v2, const glm::v void Vec3::print(const QString& label, const glm::vec3& v) { QString message = QString("%1 %2").arg(qPrintable(label)); - message = message.arg(glm::to_string(v).c_str()); + message = message.arg(glm::to_string(glm::dvec3(v)).c_str()); qCDebug(scriptengine) << message; if (ScriptEngine* scriptEngine = qobject_cast(engine())) { scriptEngine->print(message); From 25807a5258f3c6539acd6c8e739f5d50b6e354fb Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 5 May 2017 09:48:24 -0400 Subject: [PATCH 14/20] fix warning color --- interface/src/ui/JSConsole.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/ui/JSConsole.cpp b/interface/src/ui/JSConsole.cpp index 1abc5e8c2e..79314ce49a 100644 --- a/interface/src/ui/JSConsole.cpp +++ b/interface/src/ui/JSConsole.cpp @@ -29,7 +29,7 @@ const QString COMMAND_STYLE = "color: #266a9b;"; const QString RESULT_SUCCESS_STYLE = "color: #677373;"; const QString RESULT_INFO_STYLE = "color: #223bd1;"; -const QString RESULT_WARNING_STYLE = "color: #d1b322;"; +const QString RESULT_WARNING_STYLE = "color: #d13b22;"; const QString RESULT_ERROR_STYLE = "color: #d13b22;"; const QString GUTTER_PREVIOUS_COMMAND = "<"; From e882e7b648460a67c8a75eb183f740e0478d7903 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 5 May 2017 15:36:38 -0400 Subject: [PATCH 15/20] rm extra lock in audio read --- libraries/audio-client/src/AudioClient.cpp | 10 +++++++--- libraries/audio-client/src/AudioClient.h | 4 +--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index b684aac89c..fb0220c24d 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1115,7 +1115,7 @@ void AudioClient::prepareLocalAudioInjectors() { while (samplesNeeded > 0) { // lock for every write to avoid locking out the device callback // this lock is intentional - the buffer is only lock-free in its use in the device callback - RecursiveLock lock(_localAudioMutex); + Lock lock(_localAudioMutex); samplesNeeded = bufferCapacity - _localSamplesAvailable.load(std::memory_order_relaxed); if (samplesNeeded <= 0) { @@ -1452,7 +1452,7 @@ void AudioClient::outputNotify() { bool AudioClient::switchOutputToAudioDevice(const QAudioDeviceInfo& outputDeviceInfo) { bool supportedFormat = false; - RecursiveLock lock(_localAudioMutex); + Lock lock(_localAudioMutex); _localSamplesAvailable.exchange(0, std::memory_order_release); // cleanup any previously initialized device @@ -1681,8 +1681,12 @@ qint64 AudioClient::AudioOutputIODevice::readData(char * data, qint64 maxSize) { int injectorSamplesPopped = 0; { - RecursiveLock lock(_audio->_localAudioMutex); bool append = networkSamplesPopped > 0; + // this does not require a lock as of the only two functions adding to _localSamplesAvailable (samples count): + // - prepareLocalAudioInjectors will only increase samples count + // - switchOutputToAudioDevice will zero samples count + // stop the device, so that readData will exhaust the existing buffer or see a zeroed samples count + // and start the device, which can only see a zeroed samples count samplesRequested = std::min(samplesRequested, _audio->_localSamplesAvailable.load(std::memory_order_acquire)); if ((injectorSamplesPopped = _localInjectorsStream.appendSamples(mixBuffer, samplesRequested, append)) > 0) { _audio->_localSamplesAvailable.fetch_sub(injectorSamplesPopped, std::memory_order_release); diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 139749e8e8..aaedee7456 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -96,8 +96,6 @@ public: using AudioPositionGetter = std::function; using AudioOrientationGetter = std::function; - using RecursiveMutex = std::recursive_mutex; - using RecursiveLock = std::unique_lock; using Mutex = std::mutex; using Lock = std::unique_lock; @@ -345,7 +343,7 @@ private: int16_t _localScratchBuffer[AudioConstants::NETWORK_FRAME_SAMPLES_AMBISONIC]; float* _localOutputMixBuffer { NULL }; AudioInjectorsThread _localAudioThread; - RecursiveMutex _localAudioMutex; + Mutex _localAudioMutex; // for output audio (used by this thread) int _outputPeriod { 0 }; From cdf6c332a6b347eea32eeb645f5ae031edafac0c Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 5 May 2017 15:42:22 -0400 Subject: [PATCH 16/20] prevent overflow of local injectors buffer --- libraries/audio-client/src/AudioClient.cpp | 32 +++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index fb0220c24d..f61429812f 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1097,26 +1097,26 @@ void AudioClient::handleRecordedAudioInput(const QByteArray& audio) { } void AudioClient::prepareLocalAudioInjectors() { - if (_outputPeriod == 0) { - return; - } - - int bufferCapacity = _localInjectorsStream.getSampleCapacity(); - if (_localToOutputResampler) { - // avoid overwriting the buffer, - // instead of failing on writes because the buffer is used as a lock-free pipe - bufferCapacity -= - _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL) * - AudioConstants::STEREO; - bufferCapacity += 1; - } - int samplesNeeded = std::numeric_limits::max(); while (samplesNeeded > 0) { - // lock for every write to avoid locking out the device callback - // this lock is intentional - the buffer is only lock-free in its use in the device callback + // unlock between every write to allow device switching Lock lock(_localAudioMutex); + // in case of a device switch, consider bufferCapacity volatile across iterations + if (_outputPeriod == 0) { + return; + } + + int bufferCapacity = _localInjectorsStream.getSampleCapacity(); + if (_localToOutputResampler) { + // avoid overwriting the buffer, + // instead of failing on writes because the buffer is used as a lock-free pipe + bufferCapacity -= + _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL) * + AudioConstants::STEREO; + bufferCapacity += 1; + } + samplesNeeded = bufferCapacity - _localSamplesAvailable.load(std::memory_order_relaxed); if (samplesNeeded <= 0) { break; From 48af83a960bd231d2574a7de6c5a91dbc2f7c562 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 5 May 2017 17:05:40 -0400 Subject: [PATCH 17/20] guard against audio injector and local buffer datarace --- libraries/audio-client/src/AudioClient.cpp | 24 +++++++++++-------- libraries/audio/src/AbstractAudioInterface.h | 4 ++++ libraries/audio/src/AudioInjector.cpp | 23 ++++++++++++------ libraries/audio/src/AudioInjector.h | 2 ++ .../audio/src/AudioInjectorLocalBuffer.cpp | 3 +-- .../audio/src/AudioInjectorLocalBuffer.h | 2 +- 6 files changed, 38 insertions(+), 20 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index f61429812f..0710250c6a 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1168,16 +1168,18 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { memset(mixBuffer, 0, AudioConstants::NETWORK_FRAME_SAMPLES_STEREO * sizeof(float)); for (AudioInjector* injector : _activeLocalAudioInjectors) { - if (injector->getLocalBuffer()) { + // the lock guarantees that injectorBuffer, if found, is invariant + AudioInjectorLocalBuffer* injectorBuffer; + if ((injectorBuffer = injector->getLocalBuffer())) { static const int HRTF_DATASET_INDEX = 1; int numChannels = injector->isAmbisonic() ? AudioConstants::AMBISONIC : (injector->isStereo() ? AudioConstants::STEREO : AudioConstants::MONO); - qint64 bytesToRead = numChannels * AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL; + size_t bytesToRead = numChannels * AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL; // get one frame from the injector memset(_localScratchBuffer, 0, bytesToRead); - if (0 < injector->getLocalBuffer()->readData((char*)_localScratchBuffer, bytesToRead)) { + if (0 < injectorBuffer->readData((char*)_localScratchBuffer, bytesToRead)) { if (injector->isAmbisonic()) { @@ -1317,15 +1319,17 @@ void AudioClient::setIsStereoInput(bool isStereoInput) { } bool AudioClient::outputLocalInjector(AudioInjector* injector) { - Lock lock(_injectorsMutex); - if (injector->getLocalBuffer() && _audioInput ) { - // just add it to the vector of active local injectors, if - // not already there. - // Since this is invoked with invokeMethod, there _should_ be - // no reason to lock access to the vector of injectors. + AudioInjectorLocalBuffer* injectorBuffer; + if ((injectorBuffer = injector->getLocalBuffer())) { + // local injectors are on the AudioInjectorsThread, so we must guard access + Lock lock(_injectorsMutex); if (!_activeLocalAudioInjectors.contains(injector)) { qCDebug(audioclient) << "adding new injector"; _activeLocalAudioInjectors.append(injector); + + // move local buffer to the LocalAudioThread to avoid dataraces with AudioInjector (like stop()) + injectorBuffer->setParent(nullptr); + injectorBuffer->moveToThread(&_localAudioThread); } else { qCDebug(audioclient) << "injector exists in active list already"; } @@ -1333,7 +1337,7 @@ bool AudioClient::outputLocalInjector(AudioInjector* injector) { return true; } else { - // no local buffer or audio + // no local buffer return false; } } diff --git a/libraries/audio/src/AbstractAudioInterface.h b/libraries/audio/src/AbstractAudioInterface.h index d61b8d60ca..2e4611cd4e 100644 --- a/libraries/audio/src/AbstractAudioInterface.h +++ b/libraries/audio/src/AbstractAudioInterface.h @@ -33,7 +33,11 @@ public: PacketType packetType, QString codecName = QString("")); public slots: + // threadsafe + // moves injector->getLocalBuffer() to another thread (so removes its parent) + // take care to delete it when ~AudioInjector, as parenting Qt semantics will not work virtual bool outputLocalInjector(AudioInjector* injector) = 0; + virtual bool shouldLoopbackInjectors() { return false; } virtual void setIsStereoInput(bool stereo) = 0; diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 86cbc98d84..ce98225190 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -51,6 +51,10 @@ AudioInjector::AudioInjector(const QByteArray& audioData, const AudioInjectorOpt { } +AudioInjector::~AudioInjector() { + deleteLocalBuffer(); +} + bool AudioInjector::stateHas(AudioInjectorState state) const { return (_state & state) == state; } @@ -87,11 +91,7 @@ void AudioInjector::finish() { emit finished(); - if (_localBuffer) { - _localBuffer->stop(); - _localBuffer->deleteLater(); - _localBuffer = NULL; - } + deleteLocalBuffer(); if (stateHas(AudioInjectorState::PendingDelete)) { // we've been asked to delete after finishing, trigger a deleteLater here @@ -163,7 +163,7 @@ bool AudioInjector::injectLocally() { if (_localAudioInterface) { if (_audioData.size() > 0) { - _localBuffer = new AudioInjectorLocalBuffer(_audioData, this); + _localBuffer = new AudioInjectorLocalBuffer(_audioData); _localBuffer->open(QIODevice::ReadOnly); _localBuffer->setShouldLoop(_options.loop); @@ -172,7 +172,8 @@ bool AudioInjector::injectLocally() { _localBuffer->setCurrentOffset(_currentSendOffset); // call this function on the AudioClient's thread - success = QMetaObject::invokeMethod(_localAudioInterface, "outputLocalInjector", Q_ARG(AudioInjector*, this)); + // this will move the local buffer's thread to the LocalInjectorThread + success = _localAudioInterface->outputLocalInjector(this); if (!success) { qCDebug(audio) << "AudioInjector::injectLocally could not output locally via _localAudioInterface"; @@ -185,6 +186,14 @@ bool AudioInjector::injectLocally() { return success; } +void AudioInjector::deleteLocalBuffer() { + if (_localBuffer) { + _localBuffer->stop(); + _localBuffer->deleteLater(); + _localBuffer = nullptr; + } +} + const uchar MAX_INJECTOR_VOLUME = packFloatGainToByte(1.0f); static const int64_t NEXT_FRAME_DELTA_ERROR_OR_FINISHED = -1; static const int64_t NEXT_FRAME_DELTA_IMMEDIATELY = 0; diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index 7d57708738..a901c2520f 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -52,6 +52,7 @@ class AudioInjector : public QObject { public: AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions); AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions); + ~AudioInjector(); bool isFinished() const { return (stateHas(AudioInjectorState::Finished)); } @@ -99,6 +100,7 @@ private: int64_t injectNextFrame(); bool inject(bool(AudioInjectorManager::*injection)(AudioInjector*)); bool injectLocally(); + void deleteLocalBuffer(); static AbstractAudioInterface* _localAudioInterface; diff --git a/libraries/audio/src/AudioInjectorLocalBuffer.cpp b/libraries/audio/src/AudioInjectorLocalBuffer.cpp index 049adb0cc6..7834644baf 100644 --- a/libraries/audio/src/AudioInjectorLocalBuffer.cpp +++ b/libraries/audio/src/AudioInjectorLocalBuffer.cpp @@ -11,8 +11,7 @@ #include "AudioInjectorLocalBuffer.h" -AudioInjectorLocalBuffer::AudioInjectorLocalBuffer(const QByteArray& rawAudioArray, QObject* parent) : - QIODevice(parent), +AudioInjectorLocalBuffer::AudioInjectorLocalBuffer(const QByteArray& rawAudioArray) : _rawAudioArray(rawAudioArray), _shouldLoop(false), _isStopped(false), diff --git a/libraries/audio/src/AudioInjectorLocalBuffer.h b/libraries/audio/src/AudioInjectorLocalBuffer.h index 07d8ae5b9f..673966ff09 100644 --- a/libraries/audio/src/AudioInjectorLocalBuffer.h +++ b/libraries/audio/src/AudioInjectorLocalBuffer.h @@ -19,7 +19,7 @@ class AudioInjectorLocalBuffer : public QIODevice { Q_OBJECT public: - AudioInjectorLocalBuffer(const QByteArray& rawAudioArray, QObject* parent); + AudioInjectorLocalBuffer(const QByteArray& rawAudioArray); void stop(); From ed3fc004a6a2a1f55026e01032fbbc2fbf7c0abc Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 5 May 2017 18:15:56 -0400 Subject: [PATCH 18/20] avoid assignment-as-test --- libraries/audio-client/src/AudioClient.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 0710250c6a..927000d614 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1169,8 +1169,8 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { for (AudioInjector* injector : _activeLocalAudioInjectors) { // the lock guarantees that injectorBuffer, if found, is invariant - AudioInjectorLocalBuffer* injectorBuffer; - if ((injectorBuffer = injector->getLocalBuffer())) { + AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer(); + if (injectorBuffer) { static const int HRTF_DATASET_INDEX = 1; @@ -1319,8 +1319,8 @@ void AudioClient::setIsStereoInput(bool isStereoInput) { } bool AudioClient::outputLocalInjector(AudioInjector* injector) { - AudioInjectorLocalBuffer* injectorBuffer; - if ((injectorBuffer = injector->getLocalBuffer())) { + AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer(); + if (injectorBuffer) { // local injectors are on the AudioInjectorsThread, so we must guard access Lock lock(_injectorsMutex); if (!_activeLocalAudioInjectors.contains(injector)) { From 7db404eba01c7af86a400f4384523775e54b27cc Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 5 May 2017 18:16:37 -0400 Subject: [PATCH 19/20] recast avoid overwriting buffer logic --- libraries/audio-client/src/AudioClient.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 927000d614..680e9129aa 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1108,17 +1108,16 @@ void AudioClient::prepareLocalAudioInjectors() { } int bufferCapacity = _localInjectorsStream.getSampleCapacity(); + int maxOutputSamples = AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL * AudioConstants::STEREO; if (_localToOutputResampler) { - // avoid overwriting the buffer, - // instead of failing on writes because the buffer is used as a lock-free pipe - bufferCapacity -= + maxOutputSamples = _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL) * AudioConstants::STEREO; - bufferCapacity += 1; } samplesNeeded = bufferCapacity - _localSamplesAvailable.load(std::memory_order_relaxed); - if (samplesNeeded <= 0) { + if (samplesNeeded < maxOutputSamples) { + // avoid overwriting the buffer to prevent losing frames break; } From b63015eaea681b8a97c594d486be8cd7e2e3f3db Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Sat, 6 May 2017 14:34:28 -0700 Subject: [PATCH 20/20] Fix sped up animations --- interface/src/avatar/MySkeletonModel.cpp | 4 +--- .../src/avatars-renderer/SkeletonModel.cpp | 6 +++--- .../avatars-renderer/src/avatars-renderer/SkeletonModel.h | 1 + libraries/render-utils/src/CauterizedModel.h | 8 ++++---- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/interface/src/avatar/MySkeletonModel.cpp b/interface/src/avatar/MySkeletonModel.cpp index 639c9f003f..1b9aa4dc18 100644 --- a/interface/src/avatar/MySkeletonModel.cpp +++ b/interface/src/avatar/MySkeletonModel.cpp @@ -140,9 +140,6 @@ void MySkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) { auto orientation = myAvatar->getLocalOrientation(); _rig->computeMotionAnimationState(deltaTime, position, velocity, orientation, ccState); - // evaluate AnimGraph animation and update jointStates. - Model::updateRig(deltaTime, parentTransform); - Rig::EyeParameters eyeParams; eyeParams.eyeLookAt = lookAt; eyeParams.eyeSaccade = head->getSaccade(); @@ -153,6 +150,7 @@ void MySkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) { _rig->updateFromEyeParameters(eyeParams); + // evaluate AnimGraph animation and update jointStates. Parent::updateRig(deltaTime, parentTransform); } diff --git a/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.cpp b/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.cpp index 8fcc859b62..e1e5dc4282 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.cpp @@ -120,7 +120,7 @@ void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) { } // evaluate AnimGraph animation and update jointStates. - Model::updateRig(deltaTime, parentTransform); + Parent::updateRig(deltaTime, parentTransform); } void SkeletonModel::updateAttitude() { @@ -136,7 +136,7 @@ void SkeletonModel::simulate(float deltaTime, bool fullUpdate) { if (fullUpdate) { setBlendshapeCoefficients(_owningAvatar->getHead()->getSummedBlendshapeCoefficients()); - Model::simulate(deltaTime, fullUpdate); + Parent::simulate(deltaTime, fullUpdate); // let rig compute the model offset glm::vec3 registrationPoint; @@ -144,7 +144,7 @@ void SkeletonModel::simulate(float deltaTime, bool fullUpdate) { setOffset(registrationPoint); } } else { - Model::simulate(deltaTime, fullUpdate); + Parent::simulate(deltaTime, fullUpdate); } if (!isActive() || !_owningAvatar->isMyAvatar()) { diff --git a/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.h b/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.h index 23af04e964..059dd245fd 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.h +++ b/libraries/avatars-renderer/src/avatars-renderer/SkeletonModel.h @@ -23,6 +23,7 @@ using SkeletonModelWeakPointer = std::weak_ptr; /// A skeleton loaded from a model. class SkeletonModel : public CauterizedModel { + using Parent = CauterizedModel; Q_OBJECT public: diff --git a/libraries/render-utils/src/CauterizedModel.h b/libraries/render-utils/src/CauterizedModel.h index 39bcedf639..dcff7bd12d 100644 --- a/libraries/render-utils/src/CauterizedModel.h +++ b/libraries/render-utils/src/CauterizedModel.h @@ -28,10 +28,10 @@ public: const std::unordered_set& getCauterizeBoneSet() const { return _cauterizeBoneSet; } void setCauterizeBoneSet(const std::unordered_set& boneSet) { _cauterizeBoneSet = boneSet; } - void deleteGeometry() override; - bool updateGeometry() override; + void deleteGeometry() override; + bool updateGeometry() override; - void createVisibleRenderItemSet() override; + void createVisibleRenderItemSet() override; void createCollisionRenderItemSet() override; virtual void updateClusterMatrices() override; @@ -41,7 +41,7 @@ public: protected: std::unordered_set _cauterizeBoneSet; - QVector _cauterizeMeshStates; + QVector _cauterizeMeshStates; bool _isCauterized { false }; bool _enableCauterization { false }; };