From b15956278c680bf94c38dbd3b8a259f0fc431d4b Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Feb 2017 07:32:32 -0500 Subject: [PATCH 01/45] separate out supporting changes into 21114-part2 branch --- .../src/scripts/EntityScriptServer.cpp | 4 +- .../src/EntityTreeRenderer.cpp | 6 +- .../script-engine/src/BaseScriptEngine.cpp | 321 +++++++++++ .../script-engine/src/BaseScriptEngine.h | 86 +++ libraries/script-engine/src/ScriptCache.h | 3 + libraries/script-engine/src/ScriptEngine.cpp | 535 +++++++++++------- libraries/script-engine/src/ScriptEngine.h | 36 +- libraries/script-engine/src/ScriptEngines.cpp | 35 +- libraries/shared/src/PathUtils.cpp | 27 +- libraries/shared/src/PathUtils.h | 5 + 10 files changed, 834 insertions(+), 224 deletions(-) create mode 100644 libraries/script-engine/src/BaseScriptEngine.cpp create mode 100644 libraries/script-engine/src/BaseScriptEngine.h diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index 930b3946c6..866efa6328 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -395,7 +395,7 @@ void EntityScriptServer::selectAudioFormat(const QString& selectedCodecName) { } void EntityScriptServer::resetEntitiesScriptEngine() { - auto engineName = QString("Entities %1").arg(++_entitiesScriptEngineCount); + auto engineName = QString("about:Entities %1").arg(++_entitiesScriptEngineCount); auto newEngine = QSharedPointer(new ScriptEngine(ScriptEngine::ENTITY_SERVER_SCRIPT, NO_SCRIPT, engineName)); auto webSocketServerConstructorValue = newEngine->newFunction(WebSocketServerClass::constructor); @@ -477,7 +477,7 @@ void EntityScriptServer::checkAndCallPreload(const EntityItemID& entityID, const if (!scriptUrl.isEmpty()) { scriptUrl = ResourceManager::normalizeURL(scriptUrl); qCDebug(entity_script_server) << "Loading entity server script" << scriptUrl << "for" << entityID; - ScriptEngine::loadEntityScript(_entitiesScriptEngine, entityID, scriptUrl, reload); + _entitiesScriptEngine->loadEntityScript(entityID, scriptUrl, reload); } } } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 1265aabbf2..ff39536923 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -101,7 +101,7 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { // Keep a ref to oldEngine until newEngine is ready so EntityScriptingInterface has something to use auto oldEngine = _entitiesScriptEngine; - auto newEngine = new ScriptEngine(ScriptEngine::ENTITY_CLIENT_SCRIPT, NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)); + auto newEngine = new ScriptEngine(ScriptEngine::ENTITY_CLIENT_SCRIPT, NO_SCRIPT, QString("about:Entities %1").arg(++_entitiesScriptEngineCount)); _entitiesScriptEngine = QSharedPointer(newEngine, entitiesScriptEngineDeleter); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); @@ -148,7 +148,7 @@ void EntityTreeRenderer::reloadEntityScripts() { _entitiesScriptEngine->unloadAllEntityScripts(); foreach(auto entity, _entitiesInScene) { if (!entity->getScript().isEmpty()) { - ScriptEngine::loadEntityScript(_entitiesScriptEngine, entity->getEntityItemID(), entity->getScript(), true); + _entitiesScriptEngine->loadEntityScript(entity->getEntityItemID(), entity->getScript(), true); } } } @@ -950,7 +950,7 @@ void EntityTreeRenderer::checkAndCallPreload(const EntityItemID& entityID, const if (entity && entity->shouldPreloadScript() && _entitiesScriptEngine) { QString scriptUrl = entity->getScript(); scriptUrl = ResourceManager::normalizeURL(scriptUrl); - ScriptEngine::loadEntityScript(_entitiesScriptEngine, entityID, scriptUrl, reload); + _entitiesScriptEngine->loadEntityScript(entityID, scriptUrl, reload); entity->scriptHasPreloaded(); } } diff --git a/libraries/script-engine/src/BaseScriptEngine.cpp b/libraries/script-engine/src/BaseScriptEngine.cpp new file mode 100644 index 0000000000..4342205414 --- /dev/null +++ b/libraries/script-engine/src/BaseScriptEngine.cpp @@ -0,0 +1,321 @@ +// +// BaseScriptEngine.cpp +// libraries/script-engine/src +// +// Created by Timothy Dedischew on 02/01/17. +// Copyright 2017 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 "BaseScriptEngine.h" + +#include +#include +#include +#include +#include +#include + +#include "ScriptEngineLogging.h" +#include "Profile.h" + +const QString BaseScriptEngine::_SETTINGS_ENABLE_EXTENDED_EXCEPTIONS { + "com.highfidelity.experimental.enableExtendedJSExceptions" +}; + +const QString BaseScriptEngine::SCRIPT_EXCEPTION_FORMAT { "[%0] %1 in %2:%3" }; +const QString BaseScriptEngine::SCRIPT_BACKTRACE_SEP { "\n " }; + +// engine-aware JS Error copier and factory +QScriptValue BaseScriptEngine::makeError(const QScriptValue& _other, const QString& type) { + auto other = _other; + if (other.isString()) { + other = newObject(); + other.setProperty("message", _other.toString()); + } + auto proto = globalObject().property(type); + if (!proto.isFunction()) { + proto = globalObject().property(other.prototype().property("constructor").property("name").toString()); + } + if (!proto.isFunction()) { +#ifdef DEBUG_JS_EXCEPTIONS + qCDebug(scriptengine) << "BaseScriptEngine::makeError -- couldn't find constructor for" << type << " -- using Error instead"; +#endif + proto = globalObject().property("Error"); + } + if (other.engine() != this) { + // JS Objects are parented to a specific script engine instance + // -- this effectively ~clones it locally by routing through a QVariant and back + other = toScriptValue(other.toVariant()); + } + // ~ var err = new Error(other.message) + auto err = proto.construct(QScriptValueList({other.property("message")})); + + // transfer over any existing properties + QScriptValueIterator it(other); + while (it.hasNext()) { + it.next(); + err.setProperty(it.name(), it.value()); + } + return err; +} + +// check syntax and when there are issues returns an actual "SyntaxError" with the details +QScriptValue BaseScriptEngine::lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber) { + const auto syntaxCheck = checkSyntax(sourceCode); + if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) { + auto err = globalObject().property("SyntaxError") + .construct(QScriptValueList({syntaxCheck.errorMessage()})); + err.setProperty("fileName", fileName); + err.setProperty("lineNumber", syntaxCheck.errorLineNumber()); + err.setProperty("expressionBeginOffset", syntaxCheck.errorColumnNumber()); + err.setProperty("stack", currentContext()->backtrace().join(SCRIPT_BACKTRACE_SEP)); + { + const auto error = syntaxCheck.errorMessage(); + const auto line = QString::number(syntaxCheck.errorLineNumber()); + const auto column = QString::number(syntaxCheck.errorColumnNumber()); + // for compatibility with legacy reporting + const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, fileName, line, column); + err.setProperty("formatted", message); + } + return err; + } + return undefinedValue(); +} + +// this pulls from the best available information to create a detailed snapshot of the current exception +QScriptValue BaseScriptEngine::cloneUncaughtException(const QString& extraDetail) { + if (!hasUncaughtException()) { + return QScriptValue(); + } + auto exception = uncaughtException(); + // ensure the error object is engine-local + auto err = makeError(exception); + + // not sure why Qt does't offer uncaughtExceptionFileName -- but the line number + // on its own is often useless/wrong if arbitrarily married to a filename. + // when the error object already has this info, it seems to be the most reliable + auto fileName = exception.property("fileName").toString(); + auto lineNumber = exception.property("lineNumber").toInt32(); + + // the backtrace, on the other hand, seems most reliable taken from uncaughtExceptionBacktrace + auto backtrace = uncaughtExceptionBacktrace(); + if (backtrace.isEmpty()) { + // fallback to the error object + backtrace = exception.property("stack").toString().split(SCRIPT_BACKTRACE_SEP); + } + // the ad hoc "detail" property can be used now to embed additional clues + auto detail = exception.property("detail").toString(); + if (detail.isEmpty()) { + detail = extraDetail; + } else if (!extraDetail.isEmpty()) { + detail += "(" + extraDetail + ")"; + } + if (lineNumber <= 0) { + lineNumber = uncaughtExceptionLineNumber(); + } + if (fileName.isEmpty()) { + // climb the stack frames looking for something useful to display + for(auto ctx = currentContext(); ctx && fileName.isEmpty(); ctx = ctx->parentContext()) { + QScriptContextInfo info { ctx }; + if (!info.fileName().isEmpty()) { + // take fileName:lineNumber as a pair + fileName = info.fileName(); + lineNumber = info.lineNumber(); + if (backtrace.isEmpty()) { + backtrace = ctx->backtrace(); + } + break; + } + } + } + err.setProperty("fileName", fileName); + err.setProperty("lineNumber", lineNumber ); + err.setProperty("detail", detail); + err.setProperty("stack", backtrace.join(SCRIPT_BACKTRACE_SEP)); + +#ifdef DEBUG_JS_EXCEPTIONS + err.setProperty("_fileName", exception.property("fileName").toString()); + err.setProperty("_stack", uncaughtExceptionBacktrace().join(SCRIPT_BACKTRACE_SEP)); + err.setProperty("_lineNumber", uncaughtExceptionLineNumber()); +#endif + return err; +} + +QString BaseScriptEngine::formatException(const QScriptValue& exception) { + QString note { "UncaughtException" }; + QString result; + + if (!exception.isObject()) { + return result; + } + const auto message = exception.toString(); + const auto fileName = exception.property("fileName").toString(); + const auto lineNumber = exception.property("lineNumber").toString(); + const auto stacktrace = exception.property("stack").toString(); + + if (_enableExtendedJSExceptions.get()) { + // This setting toggles display of the hints now being added during the loading process. + // Example difference: + // [UncaughtExceptions] Error: Can't find variable: foobar in atp:/myentity.js\n... + // [UncaughtException (construct {1eb5d3fa-23b1-411c-af83-163af7220e3f})] Error: Can't find variable: foobar in atp:/myentity.js\n... + if (exception.property("detail").isValid()) { + note += " " + exception.property("detail").toString(); + } + } + + result = QString(SCRIPT_EXCEPTION_FORMAT).arg(note, message, fileName, lineNumber); + if (!stacktrace.isEmpty()) { + result += QString("\n[Backtrace]%1%2").arg(SCRIPT_BACKTRACE_SEP).arg(stacktrace); + } + return result; +} + +QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, const QScriptProgram& program) { + PROFILE_RANGE(script, "evaluateInClosure"); + if (QThread::currentThread() != thread()) { + qCCritical(scriptengine) << "*** CRITICAL *** ScriptEngine::evaluateInClosure() is meant to be called from engine thread only."; + // note: a recursive mutex might be needed around below code if this method ever becomes Q_INVOKABLE + return QScriptValue(); + } + + //_debugDump("evaluateInClosure.closure", closure); + + const auto fileName = program.fileName(); + const auto shortName = QUrl(fileName).fileName(); + + QScriptValue result; + QScriptValue oldGlobal; + auto global = closure.property("global"); + if (global.isObject()) { +#ifdef DEBUG_JS + qCDebug(scriptengine) << " setting global = closure.global" << shortName; +#endif + oldGlobal = globalObject(); + setGlobalObject(global); + } + + auto ctx = pushContext(); + + auto thiz = closure.property("this"); + if (thiz.isObject()) { +#ifdef DEBUG_JS + qCDebug(scriptengine) << " setting this = closure.this" << shortName; +#endif + ctx->setThisObject(thiz); + } + + ctx->pushScope(closure); +#ifdef DEBUG_JS + qCDebug(scriptengine) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName); +#endif + { + ExceptionEmitter tryCatch(this, __FUNCTION__); + result = BaseScriptEngine::evaluate(program); + if (tryCatch.hasPending()) { + auto err = tryCatch.pending(); +#ifdef DEBUG_JS_EXCEPTIONS + qCWarning(scriptengine) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString(); + err.setProperty("_result", result); +#endif + result = err; + } + } +#ifdef DEBUG_JS + qCDebug(scriptengine) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName); +#endif + popContext(); + + if (oldGlobal.isValid()) { +#ifdef DEBUG_JS + qCDebug(scriptengine) << " restoring global" << shortName; +#endif + setGlobalObject(oldGlobal); + } + + return result; +} + +// Lambda + +QScriptValue BaseScriptEngine::newLambdaFunction(std::function operation, const QScriptValue& data, const QScriptEngine::ValueOwnership& ownership) { + auto lambda = new Lambda(this, operation, data); + auto object = newQObject(lambda, ownership); + auto call = object.property("call"); + call.setPrototype(object); // ctx->callee().prototype() === Lambda QObject + call.setData(data); // ctx->callee().data() will === data param + return call; +} +QString Lambda::toString() const { + return QString("[Lambda%1]").arg(data.isValid() ? " " + data.toString() : data.toString()); +} + +Lambda::~Lambda() { +#ifdef DEBUG_JS_LAMBDA_FUNCS + qDebug() << "~Lambda" << "this" << this; +#endif +} + +Lambda::Lambda(QScriptEngine *engine, std::function operation, QScriptValue data) + : engine(engine), operation(operation), data(data) { +#ifdef DEBUG_JS_LAMBDA_FUNCS + qDebug() << "Lambda" << data.toString(); +#endif +} +QScriptValue Lambda::call() { + return operation(engine->currentContext(), engine); +} + +// BaseScriptEngine::ExceptionEmitter + +void BaseScriptEngine::_emitUnhandledException(const QScriptValue& exception) { + emit unhandledException(exception); +} +BaseScriptEngine::ExceptionEmitter::ExceptionEmitter(BaseScriptEngine* engine, const QString& debugName) + : _engine(engine), _debugName(debugName) { +} +bool BaseScriptEngine::ExceptionEmitter::hasPending() { + return _engine->hasUncaughtException(); +} +QScriptValue BaseScriptEngine::ExceptionEmitter::pending() { + return _engine->cloneUncaughtException(_debugName); +} +QScriptValue BaseScriptEngine::ExceptionEmitter::consume() { + if (hasPending()) { + _consumedException = pending(); + _engine->clearExceptions(); + } + return _consumedException; +} +bool BaseScriptEngine::ExceptionEmitter::wouldEmit() { + return !_engine->isEvaluating() && _engine->hasUncaughtException(); +} +BaseScriptEngine::ExceptionEmitter::~ExceptionEmitter() { + if (wouldEmit()) { + _engine->_emitUnhandledException(consume()); + } +} + + +#ifdef DEBUG_JS +void BaseScriptEngine::_debugDump(const QString& header, const QScriptValue& object, const QString& footer) { + if (!header.isEmpty()) { + qCDebug(scriptengine) << header; + } + if (!object.isObject()) { + qCDebug(scriptengine) << "(!isObject)" << object.toVariant().toString() << object.toString(); + return; + } + QScriptValueIterator it(object); + while (it.hasNext()) { + it.next(); + qCDebug(scriptengine) << it.name() << ":" << it.value().toString(); + } + if (!footer.isEmpty()) { + qCDebug(scriptengine) << footer; + } +} +#endif + diff --git a/libraries/script-engine/src/BaseScriptEngine.h b/libraries/script-engine/src/BaseScriptEngine.h new file mode 100644 index 0000000000..3ff4c22e4b --- /dev/null +++ b/libraries/script-engine/src/BaseScriptEngine.h @@ -0,0 +1,86 @@ +// +// BaseScriptEngine.h +// libraries/script-engine/src +// +// Created by Timothy Dedischew on 02/01/17. +// Copyright 2017 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_BaseScriptEngine_h +#define hifi_BaseScriptEngine_h + +#include +#include +#include + +#include "SettingHandle.h" + +// common base class for extending QScriptEngine itself +class BaseScriptEngine : public QScriptEngine { + Q_OBJECT +public: + static const QString SCRIPT_EXCEPTION_FORMAT; + static const QString SCRIPT_BACKTRACE_SEP; + + BaseScriptEngine() {} + + Q_INVOKABLE QScriptValue evaluateInClosure(const QScriptValue& locals, const QScriptProgram& program); + + Q_INVOKABLE QScriptValue lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber = 1); + Q_INVOKABLE QScriptValue makeError(const QScriptValue& other = QScriptValue(), const QString& type = "Error"); + Q_INVOKABLE QString formatException(const QScriptValue& exception); + QScriptValue cloneUncaughtException(const QString& detail = QString()); + +signals: + void unhandledException(const QScriptValue& exception); + +protected: + void _emitUnhandledException(const QScriptValue& exception); + QScriptValue newLambdaFunction(std::function operation, const QScriptValue& data = QScriptValue(), const QScriptEngine::ValueOwnership& ownership = QScriptEngine::AutoOwnership); + + static const QString _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS; + Setting::Handle _enableExtendedJSExceptions { _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS, true }; +#ifdef DEBUG_JS + static void _debugDump(const QString& header, const QScriptValue& object, const QString& footer = QString()); +#endif + + // ExceptionEmitter can be placed above calls that might throw JS exceptions and it'll + // automatically signal BaseScriptEngine when falling out of scope (recursion aware). + class ExceptionEmitter { + public: + ExceptionEmitter(BaseScriptEngine* engine, const QString& debugName = QString()); + // is there a pending exception? + bool hasPending(); + QScriptValue pending(); + // would it be emitted at scope's end? + bool wouldEmit(); + // consume and return the pending exception + QScriptValue consume(); + ~ExceptionEmitter(); + protected: + BaseScriptEngine* _engine; + QString _debugName; + QScriptValue _consumedException; + }; +}; + +// Lambda helps create callable QScriptValues out of std::functions: +// (just meant for use from within the script engine itself) +class Lambda : public QObject { + Q_OBJECT +public: + Lambda(QScriptEngine *engine, std::function operation, QScriptValue data); + ~Lambda(); + public slots: + QScriptValue call(); + QString toString() const; +private: + QScriptEngine* engine; + std::function operation; + QScriptValue data; +}; + +#endif // hifi_BaseScriptEngine_h diff --git a/libraries/script-engine/src/ScriptCache.h b/libraries/script-engine/src/ScriptCache.h index 6cc318cc15..511d392409 100644 --- a/libraries/script-engine/src/ScriptCache.h +++ b/libraries/script-engine/src/ScriptCache.h @@ -43,6 +43,9 @@ class ScriptCache : public QObject, public Dependency { public: static const QString STATUS_INLINE; static const QString STATUS_CACHED; + static bool isSuccessStatus(const QString& status) { + return status == "Success" || status == STATUS_INLINE || status == STATUS_CACHED; + } void clearCache(); Q_INVOKABLE void clearATPScriptsFromCache(); diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index f1ff4c4686..262823a85d 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -49,6 +49,7 @@ #include "ArrayBufferViewClass.h" #include "BatchLoader.h" +#include "BaseScriptEngine.h" #include "DataViewClass.h" #include "EventTypes.h" #include "FileScriptingInterface.h" // unzip project @@ -68,9 +69,12 @@ #include "MIDIEvent.h" -const QString BaseScriptEngine::SCRIPT_EXCEPTION_FORMAT { "[UncaughtException] %1 in %2:%3" }; static const QScriptEngine::QObjectWrapOptions DEFAULT_QOBJECT_WRAP_OPTIONS = QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects; +static const QScriptValue::PropertyFlags READONLY_PROP_FLAGS { QScriptValue::ReadOnly | QScriptValue::Undeletable }; +static const QScriptValue::PropertyFlags HIDDEN_PROP_FLAGS { READONLY_PROP_FLAGS | QScriptValue::SkipInEnumeration }; + + static const bool HIFI_AUTOREFRESH_FILE_SCRIPTS { true }; @@ -89,13 +93,9 @@ static QScriptValue debugPrint(QScriptContext* context, QScriptEngine* engine){ } qCDebug(scriptengineScript).noquote() << "script:print()<<" << message; // noquote() so that \n is treated as newline - message = message.replace("\\", "\\\\") - .replace("\n", "\\n") - .replace("\r", "\\r") - .replace("'", "\\'"); - - // FIXME - this approach neeeds revisiting. print() comes here, which ends up doing an evaluate? - engine->evaluate("Script.print('" + message + "')"); + // 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 })); return QScriptValue(); } @@ -139,52 +139,15 @@ QString encodeEntityIdIntoEntityUrl(const QString& url, const QString& entityID) return url + " [EntityID:" + entityID + "]"; } -QString BaseScriptEngine::lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber) { - const auto syntaxCheck = checkSyntax(sourceCode); - if (syntaxCheck.state() != syntaxCheck.Valid) { - const auto error = syntaxCheck.errorMessage(); - const auto line = QString::number(syntaxCheck.errorLineNumber()); - const auto column = QString::number(syntaxCheck.errorColumnNumber()); - const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, fileName, line, column); - return message; - } - return QString(); -} - -QString BaseScriptEngine::formatUncaughtException(const QString& overrideFileName) { - QString message; - if (hasUncaughtException()) { - const auto error = uncaughtException(); - const auto backtrace = uncaughtExceptionBacktrace(); - const auto exception = error.toString(); - auto filename = overrideFileName; - if (filename.isEmpty()) { - QScriptContextInfo ctx { currentContext() }; - filename = ctx.fileName(); - } - const auto line = QString::number(uncaughtExceptionLineNumber()); - - message = QString(SCRIPT_EXCEPTION_FORMAT).arg(exception, overrideFileName, line); - if (!backtrace.empty()) { - static const auto lineSeparator = "\n "; - message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator)); - } - } - return message; -} - -QString ScriptEngine::reportUncaughtException(const QString& overrideFileName) { - QString message; - if (!hasUncaughtException()) { - return message; - } - message = formatUncaughtException(overrideFileName.isEmpty() ? _fileNameString : overrideFileName); +QString ScriptEngine::logException(const QScriptValue& exception) { + auto message = formatException(exception); scriptErrorMessage(qPrintable(message)); return message; } int ScriptEngine::processLevelMaxRetries { ScriptRequest::MAX_RETRIES }; ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const QString& fileNameString) : + BaseScriptEngine(), _context(context), _scriptContents(scriptContents), _timerFunctionMap(), @@ -194,8 +157,14 @@ ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const DependencyManager::get()->addScriptEngine(this); connect(this, &QScriptEngine::signalHandlerException, this, [this](const QScriptValue& exception) { - reportUncaughtException(); - clearExceptions(); + if (hasUncaughtException()) { + // the engine's uncaughtException() seems to produce much better stack traces here + emit unhandledException(cloneUncaughtException("signalHandlerException")); + clearExceptions(); + } else { + // ... but may not always be available -- so if needed we fallback to the passed exception + emit unhandledException(exception); + } }); setProcessEventsInterval(MSECS_PER_SECOND); @@ -203,7 +172,16 @@ ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const qCDebug(scriptengine) << "isEntityServerScript() -- limiting maxRetries to 1"; processLevelMaxRetries = 1; } - qCDebug(scriptengine) << getContext() << "processLevelMaxRetries =" << processLevelMaxRetries; + //qCDebug(scriptengine) << getContext() << "processLevelMaxRetries =" << processLevelMaxRetries << fileNameString; + + // this is where all unhandled exceptions end up getting logged + connect(this, &BaseScriptEngine::unhandledException, this, [this](const QScriptValue& err) { + auto output = err.engine() == this ? err : makeError(err); + if (!output.property("detail").isValid()) { + output.setProperty("detail", "UnhandledException"); + } + logException(output); + }); } QString ScriptEngine::getContext() const { @@ -223,13 +201,22 @@ QString ScriptEngine::getContext() const { } ScriptEngine::~ScriptEngine() { + // FIXME: are these scriptInfoMessage/scriptWarningMessage segfaulting anybody else at app shutdown? +#if !defined(Q_OS_LINUX) scriptInfoMessage("Script Engine shutting down:" + getFilename()); +#else + qCDebug(scriptengine) << "~ScriptEngine()" << this; +#endif auto scriptEngines = DependencyManager::get(); if (scriptEngines) { scriptEngines->removeScriptEngine(this); } else { +#if !defined(Q_OS_LINUX) scriptWarningMessage("Script destroyed after ScriptEngines!"); +#else + qCWarning(scriptengine) << ("Script destroyed after ScriptEngines!"); +#endif } } @@ -319,9 +306,12 @@ void ScriptEngine::runDebuggable() { } } _lastUpdate = now; - // Debug and clear exceptions - if (hasUncaughtException()) { - reportUncaughtException(); + + // only clear exceptions if we are not in the middle of evaluating + if (!isEvaluating() && hasUncaughtException()) { + qCWarning(scriptengine) << __FUNCTION__ << "---------- UNCAUGHT EXCEPTION --------"; + qCWarning(scriptengine) << "runDebuggable" << uncaughtException().toString(); + logException(__FUNCTION__); clearExceptions(); } }); @@ -356,10 +346,9 @@ void ScriptEngine::runInThread() { workerThread->start(); } -void ScriptEngine::executeOnScriptThread(std::function function, bool blocking ) { +void ScriptEngine::executeOnScriptThread(std::function function, const Qt::ConnectionType& type ) { if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "executeOnScriptThread", blocking ? Qt::BlockingQueuedConnection : Qt::QueuedConnection, - Q_ARG(std::function, function)); + QMetaObject::invokeMethod(this, "executeOnScriptThread", type, Q_ARG(std::function, function)); return; } @@ -558,6 +547,7 @@ void ScriptEngine::init() { qCWarning(scriptengine) << "deletingEntity while entity script is still running!" << entityID; } _entityScripts.remove(entityID); + emit entityScriptDetailsUpdated(); } }); @@ -589,8 +579,7 @@ void ScriptEngine::init() { QScriptValue webSocketConstructorValue = newFunction(WebSocketClass::constructor); globalObject().setProperty("WebSocket", webSocketConstructorValue); - QScriptValue printConstructorValue = newFunction(debugPrint); - globalObject().setProperty("print", printConstructorValue); + globalObject().setProperty("print", newFunction(debugPrint)); QScriptValue audioEffectOptionsConstructorValue = newFunction(AudioEffectOptions::constructor); globalObject().setProperty("AudioEffectOptions", audioEffectOptionsConstructorValue); @@ -604,6 +593,7 @@ void ScriptEngine::init() { qScriptRegisterMetaType(this, wscReadyStateToScriptValue, wscReadyStateFromScriptValue); registerGlobalObject("Script", this); + registerGlobalObject("Audio", &AudioScriptingInterface::getInstance()); registerGlobalObject("Entities", entityScriptingInterface.data()); registerGlobalObject("Quat", &_quatLibrary); @@ -870,7 +860,6 @@ void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString& handlersForEvent << handlerData; // Note that the same handler can be added many times. See removeEntityEventHandler(). } - QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fileName, int lineNumber) { if (DependencyManager::get()->isStopped()) { return QScriptValue(); // bail early @@ -892,23 +881,28 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi // Check syntax auto syntaxError = lintScript(sourceCode, fileName); + if (syntaxError.isError()) { + if (isEvaluating()) { + currentContext()->throwValue(syntaxError); + } else { + syntaxError.setProperty("detail", "evaluate"); + emit unhandledException(syntaxError); + } + return syntaxError; + } QScriptProgram program { sourceCode, fileName, lineNumber }; - if (!syntaxError.isEmpty() || program.isNull()) { - scriptErrorMessage(qPrintable(syntaxError)); - return QScriptValue(); + if (program.isNull()) { + // can this happen? + auto err = makeError("could not create QScriptProgram for " + fileName); + emit unhandledException(err); + return err; } - ++_evaluatesPending; - auto result = BaseScriptEngine::evaluate(program); - --_evaluatesPending; - - if (hasUncaughtException()) { - result = uncaughtException(); - reportUncaughtException(program.fileName()); - emit evaluationFinished(result, true); - clearExceptions(); - } else { - emit evaluationFinished(result, false); + QScriptValue result; + { + // catch any exceptions throw by nested operations + ExceptionEmitter tryCatch(this, __FUNCTION__); + result = BaseScriptEngine::evaluate(program); } return result; } @@ -929,8 +923,11 @@ void ScriptEngine::run() { _isRunning = true; emit runningStateChanged(); - QScriptValue result = evaluate(_scriptContents, _fileNameString); - + { + // catch any exceptions throw by nested operations + ExceptionEmitter tryCatch(this, __FUNCTION__); + evaluate(_scriptContents, _fileNameString); + } #ifdef _WIN32 // VS13 does not sleep_until unless it uses the system_clock, see: // https://www.reddit.com/r/cpp_questions/comments/3o71ic/sleep_until_not_working_with_a_time_pointsteady/ @@ -1057,13 +1054,14 @@ void ScriptEngine::run() { } _lastUpdate = now; - // Debug and clear exceptions - if (hasUncaughtException()) { - reportUncaughtException(); + // only clear exceptions if we are not in the middle of evaluating + if (!isEvaluating() && hasUncaughtException()) { + qCWarning(scriptengine) << __FUNCTION__ << "---------- UNCAUGHT EXCEPTION --------"; + qCWarning(scriptengine) << "runInThread" << uncaughtException().toString(); + emit unhandledException(cloneUncaughtException(__FUNCTION__)); clearExceptions(); } } - scriptInfoMessage("Script Engine stopping:" + getFilename()); stopAllTimers(); // make sure all our timers are stopped if the script is ending @@ -1096,9 +1094,11 @@ void ScriptEngine::run() { // we want to only call it in our own run "shutdown" processing. void ScriptEngine::stopAllTimers() { QMutableHashIterator i(_timerFunctionMap); + int j {0}; while (i.hasNext()) { i.next(); QTimer* timer = i.key(); + qCDebug(scriptengine) << getFilename() << "stopAllTimers[" << j++ << "]"; stopTimer(timer); } } @@ -1185,6 +1185,7 @@ void ScriptEngine::timerFired() { _timerFunctionMap.remove(callingTimer); delete callingTimer; } + //qCWarning(scriptengine) << "timerFired" << timerData.function.toString(); // call the associated JS function, if it exists if (timerData.function.isValid()) { @@ -1193,11 +1194,11 @@ void ScriptEngine::timerFired() { auto postTimer = p_high_resolution_clock::now(); auto elapsed = (postTimer - preTimer); _totalTimerExecution += std::chrono::duration_cast(elapsed); - + } else { + qCWarning(scriptengine) << "timerFired -- invalid function" << timerData.function.toVariant().toString(); } } - QObject* ScriptEngine::setupTimerWithInterval(const QScriptValue& function, int intervalMS, bool isSingleShot) { // create the timer, add it to the map, and start it QTimer* newTimer = new QTimer(this); @@ -1214,7 +1215,7 @@ QObject* ScriptEngine::setupTimerWithInterval(const QScriptValue& function, int // make sure the timer stops when the script does connect(this, &ScriptEngine::scriptEnding, newTimer, &QTimer::stop); - CallbackData timerData = {function, currentEntityIdentifier, currentSandboxURL }; + CallbackData timerData = { function, currentEntityIdentifier, currentSandboxURL }; _timerFunctionMap.insert(newTimer, timerData); newTimer->start(intervalMS); @@ -1244,33 +1245,44 @@ void ScriptEngine::stopTimer(QTimer *timer) { timer->stop(); _timerFunctionMap.remove(timer); delete timer; + } else { + qCDebug(scriptengine) << "stopTimer -- not in _timerFunctionMap" << timer; } } QUrl ScriptEngine::resolvePath(const QString& include) const { QUrl url(include); // first lets check to see if it's already a full URL - if (!url.scheme().isEmpty()) { + if (include.startsWith("/") || url.scheme().length() == 1) { + url = QUrl::fromLocalFile(include); + } + if (!url.isRelative()) { return expandScriptUrl(url); } - QScriptContextInfo contextInfo { currentContext()->parentContext() }; - - // we apparently weren't a fully qualified url, so, let's assume we're relative - // to the original URL of our script - QUrl parentURL = contextInfo.fileName(); - if (parentURL.isEmpty()) { - if (_parentURL.isEmpty()) { - parentURL = QUrl(_fileNameString); - } else { - parentURL = QUrl(_parentURL); - } + // to the first absolute URL in the JS scope chain + QUrl parentURL; + auto ctx = currentContext(); + do { + QScriptContextInfo contextInfo { ctx }; + parentURL = QUrl(contextInfo.fileName()); + ctx = ctx->parentContext(); + } while (parentURL.isRelative() && ctx); + + if (parentURL.isRelative()) { + // fallback to the "include" parent (if defined, this will already be absolute) + parentURL = QUrl(_parentURL); } - // if the parent URL's scheme is empty, then this is probably a local file... - if (parentURL.scheme().isEmpty()) { - parentURL = QUrl::fromLocalFile(_fileNameString); + if (parentURL.isRelative()) { + // fallback to the original script engine URL + parentURL = QUrl(_fileNameString); + + // if still relative and path-like, then this is probably a local file... + if (parentURL.isRelative() && url.path().contains("/")) { + parentURL = QUrl::fromLocalFile(_fileNameString); + } } // at this point we should have a legitimate fully qualified URL for our parent @@ -1297,22 +1309,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac return; // bail early } QList urls; - bool knowsSensitivity = false; - Qt::CaseSensitivity sensitivity { Qt::CaseSensitive }; - auto getSensitivity = [&]() { - if (!knowsSensitivity) { - QString path = currentSandboxURL.path(); - QFileInfo upperFI(path.toUpper()); - QFileInfo lowerFI(path.toLower()); - sensitivity = (upperFI == lowerFI) ? Qt::CaseInsensitive : Qt::CaseSensitive; - knowsSensitivity = true; - } - return sensitivity; - }; - // Guard against meaningless query and fragment parts. - // Do NOT use PreferLocalFile as its behavior is unpredictable (e.g., on defaultScriptsLocation()) - const auto strippingFlags = QUrl::RemoveFilename | QUrl::RemoveQuery | QUrl::RemoveFragment; for (QString includeFile : includeFiles) { QString file = ResourceManager::normalizeURL(includeFile); QUrl thisURL; @@ -1329,10 +1326,8 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac thisURL = resolvePath(file); } - if (!isStandardLibrary && !currentSandboxURL.isEmpty() && (thisURL.scheme() == "file") && - (currentSandboxURL.scheme() != "file" || - !thisURL.toString(strippingFlags).startsWith(currentSandboxURL.toString(strippingFlags), getSensitivity()))) { - + bool disallowOutsideFiles = thisURL.isLocalFile() && !isStandardLibrary && !currentSandboxURL.isLocalFile(); + if (disallowOutsideFiles && !PathUtils::isDescendantOf(thisURL, currentSandboxURL)) { scriptWarningMessage("Script.include() ignoring file path" + thisURL.toString() + "outside of original entity script" + currentSandboxURL.toString()); } else { @@ -1368,7 +1363,17 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac evaluate(contents, url.toString()); }; + ExceptionEmitter tryCatch(this, "include"); doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation); + if (tryCatch.hasPending()) { + // match previous include behavior where possible by logging, not propagating, exceptions. + // for nested evaluations, exceptions are now allowed to propagate + auto err = tryCatch.pending(); + emit unhandledException(err); + if (tryCatch.wouldEmit()) { + tryCatch.consume(); + } + } } else { scriptWarningMessage("Script.include() skipping evaluation of previously included url:" + url.toString()); } @@ -1470,21 +1475,6 @@ int ScriptEngine::getNumRunningEntityScripts() const { return sum; } -QString ScriptEngine::getEntityScriptStatus(const EntityItemID& entityID) { - if (_entityScripts.contains(entityID)) - return EntityScriptStatus_::valueToKey(_entityScripts[entityID].status).toLower(); - return QString(); -} - -bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const { - auto it = _entityScripts.constFind(entityID); - if (it == _entityScripts.constEnd()) { - return false; - } - details = it.value(); - return true; -} - void ScriptEngine::setEntityScriptDetails(const EntityItemID& entityID, const EntityScriptDetails& details) { _entityScripts[entityID] = details; emit entityScriptDetailsUpdated(); @@ -1497,31 +1487,140 @@ void ScriptEngine::updateEntityScriptStatus(const EntityItemID& entityID, const emit entityScriptDetailsUpdated(); } -// since all of these operations can be asynch we will always do the actual work in the response handler -// for the download -void ScriptEngine::loadEntityScript(QWeakPointer theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { - auto engine = theEngine.data(); - engine->executeOnScriptThread([=]{ - EntityScriptDetails details = engine->_entityScripts[entityID]; - if (details.status == EntityScriptStatus::PENDING || details.status == EntityScriptStatus::UNLOADED) { - engine->updateEntityScriptStatus(entityID, EntityScriptStatus::LOADING, QThread::currentThread()->objectName()); - } - }); +bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const { + auto it = _entityScripts.constFind(entityID); + if (it == _entityScripts.constEnd()) { + return false; + } + details = it.value(); + return true; +} - // NOTE: If the script content is not currently in the cache, the LAMBDA here will be called on the Main Thread - // which means we're guaranteed that it's not the correct thread for the ScriptEngine. This means - // when we get into entityScriptContentAvailable() we will likely invokeMethod() to get it over - // to the "Entities" ScriptEngine thread. - DependencyManager::get()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success, const QString &status) { - QSharedPointer strongEngine = theEngine.toStrongRef(); - if (strongEngine) { -#ifdef THREAD_DEBUGGING - qCDebug(scriptengine) << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread [" - << QThread::currentThread() << "] expected thread [" << strongEngine->thread() << "]"; -#endif - strongEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success, status); +const auto RETRY_ATTEMPT_BATCH = 10; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH").toInt(), 1, 50); +const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000); + +void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { + // forceRedownload doesn't make sense yet in this scenario (which only happens when another entity has taken lead on entityScript) + if (forceRedownload) { + qCDebug(scriptengine) << "deferLoadEntityScript -- TODO: ignoring forceRedownload =" << forceRedownload; + } + + const auto needTimer = _pendingLoadEntities.isEmpty(); + _pendingLoadEntities.push_back({ entityID, entityScript/*, forceRedownload*/ }); + + auto processDeferredEntities = [this](QScriptContext* ctx, QScriptEngine*) -> QScriptValue { + //qCDebug(scriptengine) << "deferLoadEntityScript.retry #" << _pendingLoadEntities.size() << " pending loads | " << QThread::currentThread(); + + for(int i=0; i < RETRY_ATTEMPT_BATCH && !_pendingLoadEntities.isEmpty(); i++) { + auto retry = _pendingLoadEntities.takeFirst(); + qCDebug(scriptengine) << "deferLoadEntityScript.retry[" << i << "]:" << retry.entityID << retry.entityScript; + if(!_entityScripts.contains(retry.entityID)) { + qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity details gone (entity deleted?)"; + } else { + auto details = _entityScripts[retry.entityID]; + if (details.status != EntityScriptStatus::PENDING) { + qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity status no longer PENDING; " << details.status; + } else { + loadEntityScript(retry.entityID, retry.entityScript, false); + } + } } - }, forceRedownload, processLevelMaxRetries); + if (_pendingLoadEntities.isEmpty()) { + QObject *interval = ctx->callee().property("interval").toQObject(); + // we're done processing so can kill the timer + qCDebug(scriptengine) << "loadEntityScript.retry queue processing complete -- exiting" << interval; + if (interval) { + clearInterval(interval); + } + } + return QScriptValue(); + }; + + if (needTimer) { + // we were the first to get queued -- so set up an interval timer for deferred processing + qCDebug(scriptengine) << "deferLoadEntityScript -- scheduling timer " << RETRY_ATTEMPT_MSECS; + + // processDeferredEntities gets wrapped as a "regular" JavaScript callback so that it can then + // be scheduled using setInterval per usual -- allowing the script engine to own its lifecycle. + // we stil need to call clearInterval manually when done, so its passed to calle() above as a property. + + QScriptValue function = newLambdaFunction(processDeferredEntities, "processDeferredEntities-loader"); + QObject *interval = setInterval( function, RETRY_ATTEMPT_MSECS ); + + // make the interval reachable from the handler + function.setProperty("interval", newQObject(interval)); + } +} + +void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "loadEntityScript", + Q_ARG(const EntityItemID&, entityID), + Q_ARG(const QString&, entityScript), + Q_ARG(bool, forceRedownload) + ); + return; + } + PROFILE_RANGE(script, __FUNCTION__); + + if (isStopping() || DependencyManager::get()->isStopped()) { + qCDebug(scriptengine) << "loadEntityScript.start " << entityScript << entityID.toString() + << " but isStopping==" << isStopping() + << " || engines->isStopped==" << DependencyManager::get()->isStopped(); + return; + } + + if (!_entityScripts.contains(entityID)) { + // make sure EntityScriptDetails has an entry for this UUID right away + // (which allows bailing from the loading/provisioning process early if the Entity gets deleted mid-flight) + updateEntityScriptStatus(entityID, EntityScriptStatus::PENDING, "...pending..."); + } + + // This lock/defer approach allows multiple Entities to boot from the same script URL while still taking + // full advantage of cacheable require modules. This only affects Entities literally coming in back-to-back + // before the first one has time to finish loading. + auto &locker = _lockPerUniqueScriptURL[entityScript]; + if (!locker.tryLock()) { + qCDebug(scriptengine) << QString("loadEntityScript.deferring[%1] entity: %2 script: %3") + .arg( _pendingLoadEntities.size()).arg(entityID.toString()).arg(entityScript); + deferLoadEntityScript(entityID, entityScript, forceRedownload); + return; + } + +#ifdef DEBUG_ENTITY_STATES + auto previousStatus = _entityScripts.contains(entityID) ? _entityScripts[entityID].status : EntityScriptStatus::PENDING; + qCDebug(scriptengine) << "loadEntityScript.LOADING: " << entityScript << entityID.toString() + << "(previous: " << previousStatus << ")"; +#endif + + EntityScriptDetails newDetails; + newDetails.scriptText = entityScript; + newDetails.status = EntityScriptStatus::LOADING; + newDetails.definingSandboxURL = currentSandboxURL; + _entityScripts[entityID] = newDetails; + + auto scriptCache = DependencyManager::get(); + scriptCache->getScriptContents(entityScript, + [=, &locker](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { + if (isStopping()) { +#ifdef DEBUG_ENTITY_STATES + qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- stopping"; +#endif + locker.unlock(); + return; + } + executeOnScriptThread([=, &locker]{ + //qCDebug(scriptengine) << "loadEntityScript.contentAvailable" << status << QUrl(url).fileName() << entityID.toString(); + if (!isStopping() && _entityScripts.contains(entityID)) { + entityScriptContentAvailable(entityID, url, contents, isURL, success, status); + } else { +#ifdef DEBUG_ENTITY_STATES + qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- aborting"; +#endif + } + locker.unlock(); + }); + }, forceRedownload); } // since all of these operations can be asynch we will always do the actual work in the response handler @@ -1551,25 +1650,53 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co auto scriptCache = DependencyManager::get(); bool isFileUrl = isURL && scriptOrURL.startsWith("file://"); - auto fileName = isURL ? scriptOrURL : "EmbeddedEntityScript"; + auto fileName = isURL ? scriptOrURL : "about:EmbeddedEntityScript"; + const EntityScriptDetails &oldDetails = _entityScripts[entityID]; EntityScriptDetails newDetails; newDetails.scriptText = scriptOrURL; - if (!success) { - newDetails.status = EntityScriptStatus::ERROR_LOADING_SCRIPT; - newDetails.errorInfo = "Failed to load script (" + status + ")"; + // If an error happens below, we want to update newDetails with the new status info + // and also abort any pending Entity loads that are waiting on the exact same script URL. + auto setError = [&](const QString &errorInfo, const EntityScriptStatus& status) { + newDetails.errorInfo = errorInfo; + newDetails.status = status; setEntityScriptDetails(entityID, newDetails); + + QMutableListIterator i(_pendingLoadEntities); + // note: we want the *original* entityScript string (not the potentially normalized one from ScriptCache results) + const auto entityScript = oldDetails.scriptText; + while (i.hasNext()) { + auto retry = i.next(); + if (retry.entityScript == entityScript) { + qCDebug(scriptengine) << "... pending load of " << retry.entityID << " cancelled"; + i.remove(); + } + } + }; + + // NETWORK / FILESYSTEM ERRORS + if (!success) { + setError("Failed to load script (" + status + ")", EntityScriptStatus::ERROR_LOADING_SCRIPT); return; } + // SYNTAX ERRORS auto syntaxError = lintScript(contents, fileName); + if (syntaxError.isError()) { + auto message = syntaxError.property("formatted").toString(); + if (message.isEmpty()) { + message = syntaxError.toString(); + } + setError(QString("Bad syntax (%1)").arg(message), EntityScriptStatus::ERROR_RUNNING_SCRIPT); + syntaxError.setProperty("detail", entityID.toString()); + emit unhandledException(syntaxError); + return; + } QScriptProgram program { contents, fileName }; - if (!syntaxError.isNull() || program.isNull()) { - newDetails.status = EntityScriptStatus::ERROR_RUNNING_SCRIPT; - newDetails.errorInfo = QString("Bad syntax (%1)").arg(syntaxError); - setEntityScriptDetails(entityID, newDetails); - qCDebug(scriptengine) << newDetails.errorInfo << scriptOrURL; + if (program.isNull()) { + setError("Bad program (isNull)", EntityScriptStatus::ERROR_RUNNING_SCRIPT); + emit unhandledException(makeError("program.isNull")); return; // done processing script } @@ -1577,11 +1704,13 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co setParentURL(scriptOrURL); } + // SANITY/PERFORMANCE CHECK USING SANDBOX const int SANDBOX_TIMEOUT = 0.25 * MSECS_PER_SECOND; BaseScriptEngine sandbox; sandbox.setProcessEventsInterval(SANDBOX_TIMEOUT); - QScriptValue testConstructor; + QScriptValue testConstructor, exception; { + ExceptionEmitter tryCatch(&sandbox, QString("(preflight %1)").arg(entityID.toString())); QTimer timeout; timeout.setSingleShot(true); timeout.start(SANDBOX_TIMEOUT); @@ -1594,18 +1723,25 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co context->throwError(QString("Timed out (entity constructors are limited to %1ms)").arg(SANDBOX_TIMEOUT)); } }); + testConstructor = sandbox.evaluate(program); + + if (tryCatch.hasPending()) { + exception = tryCatch.consume(); + } else if (testConstructor.isError()) { + exception = testConstructor; + } } - QString exceptionMessage = sandbox.formatUncaughtException(program.fileName()); - if (!exceptionMessage.isNull()) { - newDetails.status = EntityScriptStatus::ERROR_RUNNING_SCRIPT; - newDetails.errorInfo = exceptionMessage; - setEntityScriptDetails(entityID, newDetails); - qCDebug(scriptengine) << "----- ScriptEngine::entityScriptContentAvailable -- hadUncaughtExceptions (" << scriptOrURL << ")"; + if (exception.isError()) { + // create a local copy using makeError to decouple from the sandbox engine + exception = makeError(exception); + setError(formatException(exception), EntityScriptStatus::ERROR_RUNNING_SCRIPT); + emit unhandledException(exception); return; } + // CONSTRUCTOR VIABILITY if (!testConstructor.isFunction()) { QString testConstructorType = QString(testConstructor.toVariant().typeName()); if (testConstructorType == "") { @@ -1616,32 +1752,49 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co if (testConstructorValue.size() > maxTestConstructorValueSize) { testConstructorValue = testConstructorValue.mid(0, maxTestConstructorValueSize) + "..."; } - scriptErrorMessage("Error -- ScriptEngine::loadEntityScript() entity:" + entityID.toString() - + "failed to load entity script -- expected a function, got " + testConstructorType - + "," + testConstructorValue - + "," + scriptOrURL); + auto message = QString("failed to load entity script -- expected a function, got %1, %2") + .arg(testConstructorType).arg(testConstructorValue); - newDetails.status = EntityScriptStatus::ERROR_RUNNING_SCRIPT; - newDetails.errorInfo = "Could not find constructor"; - setEntityScriptDetails(entityID, newDetails); + auto err = makeError(message); + err.setProperty("fileName", scriptOrURL); + err.setProperty("detail", "(constructor " + entityID.toString() + ")"); - qCDebug(scriptengine) << "----- ScriptEngine::entityScriptContentAvailable -- failed to run (" << scriptOrURL << ")"; + setError("Could not find constructor (" + testConstructorType + ")", EntityScriptStatus::ERROR_RUNNING_SCRIPT); + emit unhandledException(err); return; // done processing script } + // (this feeds into refreshFileScript) int64_t lastModified = 0; if (isFileUrl) { QString file = QUrl(scriptOrURL).toLocalFile(); lastModified = (quint64)QFileInfo(file).lastModified().toMSecsSinceEpoch(); } + + // THE ACTUAL EVALUATION AND CONSTRUCTION QScriptValue entityScriptConstructor, entityScriptObject; QUrl sandboxURL = currentSandboxURL.isEmpty() ? scriptOrURL : currentSandboxURL; auto initialization = [&]{ + ExceptionEmitter tryCatch(this, "(construct " + entityID.toString() + ")"); + entityScriptConstructor = evaluate(contents, fileName); entityScriptObject = entityScriptConstructor.construct(); + + if (tryCatch.hasPending()) { + entityScriptObject = tryCatch.consume(); + } }; + doWithEnvironment(entityID, sandboxURL, initialization); + if (entityScriptObject.isError()) { + auto exception = entityScriptObject; + setError(formatException(exception), EntityScriptStatus::ERROR_RUNNING_SCRIPT); + emit unhandledException(exception); + return; + } + + // ... AND WE HAVE LIFTOFF newDetails.status = EntityScriptStatus::RUNNING; newDetails.scriptObject = entityScriptObject; newDetails.lastModified = lastModified; @@ -1678,6 +1831,7 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { } EntityScriptDetails newDetails; newDetails.status = EntityScriptStatus::UNLOADED; + newDetails.lastModified = QDateTime::currentMSecsSinceEpoch(); setEntityScriptDetails(entityID, newDetails); stopAllTimersForEntityScript(entityID); } @@ -1700,15 +1854,14 @@ void ScriptEngine::unloadAllEntityScripts() { } _entityScripts.clear(); emit entityScriptDetailsUpdated(); + _lockPerUniqueScriptURL.clear(); #ifdef DEBUG_ENGINE_STATE - qCDebug(scriptengine) << "---- CURRENT STATE OF ENGINE: --------------------------"; - QScriptValueIterator it(globalObject()); - while (it.hasNext()) { - it.next(); - qCDebug(scriptengine) << it.name() << ":" << it.value().toString(); - } - qCDebug(scriptengine) << "--------------------------------------------------------"; + _debugDump( + "---- CURRENT STATE OF ENGINE: --------------------------", + globalObject(), + "--------------------------------------------------------" + ); #endif // DEBUG_ENGINE_STATE } @@ -1756,6 +1909,8 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& s currentEntityIdentifier = entityID; currentSandboxURL = sandboxURL; + ExceptionEmitter tryCatcher(this, !entityID.isNull() ? entityID.toString() : __FUNCTION__); + #if DEBUG_CURRENT_ENTITY QScriptValue oldData = this->globalObject().property("debugEntityID"); this->globalObject().setProperty("debugEntityID", entityID.toScriptValue(this)); // Make the entityID available to javascript as a global. @@ -1764,14 +1919,10 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& s #else operation(); #endif - if (hasUncaughtException()) { - reportUncaughtException(); - clearExceptions(); - } - currentEntityIdentifier = oldIdentifier; currentSandboxURL = oldSandboxURL; } + void ScriptEngine::callWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, QScriptValue function, QScriptValue thisObject, QScriptValueList args) { auto operation = [&]() { function.call(thisObject, args); @@ -1846,7 +1997,6 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS } } - void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QString& methodName, const EntityItemID& otherID, const Collision& collision) { if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING @@ -1881,3 +2031,4 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS } } } + diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index a382258973..7b2a5e8d8c 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -35,6 +35,7 @@ #include "ArrayBufferClass.h" #include "AssetScriptingInterface.h" #include "AudioScriptingInterface.h" +#include "BaseScriptEngine.h" #include "Quat.h" #include "Mat4.h" #include "ScriptCache.h" @@ -54,6 +55,13 @@ public: QUrl definingSandboxURL; }; +class DeferredLoadEntity { +public: + EntityItemID entityID; + QString entityScript; + //bool forceRedownload; +}; + typedef QList CallbackList; typedef QHash RegisteredEventHandlers; @@ -67,15 +75,7 @@ public: QString scriptText { "" }; QScriptValue scriptObject { QScriptValue() }; int64_t lastModified { 0 }; - QUrl definingSandboxURL { QUrl() }; -}; - -// common base class with just QScriptEngine-dependent helper methods -class BaseScriptEngine : public QScriptEngine { -public: - static const QString SCRIPT_EXCEPTION_FORMAT; - QString lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber = 1); - QString formatUncaughtException(const QString& overrideFileName = QString()); + QUrl definingSandboxURL { QUrl("about:EntityScript") }; }; class ScriptEngine : public BaseScriptEngine, public EntitiesScriptEngineProvider { @@ -91,14 +91,13 @@ public: }; static int processLevelMaxRetries; - ScriptEngine(Context context, const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("")); + ScriptEngine(Context context, const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("about:ScriptEngine")); ~ScriptEngine(); /// run the script in a dedicated thread. This will have the side effect of evalulating /// the current script contents and calling run(). Callers will likely want to register the script with external /// services before calling this. void runInThread(); - Q_INVOKABLE void executeOnScriptThread(std::function function, bool blocking = false); void runDebuggable(); @@ -162,16 +161,17 @@ public: Q_INVOKABLE QObject* setTimeout(const QScriptValue& function, int timeoutMS); Q_INVOKABLE void clearInterval(QObject* timer) { stopTimer(reinterpret_cast(timer)); } Q_INVOKABLE void clearTimeout(QObject* timer) { stopTimer(reinterpret_cast(timer)); } + Q_INVOKABLE void print(const QString& message); Q_INVOKABLE QUrl resolvePath(const QString& path) const; Q_INVOKABLE QUrl resourcesPath() const; // Entity Script Related methods - Q_INVOKABLE QString getEntityScriptStatus(const EntityItemID& entityID); Q_INVOKABLE bool isEntityScriptRunning(const EntityItemID& entityID) { return _entityScripts.contains(entityID) && _entityScripts[entityID].status == EntityScriptStatus::RUNNING; } - static void loadEntityScript(QWeakPointer theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload); + Q_INVOKABLE void deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload); + Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload); Q_INVOKABLE void unloadEntityScript(const EntityItemID& entityID); // will call unload method Q_INVOKABLE void unloadAllEntityScripts(); Q_INVOKABLE void callEntityScriptMethod(const EntityItemID& entityID, const QString& methodName, @@ -212,7 +212,6 @@ public: bool getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const; public slots: - int evaluatePending() const { return _evaluatesPending; } void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler); void updateMemoryCost(const qint64&); @@ -228,7 +227,6 @@ signals: void warningMessage(const QString& message); void infoMessage(const QString& message); void runningStateChanged(); - void evaluationFinished(QScriptValue result, bool isException); void loadScript(const QString& scriptName, bool isUserLoaded); void reloadScript(const QString& scriptName, bool isUserLoaded); void doneRunning(); @@ -239,8 +237,9 @@ signals: protected: void init(); + Q_INVOKABLE void executeOnScriptThread(std::function function, const Qt::ConnectionType& type = Qt::QueuedConnection ); - QString reportUncaughtException(const QString& overrideFileName = QString()); + QString logException(const QScriptValue& exception); void timerFired(); void stopAllTimers(); void stopAllTimersForEntityScript(const EntityItemID& entityID); @@ -262,17 +261,18 @@ protected: void callWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, QScriptValue function, QScriptValue thisObject, QScriptValueList args); Context _context; - QString _scriptContents; QString _parentURL; std::atomic _isFinished { false }; std::atomic _isRunning { false }; std::atomic _isStopping { false }; - int _evaluatesPending { 0 }; bool _isInitialized { false }; QHash _timerFunctionMap; QSet _includedURLs; QHash _entityScripts; + std::map _lockPerUniqueScriptURL; + QList _pendingLoadEntities; + bool _isThreaded { false }; QScriptEngineDebugger* _debugger { nullptr }; bool _debuggable { false }; diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index b2ff337fb9..57887d2d96 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -364,25 +364,43 @@ QStringList ScriptEngines::getRunningScripts() { } void ScriptEngines::stopAllScripts(bool restart) { + QVector toReload; QReadLocker lock(&_scriptEnginesHashLock); for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); it != _scriptEnginesHash.constEnd(); it++) { + ScriptEngine *scriptEngine = it.value(); // skip already stopped scripts - if (it.value()->isFinished() || it.value()->isStopping()) { + if (scriptEngine->isFinished() || scriptEngine->isStopping()) { continue; } // queue user scripts if restarting - if (restart && it.value()->isUserLoaded()) { - connect(it.value(), &ScriptEngine::finished, this, [this](QString scriptName, ScriptEngine* engine) { - reloadScript(scriptName); - }); + if (restart && scriptEngine->isUserLoaded()) { + toReload << it.key().toString(); } // stop all scripts - it.value()->stop(true); qCDebug(scriptengine) << "stopping script..." << it.key(); + scriptEngine->stop(); } + // wait for engines to stop (ie: providing time for .scriptEnding cleanup handlers to run) before + // triggering reload of any Client scripts / Entity scripts + QTimer::singleShot(500, this, [=]() { + for(const auto &scriptName : toReload) { + auto scriptEngine = getScriptEngine(scriptName); + if (scriptEngine && !scriptEngine->isFinished()) { + qCDebug(scriptengine) << "waiting on script:" << scriptName; + scriptEngine->waitTillDoneRunning(); + qCDebug(scriptengine) << "done waiting on script:" << scriptName; + } + qCDebug(scriptengine) << "reloading script..." << scriptName; + reloadScript(scriptName); + } + if (restart) { + qCDebug(scriptengine) << "stopAllScripts -- emitting scriptsReloading"; + emit scriptsReloading(); + } + }); } bool ScriptEngines::stopScript(const QString& rawScriptURL, bool restart) { @@ -421,9 +439,10 @@ void ScriptEngines::setScriptsLocation(const QString& scriptsLocation) { } void ScriptEngines::reloadAllScripts() { + qCDebug(scriptengine) << "reloadAllScripts -- clearing caches"; DependencyManager::get()->clearCache(); DependencyManager::get()->clearCache(); - emit scriptsReloading(); + qCDebug(scriptengine) << "reloadAllScripts -- stopping all scripts"; stopAllScripts(true); } @@ -456,7 +475,7 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL return scriptEngine; } - scriptEngine = new ScriptEngine(_context, NO_SCRIPT, ""); + scriptEngine = new ScriptEngine(_context, NO_SCRIPT, "about:" + scriptFilename.fileName()); scriptEngine->setUserLoaded(isUserLoaded); connect(scriptEngine, &ScriptEngine::doneRunning, this, [scriptEngine] { scriptEngine->deleteLater(); diff --git a/libraries/shared/src/PathUtils.cpp b/libraries/shared/src/PathUtils.cpp index 016b9ccfd6..265eaaa5b6 100644 --- a/libraries/shared/src/PathUtils.cpp +++ b/libraries/shared/src/PathUtils.cpp @@ -18,7 +18,7 @@ #include #include "PathUtils.h" #include - +#include // std::once const QString& PathUtils::resourcesPath() { #ifdef Q_OS_MAC @@ -82,3 +82,28 @@ QUrl defaultScriptsLocation() { QFileInfo fileInfo(path); return QUrl::fromLocalFile(fileInfo.canonicalFilePath()); } + + +QString PathUtils::stripFilename(const QUrl& url) { + // Guard against meaningless query and fragment parts. + // Do NOT use PreferLocalFile as its behavior is unpredictable (e.g., on defaultScriptsLocation()) + return url.toString(QUrl::RemoveFilename | QUrl::RemoveQuery | QUrl::RemoveFragment); +} + +Qt::CaseSensitivity PathUtils::getFSCaseSensitivity() { + static Qt::CaseSensitivity sensitivity { Qt::CaseSensitive }; + static std::once_flag once; + std::call_once(once, [&] { + QString path = defaultScriptsLocation().toLocalFile(); + QFileInfo upperFI(path.toUpper()); + QFileInfo lowerFI(path.toLower()); + sensitivity = (upperFI == lowerFI) ? Qt::CaseInsensitive : Qt::CaseSensitive; + }); + return sensitivity; +} + +bool PathUtils::isDescendantOf(const QUrl& childURL, const QUrl& parentURL) { + QString child = stripFilename(childURL); + QString parent = stripFilename(parentURL); + return child.startsWith(parent, PathUtils::getFSCaseSensitivity()); +} diff --git a/libraries/shared/src/PathUtils.h b/libraries/shared/src/PathUtils.h index 546586fb64..1f7dcbe466 100644 --- a/libraries/shared/src/PathUtils.h +++ b/libraries/shared/src/PathUtils.h @@ -28,6 +28,11 @@ class PathUtils : public QObject, public Dependency { public: static const QString& resourcesPath(); static QString getRootDataDirectory(); + + static Qt::CaseSensitivity getFSCaseSensitivity(); + static QString stripFilename(const QUrl& url); + // note: this is FS-case-sensitive version of parentURL.isParentOf(childURL) + static bool isDescendantOf(const QUrl& childURL, const QUrl& parentURL); }; QString fileNameWithoutExtension(const QString& fileName, const QVector possibleExtensions); From 417587dcda6d6064f529a18d40917c597fc9d683 Mon Sep 17 00:00:00 2001 From: humbletim Date: Wed, 22 Feb 2017 15:22:56 -0500 Subject: [PATCH 02/45] * Update per CR feedback / code standards * Cull log spam * Add tryLock to deferred processor to preempt unnecessary loadEntityContent round-trips --- .../script-engine/src/BaseScriptEngine.cpp | 18 +++++------ .../script-engine/src/BaseScriptEngine.h | 6 ++-- libraries/script-engine/src/BatchLoader.cpp | 2 +- libraries/script-engine/src/ScriptCache.cpp | 2 ++ libraries/script-engine/src/ScriptEngine.cpp | 31 +++++++++++-------- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/libraries/script-engine/src/BaseScriptEngine.cpp b/libraries/script-engine/src/BaseScriptEngine.cpp index 4342205414..fd4d829d54 100644 --- a/libraries/script-engine/src/BaseScriptEngine.cpp +++ b/libraries/script-engine/src/BaseScriptEngine.cpp @@ -118,14 +118,14 @@ QScriptValue BaseScriptEngine::cloneUncaughtException(const QString& extraDetail } if (fileName.isEmpty()) { // climb the stack frames looking for something useful to display - for(auto ctx = currentContext(); ctx && fileName.isEmpty(); ctx = ctx->parentContext()) { - QScriptContextInfo info { ctx }; + for (auto c = currentContext(); c && fileName.isEmpty(); c = c->parentContext()) { + QScriptContextInfo info { c }; if (!info.fileName().isEmpty()) { // take fileName:lineNumber as a pair fileName = info.fileName(); lineNumber = info.lineNumber(); if (backtrace.isEmpty()) { - backtrace = ctx->backtrace(); + backtrace = c->backtrace(); } break; } @@ -181,8 +181,6 @@ QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, co return QScriptValue(); } - //_debugDump("evaluateInClosure.closure", closure); - const auto fileName = program.fileName(); const auto shortName = QUrl(fileName).fileName(); @@ -197,17 +195,17 @@ QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, co setGlobalObject(global); } - auto ctx = pushContext(); + auto context = pushContext(); auto thiz = closure.property("this"); if (thiz.isObject()) { #ifdef DEBUG_JS qCDebug(scriptengine) << " setting this = closure.this" << shortName; #endif - ctx->setThisObject(thiz); + context->setThisObject(thiz); } - ctx->pushScope(closure); + context->pushScope(closure); #ifdef DEBUG_JS qCDebug(scriptengine) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName); #endif @@ -244,8 +242,8 @@ QScriptValue BaseScriptEngine::newLambdaFunction(std::functioncallee().prototype() === Lambda QObject - call.setData(data); // ctx->callee().data() will === data param + call.setPrototype(object); // context->callee().prototype() === Lambda QObject + call.setData(data); // context->callee().data() will === data param return call; } QString Lambda::toString() const { diff --git a/libraries/script-engine/src/BaseScriptEngine.h b/libraries/script-engine/src/BaseScriptEngine.h index 3ff4c22e4b..620c0915ea 100644 --- a/libraries/script-engine/src/BaseScriptEngine.h +++ b/libraries/script-engine/src/BaseScriptEngine.h @@ -39,7 +39,7 @@ signals: protected: void _emitUnhandledException(const QScriptValue& exception); - QScriptValue newLambdaFunction(std::function operation, const QScriptValue& data = QScriptValue(), const QScriptEngine::ValueOwnership& ownership = QScriptEngine::AutoOwnership); + QScriptValue newLambdaFunction(std::function operation, const QScriptValue& data = QScriptValue(), const QScriptEngine::ValueOwnership& ownership = QScriptEngine::AutoOwnership); static const QString _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS; Setting::Handle _enableExtendedJSExceptions { _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS, true }; @@ -72,14 +72,14 @@ protected: class Lambda : public QObject { Q_OBJECT public: - Lambda(QScriptEngine *engine, std::function operation, QScriptValue data); + Lambda(QScriptEngine *engine, std::function operation, QScriptValue data); ~Lambda(); public slots: QScriptValue call(); QString toString() const; private: QScriptEngine* engine; - std::function operation; + std::function operation; QScriptValue data; }; diff --git a/libraries/script-engine/src/BatchLoader.cpp b/libraries/script-engine/src/BatchLoader.cpp index eeaffff5cb..0c65d5c6f0 100644 --- a/libraries/script-engine/src/BatchLoader.cpp +++ b/libraries/script-engine/src/BatchLoader.cpp @@ -66,7 +66,7 @@ void BatchLoader::start(int maxRetries) { qCDebug(scriptengine) << "Loaded: " << url; } else { _data.insert(url, QString()); - qCDebug(scriptengine) << "Could not load: " << url; + qCDebug(scriptengine) << "Could not load: " << url << status; } if (!_finished && _urls.size() == _data.size()) { diff --git a/libraries/script-engine/src/ScriptCache.cpp b/libraries/script-engine/src/ScriptCache.cpp index 3bc780e28d..601ca6bc95 100644 --- a/libraries/script-engine/src/ScriptCache.cpp +++ b/libraries/script-engine/src/ScriptCache.cpp @@ -188,6 +188,8 @@ void ScriptCache::scriptContentAvailable(int maxRetries) { } } + } else { + qCWarning(scriptengine) << "Warning: scriptContentAvailable for inactive url: " << url; } } diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 262823a85d..0a6c3536b5 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -172,7 +172,6 @@ ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const qCDebug(scriptengine) << "isEntityServerScript() -- limiting maxRetries to 1"; processLevelMaxRetries = 1; } - //qCDebug(scriptengine) << getContext() << "processLevelMaxRetries =" << processLevelMaxRetries << fileNameString; // this is where all unhandled exceptions end up getting logged connect(this, &BaseScriptEngine::unhandledException, this, [this](const QScriptValue& err) { @@ -1185,7 +1184,6 @@ void ScriptEngine::timerFired() { _timerFunctionMap.remove(callingTimer); delete callingTimer; } - //qCWarning(scriptengine) << "timerFired" << timerData.function.toString(); // call the associated JS function, if it exists if (timerData.function.isValid()) { @@ -1500,18 +1498,14 @@ const auto RETRY_ATTEMPT_BATCH = 10; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH") const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000); void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { - // forceRedownload doesn't make sense yet in this scenario (which only happens when another entity has taken lead on entityScript) - if (forceRedownload) { - qCDebug(scriptengine) << "deferLoadEntityScript -- TODO: ignoring forceRedownload =" << forceRedownload; - } - const auto needTimer = _pendingLoadEntities.isEmpty(); + // NOTE: forceRedownload isn't forwarded here because deferLoadEntityScript() is only ever called when another entity has taken lead for the script _pendingLoadEntities.push_back({ entityID, entityScript/*, forceRedownload*/ }); - auto processDeferredEntities = [this](QScriptContext* ctx, QScriptEngine*) -> QScriptValue { - //qCDebug(scriptengine) << "deferLoadEntityScript.retry #" << _pendingLoadEntities.size() << " pending loads | " << QThread::currentThread(); + auto processDeferredEntities = [this](QScriptContext* context, QScriptEngine*) -> QScriptValue { + QList stillPending; - for(int i=0; i < RETRY_ATTEMPT_BATCH && !_pendingLoadEntities.isEmpty(); i++) { + for (int i=0; i < RETRY_ATTEMPT_BATCH && !_pendingLoadEntities.isEmpty(); i++) { auto retry = _pendingLoadEntities.takeFirst(); qCDebug(scriptengine) << "deferLoadEntityScript.retry[" << i << "]:" << retry.entityID << retry.entityScript; if(!_entityScripts.contains(retry.entityID)) { @@ -1521,12 +1515,21 @@ void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QSt if (details.status != EntityScriptStatus::PENDING) { qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity status no longer PENDING; " << details.status; } else { - loadEntityScript(retry.entityID, retry.entityScript, false); + auto &locker = _lockPerUniqueScriptURL[retry.entityScript]; + if (locker.tryLock()) { + locker.unlock(); + loadEntityScript(retry.entityID, retry.entityScript, false); + } else { + stillPending << retry; + } } } } + foreach(const DeferredLoadEntity& retry, stillPending) { + _pendingLoadEntities.push_back(retry); + } if (_pendingLoadEntities.isEmpty()) { - QObject *interval = ctx->callee().property("interval").toQObject(); + QObject *interval = context->callee().property("interval").toQObject(); // we're done processing so can kill the timer qCDebug(scriptengine) << "loadEntityScript.retry queue processing complete -- exiting" << interval; if (interval) { @@ -1610,7 +1613,9 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& return; } executeOnScriptThread([=, &locker]{ - //qCDebug(scriptengine) << "loadEntityScript.contentAvailable" << status << QUrl(url).fileName() << entityID.toString(); +#ifdef DEBUG_ENTITY_STATES + qCDebug(scriptengine) << "loadEntityScript.contentAvailable" << status << QUrl(url).fileName() << entityID.toString(); +#endif if (!isStopping() && _entityScripts.contains(entityID)) { entityScriptContentAvailable(entityID, url, contents, isURL, success, status); } else { From 132a889f7474728dcfc5facd223f800854a378ff Mon Sep 17 00:00:00 2001 From: humbletim Date: Wed, 22 Feb 2017 20:36:49 -0500 Subject: [PATCH 03/45] * Add Qt::DirectConnection to signalHandlerException * CR feedback / coding standards --- libraries/script-engine/src/ScriptEngine.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 0a6c3536b5..cee3f2344f 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -72,7 +72,7 @@ static const QScriptEngine::QObjectWrapOptions DEFAULT_QOBJECT_WRAP_OPTIONS = QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects; static const QScriptValue::PropertyFlags READONLY_PROP_FLAGS { QScriptValue::ReadOnly | QScriptValue::Undeletable }; -static const QScriptValue::PropertyFlags HIDDEN_PROP_FLAGS { READONLY_PROP_FLAGS | QScriptValue::SkipInEnumeration }; +static const QScriptValue::PropertyFlags READONLY_HIDDEN_PROP_FLAGS { READONLY_PROP_FLAGS | QScriptValue::SkipInEnumeration }; @@ -165,7 +165,7 @@ ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const // ... but may not always be available -- so if needed we fallback to the passed exception emit unhandledException(exception); } - }); + }, Qt::DirectConnection); setProcessEventsInterval(MSECS_PER_SECOND); if (isEntityServerScript()) { @@ -1250,7 +1250,7 @@ void ScriptEngine::stopTimer(QTimer *timer) { QUrl ScriptEngine::resolvePath(const QString& include) const { QUrl url(include); - // first lets check to see if it's already a full URL + // first lets check to see if it's already a full URL -- or a Windows path like "c:/" if (include.startsWith("/") || url.scheme().length() == 1) { url = QUrl::fromLocalFile(include); } @@ -1261,12 +1261,12 @@ QUrl ScriptEngine::resolvePath(const QString& include) const { // we apparently weren't a fully qualified url, so, let's assume we're relative // to the first absolute URL in the JS scope chain QUrl parentURL; - auto ctx = currentContext(); + auto context = currentContext(); do { - QScriptContextInfo contextInfo { ctx }; + QScriptContextInfo contextInfo { context }; parentURL = QUrl(contextInfo.fileName()); - ctx = ctx->parentContext(); - } while (parentURL.isRelative() && ctx); + context = context->parentContext(); + } while (parentURL.isRelative() && context); if (parentURL.isRelative()) { // fallback to the "include" parent (if defined, this will already be absolute) From f82a0196a8a7e4edfa4c255fd790caafa7130c2a Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 18:12:12 -0500 Subject: [PATCH 04/45] Always consume exceptions thrown by include's --- libraries/script-engine/src/ScriptEngine.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index cee3f2344f..56ebe0d798 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1365,12 +1365,12 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation); if (tryCatch.hasPending()) { // match previous include behavior where possible by logging, not propagating, exceptions. - // for nested evaluations, exceptions are now allowed to propagate auto err = tryCatch.pending(); emit unhandledException(err); - if (tryCatch.wouldEmit()) { - tryCatch.consume(); - } + // FIXME: to be revisited with next PR 21114-part3 for compat with require/module + //if (tryCatch.wouldEmit()) { + tryCatch.consume(); + //} } } else { scriptWarningMessage("Script.include() skipping evaluation of previously included url:" + url.toString()); From e794b64c31e748c5aaeff860b566a1d8ed2ee758 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 18:13:24 -0500 Subject: [PATCH 05/45] Add resolvePath and include characterization/unit tests --- .../tests/unit_tests/scriptTests/error.js | 6 + .../unit_tests/scriptTests/nested-error.js | 10 ++ .../scriptTests/nested-syntax-error.js | 10 ++ .../unit_tests/scriptTests/nested/error.js | 5 + .../unit_tests/scriptTests/nested/lib.js | 5 + .../unit_tests/scriptTests/nested/sibling.js | 3 + .../scriptTests/nested/syntax-error.js | 3 + .../unit_tests/scriptTests/top-level-error.js | 11 ++ .../tests/unit_tests/scriptTests/top-level.js | 5 + .../tests/unit_tests/scriptUnitTests.js | 126 ++++++++++++++++++ 10 files changed, 184 insertions(+) create mode 100644 scripts/developer/tests/unit_tests/scriptTests/error.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/nested-error.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/nested-syntax-error.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/nested/error.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/nested/lib.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/nested/sibling.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/nested/syntax-error.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/top-level-error.js create mode 100644 scripts/developer/tests/unit_tests/scriptTests/top-level.js create mode 100644 scripts/developer/tests/unit_tests/scriptUnitTests.js diff --git a/scripts/developer/tests/unit_tests/scriptTests/error.js b/scripts/developer/tests/unit_tests/scriptTests/error.js new file mode 100644 index 0000000000..6b9a7c7445 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/error.js @@ -0,0 +1,6 @@ +afterError = false; +throw new Error('error.js'); +afterError = true; + +(1,eval)('this').$finishes.push(Script.resolvePath('')); + diff --git a/scripts/developer/tests/unit_tests/scriptTests/nested-error.js b/scripts/developer/tests/unit_tests/scriptTests/nested-error.js new file mode 100644 index 0000000000..3a190545ef --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/nested-error.js @@ -0,0 +1,10 @@ +afterError = false; +outer = null; +Script.include('./nested/error.js?' + Settings.getValue('cache_buster')); +outer = { + inner: inner.lib, + sibling: sibling.lib, +}; +afterError = true; + +(1,eval)("this").$finishes.push(Script.resolvePath('')); diff --git a/scripts/developer/tests/unit_tests/scriptTests/nested-syntax-error.js b/scripts/developer/tests/unit_tests/scriptTests/nested-syntax-error.js new file mode 100644 index 0000000000..fb0e3679ff --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/nested-syntax-error.js @@ -0,0 +1,10 @@ +afterError = false; +outer = null; +Script.include('./nested/syntax-error.js?' + Settings.getValue('cache_buster')); +outer = { + inner: inner.lib, + sibling: sibling.lib, +}; +afterError = true; + +(1,eval)("this").$finishes.push(Script.resolvePath('')); diff --git a/scripts/developer/tests/unit_tests/scriptTests/nested/error.js b/scripts/developer/tests/unit_tests/scriptTests/nested/error.js new file mode 100644 index 0000000000..aeb76eec01 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/nested/error.js @@ -0,0 +1,5 @@ +afterError = false; +throw new Error('nested/error.js'); +afterError = true; + +(1,eval)("this").$finishes.push(Script.resolvePath('')); diff --git a/scripts/developer/tests/unit_tests/scriptTests/nested/lib.js b/scripts/developer/tests/unit_tests/scriptTests/nested/lib.js new file mode 100644 index 0000000000..1c2cf3b885 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/nested/lib.js @@ -0,0 +1,5 @@ +Script.include('sibling.js'); +inner = { + lib: "nested/lib.js", +}; + diff --git a/scripts/developer/tests/unit_tests/scriptTests/nested/sibling.js b/scripts/developer/tests/unit_tests/scriptTests/nested/sibling.js new file mode 100644 index 0000000000..33fa068079 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/nested/sibling.js @@ -0,0 +1,3 @@ +sibling = { + lib: "nested/sibling", +}; diff --git a/scripts/developer/tests/unit_tests/scriptTests/nested/syntax-error.js b/scripts/developer/tests/unit_tests/scriptTests/nested/syntax-error.js new file mode 100644 index 0000000000..3b578c2674 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/nested/syntax-error.js @@ -0,0 +1,3 @@ +function() { + // intentional SyntaxError... + diff --git a/scripts/developer/tests/unit_tests/scriptTests/top-level-error.js b/scripts/developer/tests/unit_tests/scriptTests/top-level-error.js new file mode 100644 index 0000000000..4ef90ec238 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/top-level-error.js @@ -0,0 +1,11 @@ +afterError = false; +outer = null; +Script.include('./nested/lib.js'); +Undefined_symbol; +outer = { + inner: inner.lib, + sibling: sibling.lib, +}; +afterError = true; + +(1,eval)("this").$finishes.push(Script.resolvePath('')); diff --git a/scripts/developer/tests/unit_tests/scriptTests/top-level.js b/scripts/developer/tests/unit_tests/scriptTests/top-level.js new file mode 100644 index 0000000000..ab55007fe9 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptTests/top-level.js @@ -0,0 +1,5 @@ +Script.include('./nested/lib.js'); +outer = { + inner: inner.lib, + sibling: sibling.lib, +}; diff --git a/scripts/developer/tests/unit_tests/scriptUnitTests.js b/scripts/developer/tests/unit_tests/scriptUnitTests.js new file mode 100644 index 0000000000..d464682ca9 --- /dev/null +++ b/scripts/developer/tests/unit_tests/scriptUnitTests.js @@ -0,0 +1,126 @@ +/* eslint-env jasmine */ + +instrument_testrunner(); + +describe('Script', function () { + // get the current filename without calling Script.resolvePath('') + try { + throw new Error('stack'); + } catch(e) { + var filename = e.fileName; + var dirname = filename.split('/').slice(0, -1).join('/') + '/'; + var parentdir = dirname.split('/').slice(0, -2).join('/') + '/'; + } + + // characterization tests + // initially these are just to capture how the app works currently + var testCases = { + '': filename, + '.': dirname, + '..': parentdir, + 'about:Entities 1': '', + 'Entities 1': dirname + 'Entities 1', + './file.js': dirname + 'file.js', + 'c:/temp/': 'file:///c:/temp/', + 'c:/temp': 'file:///c:/temp', + '/temp/': 'file:///temp/', + 'c:/': 'file:///c:/', + 'c:': 'file:///c:', + 'file:///~/libraries/a.js': 'file:///~/libraries/a.js', + '/temp/tested/../file.js': 'file:///temp/tested/../file.js', + '/~/libraries/utils.js': 'file:///~/libraries/utils.js', + '/temp/file.js': 'file:///temp/file.js', + '/~/': 'file:///~/', + }; + } + describe('resolvePath', function () { + Object.keys(testCases).forEach(function(input) { + it('(' + JSON.stringify(input) + ')', function () { + expect(Script.resolvePath(input)).toEqual(testCases[input]); + }); + }); + }); + + describe('include', function () { + var old_cache_buster; + var cache_buster = '#' + +new Date; + beforeAll(function() { + old_cache_buster = Settings.getValue('cache_buster'); + Settings.setValue('cache_buster', cache_buster); + }); + afterAll(function() { + Settings.setValue('cache_buster', old_cache_buster); + }); + beforeEach(function() { + vec3toStr = undefined; + }); + it('file:///~/system/libraries/utils.js' + cache_buster, function() { + Script.include('file:///~/system/libraries/utils.js' + cache_buster); + expect(vec3toStr).toEqual(jasmine.any(Function)); + }); + it('nested' + cache_buster, function() { + Script.include('./scriptTests/top-level.js' + cache_buster); + expect(outer).toEqual(jasmine.any(Object)); + expect(inner).toEqual(jasmine.any(Object)); + expect(sibling).toEqual(jasmine.any(Object)); + expect(outer.inner).toEqual(inner.lib); + expect(outer.sibling).toEqual(sibling.lib); + }); + describe('errors' + cache_buster, function() { + var finishes, oldFinishes; + beforeAll(function() { + oldFinishes = (1,eval)('this').$finishes; + }); + afterAll(function() { + (1,eval)('this').$finishes = oldFinishes; + }); + beforeEach(function() { + finishes = (1,eval)('this').$finishes = []; + }); + it('error', function() { + // a thrown Error in top-level include aborts that include, but does not throw the error back to JS + expect(function() { + Script.include('./scriptTests/error.js' + cache_buster); + }).not.toThrowError("error.js"); + expect(finishes.length).toBe(0); + }); + it('top-level-error', function() { + // an organice Error in a top-level include aborts that include, but does not throw the error + expect(function() { + Script.include('./scriptTests/top-level-error.js' + cache_buster); + }).not.toThrowError(/Undefined_symbol/); + expect(finishes.length).toBe(0); + }); + it('nested', function() { + // a thrown Error in a nested include aborts the nested include, but does not abort the top-level script + expect(function() { + Script.include('./scriptTests/nested-error.js' + cache_buster); + }).not.toThrowError("nested/error.js"); + expect(finishes.length).toBe(1); + }); + it('nested-syntax-error', function() { + // a SyntaxError in a nested include breaks only that include (the main script should finish unimpeded) + expect(function() { + Script.include('./scriptTests/nested-syntax-error.js' + cache_buster); + }).not.toThrowError(/SyntaxEror/); + expect(finishes.length).toBe(1); + }); + }); + }); +}); + +// enable scriptUnitTests to be loaded directly +function run() {} +function instrument_testrunner() { + if (typeof describe === 'undefined') { + print('instrumenting jasmine', Script.resolvePath('')); + Script.include('../../libraries/jasmine/jasmine.js'); + Script.include('../../libraries/jasmine/hifi-boot.js'); + jasmine.getEnv().addReporter({ jasmineDone: Script.stop }); + run = function() { + print('executing jasmine', Script.resolvePath('')); + jasmine.getEnv().execute(); + }; + } +} +run(); From 8ab7a8cad52e20ff48ccdb0c3765ab413a8ef5a0 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 18:16:41 -0500 Subject: [PATCH 06/45] fix bracket --- scripts/developer/tests/unit_tests/scriptUnitTests.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/developer/tests/unit_tests/scriptUnitTests.js b/scripts/developer/tests/unit_tests/scriptUnitTests.js index d464682ca9..63b451e97f 100644 --- a/scripts/developer/tests/unit_tests/scriptUnitTests.js +++ b/scripts/developer/tests/unit_tests/scriptUnitTests.js @@ -31,8 +31,7 @@ describe('Script', function () { '/~/libraries/utils.js': 'file:///~/libraries/utils.js', '/temp/file.js': 'file:///temp/file.js', '/~/': 'file:///~/', - }; - } + }; describe('resolvePath', function () { Object.keys(testCases).forEach(function(input) { it('(' + JSON.stringify(input) + ')', function () { From 1eeb25a9ebc884549fe6b645fa4612117d9525ba Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 19:09:27 -0500 Subject: [PATCH 07/45] Increase deferred batch processing. --- libraries/script-engine/src/ScriptEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 56ebe0d798..ff0ddc2ce3 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1494,7 +1494,7 @@ bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntitySc return true; } -const auto RETRY_ATTEMPT_BATCH = 10; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH").toInt(), 1, 50); +const auto RETRY_ATTEMPT_BATCH = 100; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH").toInt(), 1, 50); const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000); void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { From 0b73cef252f773e1e2a91a06ae301aa7d221cf83 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 19:10:44 -0500 Subject: [PATCH 08/45] Add stampede test. --- .../tests/entityStampedeTest-entity-fail.js | 3 ++ .../tests/entityStampedeTest-entity.js | 12 +++++++ scripts/developer/tests/entityStampedeTest.js | 32 +++++++++++++++++++ 3 files changed, 47 insertions(+) create mode 100644 scripts/developer/tests/entityStampedeTest-entity-fail.js create mode 100644 scripts/developer/tests/entityStampedeTest-entity.js create mode 100644 scripts/developer/tests/entityStampedeTest.js diff --git a/scripts/developer/tests/entityStampedeTest-entity-fail.js b/scripts/developer/tests/entityStampedeTest-entity-fail.js new file mode 100644 index 0000000000..53c0469055 --- /dev/null +++ b/scripts/developer/tests/entityStampedeTest-entity-fail.js @@ -0,0 +1,3 @@ +(function() { + throw new Error(Script.resolvePath('')); +}) diff --git a/scripts/developer/tests/entityStampedeTest-entity.js b/scripts/developer/tests/entityStampedeTest-entity.js new file mode 100644 index 0000000000..1f1d6bcf76 --- /dev/null +++ b/scripts/developer/tests/entityStampedeTest-entity.js @@ -0,0 +1,12 @@ +(function() { + return { + preload: function(uuid) { + var shape = Entities.getEntityProperties(uuid).shape === 'Sphere' ? 'Cube' : 'Sphere'; + + Entities.editEntity(uuid, { + shape: shape, + color: { red: 0xff, green: 0xff, blue: 0xff }, + }); + } + }; +}) diff --git a/scripts/developer/tests/entityStampedeTest.js b/scripts/developer/tests/entityStampedeTest.js new file mode 100644 index 0000000000..c5040a9796 --- /dev/null +++ b/scripts/developer/tests/entityStampedeTest.js @@ -0,0 +1,32 @@ +var NUM_ENTITIES = 100; +var RADIUS = 2; +var DIV = NUM_ENTITIES / Math.PI / 2; +var PASS_SCRIPT_URL = Script.resolvePath('').replace('.js', '-entity.js'); +var FAIL_SCRIPT_URL = Script.resolvePath('').replace('.js', '-entity-fail.js'); + +var origin = Vec3.sum(MyAvatar.position, Vec3.multiply(5, Quat.getFront(MyAvatar.orientation))); +origin.y += HMD.eyeHeight; + +var uuids = []; + +Script.scriptEnding.connect(function() { + uuids.forEach(function(id) { + Entities.deleteEntity(id); + }); +}); + +for (var i=0; i < NUM_ENTITIES; i++) { + var failGroup = i % 2; + uuids.push(Entities.addEntity({ + type: 'Shape', + shape: failGroup ? 'Sphere' : 'Icosahedron', + name: 'entityStampedeTest-' + i, + lifetime: 120, + position: Vec3.sum(origin, Vec3.multiplyQbyV( + MyAvatar.orientation, { x: Math.sin(i / DIV) * RADIUS, y: Math.cos(i / DIV) * RADIUS, z: 0 } + )), + script: (failGroup ? FAIL_SCRIPT_URL : PASS_SCRIPT_URL) + Settings.getValue('cache_buster'), + dimensions: Vec3.HALF, + color: { red: 0, green: 0, blue: 0 }, + }, !Entities.serversExist())); +} From 3ad3fd472e5720f1a67d202657ababab98b88d51 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 23 Feb 2017 19:46:28 -0500 Subject: [PATCH 09/45] * Inline / duplicate exception checking per CR feedback * Coding standards --- .../script-engine/src/BaseScriptEngine.cpp | 36 +-------------- .../script-engine/src/BaseScriptEngine.h | 19 -------- libraries/script-engine/src/ScriptEngine.cpp | 45 +++++++++---------- 3 files changed, 24 insertions(+), 76 deletions(-) diff --git a/libraries/script-engine/src/BaseScriptEngine.cpp b/libraries/script-engine/src/BaseScriptEngine.cpp index fd4d829d54..16308c0650 100644 --- a/libraries/script-engine/src/BaseScriptEngine.cpp +++ b/libraries/script-engine/src/BaseScriptEngine.cpp @@ -210,10 +210,9 @@ QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, co qCDebug(scriptengine) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName); #endif { - ExceptionEmitter tryCatch(this, __FUNCTION__); result = BaseScriptEngine::evaluate(program); - if (tryCatch.hasPending()) { - auto err = tryCatch.pending(); + if (hasUncaughtException()) { + auto err = cloneUncaughtException(__FUNCTION__); #ifdef DEBUG_JS_EXCEPTIONS qCWarning(scriptengine) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString(); err.setProperty("_result", result); @@ -266,37 +265,6 @@ QScriptValue Lambda::call() { return operation(engine->currentContext(), engine); } -// BaseScriptEngine::ExceptionEmitter - -void BaseScriptEngine::_emitUnhandledException(const QScriptValue& exception) { - emit unhandledException(exception); -} -BaseScriptEngine::ExceptionEmitter::ExceptionEmitter(BaseScriptEngine* engine, const QString& debugName) - : _engine(engine), _debugName(debugName) { -} -bool BaseScriptEngine::ExceptionEmitter::hasPending() { - return _engine->hasUncaughtException(); -} -QScriptValue BaseScriptEngine::ExceptionEmitter::pending() { - return _engine->cloneUncaughtException(_debugName); -} -QScriptValue BaseScriptEngine::ExceptionEmitter::consume() { - if (hasPending()) { - _consumedException = pending(); - _engine->clearExceptions(); - } - return _consumedException; -} -bool BaseScriptEngine::ExceptionEmitter::wouldEmit() { - return !_engine->isEvaluating() && _engine->hasUncaughtException(); -} -BaseScriptEngine::ExceptionEmitter::~ExceptionEmitter() { - if (wouldEmit()) { - _engine->_emitUnhandledException(consume()); - } -} - - #ifdef DEBUG_JS void BaseScriptEngine::_debugDump(const QString& header, const QScriptValue& object, const QString& footer) { if (!header.isEmpty()) { diff --git a/libraries/script-engine/src/BaseScriptEngine.h b/libraries/script-engine/src/BaseScriptEngine.h index 620c0915ea..27a6eff33d 100644 --- a/libraries/script-engine/src/BaseScriptEngine.h +++ b/libraries/script-engine/src/BaseScriptEngine.h @@ -46,25 +46,6 @@ protected: #ifdef DEBUG_JS static void _debugDump(const QString& header, const QScriptValue& object, const QString& footer = QString()); #endif - - // ExceptionEmitter can be placed above calls that might throw JS exceptions and it'll - // automatically signal BaseScriptEngine when falling out of scope (recursion aware). - class ExceptionEmitter { - public: - ExceptionEmitter(BaseScriptEngine* engine, const QString& debugName = QString()); - // is there a pending exception? - bool hasPending(); - QScriptValue pending(); - // would it be emitted at scope's end? - bool wouldEmit(); - // consume and return the pending exception - QScriptValue consume(); - ~ExceptionEmitter(); - protected: - BaseScriptEngine* _engine; - QString _debugName; - QScriptValue _consumedException; - }; }; // Lambda helps create callable QScriptValues out of std::functions: diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index ff0ddc2ce3..06cbc8177a 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -899,9 +899,11 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi QScriptValue result; { - // catch any exceptions throw by nested operations - ExceptionEmitter tryCatch(this, __FUNCTION__); result = BaseScriptEngine::evaluate(program); + if (!isEvaluating() && hasUncaughtException()) { + emit unhandledException(cloneUncaughtException(__FUNCTION__)); + clearExceptions(); + } } return result; } @@ -923,9 +925,11 @@ void ScriptEngine::run() { emit runningStateChanged(); { - // catch any exceptions throw by nested operations - ExceptionEmitter tryCatch(this, __FUNCTION__); evaluate(_scriptContents, _fileNameString); + if (!isEvaluating() && hasUncaughtException()) { + emit unhandledException(cloneUncaughtException(__FUNCTION__)); + clearExceptions(); + } } #ifdef _WIN32 // VS13 does not sleep_until unless it uses the system_clock, see: @@ -1361,16 +1365,10 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac evaluate(contents, url.toString()); }; - ExceptionEmitter tryCatch(this, "include"); doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation); - if (tryCatch.hasPending()) { - // match previous include behavior where possible by logging, not propagating, exceptions. - auto err = tryCatch.pending(); - emit unhandledException(err); - // FIXME: to be revisited with next PR 21114-part3 for compat with require/module - //if (tryCatch.wouldEmit()) { - tryCatch.consume(); - //} + if (hasUncaughtException()) { + emit unhandledException(cloneUncaughtException(__FUNCTION__)); + clearExceptions(); } } else { scriptWarningMessage("Script.include() skipping evaluation of previously included url:" + url.toString()); @@ -1508,7 +1506,7 @@ void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QSt for (int i=0; i < RETRY_ATTEMPT_BATCH && !_pendingLoadEntities.isEmpty(); i++) { auto retry = _pendingLoadEntities.takeFirst(); qCDebug(scriptengine) << "deferLoadEntityScript.retry[" << i << "]:" << retry.entityID << retry.entityScript; - if(!_entityScripts.contains(retry.entityID)) { + if (!_entityScripts.contains(retry.entityID)) { qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity details gone (entity deleted?)"; } else { auto details = _entityScripts[retry.entityID]; @@ -1715,7 +1713,6 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co sandbox.setProcessEventsInterval(SANDBOX_TIMEOUT); QScriptValue testConstructor, exception; { - ExceptionEmitter tryCatch(&sandbox, QString("(preflight %1)").arg(entityID.toString())); QTimer timeout; timeout.setSingleShot(true); timeout.start(SANDBOX_TIMEOUT); @@ -1731,8 +1728,9 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co testConstructor = sandbox.evaluate(program); - if (tryCatch.hasPending()) { - exception = tryCatch.consume(); + if (sandbox.hasUncaughtException()) { + exception = sandbox.cloneUncaughtException(QString("(preflight %1)").arg(entityID.toString())); + sandbox.clearExceptions(); } else if (testConstructor.isError()) { exception = testConstructor; } @@ -1780,13 +1778,12 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co QScriptValue entityScriptConstructor, entityScriptObject; QUrl sandboxURL = currentSandboxURL.isEmpty() ? scriptOrURL : currentSandboxURL; auto initialization = [&]{ - ExceptionEmitter tryCatch(this, "(construct " + entityID.toString() + ")"); - entityScriptConstructor = evaluate(contents, fileName); entityScriptObject = entityScriptConstructor.construct(); - if (tryCatch.hasPending()) { - entityScriptObject = tryCatch.consume(); + if (hasUncaughtException()) { + entityScriptObject = cloneUncaughtException("(construct " + entityID.toString() + ")"); + clearExceptions(); } }; @@ -1914,8 +1911,6 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& s currentEntityIdentifier = entityID; currentSandboxURL = sandboxURL; - ExceptionEmitter tryCatcher(this, !entityID.isNull() ? entityID.toString() : __FUNCTION__); - #if DEBUG_CURRENT_ENTITY QScriptValue oldData = this->globalObject().property("debugEntityID"); this->globalObject().setProperty("debugEntityID", entityID.toScriptValue(this)); // Make the entityID available to javascript as a global. @@ -1924,6 +1919,10 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& s #else operation(); #endif + if (!isEvaluating() && hasUncaughtException()) { + emit unhandledException(cloneUncaughtException(!entityID.isNull() ? entityID.toString() : __FUNCTION__)); + clearExceptions(); + } currentEntityIdentifier = oldIdentifier; currentSandboxURL = oldSandboxURL; } From c89d203f94c0ffa9d18b2a515117c095828076da Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 24 Feb 2017 12:27:33 -0500 Subject: [PATCH 10/45] Adapt from std::map locks to simple QHash for flagging busy scriptURLs --- libraries/script-engine/src/ScriptEngine.cpp | 69 ++++++++++++-------- libraries/script-engine/src/ScriptEngine.h | 2 +- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 06cbc8177a..cb63628d1a 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1494,6 +1494,7 @@ bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntitySc const auto RETRY_ATTEMPT_BATCH = 100; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH").toInt(), 1, 50); const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000); +const static EntityItemID BAD_SCRIPT_UUID_PLACEHOLDER { "{20170224-dead-face-0000-cee000021114}" }; void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { const auto needTimer = _pendingLoadEntities.isEmpty(); @@ -1513,12 +1514,18 @@ void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QSt if (details.status != EntityScriptStatus::PENDING) { qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity status no longer PENDING; " << details.status; } else { - auto &locker = _lockPerUniqueScriptURL[retry.entityScript]; - if (locker.tryLock()) { - locker.unlock(); + if (!_occupiedScriptURLs.contains(retry.entityScript)) { + // the URL is no longer busy with another Entity, so now it's our turn loadEntityScript(retry.entityID, retry.entityScript, false); } else { - stillPending << retry; + auto entityID = _occupiedScriptURLs[retry.entityScript]; + if (entityID == BAD_SCRIPT_UUID_PLACEHOLDER) { + // the URL was marked bad by an earlier load attempt, so abort the retry mission + qCDebug(scriptengine) << "... pending load of " << retry.entityID << " cancelled"; + } else { + // the URL is still occupied by another Entity, so add to next retry pass + stillPending << retry; + } } } } @@ -1543,7 +1550,7 @@ void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QSt // processDeferredEntities gets wrapped as a "regular" JavaScript callback so that it can then // be scheduled using setInterval per usual -- allowing the script engine to own its lifecycle. - // we stil need to call clearInterval manually when done, so its passed to calle() above as a property. + // we stil need to call clearInterval manually when done, so its passed to callee() above as a property. QScriptValue function = newLambdaFunction(processDeferredEntities, "processDeferredEntities-loader"); QObject *interval = setInterval( function, RETRY_ATTEMPT_MSECS ); @@ -1577,14 +1584,28 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& updateEntityScriptStatus(entityID, EntityScriptStatus::PENDING, "...pending..."); } - // This lock/defer approach allows multiple Entities to boot from the same script URL while still taking + // This "occupied" approach allows multiple Entities to boot from the same script URL while still taking // full advantage of cacheable require modules. This only affects Entities literally coming in back-to-back // before the first one has time to finish loading. - auto &locker = _lockPerUniqueScriptURL[entityScript]; - if (!locker.tryLock()) { - qCDebug(scriptengine) << QString("loadEntityScript.deferring[%1] entity: %2 script: %3") - .arg( _pendingLoadEntities.size()).arg(entityID.toString()).arg(entityScript); - deferLoadEntityScript(entityID, entityScript, forceRedownload); + if (!_occupiedScriptURLs.contains(entityScript)) { + // the scriptURL is available -- flag as in-use and proceed + _occupiedScriptURLs[entityScript] = entityID; + } else { + // another Entity is either still booting from this script URL or has failed and marked it "out of order" + auto currentEntityID = _occupiedScriptURLs[entityScript]; + if (currentEntityID == BAD_SCRIPT_UUID_PLACEHOLDER && !forceRedownload) { + // since not force-reloading, assume that the exact same input would produce the exact same output again + // note: this state gets reset with "reload all scripts," leaving/returning to a Domain, clear cache, etc. + qCDebug(scriptengine) << QString("loadEntityScript.cancelled entity: %1 script: %2 (previous script failure)") + .arg(entityID.toString()).arg(entityScript); + updateEntityScriptStatus(entityID, EntityScriptStatus::ERROR_LOADING_SCRIPT, + "Cached Script was marked invalid by previous Entity load failure."); + } else { + // another entity is busy loading from this script URL so wait for them to finish + qCDebug(scriptengine) << QString("loadEntityScript.deferring[%0] entity: %1 script: %2") + .arg( _pendingLoadEntities.size()).arg(entityID.toString()).arg(entityScript); + deferLoadEntityScript(entityID, entityScript, forceRedownload); + } return; } @@ -1598,19 +1619,18 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& newDetails.scriptText = entityScript; newDetails.status = EntityScriptStatus::LOADING; newDetails.definingSandboxURL = currentSandboxURL; - _entityScripts[entityID] = newDetails; + setEntityScriptDetails(entityID, newDetails); auto scriptCache = DependencyManager::get(); scriptCache->getScriptContents(entityScript, - [=, &locker](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { + [this, entityScript, entityID](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { if (isStopping()) { #ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- stopping"; #endif - locker.unlock(); return; } - executeOnScriptThread([=, &locker]{ + executeOnScriptThread([=]{ #ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << "loadEntityScript.contentAvailable" << status << QUrl(url).fileName() << entityID.toString(); #endif @@ -1621,7 +1641,10 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- aborting"; #endif } - locker.unlock(); + // recheck whether us since may have been set to BAD_SCRIPT_UUID_PLACEHOLDER in entityScriptContentAvailable + if (_occupiedScriptURLs[entityScript] == entityID) { + _occupiedScriptURLs.remove(entityScript); + } }); }, forceRedownload); } @@ -1666,16 +1689,10 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co newDetails.status = status; setEntityScriptDetails(entityID, newDetails); - QMutableListIterator i(_pendingLoadEntities); + qCDebug(scriptengine) << "entityScriptContentAvailable -- flagging " << oldDetails.scriptText << " as BAD_SCRIPT_UUID_PLACEHOLDER"; + // flag this scriptURL as unusuable // note: we want the *original* entityScript string (not the potentially normalized one from ScriptCache results) - const auto entityScript = oldDetails.scriptText; - while (i.hasNext()) { - auto retry = i.next(); - if (retry.entityScript == entityScript) { - qCDebug(scriptengine) << "... pending load of " << retry.entityID << " cancelled"; - i.remove(); - } - } + _occupiedScriptURLs[oldDetails.scriptText] = BAD_SCRIPT_UUID_PLACEHOLDER; }; // NETWORK / FILESYSTEM ERRORS @@ -1856,7 +1873,7 @@ void ScriptEngine::unloadAllEntityScripts() { } _entityScripts.clear(); emit entityScriptDetailsUpdated(); - _lockPerUniqueScriptURL.clear(); + _occupiedScriptURLs.clear(); #ifdef DEBUG_ENGINE_STATE _debugDump( diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 7b2a5e8d8c..43cb3f897d 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -270,7 +270,7 @@ protected: QHash _timerFunctionMap; QSet _includedURLs; QHash _entityScripts; - std::map _lockPerUniqueScriptURL; + QHash _occupiedScriptURLs; QList _pendingLoadEntities; bool _isThreaded { false }; From 02827552aebe347d2d80307da1439db97be77f66 Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 24 Feb 2017 12:31:18 -0500 Subject: [PATCH 11/45] Add EntityScriptServer version of the stampede test --- .../tests/entityServerStampedeTest.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 scripts/developer/tests/entityServerStampedeTest.js diff --git a/scripts/developer/tests/entityServerStampedeTest.js b/scripts/developer/tests/entityServerStampedeTest.js new file mode 100644 index 0000000000..35ecd7eb11 --- /dev/null +++ b/scripts/developer/tests/entityServerStampedeTest.js @@ -0,0 +1,33 @@ +var NUM_ENTITIES = 100; +var RADIUS = 2; +var DIV = NUM_ENTITIES / Math.PI / 2; +var PASS_SCRIPT_URL = Script.resolvePath('entityStampedeTest-entity.js'); +var FAIL_SCRIPT_URL = Script.resolvePath('entityStampedeTest-entity-fail.js'); + +var origin = Vec3.sum(MyAvatar.position, Vec3.multiply(5, Quat.getFront(MyAvatar.orientation))); +origin.y += HMD.eyeHeight; + +var uuids = []; + +Script.scriptEnding.connect(function() { + uuids.forEach(function(id) { + Entities.deleteEntity(id); + }); +}); + +for (var i=0; i < NUM_ENTITIES; i++) { + var failGroup = i % 2; + uuids.push(Entities.addEntity({ + type: 'Shape', + shape: failGroup ? 'Sphere' : 'Icosahedron', + name: 'SERVER-entityStampedeTest-' + i, + lifetime: 120, + position: Vec3.sum(origin, Vec3.multiplyQbyV( + MyAvatar.orientation, { x: Math.sin(i / DIV) * RADIUS, y: Math.cos(i / DIV) * RADIUS, z: 0 } + )), + serverScripts: (failGroup ? FAIL_SCRIPT_URL : PASS_SCRIPT_URL) + Settings.getValue('cache_buster'), + dimensions: Vec3.HALF, + color: { red: 0, green: 0, blue: 0 }, + }, !Entities.serversExist())); +} + From 3ac116545e20fcf136f45a386a662c91fe4d3fd6 Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 24 Feb 2017 16:05:11 -0500 Subject: [PATCH 12/45] * Switch trigger for deferred loads from timer to using leading entity completion * Bring back notion of tracking "bad scripts" to fail stampeding entities as a set * Update stampede JS tests to highlight/troubleshoot preexisting unload issue --- libraries/script-engine/src/ScriptEngine.cpp | 182 +++++++++--------- libraries/script-engine/src/ScriptEngine.h | 4 +- .../tests/entityServerStampedeTest-entity.js | 21 ++ .../tests/entityServerStampedeTest.js | 2 +- .../tests/entityStampedeTest-entity.js | 13 +- 5 files changed, 127 insertions(+), 95 deletions(-) create mode 100644 scripts/developer/tests/entityServerStampedeTest-entity.js diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index cb63628d1a..c0b2881b72 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1496,67 +1496,58 @@ const auto RETRY_ATTEMPT_BATCH = 100; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH" const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000); const static EntityItemID BAD_SCRIPT_UUID_PLACEHOLDER { "{20170224-dead-face-0000-cee000021114}" }; -void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { - const auto needTimer = _pendingLoadEntities.isEmpty(); - // NOTE: forceRedownload isn't forwarded here because deferLoadEntityScript() is only ever called when another entity has taken lead for the script - _pendingLoadEntities.push_back({ entityID, entityScript/*, forceRedownload*/ }); +void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const EntityItemID& leaderID) { + QList retryLoads; + QMutableListIterator i(_deferredEntityLoads); + while (i.hasNext()) { + auto retry = i.next(); + if (retry.entityScript == entityScript) { + retryLoads << retry; + i.remove(); + } + } + foreach(auto &retry, retryLoads) { + // check whether entity was since been deleted + if (!_entityScripts.contains(retry.entityID)) { + qCDebug(scriptengine) << "processDeferredEntityLoads -- entity details gone (entity deleted?)" + << retry.entityID; + continue; + } - auto processDeferredEntities = [this](QScriptContext* context, QScriptEngine*) -> QScriptValue { - QList stillPending; + // check whether entity has since been unloaded or otherwise errored-out + auto details = _entityScripts[retry.entityID]; + if (details.status != EntityScriptStatus::PENDING) { + qCDebug(scriptengine) << "processDeferredEntityLoads -- entity status no longer PENDING; " + << retry.entityID << details.status; + continue; + } - for (int i=0; i < RETRY_ATTEMPT_BATCH && !_pendingLoadEntities.isEmpty(); i++) { - auto retry = _pendingLoadEntities.takeFirst(); - qCDebug(scriptengine) << "deferLoadEntityScript.retry[" << i << "]:" << retry.entityID << retry.entityScript; - if (!_entityScripts.contains(retry.entityID)) { - qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity details gone (entity deleted?)"; + // propagate leader's failure reasons to the pending entity + const auto leaderDetails = _entityScripts[leaderID]; + if (leaderDetails.status != EntityScriptStatus::RUNNING) { + qCDebug(scriptengine) << QString("... pending load of %1 cancelled (leader: %2 status: %3)") + .arg(retry.entityID.toString()).arg(leaderID.toString()).arg(leaderDetails.status); + + auto extraDetail = "\n(propagated from " + leaderID.toString() + ")"; + if (leaderDetails.status == EntityScriptStatus::ERROR_LOADING_SCRIPT || + leaderDetails.status == EntityScriptStatus::ERROR_RUNNING_SCRIPT) { + // propagate same error so User doesn't have to hunt down stampede's leader + updateEntityScriptStatus(retry.entityID, leaderDetails.status, leaderDetails.errorInfo + extraDetail); } else { - auto details = _entityScripts[retry.entityID]; - if (details.status != EntityScriptStatus::PENDING) { - qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity status no longer PENDING; " << details.status; - } else { - if (!_occupiedScriptURLs.contains(retry.entityScript)) { - // the URL is no longer busy with another Entity, so now it's our turn - loadEntityScript(retry.entityID, retry.entityScript, false); - } else { - auto entityID = _occupiedScriptURLs[retry.entityScript]; - if (entityID == BAD_SCRIPT_UUID_PLACEHOLDER) { - // the URL was marked bad by an earlier load attempt, so abort the retry mission - qCDebug(scriptengine) << "... pending load of " << retry.entityID << " cancelled"; - } else { - // the URL is still occupied by another Entity, so add to next retry pass - stillPending << retry; - } - } - } + // the leader Entity somehow ended up in some other state (rapid-fire delete or unload could cause) + updateEntityScriptStatus(retry.entityID, EntityScriptStatus::ERROR_LOADING_SCRIPT, + "A previous Entity failed to load using this script URL; reload to try again." + extraDetail); } + continue; } - foreach(const DeferredLoadEntity& retry, stillPending) { - _pendingLoadEntities.push_back(retry); + + if (_occupiedScriptURLs.contains(retry.entityScript)) { + qCWarning(scriptengine) << "--- SHOULD NOT HAPPEN -- recursive call into processDeferredEntityLoads" << retry.entityScript; + continue; } - if (_pendingLoadEntities.isEmpty()) { - QObject *interval = context->callee().property("interval").toQObject(); - // we're done processing so can kill the timer - qCDebug(scriptengine) << "loadEntityScript.retry queue processing complete -- exiting" << interval; - if (interval) { - clearInterval(interval); - } - } - return QScriptValue(); - }; - if (needTimer) { - // we were the first to get queued -- so set up an interval timer for deferred processing - qCDebug(scriptengine) << "deferLoadEntityScript -- scheduling timer " << RETRY_ATTEMPT_MSECS; - - // processDeferredEntities gets wrapped as a "regular" JavaScript callback so that it can then - // be scheduled using setInterval per usual -- allowing the script engine to own its lifecycle. - // we stil need to call clearInterval manually when done, so its passed to callee() above as a property. - - QScriptValue function = newLambdaFunction(processDeferredEntities, "processDeferredEntities-loader"); - QObject *interval = setInterval( function, RETRY_ATTEMPT_MSECS ); - - // make the interval reachable from the handler - function.setProperty("interval", newQObject(interval)); + // if we made it here then the leading entity was successful so proceed with normal load + loadEntityScript(retry.entityID, retry.entityScript, false); } } @@ -1587,28 +1578,33 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& // This "occupied" approach allows multiple Entities to boot from the same script URL while still taking // full advantage of cacheable require modules. This only affects Entities literally coming in back-to-back // before the first one has time to finish loading. - if (!_occupiedScriptURLs.contains(entityScript)) { - // the scriptURL is available -- flag as in-use and proceed - _occupiedScriptURLs[entityScript] = entityID; - } else { - // another Entity is either still booting from this script URL or has failed and marked it "out of order" + if (_occupiedScriptURLs.contains(entityScript)) { auto currentEntityID = _occupiedScriptURLs[entityScript]; - if (currentEntityID == BAD_SCRIPT_UUID_PLACEHOLDER && !forceRedownload) { - // since not force-reloading, assume that the exact same input would produce the exact same output again - // note: this state gets reset with "reload all scripts," leaving/returning to a Domain, clear cache, etc. - qCDebug(scriptengine) << QString("loadEntityScript.cancelled entity: %1 script: %2 (previous script failure)") - .arg(entityID.toString()).arg(entityScript); - updateEntityScriptStatus(entityID, EntityScriptStatus::ERROR_LOADING_SCRIPT, - "Cached Script was marked invalid by previous Entity load failure."); + if (currentEntityID == BAD_SCRIPT_UUID_PLACEHOLDER) { + if (forceRedownload) { + // script was previously marked unusable, but we're reloading so reset it + _occupiedScriptURLs.remove(entityScript); + } else { + // since not reloading, assume that the exact same input would produce the exact same output again + // note: this state gets reset with "reload all scripts," leaving/returning to a Domain, clear cache, etc. + qCDebug(scriptengine) << QString("loadEntityScript.cancelled entity: %1 script: %2 (previous script failure)") + .arg(entityID.toString()).arg(entityScript); + updateEntityScriptStatus(entityID, EntityScriptStatus::ERROR_LOADING_SCRIPT, + "A previous Entity failed to load using this script URL; reload to try again."); + return; + } } else { // another entity is busy loading from this script URL so wait for them to finish - qCDebug(scriptengine) << QString("loadEntityScript.deferring[%0] entity: %1 script: %2") - .arg( _pendingLoadEntities.size()).arg(entityID.toString()).arg(entityScript); - deferLoadEntityScript(entityID, entityScript, forceRedownload); + qCDebug(scriptengine) << QString("loadEntityScript.deferring[%0] entity: %1 script: %2 (waiting on %3)") + .arg( _deferredEntityLoads.size()).arg(entityID.toString()).arg(entityScript).arg(currentEntityID.toString()); + _deferredEntityLoads.push_back({ entityID, entityScript }); + return; } - return; } + // the scriptURL slot is available; flag as in-use + _occupiedScriptURLs[entityScript] = entityID; + #ifdef DEBUG_ENTITY_STATES auto previousStatus = _entityScripts.contains(entityID) ? _entityScripts[entityID].status : EntityScriptStatus::PENDING; qCDebug(scriptengine) << "loadEntityScript.LOADING: " << entityScript << entityID.toString() @@ -1642,7 +1638,7 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& #endif } // recheck whether us since may have been set to BAD_SCRIPT_UUID_PLACEHOLDER in entityScriptContentAvailable - if (_occupiedScriptURLs[entityScript] == entityID) { + if (_occupiedScriptURLs.contains(entityScript) && _occupiedScriptURLs[entityScript] == entityID) { _occupiedScriptURLs.remove(entityScript); } }); @@ -1679,6 +1675,8 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co auto fileName = isURL ? scriptOrURL : "about:EmbeddedEntityScript"; const EntityScriptDetails &oldDetails = _entityScripts[entityID]; + const auto entityScript = oldDetails.scriptText; + EntityScriptDetails newDetails; newDetails.scriptText = scriptOrURL; @@ -1689,10 +1687,10 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co newDetails.status = status; setEntityScriptDetails(entityID, newDetails); - qCDebug(scriptengine) << "entityScriptContentAvailable -- flagging " << oldDetails.scriptText << " as BAD_SCRIPT_UUID_PLACEHOLDER"; - // flag this scriptURL as unusuable - // note: we want the *original* entityScript string (not the potentially normalized one from ScriptCache results) - _occupiedScriptURLs[oldDetails.scriptText] = BAD_SCRIPT_UUID_PLACEHOLDER; + qCDebug(scriptengine) << "entityScriptContentAvailable -- flagging " << entityScript << " as BAD_SCRIPT_UUID_PLACEHOLDER"; + // flag the original entityScript as unusuable + _occupiedScriptURLs[entityScript] = BAD_SCRIPT_UUID_PLACEHOLDER; + processDeferredEntityLoads(entityScript, entityID); }; // NETWORK / FILESYSTEM ERRORS @@ -1826,6 +1824,9 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co // if we got this far, then call the preload method callEntityScriptMethod(entityID, "preload"); + + _occupiedScriptURLs.remove(entityScript); + processDeferredEntityLoads(entityScript, entityID); } void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { @@ -1845,14 +1846,25 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { #endif if (_entityScripts.contains(entityID)) { + auto oldDetails = _entityScripts[entityID]; if (isEntityScriptRunning(entityID)) { callEntityScriptMethod(entityID, "unload"); + } else { + qCDebug(scriptengine) << "unload called while !running" << entityID << oldDetails.status; + } + if (oldDetails.status != EntityScriptStatus::UNLOADED) { + EntityScriptDetails newDetails; + newDetails.status = EntityScriptStatus::UNLOADED; + newDetails.lastModified = QDateTime::currentMSecsSinceEpoch(); + // keep scriptText populated for the current need to "debouce" duplicate calls to unloadEntityScript + newDetails.scriptText = oldDetails.scriptText; + setEntityScriptDetails(entityID, newDetails); } - EntityScriptDetails newDetails; - newDetails.status = EntityScriptStatus::UNLOADED; - newDetails.lastModified = QDateTime::currentMSecsSinceEpoch(); - setEntityScriptDetails(entityID, newDetails); stopAllTimersForEntityScript(entityID); + { + // FIXME: shouldn't have to do this here, but currently something seems to be firing unloads moments after firing initial load requests + processDeferredEntityLoads(oldDetails.scriptText, entityID); + } } } @@ -1902,17 +1914,7 @@ void ScriptEngine::refreshFileScript(const EntityItemID& entityID) { auto lastModified = QFileInfo(filePath).lastModified().toMSecsSinceEpoch(); if (lastModified > details.lastModified) { scriptInfoMessage("Reloading modified script " + details.scriptText); - - QFile file(filePath); - file.open(QIODevice::ReadOnly); - QString scriptContents = QTextStream(&file).readAll(); - this->unloadEntityScript(entityID); - this->entityScriptContentAvailable(entityID, details.scriptText, scriptContents, true, true, "Success"); - if (!isEntityScriptRunning(entityID)) { - scriptWarningMessage("Reload script " + details.scriptText + " failed"); - } else { - details = _entityScripts[entityID]; - } + loadEntityScript(entityID, details.scriptText, true); } } recurseGuard = false; diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 43cb3f897d..7f533d2f40 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -170,7 +170,6 @@ public: Q_INVOKABLE bool isEntityScriptRunning(const EntityItemID& entityID) { return _entityScripts.contains(entityID) && _entityScripts[entityID].status == EntityScriptStatus::RUNNING; } - Q_INVOKABLE void deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload); Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload); Q_INVOKABLE void unloadEntityScript(const EntityItemID& entityID); // will call unload method Q_INVOKABLE void unloadAllEntityScripts(); @@ -247,6 +246,7 @@ protected: void updateEntityScriptStatus(const EntityItemID& entityID, const EntityScriptStatus& status, const QString& errorInfo = QString()); void setEntityScriptDetails(const EntityItemID& entityID, const EntityScriptDetails& details); void setParentURL(const QString& parentURL) { _parentURL = parentURL; } + void processDeferredEntityLoads(const QString& entityScript, const EntityItemID& leaderID); QObject* setupTimerWithInterval(const QScriptValue& function, int intervalMS, bool isSingleShot); void stopTimer(QTimer* timer); @@ -271,7 +271,7 @@ protected: QSet _includedURLs; QHash _entityScripts; QHash _occupiedScriptURLs; - QList _pendingLoadEntities; + QList _deferredEntityLoads; bool _isThreaded { false }; QScriptEngineDebugger* _debugger { nullptr }; diff --git a/scripts/developer/tests/entityServerStampedeTest-entity.js b/scripts/developer/tests/entityServerStampedeTest-entity.js new file mode 100644 index 0000000000..781753908c --- /dev/null +++ b/scripts/developer/tests/entityServerStampedeTest-entity.js @@ -0,0 +1,21 @@ +(function() { + return { + preload: function(uuid) { + var props = Entities.getEntityProperties(uuid); + var shape = props.shape === 'Sphere' ? 'Hexagon' : 'Sphere'; + + Entities.editEntity(uuid, { + shape: shape, + color: { red: 0xff, green: 0xff, blue: 0xff }, + }); + this.name = props.name; + print("preload", this.name); + }, + unload: function(uuid) { + print("unload", this.name); + Entities.editEntity(uuid, { + color: { red: 0x0f, green: 0x0f, blue: 0xff }, + }); + }, + }; +}) diff --git a/scripts/developer/tests/entityServerStampedeTest.js b/scripts/developer/tests/entityServerStampedeTest.js index 35ecd7eb11..3fcf01bb34 100644 --- a/scripts/developer/tests/entityServerStampedeTest.js +++ b/scripts/developer/tests/entityServerStampedeTest.js @@ -1,7 +1,7 @@ var NUM_ENTITIES = 100; var RADIUS = 2; var DIV = NUM_ENTITIES / Math.PI / 2; -var PASS_SCRIPT_URL = Script.resolvePath('entityStampedeTest-entity.js'); +var PASS_SCRIPT_URL = Script.resolvePath('entityServerStampedeTest-entity.js'); var FAIL_SCRIPT_URL = Script.resolvePath('entityStampedeTest-entity-fail.js'); var origin = Vec3.sum(MyAvatar.position, Vec3.multiply(5, Quat.getFront(MyAvatar.orientation))); diff --git a/scripts/developer/tests/entityStampedeTest-entity.js b/scripts/developer/tests/entityStampedeTest-entity.js index 1f1d6bcf76..bab4efa8eb 100644 --- a/scripts/developer/tests/entityStampedeTest-entity.js +++ b/scripts/developer/tests/entityStampedeTest-entity.js @@ -1,12 +1,21 @@ (function() { return { preload: function(uuid) { - var shape = Entities.getEntityProperties(uuid).shape === 'Sphere' ? 'Cube' : 'Sphere'; + var props = Entities.getEntityProperties(uuid); + var shape = props.shape === 'Sphere' ? 'Cube' : 'Sphere'; Entities.editEntity(uuid, { shape: shape, color: { red: 0xff, green: 0xff, blue: 0xff }, }); - } + this.name = props.name; + print("preload", this.name); + }, + unload: function(uuid) { + print("unload", this.name); + Entities.editEntity(uuid, { + color: { red: 0xff, green: 0x0f, blue: 0x0f }, + }); + }, }; }) From cdbb13ecff0e76a42249a6d30fd89256a0c9f7cd Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 24 Feb 2017 17:40:06 -0500 Subject: [PATCH 13/45] Log cleanup --- libraries/script-engine/src/ScriptEngine.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index c0b2881b72..75dc83fe63 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1587,16 +1587,20 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& } else { // since not reloading, assume that the exact same input would produce the exact same output again // note: this state gets reset with "reload all scripts," leaving/returning to a Domain, clear cache, etc. +#ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << QString("loadEntityScript.cancelled entity: %1 script: %2 (previous script failure)") .arg(entityID.toString()).arg(entityScript); +#endif updateEntityScriptStatus(entityID, EntityScriptStatus::ERROR_LOADING_SCRIPT, "A previous Entity failed to load using this script URL; reload to try again."); return; } } else { // another entity is busy loading from this script URL so wait for them to finish +#ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << QString("loadEntityScript.deferring[%0] entity: %1 script: %2 (waiting on %3)") .arg( _deferredEntityLoads.size()).arg(entityID.toString()).arg(entityScript).arg(currentEntityID.toString()); +#endif _deferredEntityLoads.push_back({ entityID, entityScript }); return; } @@ -1687,7 +1691,9 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co newDetails.status = status; setEntityScriptDetails(entityID, newDetails); +#ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << "entityScriptContentAvailable -- flagging " << entityScript << " as BAD_SCRIPT_UUID_PLACEHOLDER"; +#endif // flag the original entityScript as unusuable _occupiedScriptURLs[entityScript] = BAD_SCRIPT_UUID_PLACEHOLDER; processDeferredEntityLoads(entityScript, entityID); From 731c01985bfafe6966eed5afd49c622d1a0c5087 Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 24 Feb 2017 19:21:17 -0500 Subject: [PATCH 14/45] switch from autos --- libraries/script-engine/src/ScriptEngine.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 75dc83fe63..99339834ce 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1506,7 +1506,7 @@ void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const i.remove(); } } - foreach(auto &retry, retryLoads) { + foreach(DeferredLoadEntity retry, retryLoads) { // check whether entity was since been deleted if (!_entityScripts.contains(retry.entityID)) { qCDebug(scriptengine) << "processDeferredEntityLoads -- entity details gone (entity deleted?)" @@ -1528,7 +1528,7 @@ void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const qCDebug(scriptengine) << QString("... pending load of %1 cancelled (leader: %2 status: %3)") .arg(retry.entityID.toString()).arg(leaderID.toString()).arg(leaderDetails.status); - auto extraDetail = "\n(propagated from " + leaderID.toString() + ")"; + auto extraDetail = QString("\n(propagated from %1)").arg(leaderID.toString()); if (leaderDetails.status == EntityScriptStatus::ERROR_LOADING_SCRIPT || leaderDetails.status == EntityScriptStatus::ERROR_RUNNING_SCRIPT) { // propagate same error so User doesn't have to hunt down stampede's leader @@ -1599,7 +1599,7 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& // another entity is busy loading from this script URL so wait for them to finish #ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << QString("loadEntityScript.deferring[%0] entity: %1 script: %2 (waiting on %3)") - .arg( _deferredEntityLoads.size()).arg(entityID.toString()).arg(entityScript).arg(currentEntityID.toString()); + .arg(_deferredEntityLoads.size()).arg(entityID.toString()).arg(entityScript).arg(currentEntityID.toString()); #endif _deferredEntityLoads.push_back({ entityID, entityScript }); return; @@ -1679,7 +1679,7 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co auto fileName = isURL ? scriptOrURL : "about:EmbeddedEntityScript"; const EntityScriptDetails &oldDetails = _entityScripts[entityID]; - const auto entityScript = oldDetails.scriptText; + const QString entityScript = oldDetails.scriptText; EntityScriptDetails newDetails; newDetails.scriptText = scriptOrURL; @@ -1852,7 +1852,7 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { #endif if (_entityScripts.contains(entityID)) { - auto oldDetails = _entityScripts[entityID]; + const EntityScriptDetails &oldDetails = _entityScripts[entityID]; if (isEntityScriptRunning(entityID)) { callEntityScriptMethod(entityID, "unload"); } else { From 5b11c0e00a92b55e12f4c433177d807b251b1a1c Mon Sep 17 00:00:00 2001 From: humbletim Date: Fri, 24 Feb 2017 20:45:12 -0500 Subject: [PATCH 15/45] remove unused consts --- libraries/script-engine/src/ScriptEngine.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 99339834ce..d4b6d592c0 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1492,8 +1492,6 @@ bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntitySc return true; } -const auto RETRY_ATTEMPT_BATCH = 100; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH").toInt(), 1, 50); -const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000); const static EntityItemID BAD_SCRIPT_UUID_PLACEHOLDER { "{20170224-dead-face-0000-cee000021114}" }; void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const EntityItemID& leaderID) { From 654c72fb7e19c6ee6f5d46249220b5257c5bea9e Mon Sep 17 00:00:00 2001 From: humbletim Date: Mon, 27 Feb 2017 22:58:59 -0500 Subject: [PATCH 16/45] Add weakRef guard to detect ScriptEngine deletion during ScriptCache::getScriptContents --- libraries/script-engine/src/ScriptEngine.cpp | 8 +++++++- libraries/script-engine/src/ScriptEngine.h | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index d4b6d592c0..bbe2d90ba4 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1620,8 +1620,14 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& setEntityScriptDetails(entityID, newDetails); auto scriptCache = DependencyManager::get(); + // note: see EntityTreeRenderer.cpp for shared pointer lifecycle management + QWeakPointer weakRef(sharedFromThis()); scriptCache->getScriptContents(entityScript, - [this, entityScript, entityID](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { + [this, weakRef, entityScript, entityID](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { + if (!weakRef) { + qCWarning(scriptengine) << "loadEntityScript.contentAvailable -- ScriptEngine was deleted during getScriptContents!!"; + return; + } if (isStopping()) { #ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- stopping"; diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 7f533d2f40..b988ccfe90 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -78,7 +78,7 @@ public: QUrl definingSandboxURL { QUrl("about:EntityScript") }; }; -class ScriptEngine : public BaseScriptEngine, public EntitiesScriptEngineProvider { +class ScriptEngine : public BaseScriptEngine, public EntitiesScriptEngineProvider, public QEnableSharedFromThis { Q_OBJECT Q_PROPERTY(QString context READ getContext) public: From 2505a89b5e6c3aac60420918164a070ca9bdc5fb Mon Sep 17 00:00:00 2001 From: humbletim Date: Tue, 28 Feb 2017 00:41:35 -0500 Subject: [PATCH 17/45] Per CR feedback bump to strong ref --- libraries/script-engine/src/ScriptEngine.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index bbe2d90ba4..621be00739 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1624,7 +1624,8 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& QWeakPointer weakRef(sharedFromThis()); scriptCache->getScriptContents(entityScript, [this, weakRef, entityScript, entityID](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { - if (!weakRef) { + QSharedPointer strongRef(weakRef); + if (!strongRef) { qCWarning(scriptengine) << "loadEntityScript.contentAvailable -- ScriptEngine was deleted during getScriptContents!!"; return; } From c892d8a81a21d609bea31f2533b9120cb243fd57 Mon Sep 17 00:00:00 2001 From: Bradley Austin Davis Date: Fri, 3 Mar 2017 10:41:47 -0800 Subject: [PATCH 18/45] Update Oculus SDK to 1.11 --- cmake/externals/LibOVR/CMakeLists.txt | 26 ++++++--------------- cmake/externals/LibOVR/LibOVRCMakeLists.txt | 15 ++++++++++++ 2 files changed, 22 insertions(+), 19 deletions(-) create mode 100644 cmake/externals/LibOVR/LibOVRCMakeLists.txt diff --git a/cmake/externals/LibOVR/CMakeLists.txt b/cmake/externals/LibOVR/CMakeLists.txt index 54a4a47929..d8b8307082 100644 --- a/cmake/externals/LibOVR/CMakeLists.txt +++ b/cmake/externals/LibOVR/CMakeLists.txt @@ -12,35 +12,23 @@ string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) # 0.5 public # URL http://static.oculus.com/sdk-downloads/ovr_sdk_win_0.5.0.1.zip # URL_MD5 d3fc4c02db9be5ff08af4ef4c97b32f9 -# 1.3 public -# URL http://hifi-public.s3.amazonaws.com/dependencies/ovr_sdk_win_1.3.0_public.zip -# URL_MD5 4d26faba0c1f35ff80bf674c96ed9259 if (WIN32) ExternalProject_Add( ${EXTERNAL_NAME} - URL https://hifi-public.s3.amazonaws.com/dependencies/ovr_sdk_win_1.8.0_public.zip - URL_MD5 bea17e04acc1dd8cf7cabefa1b28cc3c - CONFIGURE_COMMAND "" - BUILD_COMMAND "" - INSTALL_COMMAND "" + URL https://static.oculus.com/sdk-downloads/1.11.0/Public/1486063832/ovr_sdk_win_1.11.0_public.zip + URL_MD5 ea484403757cbfdfa743b6577fb1f9d2 + CMAKE_ARGS -DCMAKE_INSTALL_PREFIX:PATH= + PATCH_COMMAND ${CMAKE_COMMAND} -E copy "${CMAKE_CURRENT_SOURCE_DIR}/LibOVRCMakeLists.txt" /CMakeLists.txt LOG_DOWNLOAD 1 ) ExternalProject_Get_Property(${EXTERNAL_NAME} SOURCE_DIR) - message("LIBOVR dir ${SOURCE_DIR}") - set(LIBOVR_DIR ${SOURCE_DIR}/LibOVR) - if ("${CMAKE_SIZEOF_VOID_P}" EQUAL "8") - set(LIBOVR_LIB_DIR ${LIBOVR_DIR}/Lib/Windows/x64/Release/VS2013 CACHE TYPE INTERNAL) - else() - set(LIBOVR_LIB_DIR ${LIBOVR_DIR}/Lib/Windows/Win32/Release/VS2013 CACHE TYPE INTERNAL) - endif() - + ExternalProject_Get_Property(${EXTERNAL_NAME} INSTALL_DIR) + set(LIBOVR_DIR ${INSTALL_DIR}) set(${EXTERNAL_NAME_UPPER}_INCLUDE_DIRS ${LIBOVR_DIR}/Include CACHE TYPE INTERNAL) - message("LIBOVR include dir ${${EXTERNAL_NAME_UPPER}_INCLUDE_DIRS}") - set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${LIBOVR_LIB_DIR}/LibOVR.lib CACHE TYPE INTERNAL) - + set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${LIBOVR_DIR}/Lib/LibOVR.lib CACHE TYPE INTERNAL) elseif(APPLE) ExternalProject_Add( diff --git a/cmake/externals/LibOVR/LibOVRCMakeLists.txt b/cmake/externals/LibOVR/LibOVRCMakeLists.txt new file mode 100644 index 0000000000..bab6337ae2 --- /dev/null +++ b/cmake/externals/LibOVR/LibOVRCMakeLists.txt @@ -0,0 +1,15 @@ +cmake_minimum_required(VERSION 3.2) +project(LibOVR) + +#set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DOVR_BUILD_DEBUG") + +include_directories(LibOVR/Include LibOVR/Src) +#include_directories(LibOVRKernel/Src/) +file(GLOB HEADER_FILES LibOVR/Include/*.h) +file(GLOB EXTRA_HEADER_FILES LibOVR/Include/Extras/*.h) +file(GLOB_RECURSE SOURCE_FILES LibOVR/Src/*.c LibOVR/Src/*.cpp) +add_library(LibOVR STATIC ${SOURCE_FILES} ${HEADER_FILES} ${EXTRA_HEADER_FILES}) + +install(TARGETS LibOVR DESTINATION Lib) +install(FILES ${HEADER_FILES} DESTINATION Include) +install(FILES ${EXTRA_HEADER_FILES} DESTINATION Include/Extras) \ No newline at end of file From ce9d637b3f65194bb8b0c3140fe5ee84cde12bec Mon Sep 17 00:00:00 2001 From: Bradley Austin Davis Date: Fri, 3 Mar 2017 10:46:05 -0800 Subject: [PATCH 19/45] Expose ASW activation status, provide normalized present rate information --- .../src/display-plugins/OpenGLDisplayPlugin.cpp | 5 +++++ .../src/display-plugins/OpenGLDisplayPlugin.h | 2 ++ libraries/plugins/src/plugins/DisplayPlugin.h | 7 ++++++- plugins/oculus/src/OculusDisplayPlugin.cpp | 17 +++++++++++++++-- plugins/oculus/src/OculusDisplayPlugin.h | 4 +++- plugins/oculus/src/OculusHelpers.cpp | 6 +++++- 6 files changed, 36 insertions(+), 5 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp index cf6b39812a..b23b59d3f0 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp @@ -654,6 +654,11 @@ float OpenGLDisplayPlugin::presentRate() const { return _presentRate.rate(); } +void OpenGLDisplayPlugin::resetPresentRate() { + // FIXME + // _presentRate = RateCounter<100>(); +} + float OpenGLDisplayPlugin::renderRate() const { return _renderRate.rate(); } diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h index f4efc0267b..e1eea5de6c 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h @@ -59,6 +59,8 @@ public: float presentRate() const override; + void resetPresentRate() override; + float newFramePresentRate() const override; float droppedFrameRate() const override; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 2491aed817..754c919fd4 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -139,7 +139,7 @@ public: /// By default, all HMDs are stereo virtual bool isStereo() const { return isHmd(); } virtual bool isThrottled() const { return false; } - virtual float getTargetFrameRate() const { return 0.0f; } + virtual float getTargetFrameRate() const { return 1.0f; } virtual bool hasAsyncReprojection() const { return false; } /// Returns a boolean value indicating whether the display is currently visible @@ -189,6 +189,11 @@ public: virtual float renderRate() const { return -1.0f; } // Rate at which we present to the display device virtual float presentRate() const { return -1.0f; } + // Reset the present rate tracking (useful for if the target frame rate changes as in ASW for Oculus) + virtual void resetPresentRate() {} + // Return the present rate as fraction of the target present rate (hopefully 0.0 and 1.0) + virtual float normalizedPresentRate() const { return presentRate() / getTargetFrameRate(); } + // Rate at which old frames are presented to the device display virtual float stutterRate() const { return -1.0f; } // Rate at which new frames are being presented to the display device diff --git a/plugins/oculus/src/OculusDisplayPlugin.cpp b/plugins/oculus/src/OculusDisplayPlugin.cpp index b076170ae5..db8c92ac23 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.cpp +++ b/plugins/oculus/src/OculusDisplayPlugin.cpp @@ -28,6 +28,12 @@ OculusDisplayPlugin::OculusDisplayPlugin() { _compositorDroppedFrames.store(0); } +float OculusDisplayPlugin::getTargetFrameRate() const { + if (_aswActive) { + return _hmdDesc.DisplayRefreshRate / 2.0f; + } + return _hmdDesc.DisplayRefreshRate; +} bool OculusDisplayPlugin::internalActivate() { bool result = Parent::internalActivate(); @@ -185,8 +191,6 @@ void OculusDisplayPlugin::hmdPresent() { } } - - if (!OVR_SUCCESS(result)) { logWarning("Failed to present"); } @@ -195,12 +199,20 @@ void OculusDisplayPlugin::hmdPresent() { static int appDroppedFrames = 0; ovrPerfStats perfStats; ovr_GetPerfStats(_session, &perfStats); + bool shouldResetPresentRate = false; for (int i = 0; i < perfStats.FrameStatsCount; ++i) { const auto& frameStats = perfStats.FrameStats[i]; int delta = frameStats.CompositorDroppedFrameCount - compositorDroppedFrames; _stutterRate.increment(delta); compositorDroppedFrames = frameStats.CompositorDroppedFrameCount; appDroppedFrames = frameStats.AppDroppedFrameCount; + bool newAswState = ovrTrue == frameStats.AswIsActive; + if (_aswActive.exchange(newAswState) != newAswState) { + shouldResetPresentRate = true; + } + } + if (shouldResetPresentRate) { + resetPresentRate(); } _appDroppedFrames.store(appDroppedFrames); _compositorDroppedFrames.store(compositorDroppedFrames); @@ -212,6 +224,7 @@ void OculusDisplayPlugin::hmdPresent() { QJsonObject OculusDisplayPlugin::getHardwareStats() const { QJsonObject hardwareStats; + hardwareStats["asw_active"] = _aswActive.load(); hardwareStats["app_dropped_frame_count"] = _appDroppedFrames.load(); hardwareStats["compositor_dropped_frame_count"] = _compositorDroppedFrames.load(); hardwareStats["long_render_count"] = _longRenders.load(); diff --git a/plugins/oculus/src/OculusDisplayPlugin.h b/plugins/oculus/src/OculusDisplayPlugin.h index 6fc50b829f..9209fd373e 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.h +++ b/plugins/oculus/src/OculusDisplayPlugin.h @@ -20,7 +20,8 @@ public: QString getPreferredAudioInDevice() const override; QString getPreferredAudioOutDevice() const override; - + float getTargetFrameRate() const override; + virtual QJsonObject getHardwareStats() const; protected: @@ -39,6 +40,7 @@ private: gpu::FramebufferPointer _outputFramebuffer; bool _customized { false }; + std::atomic_bool _aswActive; std::atomic_int _compositorDroppedFrames; std::atomic_int _appDroppedFrames; std::atomic_int _longSubmits; diff --git a/plugins/oculus/src/OculusHelpers.cpp b/plugins/oculus/src/OculusHelpers.cpp index 340b804404..767d191c03 100644 --- a/plugins/oculus/src/OculusHelpers.cpp +++ b/plugins/oculus/src/OculusHelpers.cpp @@ -88,7 +88,11 @@ ovrSession acquireOculusSession() { } if (!session) { - if (!OVR_SUCCESS(ovr_Initialize(nullptr))) { + ovrInitParams initParams { + ovrInit_RequestVersion | ovrInit_MixedRendering, OVR_MINOR_VERSION, nullptr, 0, 0 + }; + + if (!OVR_SUCCESS(ovr_Initialize(&initParams))) { logWarning("Failed to initialize Oculus SDK"); return session; } From a715a694f0d2ffb445e3feb0438b5f2e4af3cc98 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Fri, 3 Mar 2017 16:27:38 -0800 Subject: [PATCH 20/45] fix 'banned user doesn't disappear from pal until refresh' --- interface/resources/qml/hifi/Pal.qml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 28384f9c1c..60b29960fd 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -286,7 +286,7 @@ Rectangle { HifiControls.GlyphButton { function getGlyph() { var fileName = "vol_"; - if (model["personalMute"]) { + if (model && model.personalMute) { fileName += "x_"; } fileName += (4.0*(model ? model.avgAudioLevel : 0.0)).toFixed(0); @@ -360,10 +360,8 @@ Rectangle { Users[styleData.role](model.sessionId) UserActivityLogger["palAction"](styleData.role, model.sessionId) if (styleData.role === "kick") { - // Just for now, while we cannot undo "Ban": - userModel.remove(model.userIndex) - delete userModelData[model.userIndex] // Defensive programming - sortModel() + userModelData.splice(model.userIndex, 1) + userModel.remove(model.userIndex) // after changing userModelData, b/c ListModel can frob the data } } // muted/error glyphs From 9908723bb9192da863862671d474630d02d7e952 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Fri, 3 Mar 2017 22:27:38 -0800 Subject: [PATCH 21/45] fist cut at basic TriangleSet class --- libraries/render-utils/src/Model.cpp | 65 +++++++++++++++----------- libraries/render-utils/src/Model.h | 3 +- libraries/shared/src/AABox.h | 2 + libraries/shared/src/TriangleSet.cpp | 69 ++++++++++++++++++++++++++++ libraries/shared/src/TriangleSet.h | 33 +++++++++++++ 5 files changed, 144 insertions(+), 28 deletions(-) create mode 100644 libraries/shared/src/TriangleSet.cpp create mode 100644 libraries/shared/src/TriangleSet.h diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index adfffe2614..9dd3563887 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -376,24 +376,26 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g } for (const auto& subMeshBox : _calculatedMeshBoxes) { + bool intersectedSubMesh = false; + float subMeshDistance = std::numeric_limits::max(); if (subMeshBox.findRayIntersection(origin, direction, distanceToSubMesh, subMeshFace, subMeshSurfaceNormal)) { if (distanceToSubMesh < bestDistance) { if (pickAgainstTriangles) { - // check our triangles here.... - const QVector& meshTriangles = _calculatedMeshTriangles[subMeshIndex]; - for(const auto& triangle : meshTriangles) { - float thisTriangleDistance; - if (findRayTriangleIntersection(origin, direction, triangle, thisTriangleDistance)) { - if (thisTriangleDistance < bestDistance) { - bestDistance = thisTriangleDistance; - intersectedSomething = true; - face = subMeshFace; - surfaceNormal = triangle.getNormal(); - extraInfo = geometry.getModelNameOfMesh(subMeshIndex); - } - } + + float subMeshDistance; + glm::vec3 subMeshNormal; + const auto& meshTriangleSet = _meshTriangleSets[subMeshIndex]; + bool intersectedMesh = meshTriangleSet.findRayIntersection(origin, direction, subMeshDistance, subMeshNormal); + + if (intersectedMesh && subMeshDistance < bestDistance) { + bestDistance = subMeshDistance; + intersectedSomething = true; + face = subMeshFace; + surfaceNormal = subMeshNormal; + extraInfo = geometry.getModelNameOfMesh(subMeshIndex); } + } else { // this is the non-triangle picking case... bestDistance = distanceToSubMesh; @@ -401,9 +403,13 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g face = subMeshFace; surfaceNormal = subMeshSurfaceNormal; extraInfo = geometry.getModelNameOfMesh(subMeshIndex); + + intersectedSubMesh = true; } } } + + subMeshIndex++; } _mutex.unlock(); @@ -451,18 +457,8 @@ bool Model::convexHullContains(glm::vec3 point) { int subMeshIndex = 0; foreach(const AABox& subMeshBox, _calculatedMeshBoxes) { if (subMeshBox.contains(point)) { - bool insideMesh = true; - // To be inside the sub mesh, we need to be behind every triangles' planes - const QVector& meshTriangles = _calculatedMeshTriangles[subMeshIndex]; - foreach (const Triangle& triangle, meshTriangles) { - if (!isPointBehindTrianglesPlane(point, triangle.v0, triangle.v1, triangle.v2)) { - // it's not behind at least one so we bail - insideMesh = false; - break; - } - } - if (insideMesh) { + if (_meshTriangleSets[subMeshIndex].convexHullContains(point)) { // It's inside this mesh, return true. _mutex.unlock(); return true; @@ -486,12 +482,20 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { bool calculatedMeshTrianglesNeeded = pickAgainstTriangles && !_calculatedMeshTrianglesValid; if (!_calculatedMeshBoxesValid || calculatedMeshTrianglesNeeded || (!_calculatedMeshPartBoxesValid && pickAgainstTriangles) ) { + + if (pickAgainstTriangles) { + qDebug() << "RECALCULATING triangles!"; + } else { + qDebug() << "RECALCULATING boxes!"; + } + const FBXGeometry& geometry = getFBXGeometry(); int numberOfMeshes = geometry.meshes.size(); _calculatedMeshBoxes.resize(numberOfMeshes); - _calculatedMeshTriangles.clear(); - _calculatedMeshTriangles.resize(numberOfMeshes); _calculatedMeshPartBoxes.clear(); + _meshTriangleSets.clear(); + _meshTriangleSets.resize(numberOfMeshes); + for (int i = 0; i < numberOfMeshes; i++) { const FBXMesh& mesh = geometry.meshes.at(i); Extents scaledMeshExtents = calculateScaledOffsetExtents(mesh.meshExtents, _translation, _rotation); @@ -499,6 +503,8 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { _calculatedMeshBoxes[i] = AABox(scaledMeshExtents); if (pickAgainstTriangles) { + auto& thisMeshTriangleSet = _meshTriangleSets[i]; + QVector thisMeshTriangles; for (int j = 0; j < mesh.parts.size(); j++) { const FBXMeshPart& part = mesh.parts.at(j); @@ -550,6 +556,9 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { thisMeshTriangles.push_back(tri1); thisMeshTriangles.push_back(tri2); + thisMeshTriangleSet.insertTriangle(tri1); + thisMeshTriangleSet.insertTriangle(tri2); + } } @@ -582,11 +591,13 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { Triangle tri = { v0, v1, v2 }; thisMeshTriangles.push_back(tri); + + thisMeshTriangleSet.insertTriangle(tri); + } } _calculatedMeshPartBoxes[QPair(i, j)] = thisPartBounds; } - _calculatedMeshTriangles[i] = thisMeshTriangles; _calculatedMeshPartBoxesValid = true; } } diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 7c373274e4..bc3cf0e521 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -28,6 +28,7 @@ #include #include #include +#include #include "GeometryCache.h" #include "TextureCache.h" @@ -362,8 +363,8 @@ protected: bool _calculatedMeshPartBoxesValid; QVector _calculatedMeshBoxes; // world coordinate AABoxes for all sub mesh boxes bool _calculatedMeshBoxesValid; + QVector< TriangleSet > _meshTriangleSets; // world coordinate triangles for all sub meshes - QVector< QVector > _calculatedMeshTriangles; // world coordinate triangles for all sub meshes bool _calculatedMeshTrianglesValid; QMutex _mutex; diff --git a/libraries/shared/src/AABox.h b/libraries/shared/src/AABox.h index 2f0b09d67a..ccc7b6e302 100644 --- a/libraries/shared/src/AABox.h +++ b/libraries/shared/src/AABox.h @@ -109,6 +109,8 @@ public: bool isInvalid() const { return _corner == INFINITY_VECTOR; } + void clear() { _corner = INFINITY_VECTOR; _scale = glm::vec3(0.0f); } + private: glm::vec3 getClosestPointOnFace(const glm::vec3& point, BoxFace face) const; glm::vec3 getClosestPointOnFace(const glm::vec4& origin, const glm::vec4& direction, BoxFace face) const; diff --git a/libraries/shared/src/TriangleSet.cpp b/libraries/shared/src/TriangleSet.cpp new file mode 100644 index 0000000000..3264c259d3 --- /dev/null +++ b/libraries/shared/src/TriangleSet.cpp @@ -0,0 +1,69 @@ +// +// TriangleSet.cpp +// libraries/entities/src +// +// Created by Brad Hefta-Gaub on 3/2/2017. +// Copyright 2017 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 "GLMHelpers.h" +#include "TriangleSet.h" + +void TriangleSet::insertTriangle(const Triangle& t) { + _triangles.push_back(t); + + _bounds += t.v0; + _bounds += t.v1; + _bounds += t.v2; +} + +void TriangleSet::clear() { + _triangles.clear(); + _bounds.clear(); +} + +// Determine of the given ray (origin/direction) in model space intersects with any triangles +// in the set. If an intersection occurs, the distance and surface normal will be provided. +bool TriangleSet::findRayIntersection(const glm::vec3& origin, const glm::vec3& direction, + float& distance, glm::vec3& surfaceNormal) const { + + float bestDistance = std::numeric_limits::max(); + float thisTriangleDistance; + bool intersectedSomething = false; + + for (const auto& triangle : _triangles) { + if (findRayTriangleIntersection(origin, direction, triangle, thisTriangleDistance)) { + if (thisTriangleDistance < bestDistance) { + bestDistance = thisTriangleDistance; + intersectedSomething = true; + surfaceNormal = triangle.getNormal(); + distance = bestDistance; + //face = subMeshFace; + //extraInfo = geometry.getModelNameOfMesh(subMeshIndex); + } + } + } + return intersectedSomething; +} + + +bool TriangleSet::convexHullContains(const glm::vec3& point) const { + if (!_bounds.contains(point)) { + return false; + } + + bool insideMesh = true; // optimistic + for (const auto& triangle : _triangles) { + if (!isPointBehindTrianglesPlane(point, triangle.v0, triangle.v1, triangle.v2)) { + // it's not behind at least one so we bail + insideMesh = false; + break; + } + + } + return insideMesh; +} + diff --git a/libraries/shared/src/TriangleSet.h b/libraries/shared/src/TriangleSet.h new file mode 100644 index 0000000000..3aef3966b6 --- /dev/null +++ b/libraries/shared/src/TriangleSet.h @@ -0,0 +1,33 @@ +// +// TriangleSet.h +// libraries/entities/src +// +// Created by Brad Hefta-Gaub on 3/2/2017. +// Copyright 2017 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 + +#include "AABox.h" +#include "GeometryUtil.h" + +class TriangleSet { +public: + void insertTriangle(const Triangle& t); + void clear(); + + // Determine of the given ray (origin/direction) in model space intersects with any triangles + // in the set. If an intersection occurs, the distance and surface normal will be provided. + bool findRayIntersection(const glm::vec3& origin, const glm::vec3& direction, + float& distance, glm::vec3& surfaceNormal) const; + + bool convexHullContains(const glm::vec3& point) const; // this point is "inside" all triangles + const AABox& getBounds() const; + +private: + QVector _triangles; + AABox _bounds; +}; From d85cb645b0301338f5cedad9b547772d8e4b6890 Mon Sep 17 00:00:00 2001 From: Menithal Date: Sat, 4 Mar 2017 11:42:43 +0200 Subject: [PATCH 22/45] Changed limit logic, default limit is now 0 Clones now have a named based on the original entity id Limit is now calculated from the source instance, instead of just clone name to avoid a single box being calculated as something else Default limit is now 0, which disables limit --- .../system/controllers/handControllerGrab.js | 76 +++++++++---------- scripts/system/html/js/entityProperties.js | 6 +- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index 2e1d538db9..26178773e9 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -274,24 +274,24 @@ CONTROLLER_STATE_MACHINE[STATE_OVERLAY_LASER_TOUCHING] = CONTROLLER_STATE_MACHIN // Object assign polyfill if (typeof Object.assign != 'function') { - Object.assign = function(target, varArgs) { - 'use strict'; - if (target == null) { - throw new TypeError('Cannot convert undefined or null to object'); - } - var to = Object(target); - for (var index = 1; index < arguments.length; index++) { - var nextSource = arguments[index]; - if (nextSource != null) { - for (var nextKey in nextSource) { - if (Object.prototype.hasOwnProperty.call(nextSource, nextKey)) { - to[nextKey] = nextSource[nextKey]; - } + Object.assign = function(target, varArgs) { + 'use strict'; + if (target == null) { + throw new TypeError('Cannot convert undefined or null to object'); } - } - } - return to; - }; + var to = Object(target); + for (var index = 1; index < arguments.length; index++) { + var nextSource = arguments[index]; + if (nextSource != null) { + for (var nextKey in nextSource) { + if (Object.prototype.hasOwnProperty.call(nextSource, nextKey)) { + to[nextKey] = nextSource[nextKey]; + } + } + } + } + return to; + }; } function distanceBetweenPointAndEntityBoundingBox(point, entityProps) { @@ -1524,16 +1524,16 @@ function MyController(hand) { return true; }; this.entityIsCloneable = function(entityID) { - var entityProps = entityPropertiesCache.getGrabbableProps(entityID); - var props = entityPropertiesCache.getProps(entityID); - if (!props) { - return false; - } + var entityProps = entityPropertiesCache.getGrabbableProps(entityID); + var props = entityPropertiesCache.getProps(entityID); + if (!props) { + return false; + } - if (entityProps.hasOwnProperty("cloneable")) { - return entityProps.cloneable; - } - return false; + if (entityProps.hasOwnProperty("cloneable")) { + return entityProps.cloneable; + } + return false; } this.entityIsGrabbable = function(entityID) { var grabbableProps = entityPropertiesCache.getGrabbableProps(entityID); @@ -2229,7 +2229,7 @@ function MyController(hand) { // Can't set state of other controller to STATE_DISTANCE_HOLDING because then either: // (a) The entity would jump to line up with the formerly rotating controller's orientation, or // (b) The grab beam would need an orientation offset to the controller's true orientation. - // Neither of these options is good, so instead set STATE_SEARCHING and subsequently let the formerly distance + // Neither of these options is good, so instead set STATE_SEARCHING and subsequently let the formerly distance // rotating controller start distance holding the entity if it happens to be pointing at the entity. } return; @@ -2601,31 +2601,29 @@ function MyController(hand) { var userData = JSON.parse(grabbedProperties.userData); var grabInfo = userData.grabbableKey; if (grabInfo && grabInfo.cloneable) { - // Check if - var worldEntities = Entities.findEntitiesInBox(Vec3.subtract(MyAvatar.position, {x:25,y:25, z:25}), {x:50, y: 50, z: 50}) + var worldEntities = Entities.findEntities(MyAvatar.position, 50); var count = 0; worldEntities.forEach(function(item) { var item = Entities.getEntityProperties(item, ["name"]); - if (item.name === grabbedProperties.name) { + if (item.name.indexOf('-clone-' + grabbedProperties.id) !== -1) { count++; } }) + + var limit = grabInfo.cloneLimit ? grabInfo.cloneLimit : 0; + if (count >= limit && limit !== 0) { + delete limit; + return; + } + var cloneableProps = Entities.getEntityProperties(grabbedProperties.id); + cloneableProps.name = cloneableProps.name + '-clone-' + grabbedProperties.id; var lifetime = grabInfo.cloneLifetime ? grabInfo.cloneLifetime : 300; - var limit = grabInfo.cloneLimit ? grabInfo.cloneLimit : 10; var dynamic = grabInfo.cloneDynamic ? grabInfo.cloneDynamic : false; var cUserData = Object.assign({}, userData); var cProperties = Object.assign({}, cloneableProps); isClone = true; - if (count > limit) { - delete cloneableProps; - delete lifetime; - delete cUserData; - delete cProperties; - return; - } - delete cUserData.grabbableKey.cloneLifetime; delete cUserData.grabbableKey.cloneable; delete cUserData.grabbableKey.cloneDynamic; diff --git a/scripts/system/html/js/entityProperties.js b/scripts/system/html/js/entityProperties.js index 3280e1f196..66abcaa67a 100644 --- a/scripts/system/html/js/entityProperties.js +++ b/scripts/system/html/js/entityProperties.js @@ -879,7 +879,7 @@ function loaded() { elCloneable.checked = false; elCloneableDynamic.checked = false; elCloneableGroup.style.display = elCloneable.checked ? "block": "none"; - elCloneableLimit.value = 10; + elCloneableLimit.value = 0; elCloneableLifetime.value = 300; var parsedUserData = {} @@ -899,8 +899,6 @@ function loaded() { if ("cloneable" in parsedUserData["grabbableKey"]) { elCloneable.checked = parsedUserData["grabbableKey"].cloneable; elCloneableGroup.style.display = elCloneable.checked ? "block": "none"; - elCloneableLimit.value = elCloneable.checked ? 10: 0; - elCloneableLifetime.value = elCloneable.checked ? 300: 0; elCloneableDynamic.checked = parsedUserData["grabbableKey"].cloneDynamic ? parsedUserData["grabbableKey"].cloneDynamic : properties.dynamic; elDynamic.checked = elCloneable.checked ? false: properties.dynamic; if (elCloneable.checked) { @@ -908,7 +906,7 @@ function loaded() { elCloneableLifetime.value = parsedUserData["grabbableKey"].cloneLifetime ? parsedUserData["grabbableKey"].cloneLifetime : 300; } if ("cloneLimit" in parsedUserData["grabbableKey"]) { - elCloneableLimit.value = parsedUserData["grabbableKey"].cloneLimit ? parsedUserData["grabbableKey"].cloneLimit : 10; + elCloneableLimit.value = parsedUserData["grabbableKey"].cloneLimit ? parsedUserData["grabbableKey"].cloneLimit : 0; } } } From 32add6635d8588106eddf2d1a235b6e118d2450d Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Sat, 4 Mar 2017 23:33:17 -0800 Subject: [PATCH 23/45] checkpoint on reducing raypicking recalcs --- libraries/fbx/src/FBXReader.h | 2 +- libraries/render-utils/src/Model.cpp | 98 ++++++++++++++++------------ libraries/render-utils/src/Model.h | 6 +- 3 files changed, 62 insertions(+), 44 deletions(-) diff --git a/libraries/fbx/src/FBXReader.h b/libraries/fbx/src/FBXReader.h index 5910b8d312..6e51c413dc 100644 --- a/libraries/fbx/src/FBXReader.h +++ b/libraries/fbx/src/FBXReader.h @@ -300,7 +300,7 @@ public: QHash materials; - glm::mat4 offset; + glm::mat4 offset; // This includes offset, rotation, and scale as specified by the FST file int leftEyeJointIndex = -1; int rightEyeJointIndex = -1; diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 9dd3563887..d94bb67590 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -375,6 +375,14 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g recalculateMeshBoxes(pickAgainstTriangles); } + glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); + + glm::vec3 meshFrameOrigin = glm::vec3(worldToMeshMatrix * glm::vec4(origin, 1.0f)); + glm::vec3 meshFrameDirection = glm::vec3(worldToMeshMatrix * glm::vec4(direction, 0.0f)); + + for (const auto& subMeshBox : _calculatedMeshBoxes) { bool intersectedSubMesh = false; float subMeshDistance = std::numeric_limits::max(); @@ -383,19 +391,24 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g if (distanceToSubMesh < bestDistance) { if (pickAgainstTriangles) { - float subMeshDistance; + float subMeshDistance = 0.0f; glm::vec3 subMeshNormal; - const auto& meshTriangleSet = _meshTriangleSets[subMeshIndex]; - bool intersectedMesh = meshTriangleSet.findRayIntersection(origin, direction, subMeshDistance, subMeshNormal); + const auto& meshTriangleSet = _modelSpaceMeshTriangleSets[subMeshIndex]; + bool intersectedMesh = meshTriangleSet.findRayIntersection(meshFrameOrigin, meshFrameDirection, subMeshDistance, subMeshNormal); - if (intersectedMesh && subMeshDistance < bestDistance) { - bestDistance = subMeshDistance; - intersectedSomething = true; - face = subMeshFace; - surfaceNormal = subMeshNormal; - extraInfo = geometry.getModelNameOfMesh(subMeshIndex); + if (intersectedMesh) { + glm::vec3 meshIntersectionPoint = meshFrameOrigin + (meshFrameDirection * subMeshDistance); + glm::vec3 worldIntersectionPoint = glm::vec3(meshToWorldMatrix * glm::vec4(meshIntersectionPoint, 1.0f)); + float worldDistance = glm::distance(origin, worldIntersectionPoint); + + if (worldDistance < bestDistance) { + bestDistance = subMeshDistance; + intersectedSomething = true; + face = subMeshFace; + surfaceNormal = glm::vec3(meshToWorldMatrix * glm::vec4(subMeshNormal, 0.0f)); + extraInfo = geometry.getModelNameOfMesh(subMeshIndex); + } } - } else { // this is the non-triangle picking case... bestDistance = distanceToSubMesh; @@ -458,7 +471,13 @@ bool Model::convexHullContains(glm::vec3 point) { foreach(const AABox& subMeshBox, _calculatedMeshBoxes) { if (subMeshBox.contains(point)) { - if (_meshTriangleSets[subMeshIndex].convexHullContains(point)) { + glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); + + glm::vec3 meshFramePoint = glm::vec3(worldToMeshMatrix * glm::vec4(point, 1.0f)); + + if (_modelSpaceMeshTriangleSets[subMeshIndex].convexHullContains(meshFramePoint)) { // It's inside this mesh, return true. _mutex.unlock(); return true; @@ -493,8 +512,9 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { int numberOfMeshes = geometry.meshes.size(); _calculatedMeshBoxes.resize(numberOfMeshes); _calculatedMeshPartBoxes.clear(); - _meshTriangleSets.clear(); - _meshTriangleSets.resize(numberOfMeshes); + + _modelSpaceMeshTriangleSets.clear(); + _modelSpaceMeshTriangleSets.resize(numberOfMeshes); for (int i = 0; i < numberOfMeshes; i++) { const FBXMesh& mesh = geometry.meshes.at(i); @@ -503,9 +523,7 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { _calculatedMeshBoxes[i] = AABox(scaledMeshExtents); if (pickAgainstTriangles) { - auto& thisMeshTriangleSet = _meshTriangleSets[i]; - QVector thisMeshTriangles; for (int j = 0; j < mesh.parts.size(); j++) { const FBXMeshPart& part = mesh.parts.at(j); @@ -540,24 +558,21 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { thisPartBounds += mv2; thisPartBounds += mv3; - glm::vec3 v0 = calculateScaledOffsetPoint(mv0); - glm::vec3 v1 = calculateScaledOffsetPoint(mv1); - glm::vec3 v2 = calculateScaledOffsetPoint(mv2); - glm::vec3 v3 = calculateScaledOffsetPoint(mv3); + // let's also track the model space version... (eventually using only this!) + // these points will be transformed by the FST's offset, which includes the + // scaling, rotation, and translation specified by the FST/FBX, this can't change + // at runtime, so we can safely store these in our TriangleSet + { + glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); + glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); + glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); + glm::vec3 v3 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i3], 1.0f)); - // Sam's recommended triangle slices - Triangle tri1 = { v0, v1, v3 }; - Triangle tri2 = { v1, v2, v3 }; - - // NOTE: Random guy on the internet's recommended triangle slices - //Triangle tri1 = { v0, v1, v2 }; - //Triangle tri2 = { v2, v3, v0 }; - - thisMeshTriangles.push_back(tri1); - thisMeshTriangles.push_back(tri2); - - thisMeshTriangleSet.insertTriangle(tri1); - thisMeshTriangleSet.insertTriangle(tri2); + Triangle tri1 = { v0, v1, v3 }; + Triangle tri2 = { v1, v2, v3 }; + _modelSpaceMeshTriangleSets[i].insertTriangle(tri1); + _modelSpaceMeshTriangleSets[i].insertTriangle(tri2); + } } } @@ -584,15 +599,18 @@ void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { thisPartBounds += mv1; thisPartBounds += mv2; - glm::vec3 v0 = calculateScaledOffsetPoint(mv0); - glm::vec3 v1 = calculateScaledOffsetPoint(mv1); - glm::vec3 v2 = calculateScaledOffsetPoint(mv2); + // let's also track the model space version... (eventually using only this!) + // these points will be transformed by the FST's offset, which includes the + // scaling, rotation, and translation specified by the FST/FBX, this can't change + // at runtime, so we can safely store these in our TriangleSet + { + glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); + glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); + glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); - Triangle tri = { v0, v1, v2 }; - - thisMeshTriangles.push_back(tri); - - thisMeshTriangleSet.insertTriangle(tri); + Triangle tri = { v0, v1, v2 }; + _modelSpaceMeshTriangleSets[i].insertTriangle(tri); + } } } diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index bc3cf0e521..763112cebc 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -294,10 +294,10 @@ protected: SpatiallyNestable* _spatiallyNestableOverride; - glm::vec3 _translation; + glm::vec3 _translation; // this is the translation in world coordinates to the model's registration point glm::quat _rotation; glm::vec3 _scale; - glm::vec3 _offset; + glm::vec3 _offset; // this is the translation for the minimum extent of the model (in original mesh coordinate space) to the model's registration point static float FAKE_DIMENSION_PLACEHOLDER; @@ -363,7 +363,7 @@ protected: bool _calculatedMeshPartBoxesValid; QVector _calculatedMeshBoxes; // world coordinate AABoxes for all sub mesh boxes bool _calculatedMeshBoxesValid; - QVector< TriangleSet > _meshTriangleSets; // world coordinate triangles for all sub meshes + QVector _modelSpaceMeshTriangleSets; // model space triangles for all sub meshes bool _calculatedMeshTrianglesValid; QMutex _mutex; From 87bcced409ea42d4e414ac308b0823afb1e3feed Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Sun, 5 Mar 2017 19:23:55 -0800 Subject: [PATCH 24/45] cleanup use of triangleSet for picking --- .../src/RenderableModelEntityItem.cpp | 6 + libraries/render-utils/src/Model.cpp | 333 ++++++------------ libraries/render-utils/src/Model.h | 32 +- libraries/shared/src/TriangleSet.cpp | 31 +- libraries/shared/src/TriangleSet.h | 4 +- 5 files changed, 134 insertions(+), 272 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index e6902228c5..1a6daf3d43 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -418,6 +418,12 @@ void RenderableModelEntityItem::render(RenderArgs* args) { // Enqueue updates for the next frame if (_model) { +#if 1 //def WANT_EXTRA_RENDER_DEBUGGING + // debugging... + gpu::Batch& batch = *args->_batch; + _model->renderDebugMeshBoxes(batch); +#endif + render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene(); // FIXME: this seems like it could be optimized if we tracked our last known visible state in diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index d94bb67590..172290eae5 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -96,9 +96,6 @@ Model::Model(RigPointer rig, QObject* parent, SpatiallyNestable* spatiallyNestab _isVisible(true), _blendNumber(0), _appliedBlendNumber(0), - _calculatedMeshPartBoxesValid(false), - _calculatedMeshBoxesValid(false), - _calculatedMeshTrianglesValid(false), _isWireframe(false), _rig(rig) { @@ -360,19 +357,14 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g // we can use the AABox's ray intersection by mapping our origin and direction into the model frame // and testing intersection there. if (modelFrameBox.findRayIntersection(modelFrameOrigin, modelFrameDirection, distance, face, surfaceNormal)) { + QMutexLocker locker(&_mutex); + float bestDistance = std::numeric_limits::max(); - - float distanceToSubMesh; - BoxFace subMeshFace; - glm::vec3 subMeshSurfaceNormal; int subMeshIndex = 0; - const FBXGeometry& geometry = getFBXGeometry(); - // If we hit the models box, then consider the submeshes... - _mutex.lock(); - if (!_calculatedMeshBoxesValid || (pickAgainstTriangles && !_calculatedMeshTrianglesValid)) { - recalculateMeshBoxes(pickAgainstTriangles); + if (!_triangleSetsValid) { + calculateTriangleSets(); } glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); @@ -382,50 +374,26 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g glm::vec3 meshFrameOrigin = glm::vec3(worldToMeshMatrix * glm::vec4(origin, 1.0f)); glm::vec3 meshFrameDirection = glm::vec3(worldToMeshMatrix * glm::vec4(direction, 0.0f)); + for (const auto& triangleSet : _modelSpaceMeshTriangleSets) { + float triangleSetDistance = 0.0f; + BoxFace triangleSetFace; + glm::vec3 triangleSetNormal; + if (triangleSet.findRayIntersection(meshFrameOrigin, meshFrameDirection, triangleSetDistance, triangleSetFace, triangleSetNormal, pickAgainstTriangles)) { - for (const auto& subMeshBox : _calculatedMeshBoxes) { - bool intersectedSubMesh = false; - float subMeshDistance = std::numeric_limits::max(); + glm::vec3 meshIntersectionPoint = meshFrameOrigin + (meshFrameDirection * triangleSetDistance); + glm::vec3 worldIntersectionPoint = glm::vec3(meshToWorldMatrix * glm::vec4(meshIntersectionPoint, 1.0f)); + float worldDistance = glm::distance(origin, worldIntersectionPoint); - if (subMeshBox.findRayIntersection(origin, direction, distanceToSubMesh, subMeshFace, subMeshSurfaceNormal)) { - if (distanceToSubMesh < bestDistance) { - if (pickAgainstTriangles) { - - float subMeshDistance = 0.0f; - glm::vec3 subMeshNormal; - const auto& meshTriangleSet = _modelSpaceMeshTriangleSets[subMeshIndex]; - bool intersectedMesh = meshTriangleSet.findRayIntersection(meshFrameOrigin, meshFrameDirection, subMeshDistance, subMeshNormal); - - if (intersectedMesh) { - glm::vec3 meshIntersectionPoint = meshFrameOrigin + (meshFrameDirection * subMeshDistance); - glm::vec3 worldIntersectionPoint = glm::vec3(meshToWorldMatrix * glm::vec4(meshIntersectionPoint, 1.0f)); - float worldDistance = glm::distance(origin, worldIntersectionPoint); - - if (worldDistance < bestDistance) { - bestDistance = subMeshDistance; - intersectedSomething = true; - face = subMeshFace; - surfaceNormal = glm::vec3(meshToWorldMatrix * glm::vec4(subMeshNormal, 0.0f)); - extraInfo = geometry.getModelNameOfMesh(subMeshIndex); - } - } - } else { - // this is the non-triangle picking case... - bestDistance = distanceToSubMesh; - intersectedSomething = true; - face = subMeshFace; - surfaceNormal = subMeshSurfaceNormal; - extraInfo = geometry.getModelNameOfMesh(subMeshIndex); - - intersectedSubMesh = true; - } + if (worldDistance < bestDistance) { + bestDistance = worldDistance; + intersectedSomething = true; + face = triangleSetFace; + surfaceNormal = glm::vec3(meshToWorldMatrix * glm::vec4(triangleSetNormal, 0.0f)); + extraInfo = geometry.getModelNameOfMesh(subMeshIndex); } } - - subMeshIndex++; } - _mutex.unlock(); if (intersectedSomething) { distance = bestDistance; @@ -461,182 +429,97 @@ bool Model::convexHullContains(glm::vec3 point) { // we can use the AABox's contains() by mapping our point into the model frame // and testing there. if (modelFrameBox.contains(modelFramePoint)){ - _mutex.lock(); - if (!_calculatedMeshTrianglesValid) { - recalculateMeshBoxes(true); + QMutexLocker locker(&_mutex); + + if (!_triangleSetsValid) { + calculateTriangleSets(); } // If we are inside the models box, then consider the submeshes... - int subMeshIndex = 0; - foreach(const AABox& subMeshBox, _calculatedMeshBoxes) { - if (subMeshBox.contains(point)) { + glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); + glm::vec3 meshFramePoint = glm::vec3(worldToMeshMatrix * glm::vec4(point, 1.0f)); - glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); - glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); - - glm::vec3 meshFramePoint = glm::vec3(worldToMeshMatrix * glm::vec4(point, 1.0f)); - - if (_modelSpaceMeshTriangleSets[subMeshIndex].convexHullContains(meshFramePoint)) { + for (const auto& triangleSet : _modelSpaceMeshTriangleSets) { + const AABox& box = triangleSet.getBounds(); + if (box.contains(meshFramePoint)) { + if (triangleSet.convexHullContains(meshFramePoint)) { // It's inside this mesh, return true. - _mutex.unlock(); return true; } } - subMeshIndex++; } - _mutex.unlock(); + + } // It wasn't in any mesh, return false. return false; } -// TODO: we seem to call this too often when things haven't actually changed... look into optimizing this -// Any script might trigger findRayIntersectionAgainstSubMeshes (and maybe convexHullContains), so these -// can occur multiple times. In addition, rendering does it's own ray picking in order to decide which -// entity-scripts to call. I think it would be best to do the picking once-per-frame (in cpu, or gpu if possible) -// and then the calls use the most recent such result. -void Model::recalculateMeshBoxes(bool pickAgainstTriangles) { +void Model::calculateTriangleSets() { PROFILE_RANGE(render, __FUNCTION__); - bool calculatedMeshTrianglesNeeded = pickAgainstTriangles && !_calculatedMeshTrianglesValid; - if (!_calculatedMeshBoxesValid || calculatedMeshTrianglesNeeded || (!_calculatedMeshPartBoxesValid && pickAgainstTriangles) ) { + const FBXGeometry& geometry = getFBXGeometry(); + int numberOfMeshes = geometry.meshes.size(); - if (pickAgainstTriangles) { - qDebug() << "RECALCULATING triangles!"; - } else { - qDebug() << "RECALCULATING boxes!"; - } + _triangleSetsValid = true; + _modelSpaceMeshTriangleSets.clear(); + _modelSpaceMeshTriangleSets.resize(numberOfMeshes); - const FBXGeometry& geometry = getFBXGeometry(); - int numberOfMeshes = geometry.meshes.size(); - _calculatedMeshBoxes.resize(numberOfMeshes); - _calculatedMeshPartBoxes.clear(); + for (int i = 0; i < numberOfMeshes; i++) { + const FBXMesh& mesh = geometry.meshes.at(i); - _modelSpaceMeshTriangleSets.clear(); - _modelSpaceMeshTriangleSets.resize(numberOfMeshes); + for (int j = 0; j < mesh.parts.size(); j++) { + const FBXMeshPart& part = mesh.parts.at(j); - for (int i = 0; i < numberOfMeshes; i++) { - const FBXMesh& mesh = geometry.meshes.at(i); - Extents scaledMeshExtents = calculateScaledOffsetExtents(mesh.meshExtents, _translation, _rotation); + const int INDICES_PER_TRIANGLE = 3; + const int INDICES_PER_QUAD = 4; - _calculatedMeshBoxes[i] = AABox(scaledMeshExtents); + if (part.quadIndices.size() > 0) { + int numberOfQuads = part.quadIndices.size() / INDICES_PER_QUAD; + int vIndex = 0; + for (int q = 0; q < numberOfQuads; q++) { + int i0 = part.quadIndices[vIndex++]; + int i1 = part.quadIndices[vIndex++]; + int i2 = part.quadIndices[vIndex++]; + int i3 = part.quadIndices[vIndex++]; - if (pickAgainstTriangles) { + // track the model space version... these points will be transformed by the FST's offset, + // which includes the scaling, rotation, and translation specified by the FST/FBX, + // this can't change at runtime, so we can safely store these in our TriangleSet + glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); + glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); + glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); + glm::vec3 v3 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i3], 1.0f)); - for (int j = 0; j < mesh.parts.size(); j++) { - const FBXMeshPart& part = mesh.parts.at(j); - - bool atLeastOnePointInBounds = false; - AABox thisPartBounds; - - const int INDICES_PER_TRIANGLE = 3; - const int INDICES_PER_QUAD = 4; - - if (part.quadIndices.size() > 0) { - int numberOfQuads = part.quadIndices.size() / INDICES_PER_QUAD; - int vIndex = 0; - for (int q = 0; q < numberOfQuads; q++) { - int i0 = part.quadIndices[vIndex++]; - int i1 = part.quadIndices[vIndex++]; - int i2 = part.quadIndices[vIndex++]; - int i3 = part.quadIndices[vIndex++]; - - glm::vec3 mv0 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i0], 1.0f)); - glm::vec3 mv1 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i1], 1.0f)); - glm::vec3 mv2 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i2], 1.0f)); - glm::vec3 mv3 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i3], 1.0f)); - - // track the mesh parts in model space - if (!atLeastOnePointInBounds) { - thisPartBounds.setBox(mv0, 0.0f); - atLeastOnePointInBounds = true; - } else { - thisPartBounds += mv0; - } - thisPartBounds += mv1; - thisPartBounds += mv2; - thisPartBounds += mv3; - - // let's also track the model space version... (eventually using only this!) - // these points will be transformed by the FST's offset, which includes the - // scaling, rotation, and translation specified by the FST/FBX, this can't change - // at runtime, so we can safely store these in our TriangleSet - { - glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); - glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); - glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); - glm::vec3 v3 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i3], 1.0f)); - - Triangle tri1 = { v0, v1, v3 }; - Triangle tri2 = { v1, v2, v3 }; - _modelSpaceMeshTriangleSets[i].insertTriangle(tri1); - _modelSpaceMeshTriangleSets[i].insertTriangle(tri2); - } - - } - } - - if (part.triangleIndices.size() > 0) { - int numberOfTris = part.triangleIndices.size() / INDICES_PER_TRIANGLE; - int vIndex = 0; - for (int t = 0; t < numberOfTris; t++) { - int i0 = part.triangleIndices[vIndex++]; - int i1 = part.triangleIndices[vIndex++]; - int i2 = part.triangleIndices[vIndex++]; - - glm::vec3 mv0 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i0], 1.0f)); - glm::vec3 mv1 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i1], 1.0f)); - glm::vec3 mv2 = glm::vec3(mesh.modelTransform * glm::vec4(mesh.vertices[i2], 1.0f)); - - // track the mesh parts in model space - if (!atLeastOnePointInBounds) { - thisPartBounds.setBox(mv0, 0.0f); - atLeastOnePointInBounds = true; - } else { - thisPartBounds += mv0; - } - thisPartBounds += mv1; - thisPartBounds += mv2; - - // let's also track the model space version... (eventually using only this!) - // these points will be transformed by the FST's offset, which includes the - // scaling, rotation, and translation specified by the FST/FBX, this can't change - // at runtime, so we can safely store these in our TriangleSet - { - glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); - glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); - glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); - - Triangle tri = { v0, v1, v2 }; - _modelSpaceMeshTriangleSets[i].insertTriangle(tri); - } - - } - } - _calculatedMeshPartBoxes[QPair(i, j)] = thisPartBounds; + Triangle tri1 = { v0, v1, v3 }; + Triangle tri2 = { v1, v2, v3 }; + _modelSpaceMeshTriangleSets[i].insertTriangle(tri1); + _modelSpaceMeshTriangleSets[i].insertTriangle(tri2); + } + } + + if (part.triangleIndices.size() > 0) { + int numberOfTris = part.triangleIndices.size() / INDICES_PER_TRIANGLE; + int vIndex = 0; + for (int t = 0; t < numberOfTris; t++) { + int i0 = part.triangleIndices[vIndex++]; + int i1 = part.triangleIndices[vIndex++]; + int i2 = part.triangleIndices[vIndex++]; + + // track the model space version... these points will be transformed by the FST's offset, + // which includes the scaling, rotation, and translation specified by the FST/FBX, + // this can't change at runtime, so we can safely store these in our TriangleSet + glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); + glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); + glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); + + Triangle tri = { v0, v1, v2 }; + _modelSpaceMeshTriangleSets[i].insertTriangle(tri); } - _calculatedMeshPartBoxesValid = true; } } - _calculatedMeshBoxesValid = true; - _calculatedMeshTrianglesValid = pickAgainstTriangles; - } -} - -void Model::renderSetup(RenderArgs* args) { - // set up dilated textures on first render after load/simulate - const FBXGeometry& geometry = getFBXGeometry(); - if (_dilatedTextures.isEmpty()) { - foreach (const FBXMesh& mesh, geometry.meshes) { - QVector > dilated; - dilated.resize(mesh.parts.size()); - _dilatedTextures.append(dilated); - } - } - - if (!_addedToScene && isLoaded()) { - createRenderItemSet(); } } @@ -752,7 +635,15 @@ void Model::removeFromScene(std::shared_ptr scene, render::Pendin void Model::renderDebugMeshBoxes(gpu::Batch& batch) { int colorNdx = 0; _mutex.lock(); - foreach(AABox box, _calculatedMeshBoxes) { + + glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + Transform meshToWorld(meshToWorldMatrix); + batch.setModelTransform(meshToWorld); + + for(const auto& triangleSet : _modelSpaceMeshTriangleSets) { + auto box = triangleSet.getBounds(); + if (_debugMeshBoxesID == GeometryCache::UNKNOWN_ID) { _debugMeshBoxesID = DependencyManager::get()->allocateID(); } @@ -784,8 +675,8 @@ void Model::renderDebugMeshBoxes(gpu::Batch& batch) { points << blf << tlf; glm::vec4 color[] = { - { 1.0f, 0.0f, 0.0f, 1.0f }, // red { 0.0f, 1.0f, 0.0f, 1.0f }, // green + { 1.0f, 0.0f, 0.0f, 1.0f }, // red { 0.0f, 0.0f, 1.0f, 1.0f }, // blue { 1.0f, 0.0f, 1.0f, 1.0f }, // purple { 1.0f, 1.0f, 0.0f, 1.0f }, // yellow @@ -843,37 +734,6 @@ Extents Model::getUnscaledMeshExtents() const { return scaledExtents; } -Extents Model::calculateScaledOffsetExtents(const Extents& extents, - glm::vec3 modelPosition, glm::quat modelOrientation) const { - // we need to include any fst scaling, translation, and rotation, which is captured in the offset matrix - glm::vec3 minimum = glm::vec3(getFBXGeometry().offset * glm::vec4(extents.minimum, 1.0f)); - glm::vec3 maximum = glm::vec3(getFBXGeometry().offset * glm::vec4(extents.maximum, 1.0f)); - - Extents scaledOffsetExtents = { ((minimum + _offset) * _scale), - ((maximum + _offset) * _scale) }; - - Extents rotatedExtents = scaledOffsetExtents.getRotated(modelOrientation); - - Extents translatedExtents = { rotatedExtents.minimum + modelPosition, - rotatedExtents.maximum + modelPosition }; - - return translatedExtents; -} - -/// Returns the world space equivalent of some box in model space. -AABox Model::calculateScaledOffsetAABox(const AABox& box, glm::vec3 modelPosition, glm::quat modelOrientation) const { - return AABox(calculateScaledOffsetExtents(Extents(box), modelPosition, modelOrientation)); -} - -glm::vec3 Model::calculateScaledOffsetPoint(const glm::vec3& point) const { - // we need to include any fst scaling, translation, and rotation, which is captured in the offset matrix - glm::vec3 offsetPoint = glm::vec3(getFBXGeometry().offset * glm::vec4(point, 1.0f)); - glm::vec3 scaledPoint = ((offsetPoint + _offset) * _scale); - glm::vec3 rotatedPoint = _rotation * scaledPoint; - glm::vec3 translatedPoint = rotatedPoint + _translation; - return translatedPoint; -} - void Model::clearJointState(int index) { _rig->clearJointState(index); } @@ -1159,8 +1019,11 @@ void Model::simulate(float deltaTime, bool fullUpdate) { // they really only become invalid if something about the transform to world space has changed. This is // not too bad at this point, because it doesn't impact rendering. However it does slow down ray picking // because ray picking needs valid boxes to work - _calculatedMeshBoxesValid = false; - _calculatedMeshTrianglesValid = false; + //_calculatedMeshBoxesValid = false; + //_calculatedMeshTrianglesValid = false; + + // FIXME -- if the model URL changes, then we need to recalculate the triangle sets??!!!! + onInvalidate(); // check for scale to fit diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 763112cebc..05626d1ebe 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -96,7 +96,6 @@ public: render::PendingChanges& pendingChanges, render::Item::Status::Getters& statusGetters); void removeFromScene(std::shared_ptr scene, render::PendingChanges& pendingChanges); - void renderSetup(RenderArgs* args); bool isRenderable() const; bool isVisible() const { return _isVisible; } @@ -251,6 +250,9 @@ public: uint32_t getGeometryCounter() const { return _deleteGeometryCounter; } const QMap& getRenderItems() const { return _modelMeshRenderItems; } + void renderDebugMeshBoxes(gpu::Batch& batch); + + public slots: void loadURLFinished(bool success); @@ -267,15 +269,6 @@ protected: /// Returns the unscaled extents of the model's mesh Extents getUnscaledMeshExtents() const; - /// Returns the scaled equivalent of some extents in model space. - Extents calculateScaledOffsetExtents(const Extents& extents, glm::vec3 modelPosition, glm::quat modelOrientation) const; - - /// Returns the world space equivalent of some box in model space. - AABox calculateScaledOffsetAABox(const AABox& box, glm::vec3 modelPosition, glm::quat modelOrientation) const; - - /// Returns the scaled equivalent of a point in model space. - glm::vec3 calculateScaledOffsetPoint(const glm::vec3& point) const; - /// Clear the joint states void clearJointState(int index); @@ -332,14 +325,13 @@ protected: /// Allow sub classes to force invalidating the bboxes void invalidCalculatedMeshBoxes() { - _calculatedMeshBoxesValid = false; - _calculatedMeshPartBoxesValid = false; - _calculatedMeshTrianglesValid = false; + _triangleSetsValid = false; } // hook for derived classes to be notified when setUrl invalidates the current model. virtual void onInvalidate() {}; + protected: virtual void deleteGeometry(); @@ -358,17 +350,12 @@ protected: int _blendNumber; int _appliedBlendNumber; - QHash, AABox> _calculatedMeshPartBoxes; // world coordinate AABoxes for all sub mesh part boxes - - bool _calculatedMeshPartBoxesValid; - QVector _calculatedMeshBoxes; // world coordinate AABoxes for all sub mesh boxes - bool _calculatedMeshBoxesValid; - QVector _modelSpaceMeshTriangleSets; // model space triangles for all sub meshes - - bool _calculatedMeshTrianglesValid; QMutex _mutex; - void recalculateMeshBoxes(bool pickAgainstTriangles = false); + bool _triangleSetsValid { false }; + void calculateTriangleSets(); + QVector _modelSpaceMeshTriangleSets; // model space triangles for all sub meshes + void createRenderItemSet(); virtual void createVisibleRenderItemSet(); @@ -377,7 +364,6 @@ protected: bool _isWireframe; // debug rendering support - void renderDebugMeshBoxes(gpu::Batch& batch); int _debugMeshBoxesID = GeometryCache::UNKNOWN_ID; diff --git a/libraries/shared/src/TriangleSet.cpp b/libraries/shared/src/TriangleSet.cpp index 3264c259d3..5a634acc77 100644 --- a/libraries/shared/src/TriangleSet.cpp +++ b/libraries/shared/src/TriangleSet.cpp @@ -28,24 +28,31 @@ void TriangleSet::clear() { // Determine of the given ray (origin/direction) in model space intersects with any triangles // in the set. If an intersection occurs, the distance and surface normal will be provided. bool TriangleSet::findRayIntersection(const glm::vec3& origin, const glm::vec3& direction, - float& distance, glm::vec3& surfaceNormal) const { + float& distance, BoxFace& face, glm::vec3& surfaceNormal, bool precision) const { - float bestDistance = std::numeric_limits::max(); - float thisTriangleDistance; bool intersectedSomething = false; + float boxDistance = std::numeric_limits::max(); + float bestDistance = std::numeric_limits::max(); - for (const auto& triangle : _triangles) { - if (findRayTriangleIntersection(origin, direction, triangle, thisTriangleDistance)) { - if (thisTriangleDistance < bestDistance) { - bestDistance = thisTriangleDistance; - intersectedSomething = true; - surfaceNormal = triangle.getNormal(); - distance = bestDistance; - //face = subMeshFace; - //extraInfo = geometry.getModelNameOfMesh(subMeshIndex); + if (_bounds.findRayIntersection(origin, direction, boxDistance, face, surfaceNormal)) { + if (precision) { + for (const auto& triangle : _triangles) { + float thisTriangleDistance; + if (findRayTriangleIntersection(origin, direction, triangle, thisTriangleDistance)) { + if (thisTriangleDistance < bestDistance) { + bestDistance = thisTriangleDistance; + intersectedSomething = true; + surfaceNormal = triangle.getNormal(); + distance = bestDistance; + } + } } + } else { + intersectedSomething = true; + distance = boxDistance; } } + return intersectedSomething; } diff --git a/libraries/shared/src/TriangleSet.h b/libraries/shared/src/TriangleSet.h index 3aef3966b6..8024057174 100644 --- a/libraries/shared/src/TriangleSet.h +++ b/libraries/shared/src/TriangleSet.h @@ -22,10 +22,10 @@ public: // Determine of the given ray (origin/direction) in model space intersects with any triangles // in the set. If an intersection occurs, the distance and surface normal will be provided. bool findRayIntersection(const glm::vec3& origin, const glm::vec3& direction, - float& distance, glm::vec3& surfaceNormal) const; + float& distance, BoxFace& face, glm::vec3& surfaceNormal, bool precision) const; bool convexHullContains(const glm::vec3& point) const; // this point is "inside" all triangles - const AABox& getBounds() const; + const AABox& getBounds() const { return _bounds; } private: QVector _triangles; From 2a663cbcb16352c043e38f6f6744e1615045df5d Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Sun, 5 Mar 2017 19:28:15 -0800 Subject: [PATCH 25/45] cleanup --- .../entities-renderer/src/RenderableModelEntityItem.cpp | 2 +- libraries/render-utils/src/Model.cpp | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 1a6daf3d43..935dd4e796 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -418,7 +418,7 @@ void RenderableModelEntityItem::render(RenderArgs* args) { // Enqueue updates for the next frame if (_model) { -#if 1 //def WANT_EXTRA_RENDER_DEBUGGING +#ifdef WANT_EXTRA_RENDER_DEBUGGING // debugging... gpu::Batch& batch = *args->_batch; _model->renderDebugMeshBoxes(batch); diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 172290eae5..6be23ad615 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -1015,15 +1015,6 @@ void Model::simulate(float deltaTime, bool fullUpdate) { || (_snapModelToRegistrationPoint && !_snappedToRegistrationPoint); if (isActive() && fullUpdate) { - // NOTE: This is overly aggressive and we are invalidating the MeshBoxes when in fact they may not be invalid - // they really only become invalid if something about the transform to world space has changed. This is - // not too bad at this point, because it doesn't impact rendering. However it does slow down ray picking - // because ray picking needs valid boxes to work - //_calculatedMeshBoxesValid = false; - //_calculatedMeshTrianglesValid = false; - - // FIXME -- if the model URL changes, then we need to recalculate the triangle sets??!!!! - onInvalidate(); // check for scale to fit From b6671d92c8cf533526511999c2e7b58b160295bc Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Mon, 6 Mar 2017 18:46:57 +0000 Subject: [PATCH 26/45] removed duplicated audio devices --- scripts/system/selectAudioDevice.js | 37 +++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/scripts/system/selectAudioDevice.js b/scripts/system/selectAudioDevice.js index 9b97b24455..d77bef2aec 100644 --- a/scripts/system/selectAudioDevice.js +++ b/scripts/system/selectAudioDevice.js @@ -103,6 +103,42 @@ function setupAudioMenus() { } } +function removeAudioDevices() { + Menu.removeSeparator("Audio", "Input Audio Device"); + + var inputDeviceSetting = Settings.getValue(INPUT_DEVICE_SETTING); + var inputDevices = AudioDevice.getInputDevices(); + var selectedInputDevice = AudioDevice.getInputDevice(); + if (inputDevices.indexOf(inputDeviceSetting) != -1 && selectedInputDevice != inputDeviceSetting) { + if (AudioDevice.setInputDevice(inputDeviceSetting)) { + selectedInputDevice = inputDeviceSetting; + } + } + print("audio input devices: " + inputDevices); + for(var i = 0; i < inputDevices.length; i++) { + var thisDeviceSelected = (inputDevices[i] == selectedInputDevice); + var menuItem = "Use " + inputDevices[i] + " for Input"; + Menu.removeMenuItem("Audio", menuItem); + } + + Menu.removeSeparator("Audio", "Output Audio Device"); + + var outputDeviceSetting = Settings.getValue(OUTPUT_DEVICE_SETTING); + var outputDevices = AudioDevice.getOutputDevices(); + var selectedOutputDevice = AudioDevice.getOutputDevice(); + if (outputDevices.indexOf(outputDeviceSetting) != -1 && selectedOutputDevice != outputDeviceSetting) { + if (AudioDevice.setOutputDevice(outputDeviceSetting)) { + selectedOutputDevice = outputDeviceSetting; + } + } + print("audio output devices: " + outputDevices); + for (var i = 0; i < outputDevices.length; i++) { + var thisDeviceSelected = (outputDevices[i] == selectedOutputDevice); + var menuItem = "Use " + outputDevices[i] + " for Output"; + Menu.removeMenuItem("Audio", menuItem); + } +} + function onDevicechanged() { print("audio devices changed, removing Audio > Devices menu..."); Menu.removeMenu("Audio > Devices"); @@ -218,6 +254,7 @@ Script.update.connect(checkHMDAudio); Script.scriptEnding.connect(function () { restoreAudio(); + removeAudioDevices(); Menu.menuItemEvent.disconnect(menuItemEvent); Script.update.disconnect(checkHMDAudio); }); From af97e9bdd9f1ec8bece9166253f04044b828f8c7 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Mon, 6 Mar 2017 10:49:38 -0800 Subject: [PATCH 27/45] CR feedback --- libraries/render-utils/src/Model.cpp | 21 +++++++++++++-------- libraries/render-utils/src/Model.h | 6 +++++- libraries/shared/src/TriangleSet.cpp | 2 +- libraries/shared/src/TriangleSet.h | 13 +++++++++---- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 6be23ad615..ae20823138 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -368,7 +368,7 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g } glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * createMatFromQuatAndPos(_rotation, _translation); glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); glm::vec3 meshFrameOrigin = glm::vec3(worldToMeshMatrix * glm::vec4(origin, 1.0f)); @@ -437,7 +437,7 @@ bool Model::convexHullContains(glm::vec3 point) { // If we are inside the models box, then consider the submeshes... glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * createMatFromQuatAndPos(_rotation, _translation); glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); glm::vec3 meshFramePoint = glm::vec3(worldToMeshMatrix * glm::vec4(point, 1.0f)); @@ -475,9 +475,15 @@ void Model::calculateTriangleSets() { const int INDICES_PER_TRIANGLE = 3; const int INDICES_PER_QUAD = 4; + const int TRIANGLES_PER_QUAD = 2; + + // tell our triangleSet how many triangles to expect. + int numberOfQuads = part.quadIndices.size() / INDICES_PER_QUAD; + int numberOfTris = part.triangleIndices.size() / INDICES_PER_TRIANGLE; + int totalTriangles = (numberOfQuads * TRIANGLES_PER_QUAD) + numberOfTris; + _modelSpaceMeshTriangleSets[i].reserve(totalTriangles); if (part.quadIndices.size() > 0) { - int numberOfQuads = part.quadIndices.size() / INDICES_PER_QUAD; int vIndex = 0; for (int q = 0; q < numberOfQuads; q++) { int i0 = part.quadIndices[vIndex++]; @@ -495,13 +501,12 @@ void Model::calculateTriangleSets() { Triangle tri1 = { v0, v1, v3 }; Triangle tri2 = { v1, v2, v3 }; - _modelSpaceMeshTriangleSets[i].insertTriangle(tri1); - _modelSpaceMeshTriangleSets[i].insertTriangle(tri2); + _modelSpaceMeshTriangleSets[i].insert(tri1); + _modelSpaceMeshTriangleSets[i].insert(tri2); } } if (part.triangleIndices.size() > 0) { - int numberOfTris = part.triangleIndices.size() / INDICES_PER_TRIANGLE; int vIndex = 0; for (int t = 0; t < numberOfTris; t++) { int i0 = part.triangleIndices[vIndex++]; @@ -516,7 +521,7 @@ void Model::calculateTriangleSets() { glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); Triangle tri = { v0, v1, v2 }; - _modelSpaceMeshTriangleSets[i].insertTriangle(tri); + _modelSpaceMeshTriangleSets[i].insert(tri); } } } @@ -637,7 +642,7 @@ void Model::renderDebugMeshBoxes(gpu::Batch& batch) { _mutex.lock(); glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * glm::translate(_translation) * glm::mat4_cast(_rotation); + glm::mat4 meshToWorldMatrix = meshToModelMatrix * createMatFromQuatAndPos(_rotation, _translation); Transform meshToWorld(meshToWorldMatrix); batch.setModelTransform(meshToWorld); diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 05626d1ebe..41821736f7 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -290,7 +290,11 @@ protected: glm::vec3 _translation; // this is the translation in world coordinates to the model's registration point glm::quat _rotation; glm::vec3 _scale; - glm::vec3 _offset; // this is the translation for the minimum extent of the model (in original mesh coordinate space) to the model's registration point + + // For entity models this is the translation for the minimum extent of the model (in original mesh coordinate space) + // to the model's registration point. For avatar models this is the translation from the avatar's hips, as determined + // by the default pose, to the origin. + glm::vec3 _offset; static float FAKE_DIMENSION_PLACEHOLDER; diff --git a/libraries/shared/src/TriangleSet.cpp b/libraries/shared/src/TriangleSet.cpp index 5a634acc77..cdb3fd6b2c 100644 --- a/libraries/shared/src/TriangleSet.cpp +++ b/libraries/shared/src/TriangleSet.cpp @@ -12,7 +12,7 @@ #include "GLMHelpers.h" #include "TriangleSet.h" -void TriangleSet::insertTriangle(const Triangle& t) { +void TriangleSet::insert(const Triangle& t) { _triangles.push_back(t); _bounds += t.v0; diff --git a/libraries/shared/src/TriangleSet.h b/libraries/shared/src/TriangleSet.h index 8024057174..cc9dd1cc6d 100644 --- a/libraries/shared/src/TriangleSet.h +++ b/libraries/shared/src/TriangleSet.h @@ -16,15 +16,20 @@ class TriangleSet { public: - void insertTriangle(const Triangle& t); + void reserve(size_t size) { _triangles.reserve((int)size); } // reserve space in the datastructure for size number of triangles + + void insert(const Triangle& t); void clear(); - // Determine of the given ray (origin/direction) in model space intersects with any triangles - // in the set. If an intersection occurs, the distance and surface normal will be provided. + // Determine if the given ray (origin/direction) in model space intersects with any triangles in the set. If an + // intersection occurs, the distance and surface normal will be provided. bool findRayIntersection(const glm::vec3& origin, const glm::vec3& direction, float& distance, BoxFace& face, glm::vec3& surfaceNormal, bool precision) const; - bool convexHullContains(const glm::vec3& point) const; // this point is "inside" all triangles + // Determine if a point is "inside" all the triangles of a convex hull. It is the responsibility of the caller to + // determine that the triangle set is indeed a convex hull. If the triangles added to this set are not in fact a + // convex hull, the result of this method is meaningless and undetermined. + bool convexHullContains(const glm::vec3& point) const; const AABox& getBounds() const { return _bounds; } private: From 632cdc1b38afb4a662a5c20a8356e656f3364252 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 6 Mar 2017 11:04:12 -0800 Subject: [PATCH 28/45] semicolons and whitespace per coding standard --- interface/resources/qml/hifi/Pal.qml | 816 ++++++++++++++------------- 1 file changed, 409 insertions(+), 407 deletions(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 60b29960fd..70d94908e9 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -21,45 +21,46 @@ import "../controls-uit" as HifiControls // references HMD, Users, UserActivityLogger from root context Rectangle { - id: pal + id: pal; // Size - width: parent.width - height: parent.height + width: parent.width; + height: parent.height; // Style - color: "#E3E3E3" + color: "#E3E3E3"; // Properties - property int myCardHeight: 90 - property int rowHeight: 70 - property int actionButtonWidth: 55 - property int actionButtonAllowance: actionButtonWidth * 2 - property int minNameCardWidth: palContainer.width - (actionButtonAllowance * 2) - 4 - hifi.dimensions.scrollbarBackgroundWidth - property int nameCardWidth: minNameCardWidth + (iAmAdmin ? 0 : actionButtonAllowance) - property var myData: ({displayName: "", userName: "", audioLevel: 0.0, avgAudioLevel: 0.0, admin: true}) // valid dummy until set + property int myCardHeight: 90; + property int rowHeight: 70; + property int actionButtonWidth: 55; + property int actionButtonAllowance: actionButtonWidth * 2; + property int minNameCardWidth: palContainer.width - (actionButtonAllowance * 2) - 4 - hifi.dimensions.scrollbarBackgroundWidth; + property int nameCardWidth: minNameCardWidth + (iAmAdmin ? 0 : actionButtonAllowance); + property var myData: ({displayName: "", userName: "", audioLevel: 0.0, avgAudioLevel: 0.0, admin: true}); // valid dummy until set property var ignored: ({}); // Keep a local list of ignored avatars & their data. Necessary because HashMap is slow to respond after ignoring. - property var userModelData: [] // This simple list is essentially a mirror of the userModel listModel without all the extra complexities. - property bool iAmAdmin: false + property var userModelData: []; // This simple list is essentially a mirror of the userModel listModel without all the extra complexities. + property bool iAmAdmin: false; - HifiConstants { id: hifi } + HifiConstants { id: hifi; } // The letterbox used for popup messages LetterboxMessage { - id: letterboxMessage - z: 999 // Force the popup on top of everything else + + id: letterboxMessage; + z: 999; // Force the popup on top of everything else } function letterbox(headerGlyph, headerText, message) { - letterboxMessage.headerGlyph = headerGlyph - letterboxMessage.headerText = headerText - letterboxMessage.text = message - letterboxMessage.visible = true - letterboxMessage.popupRadius = 0 + letterboxMessage.headerGlyph = headerGlyph; + letterboxMessage.headerText = headerText; + letterboxMessage.text = message; + letterboxMessage.visible = true; + letterboxMessage.popupRadius = 0; } Settings { - id: settings - category: "pal" - property bool filtered: false - property int nearDistance: 30 - property int sortIndicatorColumn: 1 - property int sortIndicatorOrder: Qt.AscendingOrder + id: settings; + category: "pal"; + property bool filtered: false; + property int nearDistance: 30; + property int sortIndicatorColumn: 1; + property int sortIndicatorOrder: Qt.AscendingOrder; } function refreshWithFilter() { // We should just be able to set settings.filtered to filter.checked, but see #3249, so send to .js for saving. @@ -68,418 +69,418 @@ Rectangle { // This is the container for the PAL Rectangle { - property bool punctuationMode: false - id: palContainer + property bool punctuationMode: false; + id: palContainer; // Size - width: pal.width - 10 - height: pal.height - 10 + width: pal.width - 10; + height: pal.height - 10; // Style - color: pal.color + color: pal.color; // Anchors - anchors.centerIn: pal + anchors.centerIn: pal; // Properties - radius: hifi.dimensions.borderRadius + radius: hifi.dimensions.borderRadius; - // This contains the current user's NameCard and will contain other information in the future - Rectangle { - id: myInfo - // Size - width: palContainer.width - height: myCardHeight - // Style - color: pal.color - // Anchors - anchors.top: palContainer.top - // Properties - radius: hifi.dimensions.borderRadius - // This NameCard refers to the current user's NameCard (the one above the table) - NameCard { - id: myCard - // Properties - displayName: myData.displayName - userName: myData.userName - audioLevel: myData.audioLevel - avgAudioLevel: myData.avgAudioLevel - isMyCard: true + // This contains the current user's NameCard and will contain other information in the future + Rectangle { + id: myInfo; // Size - width: minNameCardWidth - height: parent.height - // Anchors - anchors.left: parent.left - } - Row { - HifiControls.CheckBox { - id: filter - checked: settings.filtered - text: "in view" - boxSize: reload.height * 0.70 - onCheckedChanged: refreshWithFilter() - } - HifiControls.GlyphButton { - id: reload - glyph: hifi.glyphs.reload - width: reload.height - onClicked: refreshWithFilter() - } - spacing: 50 - anchors { - right: parent.right - top: parent.top - topMargin: 10 - } - } - } - // Rectangles used to cover up rounded edges on bottom of MyInfo Rectangle - Rectangle { - color: pal.color - width: palContainer.width - height: 10 - anchors.top: myInfo.bottom - anchors.left: parent.left - } - Rectangle { - color: pal.color - width: palContainer.width - height: 10 - anchors.bottom: table.top - anchors.left: parent.left - } - // Rectangle that houses "ADMIN" string - Rectangle { - id: adminTab - // Size - width: 2*actionButtonWidth + hifi.dimensions.scrollbarBackgroundWidth + 2 - height: 40 - // Anchors - anchors.bottom: myInfo.bottom - anchors.bottomMargin: -10 - anchors.right: myInfo.right - // Properties - visible: iAmAdmin - // Style - color: hifi.colors.tableRowLightEven - radius: hifi.dimensions.borderRadius - border.color: hifi.colors.lightGrayText - border.width: 2 - // "ADMIN" text - RalewaySemiBold { - id: adminTabText - text: "ADMIN" - // Text size - size: hifi.fontSizes.tableHeading + 2 - // Anchors - anchors.top: parent.top - anchors.topMargin: 8 - anchors.left: parent.left - anchors.right: parent.right - anchors.rightMargin: hifi.dimensions.scrollbarBackgroundWidth + width: palContainer.width; + height: myCardHeight; // Style - font.capitalization: Font.AllUppercase - color: hifi.colors.redHighlight - // Alignment - horizontalAlignment: Text.AlignHCenter - verticalAlignment: Text.AlignTop - } - } - // This TableView refers to the table (below the current user's NameCard) - HifiControls.Table { - id: table - // Size - height: palContainer.height - myInfo.height - 4 - width: palContainer.width - 4 - // Anchors - anchors.left: parent.left - anchors.top: myInfo.bottom - // Properties - centerHeaderText: true - sortIndicatorVisible: true - headerVisible: true - sortIndicatorColumn: settings.sortIndicatorColumn - sortIndicatorOrder: settings.sortIndicatorOrder - onSortIndicatorColumnChanged: { - settings.sortIndicatorColumn = sortIndicatorColumn - sortModel() - } - onSortIndicatorOrderChanged: { - settings.sortIndicatorOrder = sortIndicatorOrder - sortModel() - } - - TableViewColumn { - role: "avgAudioLevel" - title: "LOUD" - width: actionButtonWidth - movable: false - resizable: false - } - - TableViewColumn { - id: displayNameHeader - role: "displayName" - title: table.rowCount + (table.rowCount === 1 ? " NAME" : " NAMES") - width: nameCardWidth - movable: false - resizable: false - } - TableViewColumn { - role: "ignore" - title: "IGNORE" - width: actionButtonWidth - movable: false - resizable: false - } - TableViewColumn { - visible: iAmAdmin - role: "mute" - title: "SILENCE" - width: actionButtonWidth - movable: false - resizable: false - } - TableViewColumn { - visible: iAmAdmin - role: "kick" - title: "BAN" - width: actionButtonWidth - movable: false - resizable: false - } - model: ListModel { - id: userModel - } - - // This Rectangle refers to each Row in the table. - rowDelegate: Rectangle { // The only way I know to specify a row height. - // Size - height: styleData.selected ? rowHeight : rowHeight - 15 - color: styleData.selected - ? hifi.colors.orangeHighlight - : styleData.alternate ? hifi.colors.tableRowLightEven : hifi.colors.tableRowLightOdd - } - - // This Item refers to the contents of each Cell - itemDelegate: Item { - id: itemCell - property bool isCheckBox: styleData.role === "personalMute" || styleData.role === "ignore" - property bool isButton: styleData.role === "mute" || styleData.role === "kick" - property bool isAvgAudio: styleData.role === "avgAudioLevel" - - // This NameCard refers to the cell that contains an avatar's - // DisplayName and UserName + color: pal.color; + // Anchors + anchors.top: palContainer.top; + // Properties + radius: hifi.dimensions.borderRadius; + // This NameCard refers to the current user's NameCard (the one above the table) NameCard { - id: nameCard + id: myCard; // Properties - displayName: styleData.value - userName: model ? model.userName : "" - audioLevel: model ? model.audioLevel : 0.0 - avgAudioLevel: model ? model.avgAudioLevel : 0.0 - visible: !isCheckBox && !isButton && !isAvgAudio - uuid: model ? model.sessionId : "" - selected: styleData.selected - isAdmin: model && model.admin + displayName: myData.displayName; + userName: myData.userName; + audioLevel: myData.audioLevel; + avgAudioLevel: myData.avgAudioLevel; + isMyCard: true; // Size - width: nameCardWidth - height: parent.height + width: minNameCardWidth; + height: parent.height; // Anchors - anchors.left: parent.left + anchors.left: parent.left; } - HifiControls.GlyphButton { - function getGlyph() { - var fileName = "vol_"; - if (model && model.personalMute) { - fileName += "x_"; - } - fileName += (4.0*(model ? model.avgAudioLevel : 0.0)).toFixed(0); - return hifi.glyphs[fileName]; + Row { + HifiControls.CheckBox { + id: filter; + checked: settings.filtered; + text: "in view"; + boxSize: reload.height * 0.70; + onCheckedChanged: refreshWithFilter(); } - id: avgAudioVolume - visible: isAvgAudio - glyph: getGlyph() - width: 32 - size: height - anchors.verticalCenter: parent.verticalCenter - anchors.horizontalCenter: parent.horizontalCenter - onClicked: { - // cannot change mute status when ignoring - if (!model["ignore"]) { - var newValue = !model["personalMute"]; - userModel.setProperty(model.userIndex, "personalMute", newValue) - userModelData[model.userIndex]["personalMute"] = newValue // Defensive programming - Users["personalMute"](model.sessionId, newValue) - UserActivityLogger["palAction"](newValue ? "personalMute" : "un-personalMute", model.sessionId) - } + HifiControls.GlyphButton { + id: reload; + glyph: hifi.glyphs.reload; + width: reload.height; + onClicked: refreshWithFilter(); } + spacing: 50; + anchors { + right: parent.right; + top: parent.top; + topMargin: 10; + } + } + } + // Rectangles used to cover up rounded edges on bottom of MyInfo Rectangle + Rectangle { + color: pal.color; + width: palContainer.width; + height: 10; + anchors.top: myInfo.bottom; + anchors.left: parent.left; + } + Rectangle { + color: pal.color; + width: palContainer.width; + height: 10; + anchors.bottom: table.top; + anchors.left: parent.left; + } + // Rectangle that houses "ADMIN" string + Rectangle { + id: adminTab; + // Size + width: 2*actionButtonWidth + hifi.dimensions.scrollbarBackgroundWidth + 2; + height: 40; + // Anchors + anchors.bottom: myInfo.bottom; + anchors.bottomMargin: -10; + anchors.right: myInfo.right; + // Properties + visible: iAmAdmin; + // Style + color: hifi.colors.tableRowLightEven; + radius: hifi.dimensions.borderRadius; + border.color: hifi.colors.lightGrayText; + border.width: 2; + // "ADMIN" text + RalewaySemiBold { + id: adminTabText; + text: "ADMIN"; + // Text size + size: hifi.fontSizes.tableHeading + 2; + // Anchors + anchors.top: parent.top; + anchors.topMargin: 8; + anchors.left: parent.left; + anchors.right: parent.right; + anchors.rightMargin: hifi.dimensions.scrollbarBackgroundWidth; + // Style + font.capitalization: Font.AllUppercase; + color: hifi.colors.redHighlight; + // Alignment + horizontalAlignment: Text.AlignHCenter; + verticalAlignment: Text.AlignTop; + } + } + // This TableView refers to the table (below the current user's NameCard) + HifiControls.Table { + id: table; + // Size + height: palContainer.height - myInfo.height - 4; + width: palContainer.width - 4; + // Anchors + anchors.left: parent.left; + anchors.top: myInfo.bottom; + // Properties + centerHeaderText: true; + sortIndicatorVisible: true; + headerVisible: true; + sortIndicatorColumn: settings.sortIndicatorColumn; + sortIndicatorOrder: settings.sortIndicatorOrder; + onSortIndicatorColumnChanged: { + settings.sortIndicatorColumn = sortIndicatorColumn; + sortModel(); + } + onSortIndicatorOrderChanged: { + settings.sortIndicatorOrder = sortIndicatorOrder; + sortModel(); } - // This CheckBox belongs in the columns that contain the stateful action buttons ("Mute" & "Ignore" for now) - // KNOWN BUG with the Checkboxes: When clicking in the center of the sorting header, the checkbox - // will appear in the "hovered" state. Hovering over the checkbox will fix it. - // Clicking on the sides of the sorting header doesn't cause this problem. - // I'm guessing this is a QT bug and not anything I can fix. I spent too long trying to work around it... - // I'm just going to leave the minor visual bug in. - HifiControls.CheckBox { - id: actionCheckBox - visible: isCheckBox - anchors.centerIn: parent - checked: model ? model[styleData.role] : false - // If this is a "Personal Mute" checkbox, disable the checkbox if the "Ignore" checkbox is checked. - enabled: !(styleData.role === "personalMute" && (model ? model["ignore"] : true)) - boxSize: 24 - onClicked: { - var newValue = !model[styleData.role] - userModel.setProperty(model.userIndex, styleData.role, newValue) - userModelData[model.userIndex][styleData.role] = newValue // Defensive programming - Users[styleData.role](model.sessionId, newValue) - UserActivityLogger["palAction"](newValue ? styleData.role : "un-" + styleData.role, model.sessionId) - if (styleData.role === "ignore") { - userModel.setProperty(model.userIndex, "personalMute", newValue) - userModelData[model.userIndex]["personalMute"] = newValue // Defensive programming - if (newValue) { - ignored[model.sessionId] = userModelData[model.userIndex] - } else { - delete ignored[model.sessionId] - } - avgAudioVolume.glyph = avgAudioVolume.getGlyph() - } - // http://doc.qt.io/qt-5/qtqml-syntax-propertybinding.html#creating-property-bindings-from-javascript - // I'm using an explicit binding here because clicking a checkbox breaks the implicit binding as set by - // "checked:" statement above. - checked = Qt.binding(function() { return (model[styleData.role])}) - } + TableViewColumn { + role: "avgAudioLevel"; + title: "LOUD"; + width: actionButtonWidth; + movable: false; + resizable: false; } - // This Button belongs in the columns that contain the stateless action buttons ("Silence" & "Ban" for now) - HifiControls.Button { - id: actionButton - color: 2 // Red - visible: isButton - anchors.centerIn: parent - width: 32 - height: 32 - onClicked: { - Users[styleData.role](model.sessionId) - UserActivityLogger["palAction"](styleData.role, model.sessionId) - if (styleData.role === "kick") { - userModelData.splice(model.userIndex, 1) - userModel.remove(model.userIndex) // after changing userModelData, b/c ListModel can frob the data - } - } - // muted/error glyphs - HiFiGlyphs { - text: (styleData.role === "kick") ? hifi.glyphs.error : hifi.glyphs.muted + TableViewColumn { + id: displayNameHeader; + role: "displayName"; + title: table.rowCount + (table.rowCount === 1 ? " NAME" : " NAMES"); + width: nameCardWidth; + movable: false; + resizable: false; + } + TableViewColumn { + role: "ignore"; + title: "IGNORE"; + width: actionButtonWidth; + movable: false; + resizable: false; + } + TableViewColumn { + visible: iAmAdmin; + role: "mute"; + title: "SILENCE"; + width: actionButtonWidth; + movable: false; + resizable: false; + } + TableViewColumn { + visible: iAmAdmin; + role: "kick"; + title: "BAN"; + width: actionButtonWidth; + movable: false; + resizable: false; + } + model: ListModel { + id: userModel; + } + + // This Rectangle refers to each Row in the table. + rowDelegate: Rectangle { // The only way I know to specify a row height. + // Size + height: styleData.selected ? rowHeight : rowHeight - 15; + color: styleData.selected + ? hifi.colors.orangeHighlight + : styleData.alternate ? hifi.colors.tableRowLightEven : hifi.colors.tableRowLightOdd; + } + + // This Item refers to the contents of each Cell + itemDelegate: Item { + id: itemCell; + property bool isCheckBox: styleData.role === "personalMute" || styleData.role === "ignore"; + property bool isButton: styleData.role === "mute" || styleData.role === "kick"; + property bool isAvgAudio: styleData.role === "avgAudioLevel"; + + // This NameCard refers to the cell that contains an avatar's + // DisplayName and UserName + NameCard { + id: nameCard; + // Properties + displayName: styleData.value; + userName: model ? model.userName : ""; + audioLevel: model ? model.audioLevel : 0.0; + avgAudioLevel: model ? model.avgAudioLevel : 0.0; + visible: !isCheckBox && !isButton && !isAvgAudio; + uuid: model ? model.sessionId : ""; + selected: styleData.selected; + isAdmin: model && model.admin; // Size - size: parent.height*1.3 + width: nameCardWidth; + height: parent.height; // Anchors - anchors.fill: parent - // Style - horizontalAlignment: Text.AlignHCenter - color: enabled ? hifi.buttons.textColor[actionButton.color] - : hifi.buttons.disabledTextColor[actionButton.colorScheme] + anchors.left: parent.left; + } + HifiControls.GlyphButton { + function getGlyph() { + var fileName = "vol_"; + if (model && model.personalMute) { + fileName += "x_"; + } + fileName += (4.0*(model ? model.avgAudioLevel : 0.0)).toFixed(0); + return hifi.glyphs[fileName]; + } + id: avgAudioVolume; + visible: isAvgAudio; + glyph: getGlyph(); + width: 32; + size: height; + anchors.verticalCenter: parent.verticalCenter; + anchors.horizontalCenter: parent.horizontalCenter; + onClicked: { + // cannot change mute status when ignoring + if (!model["ignore"]) { + var newValue = !model["personalMute"]; + userModel.setProperty(model.userIndex, "personalMute", newValue); + userModelData[model.userIndex]["personalMute"] = newValue; // Defensive programming + Users["personalMute"](model.sessionId, newValue); + UserActivityLogger["palAction"](newValue ? "personalMute" : "un-personalMute", model.sessionId); + } + } + } + + // This CheckBox belongs in the columns that contain the stateful action buttons ("Mute" & "Ignore" for now) + // KNOWN BUG with the Checkboxes: When clicking in the center of the sorting header, the checkbox + // will appear in the "hovered" state. Hovering over the checkbox will fix it. + // Clicking on the sides of the sorting header doesn't cause this problem. + // I'm guessing this is a QT bug and not anything I can fix. I spent too long trying to work around it... + // I'm just going to leave the minor visual bug in. + HifiControls.CheckBox { + id: actionCheckBox; + visible: isCheckBox; + anchors.centerIn: parent; + checked: model ? model[styleData.role] : false; + // If this is a "Personal Mute" checkbox, disable the checkbox if the "Ignore" checkbox is checked. + enabled: !(styleData.role === "personalMute" && (model ? model["ignore"] : true)); + boxSize: 24; + onClicked: { + var newValue = !model[styleData.role]; + userModel.setProperty(model.userIndex, styleData.role, newValue); + userModelData[model.userIndex][styleData.role] = newValue; // Defensive programming + Users[styleData.role](model.sessionId, newValue); + UserActivityLogger["palAction"](newValue ? styleData.role : "un-" + styleData.role, model.sessionId); + if (styleData.role === "ignore") { + userModel.setProperty(model.userIndex, "personalMute", newValue); + userModelData[model.userIndex]["personalMute"] = newValue; // Defensive programming + if (newValue) { + ignored[model.sessionId] = userModelData[model.userIndex]; + } else { + delete ignored[model.sessionId]; + } + avgAudioVolume.glyph = avgAudioVolume.getGlyph(); + } + // http://doc.qt.io/qt-5/qtqml-syntax-propertybinding.html#creating-property-bindings-from-javascript + // I'm using an explicit binding here because clicking a checkbox breaks the implicit binding as set by + // "checked:" statement above. + checked = Qt.binding(function() { return (model[styleData.role])}); + } + } + + // This Button belongs in the columns that contain the stateless action buttons ("Silence" & "Ban" for now) + HifiControls.Button { + id: actionButton; + color: 2; // Red + visible: isButton; + anchors.centerIn: parent; + width: 32; + height: 32; + onClicked: { + Users[styleData.role](model.sessionId); + UserActivityLogger["palAction"](styleData.role, model.sessionId); + if (styleData.role === "kick") { + userModelData.splice(model.userIndex, 1); + userModel.remove(model.userIndex); // after changing userModelData, b/c ListModel can frob the data + } + } + // muted/error glyphs + HiFiGlyphs { + text: (styleData.role === "kick") ? hifi.glyphs.error : hifi.glyphs.muted; + // Size + size: parent.height*1.3; + // Anchors + anchors.fill: parent; + // Style + horizontalAlignment: Text.AlignHCenter; + color: enabled ? hifi.buttons.textColor[actionButton.color] + : hifi.buttons.disabledTextColor[actionButton.colorScheme]; + } } } } - } - // Separator between user and admin functions - Rectangle { - // Size - width: 2 - height: table.height - // Anchors - anchors.left: adminTab.left - anchors.top: table.top - // Properties - visible: iAmAdmin - color: hifi.colors.lightGrayText - } - TextMetrics { - id: displayNameHeaderMetrics - text: displayNameHeader.title - // font: displayNameHeader.font // was this always undefined? giving error now... - } - // This Rectangle refers to the [?] popup button next to "NAMES" - Rectangle { - color: hifi.colors.tableBackgroundLight - width: 20 - height: hifi.dimensions.tableHeaderHeight - 2 - anchors.left: table.left - anchors.top: table.top - anchors.topMargin: 1 - anchors.leftMargin: actionButtonWidth + nameCardWidth/2 + displayNameHeaderMetrics.width/2 + 6 - RalewayRegular { - id: helpText - text: "[?]" - size: hifi.fontSizes.tableHeading + 2 - font.capitalization: Font.AllUppercase - color: hifi.colors.darkGray - horizontalAlignment: Text.AlignHCenter - verticalAlignment: Text.AlignVCenter - anchors.fill: parent + // Separator between user and admin functions + Rectangle { + // Size + width: 2; + height: table.height; + // Anchors + anchors.left: adminTab.left; + anchors.top: table.top; + // Properties + visible: iAmAdmin; + color: hifi.colors.lightGrayText; } - MouseArea { - anchors.fill: parent - acceptedButtons: Qt.LeftButton - hoverEnabled: true - onClicked: letterbox(hifi.glyphs.question, - "Display Names", - "Bold names in the list are avatar display names.
" + - "If a display name isn't set, a unique session display name is assigned." + - "

Administrators of this domain can also see the username or machine ID associated with each avatar present.") - onEntered: helpText.color = hifi.colors.baseGrayHighlight - onExited: helpText.color = hifi.colors.darkGray + TextMetrics { + id: displayNameHeaderMetrics; + text: displayNameHeader.title; + // font: displayNameHeader.font // was this always undefined? giving error now... } - } - // This Rectangle refers to the [?] popup button next to "ADMIN" - Rectangle { - visible: iAmAdmin - color: adminTab.color - width: 20 - height: 28 - anchors.right: adminTab.right - anchors.rightMargin: 10 + hifi.dimensions.scrollbarBackgroundWidth - anchors.top: adminTab.top - anchors.topMargin: 2 - RalewayRegular { - id: adminHelpText - text: "[?]" - size: hifi.fontSizes.tableHeading + 2 - font.capitalization: Font.AllUppercase - color: hifi.colors.redHighlight - horizontalAlignment: Text.AlignHCenter - verticalAlignment: Text.AlignVCenter - anchors.fill: parent + // This Rectangle refers to the [?] popup button next to "NAMES" + Rectangle { + color: hifi.colors.tableBackgroundLight; + width: 20; + height: hifi.dimensions.tableHeaderHeight - 2; + anchors.left: table.left; + anchors.top: table.top; + anchors.topMargin: 1; + anchors.leftMargin: actionButtonWidth + nameCardWidth/2 + displayNameHeaderMetrics.width/2 + 6; + RalewayRegular { + id: helpText; + text: "[?]"; + size: hifi.fontSizes.tableHeading + 2; + font.capitalization: Font.AllUppercase; + color: hifi.colors.darkGray; + horizontalAlignment: Text.AlignHCenter; + verticalAlignment: Text.AlignVCenter; + anchors.fill: parent; + } + MouseArea { + anchors.fill: parent; + acceptedButtons: Qt.LeftButton; + hoverEnabled: true; + onClicked: letterbox(hifi.glyphs.question, + "Display Names", + "Bold names in the list are avatar display names.
" + + "If a display name isn't set, a unique session display name is assigned." + + "

Administrators of this domain can also see the username or machine ID associated with each avatar present."); + onEntered: helpText.color = hifi.colors.baseGrayHighlight; + onExited: helpText.color = hifi.colors.darkGray; + } } - MouseArea { - anchors.fill: parent - acceptedButtons: Qt.LeftButton - hoverEnabled: true - onClicked: letterbox(hifi.glyphs.question, - "Admin Actions", - "Silence mutes a user's microphone. Silenced users can unmute themselves by clicking "UNMUTE" on their toolbar.

" + - "Ban removes a user from this domain and prevents them from returning. Admins can un-ban users from the Sandbox Domain Settings page.") - onEntered: adminHelpText.color = "#94132e" - onExited: adminHelpText.color = hifi.colors.redHighlight + // This Rectangle refers to the [?] popup button next to "ADMIN" + Rectangle { + visible: iAmAdmin; + color: adminTab.color; + width: 20; + height: 28; + anchors.right: adminTab.right; + anchors.rightMargin: 10 + hifi.dimensions.scrollbarBackgroundWidth; + anchors.top: adminTab.top; + anchors.topMargin: 2; + RalewayRegular { + id: adminHelpText; + text: "[?]"; + size: hifi.fontSizes.tableHeading + 2; + font.capitalization: Font.AllUppercase; + color: hifi.colors.redHighlight; + horizontalAlignment: Text.AlignHCenter; + verticalAlignment: Text.AlignVCenter; + anchors.fill: parent; + } + MouseArea { + anchors.fill: parent; + acceptedButtons: Qt.LeftButton; + hoverEnabled: true; + onClicked: letterbox(hifi.glyphs.question, + "Admin Actions", + "Silence mutes a user's microphone. Silenced users can unmute themselves by clicking "UNMUTE" on their toolbar.

" + + "Ban removes a user from this domain and prevents them from returning. Admins can un-ban users from the Sandbox Domain Settings page."); + onEntered: adminHelpText.color = "#94132e"; + onExited: adminHelpText.color = hifi.colors.redHighlight; + } } - } - HifiControls.Keyboard { - id: keyboard - raised: myCard.currentlyEditingDisplayName && HMD.active - numeric: parent.punctuationMode - anchors { - bottom: parent.bottom - left: parent.left - right: parent.right + HifiControls.Keyboard { + id: keyboard; + raised: myCard.currentlyEditingDisplayName && HMD.active; + numeric: parent.punctuationMode; + anchors { + bottom: parent.bottom; + left: parent.left; + right: parent.right; + } } } - } // Timer used when selecting table rows that aren't yet present in the model // (i.e. when selecting avatars using edit.js or sphere overlays) Timer { - property bool selected // Selected or deselected? - property int userIndex // The userIndex of the avatar we want to select - id: selectionTimer + property bool selected; // Selected or deselected? + property int userIndex; // The userIndex of the avatar we want to select + id: selectionTimer; onTriggered: { if (selected) { table.selection.clear(); // for now, no multi-select @@ -610,7 +611,8 @@ Rectangle { } } function sortModel() { - var sortProperty = table.getColumn(table.sortIndicatorColumn).role; + var column = table.getColumn(table.sortIndicatorColumn); + var sortProperty = column ? column.role : "displayName"; var before = (table.sortIndicatorOrder === Qt.AscendingOrder) ? -1 : 1; var after = -1 * before; userModelData.sort(function (a, b) { @@ -645,7 +647,7 @@ Rectangle { pal.sendToScript({method: 'selected', params: userIds}); } Connections { - target: table.selection - onSelectionChanged: pal.noticeSelection() + target: table.selection; + onSelectionChanged: pal.noticeSelection(); } } From faba16033103ae971b91a01f8ddd739dfd7b9995 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Mon, 6 Mar 2017 11:10:04 -0800 Subject: [PATCH 29/45] more CR feedback --- libraries/shared/src/TriangleSet.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/shared/src/TriangleSet.h b/libraries/shared/src/TriangleSet.h index cc9dd1cc6d..87336c77af 100644 --- a/libraries/shared/src/TriangleSet.h +++ b/libraries/shared/src/TriangleSet.h @@ -9,14 +9,14 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include +#include #include "AABox.h" #include "GeometryUtil.h" class TriangleSet { public: - void reserve(size_t size) { _triangles.reserve((int)size); } // reserve space in the datastructure for size number of triangles + void reserve(size_t size) { _triangles.reserve(size); } // reserve space in the datastructure for size number of triangles void insert(const Triangle& t); void clear(); @@ -33,6 +33,6 @@ public: const AABox& getBounds() const { return _bounds; } private: - QVector _triangles; + std::vector _triangles; AABox _bounds; }; From fc65723d685f6376b1f88d6e8eafcc47ff00e2a3 Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Mon, 6 Mar 2017 22:14:20 +0000 Subject: [PATCH 30/45] corrected removing duplicate devices --- scripts/system/selectAudioDevice.js | 53 +++++++++-------------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/scripts/system/selectAudioDevice.js b/scripts/system/selectAudioDevice.js index d77bef2aec..f5929ce151 100644 --- a/scripts/system/selectAudioDevice.js +++ b/scripts/system/selectAudioDevice.js @@ -51,7 +51,9 @@ const OUTPUT_DEVICE_SETTING = "audio_output_device"; var selectedInputMenu = ""; var selectedOutputMenu = ""; + var audioDevicesList = []; function setupAudioMenus() { + removeAudioMenus(); Menu.addSeparator("Audio", "Input Audio Device"); var inputDeviceSetting = Settings.getValue(INPUT_DEVICE_SETTING); @@ -67,11 +69,12 @@ function setupAudioMenus() { var thisDeviceSelected = (inputDevices[i] == selectedInputDevice); var menuItem = "Use " + inputDevices[i] + " for Input"; Menu.addMenuItem({ - menuName: "Audio", - menuItemName: menuItem, - isCheckable: true, - isChecked: thisDeviceSelected - }); + menuName: "Audio", + menuItemName: menuItem, + isCheckable: true, + isChecked: thisDeviceSelected + }); + audioDevicesList.push(menuItem); if (thisDeviceSelected) { selectedInputMenu = menuItem; } @@ -97,46 +100,22 @@ function setupAudioMenus() { isCheckable: true, isChecked: thisDeviceSelected }); + audioDevicesList.push(menuItem); if (thisDeviceSelected) { selectedOutputMenu = menuItem; } } } -function removeAudioDevices() { - Menu.removeSeparator("Audio", "Input Audio Device"); - - var inputDeviceSetting = Settings.getValue(INPUT_DEVICE_SETTING); - var inputDevices = AudioDevice.getInputDevices(); - var selectedInputDevice = AudioDevice.getInputDevice(); - if (inputDevices.indexOf(inputDeviceSetting) != -1 && selectedInputDevice != inputDeviceSetting) { - if (AudioDevice.setInputDevice(inputDeviceSetting)) { - selectedInputDevice = inputDeviceSetting; - } - } - print("audio input devices: " + inputDevices); - for(var i = 0; i < inputDevices.length; i++) { - var thisDeviceSelected = (inputDevices[i] == selectedInputDevice); - var menuItem = "Use " + inputDevices[i] + " for Input"; - Menu.removeMenuItem("Audio", menuItem); - } - +function removeAudioMenus() { + Menu.removeSeparator("Audio", "Input Audio Device"); Menu.removeSeparator("Audio", "Output Audio Device"); - var outputDeviceSetting = Settings.getValue(OUTPUT_DEVICE_SETTING); - var outputDevices = AudioDevice.getOutputDevices(); - var selectedOutputDevice = AudioDevice.getOutputDevice(); - if (outputDevices.indexOf(outputDeviceSetting) != -1 && selectedOutputDevice != outputDeviceSetting) { - if (AudioDevice.setOutputDevice(outputDeviceSetting)) { - selectedOutputDevice = outputDeviceSetting; - } - } - print("audio output devices: " + outputDevices); - for (var i = 0; i < outputDevices.length; i++) { - var thisDeviceSelected = (outputDevices[i] == selectedOutputDevice); - var menuItem = "Use " + outputDevices[i] + " for Output"; - Menu.removeMenuItem("Audio", menuItem); + for (var index = 0; index < audioDevicesList.length; index++) { + Menu.removeMenuItem("Audio", audioDevicesList[index]); } + + audioDevicesList = []; } function onDevicechanged() { @@ -254,7 +233,7 @@ Script.update.connect(checkHMDAudio); Script.scriptEnding.connect(function () { restoreAudio(); - removeAudioDevices(); + removeAudioMenus(); Menu.menuItemEvent.disconnect(menuItemEvent); Script.update.disconnect(checkHMDAudio); }); From db036d997c801e22fe3ca2e3a2b51146709e65fb Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 6 Mar 2017 15:18:35 -0700 Subject: [PATCH 31/45] first pass --- interface/resources/qml/hifi/Pal.qml | 30 ++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 28384f9c1c..670a29c82d 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -63,7 +63,16 @@ Rectangle { } function refreshWithFilter() { // We should just be able to set settings.filtered to filter.checked, but see #3249, so send to .js for saving. - pal.sendToScript({method: 'refresh', params: {filter: filter.checked && {distance: settings.nearDistance}}}); + var userIds = []; + table.selection.forEach(function (userIndex) { + userIds.push(userModelData[userIndex].sessionId); + }); + table.selection.clear(); + var params = {filter: filter.checked && {distance: settings.nearDistance}}; + if (userIds.length > 0) { + params.selected = [[userIds[0]], true, true]; + } + pal.sendToScript({method: 'refresh', params: params}); } // This is the container for the PAL @@ -286,7 +295,7 @@ Rectangle { HifiControls.GlyphButton { function getGlyph() { var fileName = "vol_"; - if (model["personalMute"]) { + if (model && model["personalMute"]) { fileName += "x_"; } fileName += (4.0*(model ? model.avgAudioLevel : 0.0)).toFixed(0); @@ -611,10 +620,20 @@ Rectangle { console.log('Unrecognized message:', JSON.stringify(message)); } } + function getSelectedSessionIDs() { + var sessionIDs = []; + table.selection.forEach(function (userIndex) { + sessionIDs.push(userModelData[userIndex].sessionId); + }); + return sessionIDs; + } function sortModel() { var sortProperty = table.getColumn(table.sortIndicatorColumn).role; var before = (table.sortIndicatorOrder === Qt.AscendingOrder) ? -1 : 1; var after = -1 * before; + + // get selection(s) before sorting + var selectedIDs = getSelectedSessionIDs(); userModelData.sort(function (a, b) { var aValue = a[sortProperty].toString().toLowerCase(), bValue = b[sortProperty].toString().toLowerCase(); switch (true) { @@ -627,6 +646,7 @@ Rectangle { userModel.clear(); var userIndex = 0; + var newSelectedIndexes=[]; userModelData.forEach(function (datum) { function init(property) { if (datum[property] === undefined) { @@ -636,7 +656,13 @@ Rectangle { ['personalMute', 'ignore', 'mute', 'kick'].forEach(init); datum.userIndex = userIndex++; userModel.append(datum); + if (selectedIDs.indexOf(datum.sessionId) != -1) { + newSelectedIndexes.push(datum.userIndex); + } }); + if (newSelectedIndexes.length > 0) { + table.selection.select(newSelectedIndexes); + } } signal sendToScript(var message); function noticeSelection() { From e14a1ad1af19cc4cbdd480f09ca24d5c22f44a68 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 6 Mar 2017 15:26:03 -0700 Subject: [PATCH 32/45] minor cleanup --- interface/resources/qml/hifi/Pal.qml | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 670a29c82d..9de7817d1b 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -61,13 +61,16 @@ Rectangle { property int sortIndicatorColumn: 1 property int sortIndicatorOrder: Qt.AscendingOrder } + function getSelectedSessionIDs() { + var sessionIDs = []; + table.selection.forEach(function (userIndex) { + sessionIDs.push(userModelData[userIndex].sessionId); + }); + return sessionIDs; + } function refreshWithFilter() { // We should just be able to set settings.filtered to filter.checked, but see #3249, so send to .js for saving. - var userIds = []; - table.selection.forEach(function (userIndex) { - userIds.push(userModelData[userIndex].sessionId); - }); - table.selection.clear(); + var userIds = getSelectedSessionIDs(); var params = {filter: filter.checked && {distance: settings.nearDistance}}; if (userIds.length > 0) { params.selected = [[userIds[0]], true, true]; @@ -620,13 +623,6 @@ Rectangle { console.log('Unrecognized message:', JSON.stringify(message)); } } - function getSelectedSessionIDs() { - var sessionIDs = []; - table.selection.forEach(function (userIndex) { - sessionIDs.push(userModelData[userIndex].sessionId); - }); - return sessionIDs; - } function sortModel() { var sortProperty = table.getColumn(table.sortIndicatorColumn).role; var before = (table.sortIndicatorOrder === Qt.AscendingOrder) ? -1 : 1; From 3d39aae35b0e91a594c9a3aedf8d6548938b8654 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 6 Mar 2017 15:52:33 -0700 Subject: [PATCH 33/45] minor tweak - just in case --- interface/resources/qml/hifi/Pal.qml | 1 + 1 file changed, 1 insertion(+) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 9de7817d1b..db975f3d8a 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -658,6 +658,7 @@ Rectangle { }); if (newSelectedIndexes.length > 0) { table.selection.select(newSelectedIndexes); + table.positionViewAtRow(userIndex, ListView.Beginning); } } signal sendToScript(var message); From 10b3e7fc676736c46aafa354065bd6a879cfda8c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 6 Mar 2017 15:14:51 -0800 Subject: [PATCH 34/45] sphere shapeType for models means 'sphere' --- libraries/entities/src/ShapeEntityItem.cpp | 2 +- libraries/physics/src/ShapeFactory.cpp | 13 ++++++++++++- libraries/shared/src/ShapeInfo.h | 3 ++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/ShapeEntityItem.cpp b/libraries/entities/src/ShapeEntityItem.cpp index eaef4c5c0e..345d9e54ab 100644 --- a/libraries/entities/src/ShapeEntityItem.cpp +++ b/libraries/entities/src/ShapeEntityItem.cpp @@ -163,7 +163,7 @@ void ShapeEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBit // This value specifes how the shape should be treated by physics calculations. // For now, all polys will act as spheres ShapeType ShapeEntityItem::getShapeType() const { - return (_shape == entity::Shape::Cube) ? SHAPE_TYPE_BOX : SHAPE_TYPE_SPHERE; + return (_shape == entity::Shape::Cube) ? SHAPE_TYPE_BOX : SHAPE_TYPE_ELLIPSOID; } void ShapeEntityItem::setColor(const rgbColor& value) { diff --git a/libraries/physics/src/ShapeFactory.cpp b/libraries/physics/src/ShapeFactory.cpp index 100dab0fd1..560b0a6db2 100644 --- a/libraries/physics/src/ShapeFactory.cpp +++ b/libraries/physics/src/ShapeFactory.cpp @@ -256,9 +256,20 @@ const btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info) } break; case SHAPE_TYPE_SPHERE: { + glm::vec3 halfExtents = info.getHalfExtents(); + float radius = glm::max(halfExtents.x, glm::max(halfExtents.y, halfExtents.z)); + shape = new btSphereShape(radius); + } + break; + case SHAPE_TYPE_ELLIPSOID: { glm::vec3 halfExtents = info.getHalfExtents(); float radius = halfExtents.x; - if (radius == halfExtents.y && radius == halfExtents.z) { + const float MIN_RADIUS = 0.001f; + const float MIN_RELATIVE_SPHERICAL_ERROR = 0.001f; + if (radius > MIN_RADIUS + && fabs(radius - halfExtents.y) / radius < MIN_RELATIVE_SPHERICAL_ERROR + && fabs(radius - halfExtents.z) / radius < MIN_RELATIVE_SPHERICAL_ERROR) { + // close enough to true sphere shape = new btSphereShape(radius); } else { ShapeInfo::PointList points; diff --git a/libraries/shared/src/ShapeInfo.h b/libraries/shared/src/ShapeInfo.h index a6ff8d6d4a..98b397ee16 100644 --- a/libraries/shared/src/ShapeInfo.h +++ b/libraries/shared/src/ShapeInfo.h @@ -45,7 +45,8 @@ enum ShapeType { SHAPE_TYPE_COMPOUND, SHAPE_TYPE_SIMPLE_HULL, SHAPE_TYPE_SIMPLE_COMPOUND, - SHAPE_TYPE_STATIC_MESH + SHAPE_TYPE_STATIC_MESH, + SHAPE_TYPE_ELLIPSOID }; class ShapeInfo { From d749c0c8beb9936bfec689309a1c5c77d0dce341 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 6 Mar 2017 15:50:32 -0800 Subject: [PATCH 35/45] use fabsf() rather than fabs() --- libraries/physics/src/ShapeFactory.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/physics/src/ShapeFactory.cpp b/libraries/physics/src/ShapeFactory.cpp index 560b0a6db2..35e050024a 100644 --- a/libraries/physics/src/ShapeFactory.cpp +++ b/libraries/physics/src/ShapeFactory.cpp @@ -267,8 +267,8 @@ const btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info) const float MIN_RADIUS = 0.001f; const float MIN_RELATIVE_SPHERICAL_ERROR = 0.001f; if (radius > MIN_RADIUS - && fabs(radius - halfExtents.y) / radius < MIN_RELATIVE_SPHERICAL_ERROR - && fabs(radius - halfExtents.z) / radius < MIN_RELATIVE_SPHERICAL_ERROR) { + && fabsf(radius - halfExtents.y) / radius < MIN_RELATIVE_SPHERICAL_ERROR + && fabsf(radius - halfExtents.z) / radius < MIN_RELATIVE_SPHERICAL_ERROR) { // close enough to true sphere shape = new btSphereShape(radius); } else { From f9dcfa54aab2d5fa998dcd69df9dac87eb462640 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 6 Mar 2017 16:08:21 -0800 Subject: [PATCH 36/45] Add debug/release builds to oculus lib --- cmake/externals/LibOVR/CMakeLists.txt | 8 +++++++- cmake/externals/LibOVR/LibOVRCMakeLists.txt | 5 ++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmake/externals/LibOVR/CMakeLists.txt b/cmake/externals/LibOVR/CMakeLists.txt index d8b8307082..c98aa8a04a 100644 --- a/cmake/externals/LibOVR/CMakeLists.txt +++ b/cmake/externals/LibOVR/CMakeLists.txt @@ -28,7 +28,13 @@ if (WIN32) ExternalProject_Get_Property(${EXTERNAL_NAME} INSTALL_DIR) set(LIBOVR_DIR ${INSTALL_DIR}) set(${EXTERNAL_NAME_UPPER}_INCLUDE_DIRS ${LIBOVR_DIR}/Include CACHE TYPE INTERNAL) - set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${LIBOVR_DIR}/Lib/LibOVR.lib CACHE TYPE INTERNAL) + set(${EXTERNAL_NAME_UPPER}_LIBRARY_DEBUG ${LIBOVR_DIR}/Lib/LibOVRd.lib CACHE TYPE INTERNAL) + set(${EXTERNAL_NAME_UPPER}_LIBRARY_RELEASE ${LIBOVR_DIR}/Lib/LibOVR.lib CACHE TYPE INTERNAL) + include(SelectLibraryConfigurations) + select_library_configurations(LIBOVR) + set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${${EXTERNAL_NAME_UPPER}_LIBRARIES} CACHE TYPE INTERNAL) + message("Libs ${EXTERNAL_NAME_UPPER}_LIBRARIES ${${EXTERNAL_NAME_UPPER}_LIBRARIES}") + elseif(APPLE) ExternalProject_Add( diff --git a/cmake/externals/LibOVR/LibOVRCMakeLists.txt b/cmake/externals/LibOVR/LibOVRCMakeLists.txt index bab6337ae2..556533f0c2 100644 --- a/cmake/externals/LibOVR/LibOVRCMakeLists.txt +++ b/cmake/externals/LibOVR/LibOVRCMakeLists.txt @@ -1,14 +1,13 @@ cmake_minimum_required(VERSION 3.2) project(LibOVR) -#set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DOVR_BUILD_DEBUG") - include_directories(LibOVR/Include LibOVR/Src) -#include_directories(LibOVRKernel/Src/) file(GLOB HEADER_FILES LibOVR/Include/*.h) file(GLOB EXTRA_HEADER_FILES LibOVR/Include/Extras/*.h) file(GLOB_RECURSE SOURCE_FILES LibOVR/Src/*.c LibOVR/Src/*.cpp) +set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DOVR_BUILD_DEBUG") add_library(LibOVR STATIC ${SOURCE_FILES} ${HEADER_FILES} ${EXTRA_HEADER_FILES}) +set_target_properties(LibOVR PROPERTIES DEBUG_POSTFIX "d") install(TARGETS LibOVR DESTINATION Lib) install(FILES ${HEADER_FILES} DESTINATION Include) From 51739f2f99bbbe4a1f8a4f9ed6bb538a85fff19e Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 7 Mar 2017 15:02:26 +1300 Subject: [PATCH 37/45] Remove Developer > Network > Bandwith Details dialog --- interface/src/Application.cpp | 6 -- interface/src/Application.h | 2 +- interface/src/Menu.cpp | 2 - interface/src/Menu.h | 1 - interface/src/ui/BandwidthDialog.cpp | 135 --------------------------- interface/src/ui/BandwidthDialog.h | 94 ------------------- interface/src/ui/DialogsManager.cpp | 15 --- interface/src/ui/DialogsManager.h | 4 - interface/src/ui/HMDToolsDialog.cpp | 3 - 9 files changed, 1 insertion(+), 261 deletions(-) delete mode 100644 interface/src/ui/BandwidthDialog.cpp delete mode 100644 interface/src/ui/BandwidthDialog.h diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ba63f4f064..f870bd9f83 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4236,12 +4236,6 @@ void Application::updateDialogs(float deltaTime) const { PerformanceWarning warn(showWarnings, "Application::updateDialogs()"); auto dialogsManager = DependencyManager::get(); - // Update bandwidth dialog, if any - BandwidthDialog* bandwidthDialog = dialogsManager->getBandwidthDialog(); - if (bandwidthDialog) { - bandwidthDialog->update(); - } - QPointer octreeStatsDialog = dialogsManager->getOctreeStatsDialog(); if (octreeStatsDialog) { octreeStatsDialog->update(); diff --git a/interface/src/Application.h b/interface/src/Application.h index ec6d9b19f7..c4ba760153 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -51,6 +51,7 @@ #include #include "avatar/MyAvatar.h" +#include "BandwidthRecorder.h" #include "Bookmarks.h" #include "Camera.h" #include "ConnectionMonitor.h" @@ -61,7 +62,6 @@ #include "scripting/ControllerScriptingInterface.h" #include "scripting/DialogsManagerScriptingInterface.h" #include "ui/ApplicationOverlay.h" -#include "ui/BandwidthDialog.h" #include "ui/EntityScriptServerLogDialog.h" #include "ui/LodToolsDialog.h" #include "ui/LogDialog.h" diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index c131367aee..beacbaccab 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -566,8 +566,6 @@ Menu::Menu() { dialogsManager.data(), SLOT(toggleDiskCacheEditor())); addActionToQMenuAndActionHash(networkMenu, MenuOption::ShowDSConnectTable, 0, dialogsManager.data(), SLOT(showDomainConnectionDialog())); - addActionToQMenuAndActionHash(networkMenu, MenuOption::BandwidthDetails, 0, - dialogsManager.data(), SLOT(bandwidthDetails())); #if (PR_BUILD || DEV_BUILD) addCheckableActionToQMenuAndActionHash(networkMenu, MenuOption::SendWrongProtocolVersion, 0, false, diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 1b2564735b..c806ffa9ee 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -49,7 +49,6 @@ namespace MenuOption { const QString AutoMuteAudio = "Auto Mute Microphone"; const QString AvatarReceiveStats = "Show Receive Stats"; const QString Back = "Back"; - const QString BandwidthDetails = "Bandwidth Details"; const QString BinaryEyelidControl = "Binary Eyelid Control"; const QString BookmarkLocation = "Bookmark Location"; const QString Bookmarks = "Bookmarks"; diff --git a/interface/src/ui/BandwidthDialog.cpp b/interface/src/ui/BandwidthDialog.cpp deleted file mode 100644 index f07c844894..0000000000 --- a/interface/src/ui/BandwidthDialog.cpp +++ /dev/null @@ -1,135 +0,0 @@ -// -// BandwidthDialog.cpp -// interface/src/ui -// -// Created by Tobias Schwinger on 6/21/13. -// Copyright 2013 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 - -#include "BandwidthRecorder.h" -#include "ui/BandwidthDialog.h" - -#include -#include - -#include -#include - - -BandwidthChannelDisplay::BandwidthChannelDisplay(QVector nodeTypesToFollow, - QFormLayout* form, - char const* const caption, char const* unitCaption, - const float unitScale, unsigned colorRGBA) : - _nodeTypesToFollow(nodeTypesToFollow), - _caption(caption), - _unitCaption(unitCaption), - _unitScale(unitScale), - _colorRGBA(colorRGBA) -{ - _label = new QLabel(); - _label->setAlignment(Qt::AlignRight); - - QPalette palette = _label->palette(); - unsigned rgb = colorRGBA >> 8; - rgb = ((rgb & 0xfefefeu) >> 1) + ((rgb & 0xf8f8f8) >> 3); - palette.setColor(QPalette::WindowText, QColor::fromRgb(rgb)); - _label->setPalette(palette); - - form->addRow(QString(" ") + _caption + " Bandwidth In/Out:", _label); -} - - - -void BandwidthChannelDisplay::bandwidthAverageUpdated() { - float inTotal = 0.; - float outTotal = 0.; - - QSharedPointer bandwidthRecorder = DependencyManager::get(); - - for (int i = 0; i < _nodeTypesToFollow.size(); ++i) { - inTotal += bandwidthRecorder->getAverageInputKilobitsPerSecond(_nodeTypesToFollow.at(i)); - outTotal += bandwidthRecorder->getAverageOutputKilobitsPerSecond(_nodeTypesToFollow.at(i)); - } - - _strBuf = - QString("").setNum((int) (inTotal * _unitScale)) + "/" + - QString("").setNum((int) (outTotal * _unitScale)) + " " + _unitCaption; -} - - -void BandwidthChannelDisplay::paint() { - _label->setText(_strBuf); -} - - -BandwidthDialog::BandwidthDialog(QWidget* parent) : - QDialog(parent, Qt::Window | Qt::WindowCloseButtonHint | Qt::WindowStaysOnTopHint) { - - this->setWindowTitle("Bandwidth Details"); - - // Create layout - QFormLayout* form = new QFormLayout(); - form->setSizeConstraint(QLayout::SetFixedSize); - this->QDialog::setLayout(form); - - QSharedPointer bandwidthRecorder = DependencyManager::get(); - - _allChannelDisplays[0] = _audioChannelDisplay = - new BandwidthChannelDisplay({NodeType::AudioMixer}, form, "Audio", "Kbps", 1.0, COLOR0); - _allChannelDisplays[1] = _avatarsChannelDisplay = - new BandwidthChannelDisplay({NodeType::Agent, NodeType::AvatarMixer}, form, "Avatars", "Kbps", 1.0, COLOR1); - _allChannelDisplays[2] = _octreeChannelDisplay = - new BandwidthChannelDisplay({NodeType::EntityServer}, form, "Octree", "Kbps", 1.0, COLOR2); - _allChannelDisplays[3] = _octreeChannelDisplay = - new BandwidthChannelDisplay({NodeType::DomainServer}, form, "Domain", "Kbps", 1.0, COLOR2); - _allChannelDisplays[4] = _otherChannelDisplay = - new BandwidthChannelDisplay({NodeType::Unassigned}, form, "Other", "Kbps", 1.0, COLOR2); - _allChannelDisplays[5] = _totalChannelDisplay = - new BandwidthChannelDisplay({ - NodeType::DomainServer, NodeType::EntityServer, - NodeType::AudioMixer, NodeType::Agent, - NodeType::AvatarMixer, NodeType::Unassigned - }, form, "Total", "Kbps", 1.0, COLOR2); - - connect(averageUpdateTimer, SIGNAL(timeout()), this, SLOT(updateTimerTimeout())); - averageUpdateTimer->start(1000); -} - - -BandwidthDialog::~BandwidthDialog() { - for (unsigned int i = 0; i < _CHANNELCOUNT; i++) { - delete _allChannelDisplays[i]; - } -} - - -void BandwidthDialog::updateTimerTimeout() { - for (unsigned int i = 0; i < _CHANNELCOUNT; i++) { - _allChannelDisplays[i]->bandwidthAverageUpdated(); - } -} - - -void BandwidthDialog::paintEvent(QPaintEvent* event) { - for (unsigned int i=0; i<_CHANNELCOUNT; i++) - _allChannelDisplays[i]->paint(); - this->QDialog::paintEvent(event); -} - -void BandwidthDialog::reject() { - - // Just regularly close upon ESC - this->QDialog::close(); -} - -void BandwidthDialog::closeEvent(QCloseEvent* event) { - - this->QDialog::closeEvent(event); - emit closed(); -} - diff --git a/interface/src/ui/BandwidthDialog.h b/interface/src/ui/BandwidthDialog.h deleted file mode 100644 index a53cc21030..0000000000 --- a/interface/src/ui/BandwidthDialog.h +++ /dev/null @@ -1,94 +0,0 @@ -// -// BandwidthDialog.h -// interface/src/ui -// -// Created by Tobias Schwinger on 6/21/13. -// Copyright 2013 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_BandwidthDialog_h -#define hifi_BandwidthDialog_h - -#include -#include -#include -#include -#include - -#include "Node.h" -#include "BandwidthRecorder.h" - - -const unsigned int COLOR0 = 0x33cc99ff; -const unsigned int COLOR1 = 0xffef40c0; -const unsigned int COLOR2 = 0xd0d0d0a0; - - -class BandwidthChannelDisplay : public QObject { - Q_OBJECT - - public: - BandwidthChannelDisplay(QVector nodeTypesToFollow, - QFormLayout* form, - char const* const caption, char const* unitCaption, float unitScale, unsigned colorRGBA); - void paint(); - - private: - QVector _nodeTypesToFollow; - QLabel* _label; - QString _strBuf; - char const* const _caption; - char const* _unitCaption; - float const _unitScale; - unsigned _colorRGBA; - - - public slots: - void bandwidthAverageUpdated(); -}; - - -class BandwidthDialog : public QDialog { - Q_OBJECT -public: - BandwidthDialog(QWidget* parent); - ~BandwidthDialog(); - - void paintEvent(QPaintEvent*) override; - -private: - BandwidthChannelDisplay* _audioChannelDisplay; - BandwidthChannelDisplay* _avatarsChannelDisplay; - BandwidthChannelDisplay* _octreeChannelDisplay; - BandwidthChannelDisplay* _domainChannelDisplay; - BandwidthChannelDisplay* _otherChannelDisplay; - BandwidthChannelDisplay* _totalChannelDisplay; // sums of all the other channels - - static const unsigned int _CHANNELCOUNT = 6; - BandwidthChannelDisplay* _allChannelDisplays[_CHANNELCOUNT]; - - -signals: - - void closed(); - -public slots: - - void reject() override; - void updateTimerTimeout(); - - -protected: - - // Emits a 'closed' signal when this dialog is closed. - void closeEvent(QCloseEvent*) override; - -private: - QTimer* averageUpdateTimer = new QTimer(this); - -}; - -#endif // hifi_BandwidthDialog_h diff --git a/interface/src/ui/DialogsManager.cpp b/interface/src/ui/DialogsManager.cpp index 03c71d8573..3252fef4f0 100644 --- a/interface/src/ui/DialogsManager.cpp +++ b/interface/src/ui/DialogsManager.cpp @@ -19,7 +19,6 @@ #include #include "AddressBarDialog.h" -#include "BandwidthDialog.h" #include "CachesSizeDialog.h" #include "ConnectionFailureDialog.h" #include "DiskCacheEditor.h" @@ -108,20 +107,6 @@ void DialogsManager::cachesSizeDialog() { _cachesSizeDialog->raise(); } -void DialogsManager::bandwidthDetails() { - if (! _bandwidthDialog) { - _bandwidthDialog = new BandwidthDialog(qApp->getWindow()); - connect(_bandwidthDialog, SIGNAL(closed()), _bandwidthDialog, SLOT(deleteLater())); - - if (_hmdToolsDialog) { - _hmdToolsDialog->watchWindow(_bandwidthDialog->windowHandle()); - } - - _bandwidthDialog->show(); - } - _bandwidthDialog->raise(); -} - void DialogsManager::lodTools() { if (!_lodToolsDialog) { maybeCreateDialog(_lodToolsDialog); diff --git a/interface/src/ui/DialogsManager.h b/interface/src/ui/DialogsManager.h index c02c1fc2c3..54aef38984 100644 --- a/interface/src/ui/DialogsManager.h +++ b/interface/src/ui/DialogsManager.h @@ -21,7 +21,6 @@ class AnimationsDialog; class AttachmentsDialog; -class BandwidthDialog; class CachesSizeDialog; class DiskCacheEditor; class LodToolsDialog; @@ -36,7 +35,6 @@ class DialogsManager : public QObject, public Dependency { SINGLETON_DEPENDENCY public: - QPointer getBandwidthDialog() const { return _bandwidthDialog; } QPointer getHMDToolsDialog() const { return _hmdToolsDialog; } QPointer getLodToolsDialog() const { return _lodToolsDialog; } QPointer getOctreeStatsDialog() const { return _octreeStatsDialog; } @@ -53,7 +51,6 @@ public slots: void showLoginDialog(); void octreeStatsDetails(); void cachesSizeDialog(); - void bandwidthDetails(); void lodTools(); void hmdTools(bool showTools); void showScriptEditor(); @@ -79,7 +76,6 @@ private: QPointer _animationsDialog; QPointer _attachmentsDialog; - QPointer _bandwidthDialog; QPointer _cachesSizeDialog; QPointer _diskCacheEditor; QPointer _ircInfoBox; diff --git a/interface/src/ui/HMDToolsDialog.cpp b/interface/src/ui/HMDToolsDialog.cpp index a596403948..55c321723e 100644 --- a/interface/src/ui/HMDToolsDialog.cpp +++ b/interface/src/ui/HMDToolsDialog.cpp @@ -79,9 +79,6 @@ HMDToolsDialog::HMDToolsDialog(QWidget* parent) : // what screens we're allowed on watchWindow(windowHandle()); auto dialogsManager = DependencyManager::get(); - if (dialogsManager->getBandwidthDialog()) { - watchWindow(dialogsManager->getBandwidthDialog()->windowHandle()); - } if (dialogsManager->getOctreeStatsDialog()) { watchWindow(dialogsManager->getOctreeStatsDialog()->windowHandle()); } From ff9d6d657cc64907c3695591a8bb5e97b9ee14b6 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 6 Mar 2017 19:35:00 -0800 Subject: [PATCH 38/45] Remove weighted offset, special case downward pressure --- .../animation/src/AnimInverseKinematics.cpp | 48 +++++++------------ 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/libraries/animation/src/AnimInverseKinematics.cpp b/libraries/animation/src/AnimInverseKinematics.cpp index 173af3fdf6..fa8e4654f6 100644 --- a/libraries/animation/src/AnimInverseKinematics.cpp +++ b/libraries/animation/src/AnimInverseKinematics.cpp @@ -488,13 +488,7 @@ const AnimPoseVec& AnimInverseKinematics::overlay(const AnimVariantMap& animVars // measure new _hipsOffset for next frame // by looking for discrepancies between where a targeted endEffector is // and where it wants to be (after IK solutions are done) - - // use weighted average between HMD and other targets - float HMD_WEIGHT = 10.0f; - float OTHER_WEIGHT = 1.0f; - float totalWeight = 0.0f; - - glm::vec3 additionalHipsOffset = Vectors::ZERO; + glm::vec3 newHipsOffset = Vectors::ZERO; for (auto& target: targets) { int targetIndex = target.getIndex(); if (targetIndex == _headIndex && _headIndex != -1) { @@ -505,42 +499,34 @@ const AnimPoseVec& AnimInverseKinematics::overlay(const AnimVariantMap& animVars glm::vec3 under = _skeleton->getAbsolutePose(_headIndex, underPoses).trans(); glm::vec3 actual = _skeleton->getAbsolutePose(_headIndex, _relativePoses).trans(); const float HEAD_OFFSET_SLAVE_FACTOR = 0.65f; - additionalHipsOffset += (OTHER_WEIGHT * HEAD_OFFSET_SLAVE_FACTOR) * (under- actual); - totalWeight += OTHER_WEIGHT; + newHipsOffset += HEAD_OFFSET_SLAVE_FACTOR * (actual - under); } else if (target.getType() == IKTarget::Type::HmdHead) { + // we want to shift the hips to bring the head to its designated position glm::vec3 actual = _skeleton->getAbsolutePose(_headIndex, _relativePoses).trans(); - glm::vec3 thisOffset = target.getTranslation() - actual; - glm::vec3 futureHipsOffset = _hipsOffset + thisOffset; - if (glm::length(glm::vec2(futureHipsOffset.x, futureHipsOffset.z)) < _maxHipsOffsetLength) { - // it is imperative to shift the hips and bring the head to its designated position - // so we slam newHipsOffset here and ignore all other targets - additionalHipsOffset = futureHipsOffset - _hipsOffset; - totalWeight = 0.0f; - break; - } else { - additionalHipsOffset += HMD_WEIGHT * (target.getTranslation() - actual); - totalWeight += HMD_WEIGHT; - } + _hipsOffset += target.getTranslation() - actual; + // and ignore all other targets + newHipsOffset = _hipsOffset; + break; + } else if (target.getType() == IKTarget::Type::RotationAndPosition) { + glm::vec3 actualPosition = _skeleton->getAbsolutePose(targetIndex, _relativePoses).trans(); + glm::vec3 targetPosition = target.getTranslation(); + newHipsOffset += targetPosition - actualPosition; + + // Add downward pressure on the hips + newHipsOffset *= 0.95f; + newHipsOffset -= 1.0f; } } else if (target.getType() == IKTarget::Type::RotationAndPosition) { glm::vec3 actualPosition = _skeleton->getAbsolutePose(targetIndex, _relativePoses).trans(); glm::vec3 targetPosition = target.getTranslation(); - additionalHipsOffset += OTHER_WEIGHT * (targetPosition - actualPosition); - totalWeight += OTHER_WEIGHT; + newHipsOffset += targetPosition - actualPosition; } } - if (totalWeight > 1.0f) { - additionalHipsOffset /= totalWeight; - } - - // Add downward pressure on the hips - additionalHipsOffset *= 0.95f; - additionalHipsOffset -= 1.0f; // smooth transitions by relaxing _hipsOffset toward the new value const float HIPS_OFFSET_SLAVE_TIMESCALE = 0.10f; float tau = dt < HIPS_OFFSET_SLAVE_TIMESCALE ? dt / HIPS_OFFSET_SLAVE_TIMESCALE : 1.0f; - _hipsOffset += additionalHipsOffset * tau; + _hipsOffset += (newHipsOffset - _hipsOffset) * tau; // clamp the hips offset float hipsOffsetLength = glm::length(_hipsOffset); From 87934ee82d1c1a247311ec3886d3c278aa40d8a3 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 6 Mar 2017 21:50:32 -0800 Subject: [PATCH 39/45] fix bugs in some meshes --- libraries/render-utils/src/Model.cpp | 24 ++++++++++++++---------- libraries/shared/src/TriangleSet.h | 5 ++++- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 2af42c046e..48c1d29b68 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -368,7 +368,7 @@ bool Model::findRayIntersectionAgainstSubMeshes(const glm::vec3& origin, const g } glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * createMatFromQuatAndPos(_rotation, _translation); + glm::mat4 meshToWorldMatrix = createMatFromQuatAndPos(_rotation, _translation) * meshToModelMatrix; glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); glm::vec3 meshFrameOrigin = glm::vec3(worldToMeshMatrix * glm::vec4(origin, 1.0f)); @@ -437,7 +437,7 @@ bool Model::convexHullContains(glm::vec3 point) { // If we are inside the models box, then consider the submeshes... glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * createMatFromQuatAndPos(_rotation, _translation); + glm::mat4 meshToWorldMatrix = createMatFromQuatAndPos(_rotation, _translation) * meshToModelMatrix; glm::mat4 worldToMeshMatrix = glm::inverse(meshToWorldMatrix); glm::vec3 meshFramePoint = glm::vec3(worldToMeshMatrix * glm::vec4(point, 1.0f)); @@ -483,6 +483,8 @@ void Model::calculateTriangleSets() { int totalTriangles = (numberOfQuads * TRIANGLES_PER_QUAD) + numberOfTris; _modelSpaceMeshTriangleSets[i].reserve(totalTriangles); + auto meshTransform = getFBXGeometry().offset * mesh.modelTransform; + if (part.quadIndices.size() > 0) { int vIndex = 0; for (int q = 0; q < numberOfQuads; q++) { @@ -494,10 +496,10 @@ void Model::calculateTriangleSets() { // track the model space version... these points will be transformed by the FST's offset, // which includes the scaling, rotation, and translation specified by the FST/FBX, // this can't change at runtime, so we can safely store these in our TriangleSet - glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); - glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); - glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); - glm::vec3 v3 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i3], 1.0f)); + glm::vec3 v0 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i0], 1.0f)); + glm::vec3 v1 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i1], 1.0f)); + glm::vec3 v2 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i2], 1.0f)); + glm::vec3 v3 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i3], 1.0f)); Triangle tri1 = { v0, v1, v3 }; Triangle tri2 = { v1, v2, v3 }; @@ -516,9 +518,9 @@ void Model::calculateTriangleSets() { // track the model space version... these points will be transformed by the FST's offset, // which includes the scaling, rotation, and translation specified by the FST/FBX, // this can't change at runtime, so we can safely store these in our TriangleSet - glm::vec3 v0 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i0], 1.0f)); - glm::vec3 v1 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i1], 1.0f)); - glm::vec3 v2 = glm::vec3(getFBXGeometry().offset * glm::vec4(mesh.vertices[i2], 1.0f)); + glm::vec3 v0 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i0], 1.0f)); + glm::vec3 v1 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i1], 1.0f)); + glm::vec3 v2 = glm::vec3(meshTransform * glm::vec4(mesh.vertices[i2], 1.0f)); Triangle tri = { v0, v1, v2 }; _modelSpaceMeshTriangleSets[i].insert(tri); @@ -642,10 +644,12 @@ void Model::renderDebugMeshBoxes(gpu::Batch& batch) { _mutex.lock(); glm::mat4 meshToModelMatrix = glm::scale(_scale) * glm::translate(_offset); - glm::mat4 meshToWorldMatrix = meshToModelMatrix * createMatFromQuatAndPos(_rotation, _translation); + glm::mat4 meshToWorldMatrix = createMatFromQuatAndPos(_rotation, _translation) * meshToModelMatrix; Transform meshToWorld(meshToWorldMatrix); batch.setModelTransform(meshToWorld); + DependencyManager::get()->bindSimpleProgram(batch, false, false, false, true, true); + for(const auto& triangleSet : _modelSpaceMeshTriangleSets) { auto box = triangleSet.getBounds(); diff --git a/libraries/shared/src/TriangleSet.h b/libraries/shared/src/TriangleSet.h index 87336c77af..b54f1a642a 100644 --- a/libraries/shared/src/TriangleSet.h +++ b/libraries/shared/src/TriangleSet.h @@ -16,7 +16,10 @@ class TriangleSet { public: - void reserve(size_t size) { _triangles.reserve(size); } // reserve space in the datastructure for size number of triangles + void reserve(size_t size) { _triangles.reserve(size); } // reserve space in the datastructure for size number of triangles + size_t size() const { return _triangles.size(); } + + const Triangle& getTriangle(size_t t) const { return _triangles[t]; } void insert(const Triangle& t); void clear(); From 2d2b2094fd7d79c9c636de3f9fcd765774f400a3 Mon Sep 17 00:00:00 2001 From: Vladyslav Stelmakhovskyi Date: Tue, 7 Mar 2017 10:11:09 +0100 Subject: [PATCH 40/45] Adding ability for QML/JS file to be visible in QtCreator. Also changed files will be installed to destination --- CMakeLists.txt | 11 +++++++++++ interface/CMakeLists.txt | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1ab7e55343..0703866ac6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -206,6 +206,17 @@ foreach(CUSTOM_MACRO ${HIFI_CUSTOM_MACROS}) include(${CUSTOM_MACRO}) endforeach() +file(GLOB_RECURSE JS_SRC scripts/*.js) +add_custom_target(js SOURCES ${JS_SRC}) + +if (UNIX) + install( + DIRECTORY "${CMAKE_SOURCE_DIR}/scripts" + DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/interface + COMPONENT ${CLIENT_COMPONENT} + ) +endif() + if (ANDROID) file(GLOB ANDROID_CUSTOM_MACROS "cmake/android/*.cmake") foreach(CUSTOM_MACRO ${ANDROID_CUSTOM_MACROS}) diff --git a/interface/CMakeLists.txt b/interface/CMakeLists.txt index 868a2cf933..dbc484d0b9 100644 --- a/interface/CMakeLists.txt +++ b/interface/CMakeLists.txt @@ -63,6 +63,17 @@ qt5_wrap_ui(QT_UI_HEADERS "${QT_UI_FILES}") # add them to the interface source files set(INTERFACE_SRCS ${INTERFACE_SRCS} "${QT_UI_HEADERS}" "${QT_RESOURCES}") +file(GLOB_RECURSE QML_SRC resources/qml/*.qml resources/qml/*.js) +add_custom_target(qml SOURCES ${QML_SRC}) + +if (UNIX) + install( + DIRECTORY "${CMAKE_SOURCE_DIR}/interface/resources/qml" + DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/resources + COMPONENT ${CLIENT_COMPONENT} + ) +endif() + # translation disabled until we strip out the line numbers # set(QM ${TARGET_NAME}_en.qm) # set(TS ${TARGET_NAME}_en.ts) From c6dbfc85f055bc95646b04b36b38c7b3526a00ad Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 7 Mar 2017 09:54:00 -0700 Subject: [PATCH 41/45] cr feedback --- interface/resources/qml/hifi/Pal.qml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index db975f3d8a..3608c03e79 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -658,7 +658,7 @@ Rectangle { }); if (newSelectedIndexes.length > 0) { table.selection.select(newSelectedIndexes); - table.positionViewAtRow(userIndex, ListView.Beginning); + table.positionViewAtRow(newSelectedIndexes[0], ListView.Beginning); } } signal sendToScript(var message); From b760e8622606c91be1eac4995c30426ff56fb30a Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 7 Mar 2017 10:17:32 -0700 Subject: [PATCH 42/45] rest of cr feedback --- interface/resources/qml/hifi/Pal.qml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 3608c03e79..7f9de35c90 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -642,7 +642,7 @@ Rectangle { userModel.clear(); var userIndex = 0; - var newSelectedIndexes=[]; + var newSelectedIndexes = []; userModelData.forEach(function (datum) { function init(property) { if (datum[property] === undefined) { From 694fc5837425251b3e0442f35975c0fbb8f2a6b6 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 7 Mar 2017 12:07:24 -0800 Subject: [PATCH 43/45] restore missing shading pipeline for simple opaque in deferred --- libraries/render-utils/src/RenderPipelines.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/render-utils/src/RenderPipelines.cpp b/libraries/render-utils/src/RenderPipelines.cpp index 3b279ff6d9..4fbac4170e 100644 --- a/libraries/render-utils/src/RenderPipelines.cpp +++ b/libraries/render-utils/src/RenderPipelines.cpp @@ -165,6 +165,9 @@ void initDeferredPipelines(render::ShapePlumber& plumber) { addPipeline( Key::Builder().withMaterial(), modelVertex, modelPixel); + addPipeline( + Key::Builder(), + modelVertex, modelPixel); addPipeline( Key::Builder().withMaterial().withUnlit(), modelVertex, modelUnlitPixel); From cb41b9135bd7feca8f984bf7e48354030641d38b Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 7 Mar 2017 12:10:52 -0800 Subject: [PATCH 44/45] fix broken merge --- interface/resources/qml/hifi/Pal.qml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index e3fe8152da..25362d98f1 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -627,6 +627,8 @@ Rectangle { var sortProperty = column ? column.role : "displayName"; var before = (table.sortIndicatorOrder === Qt.AscendingOrder) ? -1 : 1; var after = -1 * before; + // get selection(s) before sorting + var selectedIDs = getSelectedSessionIDs(); userModelData.sort(function (a, b) { var aValue = a[sortProperty].toString().toLowerCase(), bValue = b[sortProperty].toString().toLowerCase(); switch (true) { @@ -639,7 +641,6 @@ Rectangle { userModel.clear(); var userIndex = 0; - // get selection(s) before sorting var newSelectedIndexes = []; userModelData.forEach(function (datum) { function init(property) { From d2c3b0b7066ccb6fcf5e752376df27abf5c5e5bb Mon Sep 17 00:00:00 2001 From: Thijs Wenker Date: Tue, 7 Mar 2017 22:37:54 +0100 Subject: [PATCH 45/45] Fix coding standards link --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 86ea351609..a0d867ade9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ Contributing git checkout -b new_branch_name ``` 4. Code - * Follow the [coding standard](https://readme.highfidelity.com/v1.0/docs/coding-standard) + * Follow the [coding standard](https://wiki.highfidelity.com/wiki/Coding_Standards) 5. Commit * Use [well formed commit messages](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) 6. Update your branch