From 554247193c5a67506b0bfd16ee5767dc6f297f09 Mon Sep 17 00:00:00 2001 From: samcake Date: Fri, 15 Apr 2016 19:55:52 -0700 Subject: [PATCH 01/28] Trying SSBO for the object matrices --- libraries/gl/src/gl/Config.h | 2 +- libraries/gpu/src/gpu/Config.slh | 2 +- libraries/gpu/src/gpu/GLBackend.h | 3 ++- libraries/gpu/src/gpu/GLBackendTransform.cpp | 11 +++++++---- libraries/gpu/src/gpu/Transform.slh | 6 +++--- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/libraries/gl/src/gl/Config.h b/libraries/gl/src/gl/Config.h index 8f7582c271..a7d2c27db2 100644 --- a/libraries/gl/src/gl/Config.h +++ b/libraries/gl/src/gl/Config.h @@ -51,7 +51,7 @@ #if (GPU_INPUT_PROFILE == GPU_CORE_43) // Deactivate SSBO for now, we've run into some issues // on GL 4.3 capable GPUs not behaving as expected -//#define GPU_SSBO_DRAW_CALL_INFO +#define GPU_SSBO_DRAW_CALL_INFO #endif diff --git a/libraries/gpu/src/gpu/Config.slh b/libraries/gpu/src/gpu/Config.slh index 3da92df7c1..5a96a7eafe 100644 --- a/libraries/gpu/src/gpu/Config.slh +++ b/libraries/gpu/src/gpu/Config.slh @@ -13,7 +13,7 @@ <@if GLPROFILE == PC_GL @> <@def GPU_FEATURE_PROFILE GPU_CORE@> - <@def VERSION_HEADER #version 410 core@> + <@def VERSION_HEADER #version 430 core@> <@elif GLPROFILE == MAC_GL @> <@def GPU_FEATURE_PROFILE GPU_CORE@> <@def VERSION_HEADER #version 410 core@> diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index aabd84fbfb..8ed7a70788 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -386,7 +386,8 @@ protected: mutable std::map _drawCallInfoOffsets; - GLuint _objectBuffer { 0 }; + mutable int batchNum{ 0 }; + GLuint _objectBuffer[2]; GLuint _cameraBuffer { 0 }; GLuint _drawCallInfoBuffer { 0 }; GLuint _objectBufferTexture { 0 }; diff --git a/libraries/gpu/src/gpu/GLBackendTransform.cpp b/libraries/gpu/src/gpu/GLBackendTransform.cpp index 19cb7fb2a9..8d72f93d07 100755 --- a/libraries/gpu/src/gpu/GLBackendTransform.cpp +++ b/libraries/gpu/src/gpu/GLBackendTransform.cpp @@ -59,7 +59,7 @@ void GLBackend::do_setDepthRangeTransform(Batch& batch, size_t paramOffset) { } void GLBackend::initTransform() { - glGenBuffers(1, &_transform._objectBuffer); + glGenBuffers(2, _transform._objectBuffer); glGenBuffers(1, &_transform._cameraBuffer); glGenBuffers(1, &_transform._drawCallInfoBuffer); #ifndef GPU_SSBO_DRAW_CALL_INFO @@ -72,7 +72,8 @@ void GLBackend::initTransform() { } void GLBackend::killTransform() { - glDeleteBuffers(1, &_transform._objectBuffer); + glDeleteBuffers(2, _transform._objectBuffer); + // glDeleteBuffers(1, &_transform._objectBuffer); glDeleteBuffers(1, &_transform._cameraBuffer); glDeleteBuffers(1, &_transform._drawCallInfoBuffer); #ifndef GPU_SSBO_DRAW_CALL_INFO @@ -145,7 +146,9 @@ void GLBackend::TransformStageState::transfer(const Batch& batch) const { memcpy(bufferData.data(), batch._objects.data(), byteSize); #ifdef GPU_SSBO_DRAW_CALL_INFO - glBindBuffer(GL_SHADER_STORAGE_BUFFER, _objectBuffer); + batchNum++; + // glBindBuffer(GL_SHADER_STORAGE_BUFFER, _objectBuffer); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, _objectBuffer[batchNum % 2]); glBufferData(GL_SHADER_STORAGE_BUFFER, bufferData.size(), bufferData.data(), GL_DYNAMIC_DRAW); glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0); #else @@ -171,7 +174,7 @@ void GLBackend::TransformStageState::transfer(const Batch& batch) const { } #ifdef GPU_SSBO_DRAW_CALL_INFO - glBindBufferBase(GL_SHADER_STORAGE_BUFFER, TRANSFORM_OBJECT_SLOT, _objectBuffer); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, TRANSFORM_OBJECT_SLOT, _objectBuffer[batchNum % 2]); #else glActiveTexture(GL_TEXTURE0 + TRANSFORM_OBJECT_SLOT); glBindTexture(GL_TEXTURE_BUFFER, _objectBufferTexture); diff --git a/libraries/gpu/src/gpu/Transform.slh b/libraries/gpu/src/gpu/Transform.slh index 52ddf92158..d2d8e7af34 100644 --- a/libraries/gpu/src/gpu/Transform.slh +++ b/libraries/gpu/src/gpu/Transform.slh @@ -37,7 +37,7 @@ struct TransformObject { layout(location=15) in ivec2 _drawCallInfo; -<@if FALSE @> +!> // Disable SSBOs for now layout(std140) buffer transformObjectBuffer { TransformObject _object[]; @@ -45,7 +45,7 @@ layout(std140) buffer transformObjectBuffer { TransformObject getTransformObject() { return _object[_drawCallInfo.x]; } -<@else@> + uniform samplerBuffer transformObjectBuffer; TransformObject getTransformObject() { @@ -63,7 +63,7 @@ TransformObject getTransformObject() { return object; } -<@endif@> +<@endif@> !> <@endfunc@> From 87078b1ea29931e9de641a92876382f1bc710aad Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Sat, 16 Apr 2016 11:09:23 -0700 Subject: [PATCH 02/28] first cut at resetting entities script engine on domain change --- interface/src/Application.cpp | 40 +++++++++++++++++-- .../src/EntityTreeRenderer.cpp | 38 +++++++++++++++++- libraries/script-engine/src/ScriptEngine.cpp | 16 ++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 3a0b0998c5..11c68a8878 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1156,6 +1156,9 @@ void Application::aboutToQuit() { } void Application::cleanupBeforeQuit() { + qDebug() << __FUNCTION__ << "----- START -----"; + + // add a logline indicating if QTWEBENGINE_REMOTE_DEBUGGING is set or not QString webengineRemoteDebugging = QProcessEnvironment::systemEnvironment().value("QTWEBENGINE_REMOTE_DEBUGGING", "false"); qCDebug(interfaceapp) << "QTWEBENGINE_REMOTE_DEBUGGING =" << webengineRemoteDebugging; @@ -1178,8 +1181,6 @@ void Application::cleanupBeforeQuit() { } _keyboardFocusHighlight = nullptr; - getEntities()->clear(); // this will allow entity scripts to properly shutdown - auto nodeList = DependencyManager::get(); // send the domain a disconnect packet, force stoppage of domain-server check-ins @@ -1189,10 +1190,18 @@ void Application::cleanupBeforeQuit() { // tell the packet receiver we're shutting down, so it can drop packets nodeList->getPacketReceiver().setShouldDropPackets(true); + qDebug() << __FUNCTION__ << "about to call getEntities()->shutdown();"; getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts + + getEntities()->clear(); // this will allow entity scripts to properly shutdown + //DependencyManager::destroy(); + qDebug() << __FUNCTION__ << "AFTER call to getEntities()->shutdown();"; + + qDebug() << __FUNCTION__ << "about to shutdown ScriptEngines..."; DependencyManager::get()->saveScripts(); DependencyManager::get()->shutdownScripting(); // stop all currently running global scripts DependencyManager::destroy(); + qDebug() << __FUNCTION__ << "AFTER shutdown ScriptEngines..."; // first stop all timers directly or by invokeMethod // depending on what thread they run in @@ -1202,11 +1211,15 @@ void Application::cleanupBeforeQuit() { pingTimer.stop(); QMetaObject::invokeMethod(&_settingsTimer, "stop", Qt::BlockingQueuedConnection); + qDebug() << __FUNCTION__ << " line:" << __LINE__; + // save state _settingsThread.quit(); saveSettings(); _window->saveGeometry(); + qDebug() << __FUNCTION__ << " line:" << __LINE__; + // stop the AudioClient QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); @@ -1214,10 +1227,14 @@ void Application::cleanupBeforeQuit() { // destroy the AudioClient so it and its thread have a chance to go down safely DependencyManager::destroy(); + qDebug() << __FUNCTION__ << " line:" << __LINE__; + // destroy the AudioInjectorManager so it and its thread have a chance to go down safely // this will also stop any ongoing network injectors DependencyManager::destroy(); + qDebug() << __FUNCTION__ << " line:" << __LINE__; + // Destroy third party processes after scripts have finished using them. #ifdef HAVE_DDE DependencyManager::destroy(); @@ -1226,7 +1243,11 @@ void Application::cleanupBeforeQuit() { DependencyManager::destroy(); #endif + qDebug() << __FUNCTION__ << " line:" << __LINE__; + DependencyManager::destroy(); + + qDebug() << __FUNCTION__ << " ----- END -----"; } Application::~Application() { @@ -4174,6 +4195,14 @@ void Application::updateWindowTitle() const { } void Application::clearDomainOctreeDetails() { + + // if we're about to quit, we really don't need to do any of these things... + if (_aboutToQuit) { + qCDebug(interfaceapp) << "No need to clear domain octree details we are shutting down..."; + return; + } + + qCDebug(interfaceapp) << "Clearing domain octree details..."; // reset the environment so that we don't erroneously end up with multiple @@ -4189,7 +4218,9 @@ void Application::clearDomainOctreeDetails() { }); // reset the model renderer + qCDebug(interfaceapp) << __FUNCTION__ << " --- BEFORE getEntities()->clear() --- _aboutToQuit:" << _aboutToQuit; getEntities()->clear(); + qCDebug(interfaceapp) << __FUNCTION__ << " --- AFTER getEntities()->clear() ---"; auto skyStage = DependencyManager::get()->getSkyStage(); skyStage->setBackgroundMode(model::SunSkyStage::SKY_DOME); @@ -4198,10 +4229,13 @@ void Application::clearDomainOctreeDetails() { } void Application::domainChanged(const QString& domainHostname) { + qCDebug(interfaceapp) << __FUNCTION__ << " --- BEGIN --- domainHostname:" << domainHostname; + updateWindowTitle(); - clearDomainOctreeDetails(); + //clearDomainOctreeDetails(); // disable physics until we have enough information about our new location to not cause craziness. resetPhysicsReadyInformation(); + qCDebug(interfaceapp) << __FUNCTION__ << " --- END ---"; } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index eab28f500a..2544eb970d 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -71,13 +71,46 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf } EntityTreeRenderer::~EntityTreeRenderer() { + qDebug() << __FUNCTION__ << "---- BEGIN ----"; // NOTE: We don't need to delete _entitiesScriptEngine because // it is registered with ScriptEngines, which will call deleteLater for us. + + /* + if (_entitiesScriptEngine) { + qDebug() << __FUNCTION__ << "about to stop entity script engine."; + _entitiesScriptEngine->stop(); + qDebug() << __FUNCTION__ << "about to delete entity script engine."; + delete _entitiesScriptEngine; + _entitiesScriptEngine = nullptr; + } + */ + qDebug() << __FUNCTION__ << "---- END ----"; } +static int entititesScriptEngineCount = 0; + void EntityTreeRenderer::clear() { + qDebug() << __FUNCTION__ << "---- BEGIN ----"; leaveAllEntities(); - _entitiesScriptEngine->unloadAllEntityScripts(); + if (_entitiesScriptEngine) { + _entitiesScriptEngine->unloadAllEntityScripts(); + } + + // this would be a good place to actuall delete and recreate the _entitiesScriptEngine + qDebug() << __FUNCTION__ << "_shuttingDown:" << _shuttingDown; + if (_wantScripts && !_shuttingDown) { + qDebug() << __FUNCTION__ << " about to stop/delete current _entitiesScriptEngine"; + _entitiesScriptEngine->stop(); + _entitiesScriptEngine->deleteLater(); + qDebug() << __FUNCTION__ << " AFTER stop/delete current _entitiesScriptEngine"; + + qDebug() << __FUNCTION__ << " about to create new _entitiesScriptEngine"; + _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)); + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine); + _entitiesScriptEngine->runInThread(); + DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine); + qDebug() << __FUNCTION__ << " AFTER create new _entitiesScriptEngine"; + } auto scene = _viewState->getMain3DScene(); render::PendingChanges pendingChanges; @@ -88,6 +121,7 @@ void EntityTreeRenderer::clear() { _entitiesInScene.clear(); OctreeRenderer::clear(); + qDebug() << __FUNCTION__ << "---- END ----"; } void EntityTreeRenderer::reloadEntityScripts() { @@ -105,7 +139,7 @@ void EntityTreeRenderer::init() { entityTree->setFBXService(this); if (_wantScripts) { - _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, "Entities"); + _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine); _entitiesScriptEngine->runInThread(); DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine); diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 506bdb9463..1b3ad09d78 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -142,14 +142,18 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam } ScriptEngine::~ScriptEngine() { + qCDebug(scriptengine) << __FUNCTION__ << "--- BEGIN --- script:" << getFilename(); qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename(); auto scriptEngines = DependencyManager::get(); if (scriptEngines) { + qCDebug(scriptengine) << "About to remove ScriptEngine [" << getFilename() << "] from ScriptEngines!"; scriptEngines->removeScriptEngine(this); + qCDebug(scriptengine) << "AFTER remove ScriptEngine [" << getFilename() << "] from ScriptEngines!"; } else { qCWarning(scriptengine) << "Script destroyed after ScriptEngines!"; } + qCDebug(scriptengine) << __FUNCTION__ << "--- END --- script:" << getFilename(); } void ScriptEngine::disconnectNonEssentialSignals() { @@ -1197,6 +1201,8 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { } } +#include + void ScriptEngine::unloadAllEntityScripts() { if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING @@ -1213,6 +1219,16 @@ void ScriptEngine::unloadAllEntityScripts() { callEntityScriptMethod(entityID, "unload"); } _entityScripts.clear(); + +#ifdef DEBUG_ENGINE_STATE + qDebug() << "---- CURRENT STATE OF ENGINE: --------------------------"; + QScriptValueIterator it(globalObject()); + while (it.hasNext()) { + it.next(); + qDebug() << it.name() << ":" << it.value().toString(); + } + qDebug() << "--------------------------------------------------------"; +#endif // DEBUG_ENGINE_STATE } void ScriptEngine::refreshFileScript(const EntityItemID& entityID) { From 71d57295eb968cd6f66364253b6d75893d77cc34 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Sat, 16 Apr 2016 19:39:45 -0700 Subject: [PATCH 03/28] handle possibly deleted ScriptEngine in getScriptContents --- libraries/script-engine/src/ScriptEngine.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 1b3ad09d78..cff0c569bf 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1077,13 +1077,21 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& qDebug() << "ScriptEngine::loadEntityScript() calling scriptCache->getScriptContents() on thread [" << QThread::currentThread() << "] expected thread [" << thread() << "]"; #endif - DependencyManager::get()->getScriptContents(entityScript, [=](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) { + + QPointer theEngine(this); + + DependencyManager::get()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) { #ifdef THREAD_DEBUGGING qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread [" << QThread::currentThread() << "] expected thread [" << thread() << "]"; #endif - this->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success); + if (!theEngine.isNull()) { + qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()"; + theEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success); + } else { + qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted..."; + } }, forceRedownload); } From 9db0fe0d11594ac658e6a4d8afa08c19a8b6885c Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 09:46:04 -0700 Subject: [PATCH 04/28] switch to using QSharedPointer for _entitiesScriptEngine --- .../src/EntityTreeRenderer.cpp | 28 ++++++++++------ .../src/EntityTreeRenderer.h | 2 +- libraries/script-engine/src/ScriptEngine.cpp | 32 ++++++++++++------- libraries/script-engine/src/ScriptEngine.h | 15 ++++++++- 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 2544eb970d..3edc9ba75a 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -99,16 +99,24 @@ void EntityTreeRenderer::clear() { // this would be a good place to actuall delete and recreate the _entitiesScriptEngine qDebug() << __FUNCTION__ << "_shuttingDown:" << _shuttingDown; if (_wantScripts && !_shuttingDown) { - qDebug() << __FUNCTION__ << " about to stop/delete current _entitiesScriptEngine"; + qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->stop()"; _entitiesScriptEngine->stop(); + qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->stop()"; + + qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->deleteLater()"; + // NOTE: you can't actually delete it here we need to let Qt unwind and delete the object safely when ready + qDebug() << __FUNCTION__ << " about to call _entitiesScriptEngine->deleteLater() on thread[" << QThread::currentThread() << "], _entitiesScriptEngine's thread [" << _entitiesScriptEngine->thread() << "]"; _entitiesScriptEngine->deleteLater(); - qDebug() << __FUNCTION__ << " AFTER stop/delete current _entitiesScriptEngine"; + //QMetaObject::invokeMethod(_entitiesScriptEngine, "deleteLater"); + qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->deleteLater()"; qDebug() << __FUNCTION__ << " about to create new _entitiesScriptEngine"; - _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)); - _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine); + _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater); + + + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine); + DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); qDebug() << __FUNCTION__ << " AFTER create new _entitiesScriptEngine"; } @@ -128,7 +136,7 @@ void EntityTreeRenderer::reloadEntityScripts() { _entitiesScriptEngine->unloadAllEntityScripts(); foreach(auto entity, _entitiesInScene) { if (!entity->getScript().isEmpty()) { - _entitiesScriptEngine->loadEntityScript(entity->getEntityItemID(), entity->getScript(), true); + ScriptEngine::loadEntityScript(_entitiesScriptEngine, entity->getEntityItemID(), entity->getScript(), true); } } } @@ -139,10 +147,10 @@ void EntityTreeRenderer::init() { entityTree->setFBXService(this); if (_wantScripts) { - _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)); - _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine); + _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater); + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine); + DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); } forceRecheckEntities(); // setup our state to force checking our inside/outsideness of entities @@ -797,7 +805,7 @@ void EntityTreeRenderer::checkAndCallPreload(const EntityItemID& entityID, const if (entity && entity->shouldPreloadScript()) { QString scriptUrl = entity->getScript(); scriptUrl = ResourceManager::normalizeURL(scriptUrl); - _entitiesScriptEngine->loadEntityScript(entityID, scriptUrl, reload); + ScriptEngine::loadEntityScript(_entitiesScriptEngine, entityID, scriptUrl, reload); entity->scriptHasPreloaded(); } } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index 0fd5adc3f0..42e8ddeddf 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -155,7 +155,7 @@ private: NetworkTexturePointer _ambientTexture; bool _wantScripts; - ScriptEngine* _entitiesScriptEngine; + QSharedPointer _entitiesScriptEngine; bool isCollisionOwner(const QUuid& myNodeID, EntityTreePointer entityTree, const EntityItemID& id, const Collision& collision); diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index cff0c569bf..6b2d9e6854 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -143,6 +143,7 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam ScriptEngine::~ScriptEngine() { qCDebug(scriptengine) << __FUNCTION__ << "--- BEGIN --- script:" << getFilename(); + qCDebug(scriptengine) << __FUNCTION__ << "called on thread[" << QThread::currentThread() << "], object's thread [" << thread() << "] script:" << getFilename(); qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename(); auto scriptEngines = DependencyManager::get(); @@ -1051,7 +1052,10 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin // 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(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { +void ScriptEngine::loadEntityScript(QWeakPointer theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) { + + +/***** if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING qDebug() << "*** WARNING *** ScriptEngine::loadEntityScript() called on wrong thread [" @@ -1070,25 +1074,30 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& "entityID:" << entityID << "entityScript:" << entityScript << "forceRedownload:" << forceRedownload; #endif - // If we've been called our known entityScripts should not know about us.. - assert(!_entityScripts.contains(entityID)); + // If we've been called our known entityScripts should not know about us.. + assert(!_entityScripts.contains(entityID)); #ifdef THREAD_DEBUGGING qDebug() << "ScriptEngine::loadEntityScript() calling scriptCache->getScriptContents() on thread [" << QThread::currentThread() << "] expected thread [" << thread() << "]"; #endif +******/ - QPointer theEngine(this); - + // NOTE: If the script content is not currently in the caceh, The LAMBDA here, will be called on the Main Thread + // which means we're guarenteed 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) { -#ifdef THREAD_DEBUGGING - qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread [" - << QThread::currentThread() << "] expected thread [" << thread() << "]"; + QSharedPointer strongEngine = theEngine.toStrongRef(); + if (strongEngine) { + qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()"; +#if 1//def THREAD_DEBUGGING + qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread [" + << QThread::currentThread() << "] expected thread [" << strongEngine->thread() << "]"; #endif - if (!theEngine.isNull()) { - qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()"; - theEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success); + + strongEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success); } else { qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted..."; } @@ -1098,6 +1107,7 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& // since all of these operations can be asynch we will always do the actual work in the response handler // for the download void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, const QString& scriptOrURL, const QString& contents, bool isURL, bool success) { + if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING qDebug() << "*** WARNING *** ScriptEngine::entityScriptContentAvailable() called on wrong thread [" diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index a6a623e751..c5ed5b56fe 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -70,6 +70,9 @@ public: ~ScriptEngine(); + // Useful for QSharePointer + static void doDeleteLater(ScriptEngine* obj) { obj->deleteLater(); } + /// 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. @@ -124,7 +127,10 @@ public: Q_INVOKABLE QUrl resolvePath(const QString& path) const; // Entity Script Related methods - Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload = false); // will call the preload method once loaded + //Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload = false); // will call the preload method once loaded + + static void loadEntityScript(QWeakPointer theEngine, 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, const QStringList& params = QStringList()); @@ -224,4 +230,11 @@ protected: static std::atomic _stoppingAllScripts; }; +/* +class ScriptEngine : public QObject { + Q_OBJECT +}; +*/ + + #endif // hifi_ScriptEngine_h From a4f562672acf14f3b944dcdf9c0d0e3ee8908d99 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 10:08:33 -0700 Subject: [PATCH 05/28] cleanup --- .../src/EntityTreeRenderer.cpp | 48 +++++++------------ .../src/EntityTreeRenderer.h | 4 ++ 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 3edc9ba75a..131784cc94 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -71,23 +71,21 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf } EntityTreeRenderer::~EntityTreeRenderer() { - qDebug() << __FUNCTION__ << "---- BEGIN ----"; + qDebug() << __FUNCTION__; // NOTE: We don't need to delete _entitiesScriptEngine because // it is registered with ScriptEngines, which will call deleteLater for us. - - /* - if (_entitiesScriptEngine) { - qDebug() << __FUNCTION__ << "about to stop entity script engine."; - _entitiesScriptEngine->stop(); - qDebug() << __FUNCTION__ << "about to delete entity script engine."; - delete _entitiesScriptEngine; - _entitiesScriptEngine = nullptr; - } - */ - qDebug() << __FUNCTION__ << "---- END ----"; } -static int entititesScriptEngineCount = 0; +int EntityTreeRenderer::_entititesScriptEngineCount = 0; + +void EntityTreeRenderer::setupEntitiesScriptEngine() { + qDebug() << __FUNCTION__ << "---- BEGIN ----"; + _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater); + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); + _entitiesScriptEngine->runInThread(); + DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); + qDebug() << __FUNCTION__ << "---- END ----"; +} void EntityTreeRenderer::clear() { qDebug() << __FUNCTION__ << "---- BEGIN ----"; @@ -103,21 +101,10 @@ void EntityTreeRenderer::clear() { _entitiesScriptEngine->stop(); qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->stop()"; - qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->deleteLater()"; - // NOTE: you can't actually delete it here we need to let Qt unwind and delete the object safely when ready - qDebug() << __FUNCTION__ << " about to call _entitiesScriptEngine->deleteLater() on thread[" << QThread::currentThread() << "], _entitiesScriptEngine's thread [" << _entitiesScriptEngine->thread() << "]"; - _entitiesScriptEngine->deleteLater(); - //QMetaObject::invokeMethod(_entitiesScriptEngine, "deleteLater"); - qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->deleteLater()"; - - qDebug() << __FUNCTION__ << " about to create new _entitiesScriptEngine"; - _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater); - - - _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); - _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); - qDebug() << __FUNCTION__ << " AFTER create new _entitiesScriptEngine"; + // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will + // assign a new instance to our shared pointer, which will deref the old instance and ultimately call + // the custom deleter which calls deleteLater + setupEntitiesScriptEngine(); } auto scene = _viewState->getMain3DScene(); @@ -147,10 +134,7 @@ void EntityTreeRenderer::init() { entityTree->setFBXService(this); if (_wantScripts) { - _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater); - _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); - _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); + setupEntitiesScriptEngine(); } forceRecheckEntities(); // setup our state to force checking our inside/outsideness of entities diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index 42e8ddeddf..f1b581d8ef 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -126,6 +126,8 @@ protected: } private: + void setupEntitiesScriptEngine(); + void addEntityToScene(EntityItemPointer entity); bool findBestZoneAndMaybeContainingEntities(const glm::vec3& avatarPosition, QVector* entitiesContainingAvatar); @@ -196,6 +198,8 @@ private: QHash _entitiesInScene; // For Scene.shouldRenderEntities QList _entityIDsLastInScene; + + static int _entititesScriptEngineCount; }; From d1f6b371c977fae41ebbcdd909db9f5e344bec20 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 10:27:12 -0700 Subject: [PATCH 06/28] cleanup --- interface/src/Application.cpp | 27 ------------ .../src/EntityTreeRenderer.cpp | 8 ---- libraries/script-engine/src/ScriptEngine.cpp | 42 ++----------------- 3 files changed, 4 insertions(+), 73 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 11c68a8878..d7702d0091 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1156,9 +1156,6 @@ void Application::aboutToQuit() { } void Application::cleanupBeforeQuit() { - qDebug() << __FUNCTION__ << "----- START -----"; - - // add a logline indicating if QTWEBENGINE_REMOTE_DEBUGGING is set or not QString webengineRemoteDebugging = QProcessEnvironment::systemEnvironment().value("QTWEBENGINE_REMOTE_DEBUGGING", "false"); qCDebug(interfaceapp) << "QTWEBENGINE_REMOTE_DEBUGGING =" << webengineRemoteDebugging; @@ -1190,18 +1187,12 @@ void Application::cleanupBeforeQuit() { // tell the packet receiver we're shutting down, so it can drop packets nodeList->getPacketReceiver().setShouldDropPackets(true); - qDebug() << __FUNCTION__ << "about to call getEntities()->shutdown();"; getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts - getEntities()->clear(); // this will allow entity scripts to properly shutdown - //DependencyManager::destroy(); - qDebug() << __FUNCTION__ << "AFTER call to getEntities()->shutdown();"; - qDebug() << __FUNCTION__ << "about to shutdown ScriptEngines..."; DependencyManager::get()->saveScripts(); DependencyManager::get()->shutdownScripting(); // stop all currently running global scripts DependencyManager::destroy(); - qDebug() << __FUNCTION__ << "AFTER shutdown ScriptEngines..."; // first stop all timers directly or by invokeMethod // depending on what thread they run in @@ -1211,15 +1202,11 @@ void Application::cleanupBeforeQuit() { pingTimer.stop(); QMetaObject::invokeMethod(&_settingsTimer, "stop", Qt::BlockingQueuedConnection); - qDebug() << __FUNCTION__ << " line:" << __LINE__; - // save state _settingsThread.quit(); saveSettings(); _window->saveGeometry(); - qDebug() << __FUNCTION__ << " line:" << __LINE__; - // stop the AudioClient QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); @@ -1227,14 +1214,10 @@ void Application::cleanupBeforeQuit() { // destroy the AudioClient so it and its thread have a chance to go down safely DependencyManager::destroy(); - qDebug() << __FUNCTION__ << " line:" << __LINE__; - // destroy the AudioInjectorManager so it and its thread have a chance to go down safely // this will also stop any ongoing network injectors DependencyManager::destroy(); - qDebug() << __FUNCTION__ << " line:" << __LINE__; - // Destroy third party processes after scripts have finished using them. #ifdef HAVE_DDE DependencyManager::destroy(); @@ -1243,11 +1226,7 @@ void Application::cleanupBeforeQuit() { DependencyManager::destroy(); #endif - qDebug() << __FUNCTION__ << " line:" << __LINE__; - DependencyManager::destroy(); - - qDebug() << __FUNCTION__ << " ----- END -----"; } Application::~Application() { @@ -4218,9 +4197,7 @@ void Application::clearDomainOctreeDetails() { }); // reset the model renderer - qCDebug(interfaceapp) << __FUNCTION__ << " --- BEFORE getEntities()->clear() --- _aboutToQuit:" << _aboutToQuit; getEntities()->clear(); - qCDebug(interfaceapp) << __FUNCTION__ << " --- AFTER getEntities()->clear() ---"; auto skyStage = DependencyManager::get()->getSkyStage(); skyStage->setBackgroundMode(model::SunSkyStage::SKY_DOME); @@ -4229,13 +4206,9 @@ void Application::clearDomainOctreeDetails() { } void Application::domainChanged(const QString& domainHostname) { - qCDebug(interfaceapp) << __FUNCTION__ << " --- BEGIN --- domainHostname:" << domainHostname; - updateWindowTitle(); - //clearDomainOctreeDetails(); // disable physics until we have enough information about our new location to not cause craziness. resetPhysicsReadyInformation(); - qCDebug(interfaceapp) << __FUNCTION__ << " --- END ---"; } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 131784cc94..f30261b179 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -71,7 +71,6 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf } EntityTreeRenderer::~EntityTreeRenderer() { - qDebug() << __FUNCTION__; // NOTE: We don't need to delete _entitiesScriptEngine because // it is registered with ScriptEngines, which will call deleteLater for us. } @@ -79,27 +78,21 @@ EntityTreeRenderer::~EntityTreeRenderer() { int EntityTreeRenderer::_entititesScriptEngineCount = 0; void EntityTreeRenderer::setupEntitiesScriptEngine() { - qDebug() << __FUNCTION__ << "---- BEGIN ----"; _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); - qDebug() << __FUNCTION__ << "---- END ----"; } void EntityTreeRenderer::clear() { - qDebug() << __FUNCTION__ << "---- BEGIN ----"; leaveAllEntities(); if (_entitiesScriptEngine) { _entitiesScriptEngine->unloadAllEntityScripts(); } // this would be a good place to actuall delete and recreate the _entitiesScriptEngine - qDebug() << __FUNCTION__ << "_shuttingDown:" << _shuttingDown; if (_wantScripts && !_shuttingDown) { - qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->stop()"; _entitiesScriptEngine->stop(); - qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->stop()"; // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will // assign a new instance to our shared pointer, which will deref the old instance and ultimately call @@ -116,7 +109,6 @@ void EntityTreeRenderer::clear() { _entitiesInScene.clear(); OctreeRenderer::clear(); - qDebug() << __FUNCTION__ << "---- END ----"; } void EntityTreeRenderer::reloadEntityScripts() { diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 6b2d9e6854..915eae84f8 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -142,10 +142,7 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam } ScriptEngine::~ScriptEngine() { - qCDebug(scriptengine) << __FUNCTION__ << "--- BEGIN --- script:" << getFilename(); - qCDebug(scriptengine) << __FUNCTION__ << "called on thread[" << QThread::currentThread() << "], object's thread [" << thread() << "] script:" << getFilename(); qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename(); - auto scriptEngines = DependencyManager::get(); if (scriptEngines) { qCDebug(scriptengine) << "About to remove ScriptEngine [" << getFilename() << "] from ScriptEngines!"; @@ -154,7 +151,6 @@ ScriptEngine::~ScriptEngine() { } else { qCWarning(scriptengine) << "Script destroyed after ScriptEngines!"; } - qCDebug(scriptengine) << __FUNCTION__ << "--- END --- script:" << getFilename(); } void ScriptEngine::disconnectNonEssentialSignals() { @@ -1053,36 +1049,6 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin // 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) { - - -/***** - if (QThread::currentThread() != thread()) { -#ifdef THREAD_DEBUGGING - qDebug() << "*** WARNING *** ScriptEngine::loadEntityScript() called on wrong thread [" - << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] " - "entityID:" << entityID << "entityScript:" << entityScript <<"forceRedownload:" << forceRedownload; -#endif - - QMetaObject::invokeMethod(this, "loadEntityScript", - Q_ARG(const EntityItemID&, entityID), - Q_ARG(const QString&, entityScript), - Q_ARG(bool, forceRedownload)); - return; - } -#ifdef THREAD_DEBUGGING - qDebug() << "ScriptEngine::loadEntityScript() called on correct thread [" << thread() << "] " - "entityID:" << entityID << "entityScript:" << entityScript << "forceRedownload:" << forceRedownload; -#endif - - // If we've been called our known entityScripts should not know about us.. - assert(!_entityScripts.contains(entityID)); - -#ifdef THREAD_DEBUGGING - qDebug() << "ScriptEngine::loadEntityScript() calling scriptCache->getScriptContents() on thread [" - << QThread::currentThread() << "] expected thread [" << thread() << "]"; -#endif -******/ - // NOTE: If the script content is not currently in the caceh, The LAMBDA here, will be called on the Main Thread // which means we're guarenteed 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 @@ -1090,15 +1056,15 @@ void ScriptEngine::loadEntityScript(QWeakPointer theEngine, const DependencyManager::get()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) { QSharedPointer strongEngine = theEngine.toStrongRef(); if (strongEngine) { - qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()"; -#if 1//def THREAD_DEBUGGING +#ifdef THREAD_DEBUGGING qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread [" << QThread::currentThread() << "] expected thread [" << strongEngine->thread() << "]"; #endif - - strongEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success); } else { + // FIXME - I'm leaving this in for testing, so that QA can confirm that sometimes the script contents + // returns after the ScriptEngine has been deleted, we can remove this after QA verifies the + // repro case. qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted..."; } }, forceRedownload); From 0520363da85c8dfd298b52038388f5474e3e2f05 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 10:37:15 -0700 Subject: [PATCH 07/28] cleanup --- interface/src/Application.cpp | 4 +--- libraries/script-engine/src/ScriptEngine.cpp | 3 +-- libraries/script-engine/src/ScriptEngine.h | 10 ---------- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index d7702d0091..32b8af6ce8 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4177,14 +4177,12 @@ void Application::clearDomainOctreeDetails() { // if we're about to quit, we really don't need to do any of these things... if (_aboutToQuit) { - qCDebug(interfaceapp) << "No need to clear domain octree details we are shutting down..."; return; } - qCDebug(interfaceapp) << "Clearing domain octree details..."; - // reset the environment so that we don't erroneously end up with multiple + // reset the environment so that we don't erroneously end up with multiple resetPhysicsReadyInformation(); // reset our node to stats and node to jurisdiction maps... since these must be changing... diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 915eae84f8..2e9ca88908 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -145,9 +145,7 @@ ScriptEngine::~ScriptEngine() { qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename(); auto scriptEngines = DependencyManager::get(); if (scriptEngines) { - qCDebug(scriptengine) << "About to remove ScriptEngine [" << getFilename() << "] from ScriptEngines!"; scriptEngines->removeScriptEngine(this); - qCDebug(scriptengine) << "AFTER remove ScriptEngine [" << getFilename() << "] from ScriptEngines!"; } else { qCWarning(scriptengine) << "Script destroyed after ScriptEngines!"; } @@ -759,6 +757,7 @@ void ScriptEngine::stop() { QMetaObject::invokeMethod(this, "stop"); return; } + abortEvaluation(); // abort any script evaluation that may be active _isFinished = true; if (_wantSignals) { emit runningStateChanged(); diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index c5ed5b56fe..2af4975a8e 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -127,10 +127,7 @@ public: Q_INVOKABLE QUrl resolvePath(const QString& path) const; // Entity Script Related methods - //Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload = false); // will call the preload method once loaded - static void loadEntityScript(QWeakPointer theEngine, 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, const QStringList& params = QStringList()); @@ -230,11 +227,4 @@ protected: static std::atomic _stoppingAllScripts; }; -/* -class ScriptEngine : public QObject { - Q_OBJECT -}; -*/ - - #endif // hifi_ScriptEngine_h From f6c75d0530c3b41dfc9af287ac4318ec421dadcf Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 10:41:54 -0700 Subject: [PATCH 08/28] cleanup --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 3 +-- libraries/script-engine/src/ScriptEngine.cpp | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index f30261b179..da860cd834 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -88,12 +88,11 @@ void EntityTreeRenderer::clear() { leaveAllEntities(); if (_entitiesScriptEngine) { _entitiesScriptEngine->unloadAllEntityScripts(); + _entitiesScriptEngine->stop(); } // this would be a good place to actuall delete and recreate the _entitiesScriptEngine if (_wantScripts && !_shuttingDown) { - _entitiesScriptEngine->stop(); - // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will // assign a new instance to our shared pointer, which will deref the old instance and ultimately call // the custom deleter which calls deleteLater diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 2e9ca88908..2aab250ef4 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -1072,7 +1073,6 @@ void ScriptEngine::loadEntityScript(QWeakPointer theEngine, const // since all of these operations can be asynch we will always do the actual work in the response handler // for the download void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, const QString& scriptOrURL, const QString& contents, bool isURL, bool success) { - if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING qDebug() << "*** WARNING *** ScriptEngine::entityScriptContentAvailable() called on wrong thread [" @@ -1184,8 +1184,6 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { } } -#include - void ScriptEngine::unloadAllEntityScripts() { if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING From 47900df3e776bbb7b0dd05da3b4d8a0b2a890cd9 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 10:43:08 -0700 Subject: [PATCH 09/28] cleanup --- libraries/script-engine/src/ScriptEngine.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 2af4975a8e..7a021dde4f 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -71,7 +71,7 @@ public: ~ScriptEngine(); // Useful for QSharePointer - static void doDeleteLater(ScriptEngine* obj) { obj->deleteLater(); } + static void doDeleteLater(ScriptEngine* engine) { engine->deleteLater(); } /// 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 From 1c111e20bd6be7bdd201d5a4052111e076a2f920 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 11:20:26 -0700 Subject: [PATCH 10/28] fix possible thread issue with Entities.callEntityMethod() --- libraries/entities/src/EntityScriptingInterface.cpp | 6 ++++++ libraries/entities/src/EntityScriptingInterface.h | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 540cc68689..697119cf9c 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -414,7 +414,13 @@ void EntityScriptingInterface::deleteEntity(QUuid id) { } } +void EntityScriptingInterface::setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) { + std::unique_lock lock(_entitiesScriptEngineLock); + _entitiesScriptEngine = engine; +} + void EntityScriptingInterface::callEntityMethod(QUuid id, const QString& method, const QStringList& params) { + std::unique_lock lock(_entitiesScriptEngineLock); if (_entitiesScriptEngine) { EntityItemID entityID{ id }; _entitiesScriptEngine->callEntityScriptMethod(entityID, method, params); diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h index b65bc55dbb..ca6e6fd9ac 100644 --- a/libraries/entities/src/EntityScriptingInterface.h +++ b/libraries/entities/src/EntityScriptingInterface.h @@ -71,7 +71,7 @@ public: void setEntityTree(EntityTreePointer modelTree); EntityTreePointer getEntityTree() { return _entityTree; } - void setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) { _entitiesScriptEngine = engine; } + void setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine); float calculateCost(float mass, float oldVelocity, float newVelocity); public slots: @@ -214,6 +214,8 @@ private: bool precisionPicking, const QVector& entityIdsToInclude, const QVector& entityIdsToDiscard); EntityTreePointer _entityTree; + + std::mutex _entitiesScriptEngineLock; EntitiesScriptEngineProvider* _entitiesScriptEngine { nullptr }; bool _bidOnSimulationOwnership { false }; From c26115cf2ef72698921b5eeab8a49d88ced2f2f6 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 11:40:17 -0700 Subject: [PATCH 11/28] one small tweak --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index da860cd834..d0b2c67c95 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -78,6 +78,7 @@ EntityTreeRenderer::~EntityTreeRenderer() { int EntityTreeRenderer::_entititesScriptEngineCount = 0; void EntityTreeRenderer::setupEntitiesScriptEngine() { + QSharedPointer oldEngine = _entitiesScriptEngine; // save the old engine through this function, so the EntityScriptingInterface doesn't have problems with it. _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); From 0d20f2468e84caf51735dacaeace03dd719eec26 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 12:13:21 -0700 Subject: [PATCH 12/28] CR feedback --- interface/src/Application.cpp | 1 - libraries/entities-renderer/src/EntityTreeRenderer.cpp | 4 ++-- libraries/entities-renderer/src/EntityTreeRenderer.h | 2 +- libraries/script-engine/src/ScriptEngine.h | 3 --- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 32b8af6ce8..eeed11deee 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4182,7 +4182,6 @@ void Application::clearDomainOctreeDetails() { qCDebug(interfaceapp) << "Clearing domain octree details..."; - // reset the environment so that we don't erroneously end up with multiple resetPhysicsReadyInformation(); // reset our node to stats and node to jurisdiction maps... since these must be changing... diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index d0b2c67c95..9de2fc1525 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -75,11 +75,11 @@ EntityTreeRenderer::~EntityTreeRenderer() { // it is registered with ScriptEngines, which will call deleteLater for us. } -int EntityTreeRenderer::_entititesScriptEngineCount = 0; +int EntityTreeRenderer::_entitiesScriptEngineCount = 0; void EntityTreeRenderer::setupEntitiesScriptEngine() { QSharedPointer oldEngine = _entitiesScriptEngine; // save the old engine through this function, so the EntityScriptingInterface doesn't have problems with it. - _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater); + _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)), &QObject::deleteLater); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index f1b581d8ef..a6fc58e5f1 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -199,7 +199,7 @@ private: // For Scene.shouldRenderEntities QList _entityIDsLastInScene; - static int _entititesScriptEngineCount; + static int _entitiesScriptEngineCount; }; diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 7a021dde4f..e8ce00c66c 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -70,9 +70,6 @@ public: ~ScriptEngine(); - // Useful for QSharePointer - static void doDeleteLater(ScriptEngine* engine) { engine->deleteLater(); } - /// 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. From 374ba105249284c6e7a66e2882a6542041e4ef49 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 12:23:36 -0700 Subject: [PATCH 13/28] more CR feedback --- interface/src/Application.cpp | 1 - libraries/entities-renderer/src/EntityTreeRenderer.cpp | 2 ++ libraries/entities/src/EntityScriptingInterface.cpp | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index eeed11deee..ce673f444f 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1188,7 +1188,6 @@ void Application::cleanupBeforeQuit() { nodeList->getPacketReceiver().setShouldDropPackets(true); getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts - getEntities()->clear(); // this will allow entity scripts to properly shutdown DependencyManager::get()->saveScripts(); DependencyManager::get()->shutdownScripting(); // stop all currently running global scripts diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 9de2fc1525..abba0fe7fe 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -140,6 +140,8 @@ void EntityTreeRenderer::init() { void EntityTreeRenderer::shutdown() { _entitiesScriptEngine->disconnectNonEssentialSignals(); // disconnect all slots/signals from the script engine, except essential _shuttingDown = true; + + clear(); // always clear() on shutdown } void EntityTreeRenderer::setTree(OctreePointer newTree) { diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 697119cf9c..8213316b7b 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -415,12 +415,12 @@ void EntityScriptingInterface::deleteEntity(QUuid id) { } void EntityScriptingInterface::setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) { - std::unique_lock lock(_entitiesScriptEngineLock); + std::lock_guard lock(_entitiesScriptEngineLock); _entitiesScriptEngine = engine; } void EntityScriptingInterface::callEntityMethod(QUuid id, const QString& method, const QStringList& params) { - std::unique_lock lock(_entitiesScriptEngineLock); + std::lock_guard lock(_entitiesScriptEngineLock); if (_entitiesScriptEngine) { EntityItemID entityID{ id }; _entitiesScriptEngine->callEntityScriptMethod(entityID, method, params); From 9e13b1bbaef7fd3cbb06d62cfcc01a7396c9cf10 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 12:25:46 -0700 Subject: [PATCH 14/28] more CR feedback --- libraries/script-engine/src/ScriptEngine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 2aab250ef4..610f542397 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1049,8 +1049,8 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin // 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) { - // NOTE: If the script content is not currently in the caceh, The LAMBDA here, will be called on the Main Thread - // which means we're guarenteed that it's not the correct thread for the ScriptEngine. This means + // 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) { From 55010cebb70a0898b5552dd1346aa558acd582c8 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 12:43:32 -0700 Subject: [PATCH 15/28] remove unhelpful comment --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index abba0fe7fe..09f50c5de3 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -92,7 +92,6 @@ void EntityTreeRenderer::clear() { _entitiesScriptEngine->stop(); } - // this would be a good place to actuall delete and recreate the _entitiesScriptEngine if (_wantScripts && !_shuttingDown) { // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will // assign a new instance to our shared pointer, which will deref the old instance and ultimately call From bf8cdabd1bf965e80f35728ece99c58a006dacb0 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 12:50:29 -0700 Subject: [PATCH 16/28] CR feedback --- 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 610f542397..cbbc8832b6 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1065,7 +1065,7 @@ void ScriptEngine::loadEntityScript(QWeakPointer theEngine, const // FIXME - I'm leaving this in for testing, so that QA can confirm that sometimes the script contents // returns after the ScriptEngine has been deleted, we can remove this after QA verifies the // repro case. - qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted..."; + qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted... script:" << scriptOrURL; } }, forceRedownload); } From 6ac173d19701fa5087db1e5e4988950c04d5e257 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 13:27:05 -0700 Subject: [PATCH 17/28] add an example script --- examples/entityScripts/exampleUpdate.js | 54 +++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 examples/entityScripts/exampleUpdate.js diff --git a/examples/entityScripts/exampleUpdate.js b/examples/entityScripts/exampleUpdate.js new file mode 100644 index 0000000000..c8a18ac774 --- /dev/null +++ b/examples/entityScripts/exampleUpdate.js @@ -0,0 +1,54 @@ +// +// exampleUpdate.js +// examples/entityScripts +// +// Created by Brad Hefta-Gaub on 4/18/16. +// Copyright 2016 High Fidelity, Inc. +// +// This is an example of an entity script which hooks the update signal +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +(function() { + var _this; + + // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember + // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals) + ExampleUpdate = function() { + _this = this; + }; + + ExampleUpdate.prototype = { + + // update() will be called regulary, because we've hooked the update signal in our preload() function. + // we will check the avatars hand positions and if either hand is in our bounding box, we will notice that + update: function() { + // because the update() signal doesn't have a valid this, we need to use our memorized _this to access our entityID + var entityID = _this.entityID; + print("update in entityID:" + entityID); + }, + + // preload() will be called when the entity has become visible (or known) to the interface + // it gives us a chance to set our local JavaScript object up. In this case it means: + // * remembering our entityID, so we can access it in cases where we're called without an entityID + // * connecting to the update signal so we can check our grabbed state + preload: function(entityID) { + print("preload - entityID:" + entityID); + this.entityID = entityID; + Script.update.connect(this.update); + }, + + // unload() will be called when our entity is no longer available. It may be because we were deleted, + // or because we've left the domain or quit the application. In all cases we want to unhook our connection + // to the update signal + unload: function(entityID) { + print("unload - entityID:" + entityID); + Script.update.disconnect(this.update); + }, + }; + + // entity scripts always need to return a newly constructed object of our type + return new ExampleUpdate(); +}) From 24003840df35ceb70fd9cc90062789c704a5dab3 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 13:33:08 -0700 Subject: [PATCH 18/28] add an example script --- .../exampleUpdateNoDisconnect.js | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 examples/entityScripts/exampleUpdateNoDisconnect.js diff --git a/examples/entityScripts/exampleUpdateNoDisconnect.js b/examples/entityScripts/exampleUpdateNoDisconnect.js new file mode 100644 index 0000000000..81bb742ca2 --- /dev/null +++ b/examples/entityScripts/exampleUpdateNoDisconnect.js @@ -0,0 +1,54 @@ +// +// exampleUpdateNoDisconnect.js +// examples/entityScripts +// +// Created by Brad Hefta-Gaub on 4/18/16. +// Copyright 2016 High Fidelity, Inc. +// +// This is an example of an entity script which hooks the update signal +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +(function() { + var _this; + + // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember + // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals) + ExampleUpdate = function() { + _this = this; + }; + + ExampleUpdate.prototype = { + + // update() will be called regulary, because we've hooked the update signal in our preload() function. + // we will check the avatars hand positions and if either hand is in our bounding box, we will notice that + update: function() { + // because the update() signal doesn't have a valid this, we need to use our memorized _this to access our entityID + var entityID = _this.entityID; + print("update in entityID:" + entityID); + }, + + // preload() will be called when the entity has become visible (or known) to the interface + // it gives us a chance to set our local JavaScript object up. In this case it means: + // * remembering our entityID, so we can access it in cases where we're called without an entityID + // * connecting to the update signal so we can check our grabbed state + preload: function(entityID) { + print("preload - entityID:" + entityID); + this.entityID = entityID; + Script.update.connect(this.update); + }, + + // unload() will be called when our entity is no longer available. It may be because we were deleted, + // or because we've left the domain or quit the application. In all cases we want to unhook our connection + // to the update signal + unload: function(entityID) { + print("unload - entityID:" + entityID); + print("NOTE --- WE DID NOT CALL Script.update.disconnect()"); + }, + }; + + // entity scripts always need to return a newly constructed object of our type + return new ExampleUpdate(); +}) From d6fa58c39c9feeb4b4a201f8dd9324472bb3e3d7 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 13:42:58 -0700 Subject: [PATCH 19/28] add an example script --- .../entityScripts/exampleTimeoutNoCleanup.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 examples/entityScripts/exampleTimeoutNoCleanup.js diff --git a/examples/entityScripts/exampleTimeoutNoCleanup.js b/examples/entityScripts/exampleTimeoutNoCleanup.js new file mode 100644 index 0000000000..c05a7223ec --- /dev/null +++ b/examples/entityScripts/exampleTimeoutNoCleanup.js @@ -0,0 +1,50 @@ +// +// exampleTimeoutNoCleanup.js +// examples/entityScripts +// +// Created by Brad Hefta-Gaub on 4/18/16. +// Copyright 2016 High Fidelity, Inc. +// +// This is an example of an entity script which hooks the update signal +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +(function() { + var _this; + + // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember + // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals) + ExampleUpdate = function() { + _this = this; + }; + + ExampleUpdate.prototype = { + + // preload() will be called when the entity has become visible (or known) to the interface + // it gives us a chance to set our local JavaScript object up. In this case it means: + // * remembering our entityID, so we can access it in cases where we're called without an entityID + // * connecting to the update signal so we can check our grabbed state + preload: function(entityID) { + print("preload - entityID:" + entityID); + this.entityID = entityID; + + Script.setTimeout(function() { + var entityID = _this.entityID; + print("timer timeout in entityID:" + entityID); + }, 3000); + }, + + // unload() will be called when our entity is no longer available. It may be because we were deleted, + // or because we've left the domain or quit the application. In all cases we want to unhook our connection + // to the update signal + unload: function(entityID) { + print("unload - entityID:" + entityID); + print("NOTE --- WE DID NOT CALL clear our timeout"); + }, + }; + + // entity scripts always need to return a newly constructed object of our type + return new ExampleUpdate(); +}) From ab7f3d573984ae41f1b772c4fa3f1b392a036e4f Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 13:58:40 -0700 Subject: [PATCH 20/28] change the timeout to an interval so it constantly runs --- examples/entityScripts/exampleTimeoutNoCleanup.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/entityScripts/exampleTimeoutNoCleanup.js b/examples/entityScripts/exampleTimeoutNoCleanup.js index c05a7223ec..99b8816388 100644 --- a/examples/entityScripts/exampleTimeoutNoCleanup.js +++ b/examples/entityScripts/exampleTimeoutNoCleanup.js @@ -30,9 +30,9 @@ print("preload - entityID:" + entityID); this.entityID = entityID; - Script.setTimeout(function() { + Script.setInterval(function() { var entityID = _this.entityID; - print("timer timeout in entityID:" + entityID); + print("timer interval in entityID:" + entityID); }, 3000); }, From 62c252e8ef8e7d10b121d65810008e6b75f68b39 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 18 Apr 2016 13:26:40 -0700 Subject: [PATCH 21/28] Set GL in QML immediately --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 38 +++++++++++++++------ 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 9dc475ab27..e90ef9e782 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -83,11 +83,18 @@ protected: }; friend class OffscreenQmlSurface; + + QJsonObject getGLContextData(); + Queue _queue; QMutex _mutex; QWaitCondition _waitCondition; std::atomic _rendering { false }; + QJsonObject _glData; + QMutex _glMutex; + QWaitCondition _glWait; + private: // Event-driven methods void init(); @@ -211,22 +218,31 @@ void OffscreenQmlRenderThread::setupFbo() { } } +QJsonObject OffscreenQmlRenderThread::getGLContextData() { + _glMutex.lock(); + if (_glData.isEmpty()) { + _glWait.wait(&_glMutex); + } + _glMutex.unlock(); + return _glData; +} + void OffscreenQmlRenderThread::init() { qDebug() << "Initializing QML Renderer"; - connect(_renderControl, &QQuickRenderControl::renderRequested, _surface, &OffscreenQmlSurface::requestRender); - connect(_renderControl, &QQuickRenderControl::sceneChanged, _surface, &OffscreenQmlSurface::requestUpdate); - if (!_canvas.makeCurrent()) { qWarning("Failed to make context current on QML Renderer Thread"); _quit = true; return; } - // Expose GL data to QML - auto glData = getGLContextData(); - auto setGL = [=]{ _surface->getRootContext()->setContextProperty("GL", glData); }; - _surface->executeOnUiThread(setGL); + _glMutex.lock(); + _glData = ::getGLContextData(); + _glMutex.unlock(); + _glWait.wakeAll(); + + connect(_renderControl, &QQuickRenderControl::renderRequested, _surface, &OffscreenQmlSurface::requestRender); + connect(_renderControl, &QQuickRenderControl::sceneChanged, _surface, &OffscreenQmlSurface::requestUpdate); _renderControl->initialize(_canvas.getContext()); setupFbo(); @@ -386,14 +402,16 @@ void OffscreenQmlSurface::create(QOpenGLContext* shareContext) { _qmlEngine->setIncubationController(_renderer->_quickWindow->incubationController()); } + _qmlEngine->rootContext()->setContextProperty("GL", _renderer->getGLContextData()); + _qmlEngine->rootContext()->setContextProperty("offscreenWindow", QVariant::fromValue(getWindow())); + _qmlComponent = new QQmlComponent(_qmlEngine); + // When Quick says there is a need to render, we will not render immediately. Instead, // a timer with a small interval is used to get better performance. - _updateTimer.setInterval(MIN_TIMER_MS); QObject::connect(&_updateTimer, &QTimer::timeout, this, &OffscreenQmlSurface::updateQuick); QObject::connect(qApp, &QCoreApplication::aboutToQuit, this, &OffscreenQmlSurface::onAboutToQuit); + _updateTimer.setInterval(MIN_TIMER_MS); _updateTimer.start(); - _qmlComponent = new QQmlComponent(_qmlEngine); - _qmlEngine->rootContext()->setContextProperty("offscreenWindow", QVariant::fromValue(getWindow())); } void OffscreenQmlSurface::resize(const QSize& newSize_) { From c3a86f46d07f1bee738f19b8f8cbc5b1e2d362de Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 14:55:23 -0700 Subject: [PATCH 22/28] change the timeout to an interval so it constantly runs --- .../exampleSelfCallingTimeoutNoCleanup.js | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js diff --git a/examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js b/examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js new file mode 100644 index 0000000000..a4a66fc785 --- /dev/null +++ b/examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js @@ -0,0 +1,56 @@ +// +// exampleSelfCallingTimeoutNoCleanup.js +// examples/entityScripts +// +// Created by Brad Hefta-Gaub on 4/18/16. +// Copyright 2016 High Fidelity, Inc. +// +// This is an example of an entity script which hooks the update signal +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +(function() { + var _this; + + // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember + // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals) + ExampleUpdate = function() { + _this = this; + }; + + ExampleUpdate.prototype = { + + timeOutFunction: function() { + var entityID = _this.entityID; + print("timeOutFunction in entityID:" + entityID); + Script.setTimeout(function() { + _this.timeOutFunction(); + }, 3000); + }, + + // preload() will be called when the entity has become visible (or known) to the interface + // it gives us a chance to set our local JavaScript object up. In this case it means: + // * remembering our entityID, so we can access it in cases where we're called without an entityID + // * connecting to the update signal so we can check our grabbed state + preload: function(entityID) { + print("preload - entityID:" + entityID); + this.entityID = entityID; + + print("preload - entityID:" + entityID + "-- calling timeOutFunction()...."); + _this.timeOutFunction(); + }, + + // unload() will be called when our entity is no longer available. It may be because we were deleted, + // or because we've left the domain or quit the application. In all cases we want to unhook our connection + // to the update signal + unload: function(entityID) { + print("unload - entityID:" + entityID); + print("NOTE --- WE DID NOT CALL clear our timeout"); + }, + }; + + // entity scripts always need to return a newly constructed object of our type + return new ExampleUpdate(); +}) From d5891e9073ec21abeae0448de448e312a2dc5f86 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 18 Apr 2016 15:28:33 -0700 Subject: [PATCH 23/28] remove abortEvaluation() since it has no effect for the test cases and might cause unexpected side effects --- libraries/script-engine/src/ScriptEngine.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index cbbc8832b6..15f3ebb985 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -758,7 +758,6 @@ void ScriptEngine::stop() { QMetaObject::invokeMethod(this, "stop"); return; } - abortEvaluation(); // abort any script evaluation that may be active _isFinished = true; if (_wantSignals) { emit runningStateChanged(); From d25a36696267589ea8770933e139088a344c5ace Mon Sep 17 00:00:00 2001 From: samcake Date: Mon, 18 Apr 2016 17:23:57 -0700 Subject: [PATCH 24/28] Resoving to master... --- libraries/gl/src/gl/Config.h | 2 +- libraries/gpu/src/gpu/Config.slh | 2 +- libraries/gpu/src/gpu/GLBackend.h | 3 +-- libraries/gpu/src/gpu/Transform.slh | 6 +++--- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/libraries/gl/src/gl/Config.h b/libraries/gl/src/gl/Config.h index a7d2c27db2..8f7582c271 100644 --- a/libraries/gl/src/gl/Config.h +++ b/libraries/gl/src/gl/Config.h @@ -51,7 +51,7 @@ #if (GPU_INPUT_PROFILE == GPU_CORE_43) // Deactivate SSBO for now, we've run into some issues // on GL 4.3 capable GPUs not behaving as expected -#define GPU_SSBO_DRAW_CALL_INFO +//#define GPU_SSBO_DRAW_CALL_INFO #endif diff --git a/libraries/gpu/src/gpu/Config.slh b/libraries/gpu/src/gpu/Config.slh index 5a96a7eafe..3da92df7c1 100644 --- a/libraries/gpu/src/gpu/Config.slh +++ b/libraries/gpu/src/gpu/Config.slh @@ -13,7 +13,7 @@ <@if GLPROFILE == PC_GL @> <@def GPU_FEATURE_PROFILE GPU_CORE@> - <@def VERSION_HEADER #version 430 core@> + <@def VERSION_HEADER #version 410 core@> <@elif GLPROFILE == MAC_GL @> <@def GPU_FEATURE_PROFILE GPU_CORE@> <@def VERSION_HEADER #version 410 core@> diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index 0df49c2a6a..a9d80e0be8 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -386,8 +386,7 @@ protected: mutable std::map _drawCallInfoOffsets; - mutable int batchNum{ 0 }; - GLuint _objectBuffer[2]; + GLuint _objectBuffer{ 0 }; GLuint _cameraBuffer { 0 }; GLuint _drawCallInfoBuffer { 0 }; GLuint _objectBufferTexture { 0 }; diff --git a/libraries/gpu/src/gpu/Transform.slh b/libraries/gpu/src/gpu/Transform.slh index d2d8e7af34..52ddf92158 100644 --- a/libraries/gpu/src/gpu/Transform.slh +++ b/libraries/gpu/src/gpu/Transform.slh @@ -37,7 +37,7 @@ struct TransformObject { layout(location=15) in ivec2 _drawCallInfo; -!> +<@if FALSE @> // Disable SSBOs for now layout(std140) buffer transformObjectBuffer { TransformObject _object[]; @@ -45,7 +45,7 @@ layout(std140) buffer transformObjectBuffer { TransformObject getTransformObject() { return _object[_drawCallInfo.x]; } - +<@else@> uniform samplerBuffer transformObjectBuffer; TransformObject getTransformObject() { @@ -63,7 +63,7 @@ TransformObject getTransformObject() { return object; } -<@endif@> !> +<@endif@> <@endfunc@> From c6deff16d6499fb229d2be2d40a4dc32c78cfbd8 Mon Sep 17 00:00:00 2001 From: samcake Date: Mon, 18 Apr 2016 17:24:55 -0700 Subject: [PATCH 25/28] Resoving to master... --- libraries/gpu/src/gpu/GLBackend.h | 2 +- libraries/gpu/src/gpu/GLBackendTransform.cpp | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index a9d80e0be8..9806c17db4 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -386,7 +386,7 @@ protected: mutable std::map _drawCallInfoOffsets; - GLuint _objectBuffer{ 0 }; + GLuint _objectBuffer { 0 }; GLuint _cameraBuffer { 0 }; GLuint _drawCallInfoBuffer { 0 }; GLuint _objectBufferTexture { 0 }; diff --git a/libraries/gpu/src/gpu/GLBackendTransform.cpp b/libraries/gpu/src/gpu/GLBackendTransform.cpp index 8d72f93d07..19cb7fb2a9 100755 --- a/libraries/gpu/src/gpu/GLBackendTransform.cpp +++ b/libraries/gpu/src/gpu/GLBackendTransform.cpp @@ -59,7 +59,7 @@ void GLBackend::do_setDepthRangeTransform(Batch& batch, size_t paramOffset) { } void GLBackend::initTransform() { - glGenBuffers(2, _transform._objectBuffer); + glGenBuffers(1, &_transform._objectBuffer); glGenBuffers(1, &_transform._cameraBuffer); glGenBuffers(1, &_transform._drawCallInfoBuffer); #ifndef GPU_SSBO_DRAW_CALL_INFO @@ -72,8 +72,7 @@ void GLBackend::initTransform() { } void GLBackend::killTransform() { - glDeleteBuffers(2, _transform._objectBuffer); - // glDeleteBuffers(1, &_transform._objectBuffer); + glDeleteBuffers(1, &_transform._objectBuffer); glDeleteBuffers(1, &_transform._cameraBuffer); glDeleteBuffers(1, &_transform._drawCallInfoBuffer); #ifndef GPU_SSBO_DRAW_CALL_INFO @@ -146,9 +145,7 @@ void GLBackend::TransformStageState::transfer(const Batch& batch) const { memcpy(bufferData.data(), batch._objects.data(), byteSize); #ifdef GPU_SSBO_DRAW_CALL_INFO - batchNum++; - // glBindBuffer(GL_SHADER_STORAGE_BUFFER, _objectBuffer); - glBindBuffer(GL_SHADER_STORAGE_BUFFER, _objectBuffer[batchNum % 2]); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, _objectBuffer); glBufferData(GL_SHADER_STORAGE_BUFFER, bufferData.size(), bufferData.data(), GL_DYNAMIC_DRAW); glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0); #else @@ -174,7 +171,7 @@ void GLBackend::TransformStageState::transfer(const Batch& batch) const { } #ifdef GPU_SSBO_DRAW_CALL_INFO - glBindBufferBase(GL_SHADER_STORAGE_BUFFER, TRANSFORM_OBJECT_SLOT, _objectBuffer[batchNum % 2]); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, TRANSFORM_OBJECT_SLOT, _objectBuffer); #else glActiveTexture(GL_TEXTURE0 + TRANSFORM_OBJECT_SLOT); glBindTexture(GL_TEXTURE_BUFFER, _objectBufferTexture); From cadb53b70356b1d3ddf6d95aeb964ea94a2887c2 Mon Sep 17 00:00:00 2001 From: samcake Date: Mon, 18 Apr 2016 18:24:27 -0700 Subject: [PATCH 26/28] Adding shader versioning and defines from GLBackendShader allowing for runtime defines injection to shaders and more --- .../src/RenderableProceduralItemShader.h | 2 +- libraries/gpu/src/gpu/Config.slh | 6 +- libraries/gpu/src/gpu/GLBackend.cpp | 46 +- libraries/gpu/src/gpu/GLBackend.h | 35 +- libraries/gpu/src/gpu/GLBackendPipeline.cpp | 8 +- libraries/gpu/src/gpu/GLBackendShader.cpp | 507 ++++++++++-------- .../render-utils/src/ToneMappingEffect.cpp | 2 +- 7 files changed, 334 insertions(+), 272 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableProceduralItemShader.h b/libraries/entities-renderer/src/RenderableProceduralItemShader.h index a01a6b8571..1afd3bc608 100644 --- a/libraries/entities-renderer/src/RenderableProceduralItemShader.h +++ b/libraries/entities-renderer/src/RenderableProceduralItemShader.h @@ -18,7 +18,7 @@ // -const QString SHADER_COMMON = R"SHADER(#version 410 core +const QString SHADER_COMMON = R"SHADER( layout(location = 0) out vec4 _fragColor0; layout(location = 1) out vec4 _fragColor1; layout(location = 2) out vec4 _fragColor2; diff --git a/libraries/gpu/src/gpu/Config.slh b/libraries/gpu/src/gpu/Config.slh index 3da92df7c1..ae556a3828 100644 --- a/libraries/gpu/src/gpu/Config.slh +++ b/libraries/gpu/src/gpu/Config.slh @@ -13,11 +13,11 @@ <@if GLPROFILE == PC_GL @> <@def GPU_FEATURE_PROFILE GPU_CORE@> - <@def VERSION_HEADER #version 410 core@> + <@def VERSION_HEADER //PC 410 core@> <@elif GLPROFILE == MAC_GL @> <@def GPU_FEATURE_PROFILE GPU_CORE@> - <@def VERSION_HEADER #version 410 core@> + <@def VERSION_HEADER //MAC 410 core@> <@else@> <@def GPU_FEATURE_PROFILE GPU_CORE@> - <@def VERSION_HEADER #version 410 core@> + <@def VERSION_HEADER //410 core@> <@endif@> diff --git a/libraries/gpu/src/gpu/GLBackend.cpp b/libraries/gpu/src/gpu/GLBackend.cpp index f51448f8fd..c27da793d2 100644 --- a/libraries/gpu/src/gpu/GLBackend.cpp +++ b/libraries/gpu/src/gpu/GLBackend.cpp @@ -461,8 +461,7 @@ void GLBackend::resetStages() { #define ADD_COMMAND_GL(call) _commands.push_back(COMMAND_##call); _commandOffsets.push_back(_params.size()); -//#define DO_IT_NOW(call, offset) runLastCommand(); -#define DO_IT_NOW(call, offset) +#define GET_UNIFORM_LOCATION(shaderUniformLoc) shaderUniformLoc void Batch::_glActiveBindTexture(GLenum unit, GLenum target, GLuint texture) { // clean the cache on the texture unit we are going to use so the next call to setResourceTexture() at the same slot works fine @@ -472,14 +471,11 @@ void Batch::_glActiveBindTexture(GLenum unit, GLenum target, GLuint texture) { _params.push_back(texture); _params.push_back(target); _params.push_back(unit); - - - DO_IT_NOW(_glActiveBindTexture, 3); } void GLBackend::do_glActiveBindTexture(Batch& batch, size_t paramOffset) { glActiveTexture(batch._params[paramOffset + 2]._uint); glBindTexture( - batch._params[paramOffset + 1]._uint, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 1]._uint), batch._params[paramOffset + 0]._uint); (void) CHECK_GL_ERROR(); @@ -492,8 +488,6 @@ void Batch::_glUniform1i(GLint location, GLint v0) { ADD_COMMAND_GL(glUniform1i); _params.push_back(v0); _params.push_back(location); - - DO_IT_NOW(_glUniform1i, 1); } void GLBackend::do_glUniform1i(Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -503,7 +497,7 @@ void GLBackend::do_glUniform1i(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniform1f( - batch._params[paramOffset + 1]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 1]._int), batch._params[paramOffset + 0]._int); (void) CHECK_GL_ERROR(); } @@ -515,8 +509,6 @@ void Batch::_glUniform1f(GLint location, GLfloat v0) { ADD_COMMAND_GL(glUniform1f); _params.push_back(v0); _params.push_back(location); - - DO_IT_NOW(_glUniform1f, 1); } void GLBackend::do_glUniform1f(Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -527,7 +519,7 @@ void GLBackend::do_glUniform1f(Batch& batch, size_t paramOffset) { updatePipeline(); glUniform1f( - batch._params[paramOffset + 1]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 1]._int), batch._params[paramOffset + 0]._float); (void) CHECK_GL_ERROR(); } @@ -538,8 +530,6 @@ void Batch::_glUniform2f(GLint location, GLfloat v0, GLfloat v1) { _params.push_back(v1); _params.push_back(v0); _params.push_back(location); - - DO_IT_NOW(_glUniform2f, 1); } void GLBackend::do_glUniform2f(Batch& batch, size_t paramOffset) { @@ -550,7 +540,7 @@ void GLBackend::do_glUniform2f(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniform2f( - batch._params[paramOffset + 2]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 2]._int), batch._params[paramOffset + 1]._float, batch._params[paramOffset + 0]._float); (void) CHECK_GL_ERROR(); @@ -563,8 +553,6 @@ void Batch::_glUniform3f(GLint location, GLfloat v0, GLfloat v1, GLfloat v2) { _params.push_back(v1); _params.push_back(v0); _params.push_back(location); - - DO_IT_NOW(_glUniform3f, 1); } void GLBackend::do_glUniform3f(Batch& batch, size_t paramOffset) { @@ -575,7 +563,7 @@ void GLBackend::do_glUniform3f(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniform3f( - batch._params[paramOffset + 3]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 3]._int), batch._params[paramOffset + 2]._float, batch._params[paramOffset + 1]._float, batch._params[paramOffset + 0]._float); @@ -591,8 +579,6 @@ void Batch::_glUniform4f(GLint location, GLfloat v0, GLfloat v1, GLfloat v2, GLf _params.push_back(v1); _params.push_back(v0); _params.push_back(location); - - DO_IT_NOW(_glUniform4f, 1); } @@ -604,7 +590,7 @@ void GLBackend::do_glUniform4f(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniform4f( - batch._params[paramOffset + 4]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 4]._int), batch._params[paramOffset + 3]._float, batch._params[paramOffset + 2]._float, batch._params[paramOffset + 1]._float, @@ -619,8 +605,6 @@ void Batch::_glUniform3fv(GLint location, GLsizei count, const GLfloat* value) { _params.push_back(cacheData(count * VEC3_SIZE, value)); _params.push_back(count); _params.push_back(location); - - DO_IT_NOW(_glUniform3fv, 3); } void GLBackend::do_glUniform3fv(Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -630,7 +614,7 @@ void GLBackend::do_glUniform3fv(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniform3fv( - batch._params[paramOffset + 2]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 2]._int), batch._params[paramOffset + 1]._uint, (const GLfloat*)batch.editData(batch._params[paramOffset + 0]._uint)); @@ -645,8 +629,6 @@ void Batch::_glUniform4fv(GLint location, GLsizei count, const GLfloat* value) { _params.push_back(cacheData(count * VEC4_SIZE, value)); _params.push_back(count); _params.push_back(location); - - DO_IT_NOW(_glUniform4fv, 3); } void GLBackend::do_glUniform4fv(Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -656,7 +638,7 @@ void GLBackend::do_glUniform4fv(Batch& batch, size_t paramOffset) { } updatePipeline(); - GLint location = batch._params[paramOffset + 2]._int; + GLint location = GET_UNIFORM_LOCATION(batch._params[paramOffset + 2]._int); GLsizei count = batch._params[paramOffset + 1]._uint; const GLfloat* value = (const GLfloat*)batch.editData(batch._params[paramOffset + 0]._uint); glUniform4fv(location, count, value); @@ -671,8 +653,6 @@ void Batch::_glUniform4iv(GLint location, GLsizei count, const GLint* value) { _params.push_back(cacheData(count * VEC4_SIZE, value)); _params.push_back(count); _params.push_back(location); - - DO_IT_NOW(_glUniform4iv, 3); } void GLBackend::do_glUniform4iv(Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -682,7 +662,7 @@ void GLBackend::do_glUniform4iv(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniform4iv( - batch._params[paramOffset + 2]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 2]._int), batch._params[paramOffset + 1]._uint, (const GLint*)batch.editData(batch._params[paramOffset + 0]._uint)); @@ -697,8 +677,6 @@ void Batch::_glUniformMatrix4fv(GLint location, GLsizei count, GLboolean transpo _params.push_back(transpose); _params.push_back(count); _params.push_back(location); - - DO_IT_NOW(_glUniformMatrix4fv, 4); } void GLBackend::do_glUniformMatrix4fv(Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -708,7 +686,7 @@ void GLBackend::do_glUniformMatrix4fv(Batch& batch, size_t paramOffset) { } updatePipeline(); glUniformMatrix4fv( - batch._params[paramOffset + 3]._int, + GET_UNIFORM_LOCATION(batch._params[paramOffset + 3]._int), batch._params[paramOffset + 2]._uint, batch._params[paramOffset + 1]._uint, (const GLfloat*)batch.editData(batch._params[paramOffset + 0]._uint)); @@ -722,8 +700,6 @@ void Batch::_glColor4f(GLfloat red, GLfloat green, GLfloat blue, GLfloat alpha) _params.push_back(blue); _params.push_back(green); _params.push_back(red); - - DO_IT_NOW(_glColor4f, 4); } void GLBackend::do_glColor4f(Batch& batch, size_t paramOffset) { diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index 9806c17db4..7b9d69fbbd 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -153,17 +153,40 @@ public: class GLShader : public GPUObject { public: - GLuint _shader; - GLuint _program; + enum Version { + Mono = 0, + + NumVersions + }; + + struct ShaderObject { + GLuint glshader{ 0 }; + GLuint glprogram{ 0 }; + GLint transformCameraSlot{ -1 }; + GLint transformObjectSlot{ -1 }; + }; + + using ShaderObjects = std::array< ShaderObject, NumVersions >; + using UniformMapping = std::vector; + using UniformMappingVersions = std::vector; - GLint _transformCameraSlot = -1; - GLint _transformObjectSlot = -1; GLShader(); ~GLShader(); + + ShaderObjects _shaderObjects; + UniformMappingVersions _uniformMappings; + + GLuint getProgram() const { + return _shaderObjects[Mono].glprogram; + } + + GLint getUniformLocation(GLint srcLoc) { + return _uniformMappings[Mono][srcLoc]; + } + }; static GLShader* syncGPUObject(const Shader& shader); - static GLuint getShaderID(const ShaderPointer& shader); class GLState : public GPUObject { public: @@ -464,6 +487,7 @@ protected: PipelinePointer _pipeline; GLuint _program; + GLShader* _programShader; bool _invalidProgram; State::Data _stateCache; @@ -475,6 +499,7 @@ protected: PipelineStageState() : _pipeline(), _program(0), + _programShader(nullptr), _invalidProgram(false), _stateCache(State::DEFAULT), _stateSignatureCache(0), diff --git a/libraries/gpu/src/gpu/GLBackendPipeline.cpp b/libraries/gpu/src/gpu/GLBackendPipeline.cpp index 60ec478a8e..d36d37d597 100755 --- a/libraries/gpu/src/gpu/GLBackendPipeline.cpp +++ b/libraries/gpu/src/gpu/GLBackendPipeline.cpp @@ -69,6 +69,7 @@ void GLBackend::do_setPipeline(Batch& batch, size_t paramOffset) { _pipeline._pipeline.reset(); _pipeline._program = 0; + _pipeline._programShader = nullptr; _pipeline._invalidProgram = true; _pipeline._state = nullptr; @@ -80,8 +81,10 @@ void GLBackend::do_setPipeline(Batch& batch, size_t paramOffset) { } // check the program cache - if (_pipeline._program != pipelineObject->_program->_program) { - _pipeline._program = pipelineObject->_program->_program; + GLuint glprogram = pipelineObject->_program->getProgram(); + if (_pipeline._program != glprogram) { + _pipeline._program = glprogram; + _pipeline._programShader = pipelineObject->_program; _pipeline._invalidProgram = true; } @@ -142,6 +145,7 @@ void GLBackend::resetPipelineStage() { // Second the shader side _pipeline._invalidProgram = false; _pipeline._program = 0; + _pipeline._programShader = nullptr; _pipeline._pipeline.reset(); glUseProgram(0); } diff --git a/libraries/gpu/src/gpu/GLBackendShader.cpp b/libraries/gpu/src/gpu/GLBackendShader.cpp index c27c5dd97d..86149c3f07 100755 --- a/libraries/gpu/src/gpu/GLBackendShader.cpp +++ b/libraries/gpu/src/gpu/GLBackendShader.cpp @@ -13,27 +13,205 @@ using namespace gpu; -GLBackend::GLShader::GLShader() : - _shader(0), - _program(0) -{} +GLBackend::GLShader::GLShader() +{ +} GLBackend::GLShader::~GLShader() { - if (_shader != 0) { - glDeleteShader(_shader); - } - if (_program != 0) { - glDeleteProgram(_program); + for (auto& so : _shaderObjects) { + if (so.glshader != 0) { + glDeleteShader(so.glshader); + } + if (so.glprogram != 0) { + glDeleteProgram(so.glprogram); + } } } -void makeBindings(GLBackend::GLShader* shader) { - if(!shader || !shader->_program) { +bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, GLuint &programObject) { + if (shaderSource.empty()) { + qCDebug(gpulogging) << "GLShader::compileShader - no GLSL shader source code ? so failed to create"; + return false; + } + + // Create the shader object + GLuint glshader = glCreateShader(shaderDomain); + if (!glshader) { + qCDebug(gpulogging) << "GLShader::compileShader - failed to create the gl shader object"; + return nullptr; + } + + // Assign the source + const int NUM_SOURCE_STRINGS = 2; + const GLchar* srcstr[] = { defines.c_str(), shaderSource.c_str() }; + glShaderSource(glshader, NUM_SOURCE_STRINGS, srcstr, NULL); + + // Compile ! + glCompileShader(glshader); + + // check if shader compiled + GLint compiled = 0; + glGetShaderiv(glshader, GL_COMPILE_STATUS, &compiled); + + // if compilation fails + if (!compiled) { + // save the source code to a temp file so we can debug easily + /* std::ofstream filestream; + filestream.open("debugshader.glsl"); + if (filestream.is_open()) { + filestream << shaderSource->source; + filestream.close(); + } + */ + + GLint infoLength = 0; + glGetShaderiv(glshader, GL_INFO_LOG_LENGTH, &infoLength); + + char* temp = new char[infoLength]; + glGetShaderInfoLog(glshader, infoLength, NULL, temp); + + + /* + filestream.open("debugshader.glsl.info.txt"); + if (filestream.is_open()) { + filestream << std::string(temp); + filestream.close(); + } + */ + + qCWarning(gpulogging) << "GLShader::compileShader - failed to compile the gl shader object:"; + for (auto s : srcstr) { + qCWarning(gpulogging) << s; + } + qCWarning(gpulogging) << "GLShader::compileShader - errors:"; + qCWarning(gpulogging) << temp; + delete[] temp; + + glDeleteShader(glshader); + return false; + } + + GLuint glprogram = 0; +#ifdef SEPARATE_PROGRAM + // so far so good, program is almost done, need to link: + GLuint glprogram = glCreateProgram(); + if (!glprogram) { + qCDebug(gpulogging) << "GLShader::compileShader - failed to create the gl shader & gl program object"; + return false; + } + + glProgramParameteri(glprogram, GL_PROGRAM_SEPARABLE, GL_TRUE); + glAttachShader(glprogram, glshader); + glLinkProgram(glprogram); + + GLint linked = 0; + glGetProgramiv(glprogram, GL_LINK_STATUS, &linked); + + if (!linked) { + /* + // save the source code to a temp file so we can debug easily + std::ofstream filestream; + filestream.open("debugshader.glsl"); + if (filestream.is_open()) { + filestream << shaderSource->source; + filestream.close(); + } + */ + + GLint infoLength = 0; + glGetProgramiv(glprogram, GL_INFO_LOG_LENGTH, &infoLength); + + char* temp = new char[infoLength]; + glGetProgramInfoLog(glprogram, infoLength, NULL, temp); + + qCDebug(gpulogging) << "GLShader::compileShader - failed to LINK the gl program object :"; + qCDebug(gpulogging) << temp; + + /* + filestream.open("debugshader.glsl.info.txt"); + if (filestream.is_open()) { + filestream << String(temp); + filestream.close(); + } + */ + delete[] temp; + + glDeleteShader(glshader); + glDeleteProgram(glprogram); + return false; + } +#endif + + shaderObject = glshader; + programObject = glprogram; + + return true; +} + +GLuint compileProgram(const std::vector& glshaders) { + // A brand new program: + GLuint glprogram = glCreateProgram(); + if (!glprogram) { + qCDebug(gpulogging) << "GLShader::compileProgram - failed to create the gl program object"; + return 0; + } + + // glProgramParameteri(glprogram, GL_PROGRAM_, GL_TRUE); + // Create the program from the sub shaders + for (auto so : glshaders) { + glAttachShader(glprogram, so); + } + + // Link! + glLinkProgram(glprogram); + + GLint linked = 0; + glGetProgramiv(glprogram, GL_LINK_STATUS, &linked); + + if (!linked) { + /* + // save the source code to a temp file so we can debug easily + std::ofstream filestream; + filestream.open("debugshader.glsl"); + if (filestream.is_open()) { + filestream << shaderSource->source; + filestream.close(); + } + */ + + GLint infoLength = 0; + glGetProgramiv(glprogram, GL_INFO_LOG_LENGTH, &infoLength); + + char* temp = new char[infoLength]; + glGetProgramInfoLog(glprogram, infoLength, NULL, temp); + + qCDebug(gpulogging) << "GLShader::compileProgram - failed to LINK the gl program object :"; + qCDebug(gpulogging) << temp; + + /* + filestream.open("debugshader.glsl.info.txt"); + if (filestream.is_open()) { + filestream << std::string(temp); + filestream.close(); + } + */ + delete[] temp; + + glDeleteProgram(glprogram); + return 0; + } + + return glprogram; +} + + +void makeProgramBindings(GLBackend::GLShader::ShaderObject& shaderObject) { + if (!shaderObject.glprogram) { return; } - GLuint glprogram = shader->_program; + GLuint glprogram = shaderObject.glprogram; GLint loc = -1; - + //Check for gpu specific attribute slotBindings loc = glGetAttribLocation(glprogram, "inPosition"); if (loc >= 0 && loc != gpu::Stream::POSITION) { @@ -96,226 +274,111 @@ void makeBindings(GLBackend::GLShader* shader) { loc = glGetProgramResourceIndex(glprogram, GL_SHADER_STORAGE_BLOCK, "transformObjectBuffer"); if (loc >= 0) { glShaderStorageBlockBinding(glprogram, loc, gpu::TRANSFORM_OBJECT_SLOT); - shader->_transformObjectSlot = gpu::TRANSFORM_OBJECT_SLOT; + shaderObject.transformObjectSlot = gpu::TRANSFORM_OBJECT_SLOT; } #else loc = glGetUniformLocation(glprogram, "transformObjectBuffer"); if (loc >= 0) { glProgramUniform1i(glprogram, loc, gpu::TRANSFORM_OBJECT_SLOT); - shader->_transformObjectSlot = gpu::TRANSFORM_OBJECT_SLOT; + shaderObject.transformObjectSlot = gpu::TRANSFORM_OBJECT_SLOT; } #endif loc = glGetUniformBlockIndex(glprogram, "transformCameraBuffer"); if (loc >= 0) { glUniformBlockBinding(glprogram, loc, gpu::TRANSFORM_CAMERA_SLOT); - shader->_transformCameraSlot = gpu::TRANSFORM_CAMERA_SLOT; + shaderObject.transformCameraSlot = gpu::TRANSFORM_CAMERA_SLOT; } (void)CHECK_GL_ERROR(); } -GLBackend::GLShader* compileShader(const Shader& shader) { +GLBackend::GLShader* compileBackendShader(const Shader& shader) { // Any GLSLprogram ? normally yes... const std::string& shaderSource = shader.getSource().getCode(); - if (shaderSource.empty()) { - qCDebug(gpulogging) << "GLShader::compileShader - no GLSL shader source code ? so failed to create"; - return nullptr; - } + + // GLSL version + const std::string glslVersion = { + "#version 410 core" + }; // Shader domain - const GLenum SHADER_DOMAINS[2] = { GL_VERTEX_SHADER, GL_FRAGMENT_SHADER }; + const int NUM_SHADER_DOMAINS = 2; + const GLenum SHADER_DOMAINS[NUM_SHADER_DOMAINS] = { + GL_VERTEX_SHADER, + GL_FRAGMENT_SHADER + }; GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; - // Create the shader object - GLuint glshader = glCreateShader(shaderDomain); - if (!glshader) { - qCDebug(gpulogging) << "GLShader::compileShader - failed to create the gl shader object"; - return nullptr; - } + // Domain specific defines + const std::string domainDefines[NUM_SHADER_DOMAINS] = { + "#define VERTEX_SHADER", + "#define PIXEL_SHADER" + }; - // Assign the source - const GLchar* srcstr = shaderSource.c_str(); - glShaderSource(glshader, 1, &srcstr, NULL); - // Compile ! - glCompileShader(glshader); - // check if shader compiled - GLint compiled = 0; - glGetShaderiv(glshader, GL_COMPILE_STATUS, &compiled); + // Versions specific of the shader + const std::string versionDefines[GLBackend::GLShader::NumVersions] = { + "" + }; - // if compilation fails - if (!compiled) { - // save the source code to a temp file so we can debug easily - /* std::ofstream filestream; - filestream.open("debugshader.glsl"); - if (filestream.is_open()) { - filestream << shaderSource->source; - filestream.close(); + GLBackend::GLShader::ShaderObjects shaderObjects; + + for (int version = 0; version < GLBackend::GLShader::NumVersions; version++) { + auto& shaderObject = shaderObjects[version]; + + std::string shaderDefines = glslVersion + "\n" + domainDefines[shader.getType()] + "\n" + versionDefines[version]; + + bool result = compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, shaderObject.glprogram); + if (!result) { + return nullptr; } - */ - - GLint infoLength = 0; - glGetShaderiv(glshader, GL_INFO_LOG_LENGTH, &infoLength); - - char* temp = new char[infoLength] ; - glGetShaderInfoLog(glshader, infoLength, NULL, temp); - - - /* - filestream.open("debugshader.glsl.info.txt"); - if (filestream.is_open()) { - filestream << std::string(temp); - filestream.close(); - } - */ - - qCWarning(gpulogging) << "GLShader::compileShader - failed to compile the gl shader object:"; - qCWarning(gpulogging) << srcstr; - qCWarning(gpulogging) << "GLShader::compileShader - errors:"; - qCWarning(gpulogging) << temp; - delete[] temp; - - glDeleteShader(glshader); - return nullptr; } - GLuint glprogram = 0; -#ifdef SEPARATE_PROGRAM - // so far so good, program is almost done, need to link: - GLuint glprogram = glCreateProgram(); - if (!glprogram) { - qCDebug(gpulogging) << "GLShader::compileShader - failed to create the gl shader & gl program object"; - return nullptr; - } - - glProgramParameteri(glprogram, GL_PROGRAM_SEPARABLE, GL_TRUE); - glAttachShader(glprogram, glshader); - glLinkProgram(glprogram); - - GLint linked = 0; - glGetProgramiv(glprogram, GL_LINK_STATUS, &linked); - - if (!linked) { - /* - // save the source code to a temp file so we can debug easily - std::ofstream filestream; - filestream.open("debugshader.glsl"); - if (filestream.is_open()) { - filestream << shaderSource->source; - filestream.close(); - } - */ - - GLint infoLength = 0; - glGetProgramiv(glprogram, GL_INFO_LOG_LENGTH, &infoLength); - - char* temp = new char[infoLength] ; - glGetProgramInfoLog(glprogram, infoLength, NULL, temp); - - qCDebug(gpulogging) << "GLShader::compileShader - failed to LINK the gl program object :"; - qCDebug(gpulogging) << temp; - - /* - filestream.open("debugshader.glsl.info.txt"); - if (filestream.is_open()) { - filestream << String(temp); - filestream.close(); - } - */ - delete[] temp; - - glDeleteShader(glshader); - glDeleteProgram(glprogram); - return nullptr; - } -#endif - // So far so good, the shader is created successfully GLBackend::GLShader* object = new GLBackend::GLShader(); - object->_shader = glshader; - object->_program = glprogram; - - makeBindings(object); + object->_shaderObjects = shaderObjects; return object; } -GLBackend::GLShader* compileProgram(const Shader& program) { - if(!program.isProgram()) { +GLBackend::GLShader* compileBackendProgram(const Shader& program) { + if (!program.isProgram()) { return nullptr; } - // Let's go through every shaders and make sure they are ready to go - std::vector< GLuint > shaderObjects; - for (auto subShader : program.getShaders()) { - GLuint so = GLBackend::getShaderID(subShader); - if (!so) { - qCDebug(gpulogging) << "GLShader::compileProgram - One of the shaders of the program is not compiled?"; + GLBackend::GLShader::ShaderObjects programObjects; + + for (int version = 0; version < GLBackend::GLShader::NumVersions; version++) { + auto& programObject = programObjects[version]; + + // Let's go through every shaders and make sure they are ready to go + std::vector< GLuint > shaderGLObjects; + for (auto subShader : program.getShaders()) { + auto object = GLBackend::syncGPUObject(*subShader); + if (object) { + shaderGLObjects.push_back(object->_shaderObjects[version].glshader); + } else { + qCDebug(gpulogging) << "GLShader::compileBackendProgram - One of the shaders of the program is not compiled?"; + return nullptr; + } + } + + GLuint glprogram = compileProgram(shaderGLObjects); + if (glprogram == 0) { return nullptr; } - shaderObjects.push_back(so); + + programObject.glprogram = glprogram; + + makeProgramBindings(programObject); } - // so far so good, program is almost done, need to link: - GLuint glprogram = glCreateProgram(); - if (!glprogram) { - qCDebug(gpulogging) << "GLShader::compileProgram - failed to create the gl program object"; - return nullptr; - } - // glProgramParameteri(glprogram, GL_PROGRAM_, GL_TRUE); - // Create the program from the sub shaders - for (auto so : shaderObjects) { - glAttachShader(glprogram, so); - } - - // Link! - glLinkProgram(glprogram); - - GLint linked = 0; - glGetProgramiv(glprogram, GL_LINK_STATUS, &linked); - - if (!linked) { - /* - // save the source code to a temp file so we can debug easily - std::ofstream filestream; - filestream.open("debugshader.glsl"); - if (filestream.is_open()) { - filestream << shaderSource->source; - filestream.close(); - } - */ - - GLint infoLength = 0; - glGetProgramiv(glprogram, GL_INFO_LOG_LENGTH, &infoLength); - - char* temp = new char[infoLength] ; - glGetProgramInfoLog(glprogram, infoLength, NULL, temp); - - qCDebug(gpulogging) << "GLShader::compileProgram - failed to LINK the gl program object :"; - qCDebug(gpulogging) << temp; - - /* - filestream.open("debugshader.glsl.info.txt"); - if (filestream.is_open()) { - filestream << std::string(temp); - filestream.close(); - } - */ - delete[] temp; - - glDeleteProgram(glprogram); - return nullptr; - } - - // So far so good, the program is created successfully + // So far so good, the program versions have all been created successfully GLBackend::GLShader* object = new GLBackend::GLShader(); - object->_shader = 0; - object->_program = glprogram; - - makeBindings(object); + object->_shaderObjects = programObjects; return object; } @@ -329,14 +392,14 @@ GLBackend::GLShader* GLBackend::syncGPUObject(const Shader& shader) { } // need to have a gpu object? if (shader.isProgram()) { - GLShader* tempObject = compileProgram(shader); - if (tempObject) { + GLShader* tempObject = compileBackendProgram(shader); + if (tempObject) { object = tempObject; Backend::setGPUObject(shader, object); } } else if (shader.isDomain()) { - GLShader* tempObject = compileShader(shader); - if (tempObject) { + GLShader* tempObject = compileBackendShader(shader); + if (tempObject) { object = tempObject; Backend::setGPUObject(shader, object); } @@ -345,23 +408,6 @@ GLBackend::GLShader* GLBackend::syncGPUObject(const Shader& shader) { return object; } - -GLuint GLBackend::getShaderID(const ShaderPointer& shader) { - if (!shader) { - return 0; - } - GLShader* object = GLBackend::syncGPUObject(*shader); - if (object) { - if (shader->isProgram()) { - return object->_program; - } else { - return object->_shader; - } - } else { - return 0; - } -} - class ElementResource { public: gpu::Element _element; @@ -714,27 +760,38 @@ bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindin return false; } - if (object->_program) { - Shader::SlotSet buffers; - makeUniformBlockSlots(object->_program, slotBindings, buffers); + // Apply bindings to all program versions and generate list of slots from default version + for (int version = 0; version < GLBackend::GLShader::NumVersions; version++) { + auto& shaderObject = object->_shaderObjects[version]; + if (shaderObject.glprogram) { + Shader::SlotSet buffers; + makeUniformBlockSlots(shaderObject.glprogram, slotBindings, buffers); - Shader::SlotSet uniforms; - Shader::SlotSet textures; - Shader::SlotSet samplers; - makeUniformSlots(object->_program, slotBindings, uniforms, textures, samplers); - - Shader::SlotSet inputs; - makeInputSlots(object->_program, slotBindings, inputs); + Shader::SlotSet uniforms; + Shader::SlotSet textures; + Shader::SlotSet samplers; + makeUniformSlots(shaderObject.glprogram, slotBindings, uniforms, textures, samplers); - Shader::SlotSet outputs; - makeOutputSlots(object->_program, slotBindings, outputs); + Shader::SlotSet inputs; + makeInputSlots(shaderObject.glprogram, slotBindings, inputs); - shader.defineSlots(uniforms, buffers, textures, samplers, inputs, outputs); - - } else if (object->_shader) { + Shader::SlotSet outputs; + makeOutputSlots(shaderObject.glprogram, slotBindings, outputs); + // Define the public slots only from the default version + if (version == 0) { + shader.defineSlots(uniforms, buffers, textures, samplers, inputs, outputs); + } else { + GLShader::UniformMapping mapping; + for (auto srcUniform : shader.getUniforms()) { + mapping[srcUniform._location] = uniforms.findLocation(srcUniform._name); + } + object->_uniformMappings.push_back(mapping); + } + } } + return true; } diff --git a/libraries/render-utils/src/ToneMappingEffect.cpp b/libraries/render-utils/src/ToneMappingEffect.cpp index da11507965..b28f271f9d 100644 --- a/libraries/render-utils/src/ToneMappingEffect.cpp +++ b/libraries/render-utils/src/ToneMappingEffect.cpp @@ -27,7 +27,7 @@ ToneMappingEffect::ToneMappingEffect() { } void ToneMappingEffect::init() { - const char BlitTextureGamma_frag[] = R"SCRIBE(#version 410 core + const char BlitTextureGamma_frag[] = R"SCRIBE( // Generated on Sat Oct 24 09:34:37 2015 // // Draw texture 0 fetched at texcoord.xy From 4bbe0ddc6dd5b8ba65d0c56ae6cf1716c8c54098 Mon Sep 17 00:00:00 2001 From: samcake Date: Mon, 18 Apr 2016 19:01:15 -0700 Subject: [PATCH 27/28] Go simple with a a true map for the unifrom mappings --- libraries/gpu/src/gpu/GLBackend.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index 7b9d69fbbd..9e9885c662 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -167,7 +167,7 @@ public: }; using ShaderObjects = std::array< ShaderObject, NumVersions >; - using UniformMapping = std::vector; + using UniformMapping = std::map; using UniformMappingVersions = std::vector; From 7c7cd6e9e56cfc75640c20348837c54d1e9149ff Mon Sep 17 00:00:00 2001 From: samcake Date: Mon, 18 Apr 2016 19:05:49 -0700 Subject: [PATCH 28/28] Go simple with a a true map for the unifrom mappings --- libraries/gpu/src/gpu/GLBackend.cpp | 3 +++ libraries/gpu/src/gpu/GLBackend.h | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/gpu/src/gpu/GLBackend.cpp b/libraries/gpu/src/gpu/GLBackend.cpp index c27da793d2..bd7b86adfd 100644 --- a/libraries/gpu/src/gpu/GLBackend.cpp +++ b/libraries/gpu/src/gpu/GLBackend.cpp @@ -462,6 +462,9 @@ void GLBackend::resetStages() { #define ADD_COMMAND_GL(call) _commands.push_back(COMMAND_##call); _commandOffsets.push_back(_params.size()); #define GET_UNIFORM_LOCATION(shaderUniformLoc) shaderUniformLoc +// THis will be used in the next PR +// #define GET_UNIFORM_LOCATION(shaderUniformLoc) _pipeline._programShader->getUniformLocation(shaderUniformLoc) + void Batch::_glActiveBindTexture(GLenum unit, GLenum target, GLuint texture) { // clean the cache on the texture unit we are going to use so the next call to setResourceTexture() at the same slot works fine diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index 9e9885c662..05afb28bf1 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -182,7 +182,9 @@ public: } GLint getUniformLocation(GLint srcLoc) { - return _uniformMappings[Mono][srcLoc]; + return srcLoc; + // THIS will be used in the next PR + // return _uniformMappings[Mono][srcLoc]; } };