From 7bfc7477486caf05fbeb29d812541e467e96fde7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Mar 2015 12:13:32 -0700 Subject: [PATCH 1/3] Revert "Revert "NOT MERGEABLE: graceful cleanup on Application dtor for NodeList"" This reverts commit 9269b2a0b2ae9d05abf721f32b3f3f91ad844b16. --- domain-server/src/DomainServer.cpp | 4 -- interface/src/Application.cpp | 40 +++++++++---------- interface/src/Application.h | 6 +-- libraries/networking/src/LimitedNodeList.cpp | 5 +++ libraries/networking/src/LimitedNodeList.h | 2 + .../networking/src/ThreadedAssignment.cpp | 4 -- 6 files changed, 27 insertions(+), 34 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index d02ad73b47..7af9ffd85c 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -267,10 +267,6 @@ void DomainServer::setupNodeListAndAssignments(const QUuid& sessionUUID) { connect(nodeList.data(), &LimitedNodeList::nodeAdded, this, &DomainServer::nodeAdded); connect(nodeList.data(), &LimitedNodeList::nodeKilled, this, &DomainServer::nodeKilled); - QTimer* silentNodeTimer = new QTimer(this); - connect(silentNodeTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); - silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), SLOT(readAvailableDatagrams())); // add whatever static assignments that have been parsed to the queue diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 141a58b317..5a84c7895a 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -146,7 +146,6 @@ const qint64 MAXIMUM_CACHE_SIZE = 10 * BYTES_PER_GIGABYTES; // 10GB static QTimer* locationUpdateTimer = NULL; static QTimer* balanceUpdateTimer = NULL; -static QTimer* silentNodeTimer = NULL; static QTimer* identityPacketTimer = NULL; static QTimer* billboardPacketTimer = NULL; static QTimer* checkFPStimer = NULL; @@ -258,7 +257,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _dependencyManagerIsSetup(setupEssentials(argc, argv)), _window(new MainWindow(desktop())), _toolWindow(NULL), - _nodeThread(new QThread(this)), _datagramProcessor(), _undoStack(), _undoStackScriptingInterface(&_undoStack), @@ -329,18 +327,20 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _runningScriptsWidget = new RunningScriptsWidget(_window); // start the nodeThread so its event loop is running - _nodeThread->setObjectName("Datagram Processor Thread"); - _nodeThread->start(); + QThread* nodeThread = new QThread(this); + nodeThread->setObjectName("Datagram Processor Thread"); + nodeThread->start(); // make sure the node thread is given highest priority - _nodeThread->setPriority(QThread::TimeCriticalPriority); + nodeThread->setPriority(QThread::TimeCriticalPriority); + + _datagramProcessor = new DatagramProcessor(nodeList.data()); // put the NodeList and datagram processing on the node thread - nodeList->moveToThread(_nodeThread); - _datagramProcessor.moveToThread(_nodeThread); + nodeList->moveToThread(nodeThread); // connect the DataProcessor processDatagrams slot to the QUDPSocket readyRead() signal - connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), &_datagramProcessor, SLOT(processDatagrams())); + connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _datagramProcessor, &DatagramProcessor::processDatagrams); // put the audio processing on a separate thread QThread* audioThread = new QThread(); @@ -427,12 +427,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : // connect to the packet sent signal of the _entityEditSender connect(&_entityEditSender, &EntityEditPacketSender::packetSent, this, &Application::packetSent); - // move the silentNodeTimer to the _nodeThread - silentNodeTimer = new QTimer(); - connect(silentNodeTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); - silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - silentNodeTimer->moveToThread(_nodeThread); - // send the identity packet for our avatar each second to our avatar mixer identityPacketTimer = new QTimer(); connect(identityPacketTimer, &QTimer::timeout, _myAvatar, &MyAvatar::sendIdentityPacket); @@ -547,7 +541,7 @@ void Application::aboutToQuit() { } void Application::cleanupBeforeQuit() { - _datagramProcessor.shutdown(); // tell the datagram processor we're shutting down, so it can short circuit + _datagramProcessor->shutdown(); // tell the datagram processor we're shutting down, so it can short circuit _entities.shutdown(); // tell the entities system we're shutting down, so it will stop running scripts ScriptEngine::stopAllScripts(this); // stop all currently running global scripts @@ -555,7 +549,6 @@ void Application::cleanupBeforeQuit() { // depending on what thread they run in locationUpdateTimer->stop(); balanceUpdateTimer->stop(); - QMetaObject::invokeMethod(silentNodeTimer, "stop", Qt::BlockingQueuedConnection); identityPacketTimer->stop(); billboardPacketTimer->stop(); checkFPStimer->stop(); @@ -565,7 +558,6 @@ void Application::cleanupBeforeQuit() { // and then delete those that got created by "new" delete locationUpdateTimer; delete balanceUpdateTimer; - delete silentNodeTimer; delete identityPacketTimer; delete billboardPacketTimer; delete checkFPStimer; @@ -597,10 +589,6 @@ Application::~Application() { tree->lockForWrite(); _entities.getTree()->setSimulation(NULL); tree->unlock(); - - // ask the datagram processing thread to quit and wait until it is done - _nodeThread->quit(); - _nodeThread->wait(); _octreeProcessor.terminate(); _entityEditSender.terminate(); @@ -620,6 +608,14 @@ Application::~Application() { DependencyManager::destroy(); //DependencyManager::destroy(); DependencyManager::destroy(); + + auto nodeList = DependencyManager::get(); + QThread* nodeThread = nodeList->thread(); + nodeList->deleteLater(); + + // ask the node thread to quit and wait until it is done + nodeThread->quit(); + nodeThread->wait(); qInstallMessageHandler(NULL); // NOTE: Do this as late as possible so we continue to get our log messages } @@ -1498,7 +1494,7 @@ void Application::checkFPS() { _fps = (float)_frameCount / diffTime; _frameCount = 0; - _datagramProcessor.resetCounters(); + _datagramProcessor->resetCounters(); _timerStart.start(); // ask the node list to check in with the domain server diff --git a/interface/src/Application.h b/interface/src/Application.h index bcd31fcd51..248aaa0f6a 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -445,10 +445,8 @@ private: MainWindow* _window; ToolWindow* _toolWindow; - - - QThread* _nodeThread; - DatagramProcessor _datagramProcessor; + + DatagramProcessor* _datagramProcessor; QUndoStack _undoStack; UndoStackScriptingInterface _undoStackScriptingInterface; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 279d958082..520dc650ed 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -80,6 +80,10 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short connect(localSocketUpdate, &QTimer::timeout, this, &LimitedNodeList::updateLocalSockAddr); localSocketUpdate->start(LOCAL_SOCKET_UPDATE_INTERVAL_MSECS); + QTimer* silentNodeTimer = new QTimer(this); + connect(silentNodeTimer, &QTimer::timeout, this, &LimitedNodeList::removeSilentNodes); + silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); + // check the local socket right now updateLocalSockAddr(); @@ -500,6 +504,7 @@ void LimitedNodeList::resetPacketStats() { } void LimitedNodeList::removeSilentNodes() { + QSet killedNodes; eachNodeHashIterator([&](NodeHash::iterator& it){ diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index afbdf23fba..a071eced31 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -223,6 +223,8 @@ protected: HifiSockAddr _localSockAddr; HifiSockAddr _publicSockAddr; HifiSockAddr _stunSockAddr; + + QTimer* _silentNodeTimer; // XXX can BandwidthRecorder be used for this? int _numCollectedPackets; diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index ea94a8e22c..79b4e7f437 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -67,10 +67,6 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy connect(domainServerTimer, SIGNAL(timeout()), this, SLOT(checkInWithDomainServerOrExit())); domainServerTimer->start(DOMAIN_SERVER_CHECK_IN_MSECS); - QTimer* silentNodeRemovalTimer = new QTimer(this); - connect(silentNodeRemovalTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); - silentNodeRemovalTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - if (shouldSendStats) { // send a stats packet every 1 second QTimer* statsTimer = new QTimer(this); From 09e2c0987e6af3155b0b668b76ffc5ab2b4b0820 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Mar 2015 12:27:48 -0700 Subject: [PATCH 2/3] use Dependency customDeleter for Application NL --- interface/src/Application.cpp | 10 +++++++--- libraries/networking/src/NodeList.h | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 5a84c7895a..baa7dd633d 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -336,6 +336,11 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _datagramProcessor = new DatagramProcessor(nodeList.data()); + // have the NodeList use deleteLater from DM customDeleter + nodeList->setCustomDeleter([](Dependency* dependency) { + static_cast(dependency)->deleteLater(); + }); + // put the NodeList and datagram processing on the node thread nodeList->moveToThread(nodeThread); @@ -609,9 +614,8 @@ Application::~Application() { //DependencyManager::destroy(); DependencyManager::destroy(); - auto nodeList = DependencyManager::get(); - QThread* nodeThread = nodeList->thread(); - nodeList->deleteLater(); + QThread* nodeThread = DependencyManager::get()->thread(); + DependencyManager::destroy(); // ask the node thread to quit and wait until it is done nodeThread->quit(); diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 1c6de4bb6c..ccfaa7a4cf 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -37,6 +37,7 @@ const quint64 DOMAIN_SERVER_CHECK_IN_MSECS = 1 * 1000; const int MAX_SILENT_DOMAIN_SERVER_CHECK_INS = 5; +class Application; class Assignment; class NodeList : public LimitedNodeList { @@ -95,6 +96,8 @@ private: HifiSockAddr _assignmentServerSocket; bool _hasCompletedInitialSTUNFailure; unsigned int _stunRequestsSinceSuccess; + + friend class Application; }; #endif // hifi_NodeList_h From 471e55c8cedf7ab321aaed72b84dfe55c741c3a9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Mar 2015 12:36:39 -0700 Subject: [PATCH 3/3] fix some lingering warnings --- interface/src/Application.cpp | 1 - interface/src/avatar/MyAvatar.cpp | 2 - interface/src/avatar/SkeletonModel.cpp | 2 - libraries/entities/src/LightEntityItem.cpp | 1 - libraries/model/src/model/Stage.cpp | 86 +++++++++++----------- libraries/physics/src/MeshInfo.cpp | 8 +- 6 files changed, 48 insertions(+), 52 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index baa7dd633d..72c17ed09b 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2739,7 +2739,6 @@ const GLfloat WORLD_DIFFUSE_COLOR[] = { 0.6f, 0.525f, 0.525f }; const GLfloat WORLD_SPECULAR_COLOR[] = { 0.94f, 0.94f, 0.737f, 1.0f }; const glm::vec3 GLOBAL_LIGHT_COLOR = { 0.6f, 0.525f, 0.525f }; -const float GLOBAL_LIGHT_INTENSITY = 1.0f; void Application::setupWorldLight() { diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 9ecc0a3798..d9c9ff3ad1 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -49,8 +49,6 @@ using namespace std; const glm::vec3 DEFAULT_UP_DIRECTION(0.0f, 1.0f, 0.0f); const float YAW_SPEED = 500.0f; // degrees/sec const float PITCH_SPEED = 100.0f; // degrees/sec -const float COLLISION_RADIUS_SCALAR = 1.2f; // pertains to avatar-to-avatar collisions -const float COLLISION_RADIUS_SCALE = 0.125f; const float DEFAULT_REAL_WORLD_FIELD_OF_VIEW_DEGREES = 30.0f; const float MAX_WALKING_SPEED = 2.5f; // human walking speed diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp index d083116ecd..52dd424b71 100644 --- a/interface/src/avatar/SkeletonModel.cpp +++ b/interface/src/avatar/SkeletonModel.cpp @@ -791,8 +791,6 @@ void SkeletonModel::renderBoundingCollisionShapes(float alpha) { glPopMatrix(); } -const int BALL_SUBDIVISIONS = 10; - bool SkeletonModel::hasSkeleton() { return isActive() ? _geometry->getFBXGeometry().rootJointIndex != -1 : false; } diff --git a/libraries/entities/src/LightEntityItem.cpp b/libraries/entities/src/LightEntityItem.cpp index 3265891b18..62a44c7e21 100644 --- a/libraries/entities/src/LightEntityItem.cpp +++ b/libraries/entities/src/LightEntityItem.cpp @@ -32,7 +32,6 @@ LightEntityItem::LightEntityItem(const EntityItemID& entityItemID, const EntityI _type = EntityTypes::Light; // default property values - const quint8 MAX_COLOR = 255; _color[RED_INDEX] = _color[GREEN_INDEX] = _color[BLUE_INDEX] = 0; _intensity = 1.0f; _exponent = 0.0f; diff --git a/libraries/model/src/model/Stage.cpp b/libraries/model/src/model/Stage.cpp index 970539a908..1c171eee76 100644 --- a/libraries/model/src/model/Stage.cpp +++ b/libraries/model/src/model/Stage.cpp @@ -134,55 +134,55 @@ void EarthSunModel::setSunLongitude(float lon) { _sunLongitude = validateLongitude(lon); invalidate(); } - -Atmosphere::Atmosphere() { - // only if created from nothing shall we create the Buffer to store the properties - Data data; - _dataBuffer = gpu::BufferView(new gpu::Buffer(sizeof(Data), (const gpu::Buffer::Byte*) &data)); - + +Atmosphere::Atmosphere() { + // only if created from nothing shall we create the Buffer to store the properties + Data data; + _dataBuffer = gpu::BufferView(new gpu::Buffer(sizeof(Data), (const gpu::Buffer::Byte*) &data)); + setScatteringWavelength(_scatteringWavelength); setRayleighScattering(_rayleighScattering); setInnerOuterRadiuses(getInnerRadius(), getOuterRadius()); } -void Atmosphere::setScatteringWavelength(Vec3 wavelength) { - _scatteringWavelength = wavelength; - Data& data = editData(); - data._invWaveLength = Vec4(1.0f / powf(wavelength.x, 4.0f), 1.0f / powf(wavelength.y, 4.0f), 1.0f / powf(wavelength.z, 4.0f), 0.0f); -} - -void Atmosphere::setRayleighScattering(float scattering) { - _rayleighScattering = scattering; - updateScattering(); -} - -void Atmosphere::setMieScattering(float scattering) { - _mieScattering = scattering; - updateScattering(); -} - -void Atmosphere::setSunBrightness(float brightness) { - _sunBrightness = brightness; - updateScattering(); -} +void Atmosphere::setScatteringWavelength(Vec3 wavelength) { + _scatteringWavelength = wavelength; + Data& data = editData(); + data._invWaveLength = Vec4(1.0f / powf(wavelength.x, 4.0f), 1.0f / powf(wavelength.y, 4.0f), 1.0f / powf(wavelength.z, 4.0f), 0.0f); +} -void Atmosphere::updateScattering() { - Data& data = editData(); - - data._scatterings.x = getRayleighScattering() * getSunBrightness(); - data._scatterings.y = getMieScattering() * getSunBrightness(); - - data._scatterings.z = getRayleighScattering() * 4.0f * glm::pi(); - data._scatterings.w = getMieScattering() * 4.0f * glm::pi(); -} +void Atmosphere::setRayleighScattering(float scattering) { + _rayleighScattering = scattering; + updateScattering(); +} -void Atmosphere::setInnerOuterRadiuses(float inner, float outer) { - Data& data = editData(); - data._radiuses.x = inner; - data._radiuses.y = outer; - data._scales.x = 1.0f / (outer - inner); - data._scales.z = data._scales.x / data._scales.y; -} +void Atmosphere::setMieScattering(float scattering) { + _mieScattering = scattering; + updateScattering(); +} + +void Atmosphere::setSunBrightness(float brightness) { + _sunBrightness = brightness; + updateScattering(); +} + +void Atmosphere::updateScattering() { + Data& data = editData(); + + data._scatterings.x = getRayleighScattering() * getSunBrightness(); + data._scatterings.y = getMieScattering() * getSunBrightness(); + + data._scatterings.z = getRayleighScattering() * 4.0f * glm::pi(); + data._scatterings.w = getMieScattering() * 4.0f * glm::pi(); +} + +void Atmosphere::setInnerOuterRadiuses(float inner, float outer) { + Data& data = editData(); + data._radiuses.x = inner; + data._radiuses.y = outer; + data._scales.x = 1.0f / (outer - inner); + data._scales.z = data._scales.x / data._scales.y; +} const int NUM_DAYS_PER_YEAR = 365; @@ -267,7 +267,7 @@ void SunSkyStage::updateGraphicsObject() const { static int firstTime = 0; if (firstTime == 0) { firstTime++; - bool result = gpu::Shader::makeProgram(*(_skyPipeline->getProgram())); + gpu::Shader::makeProgram(*(_skyPipeline->getProgram())); } diff --git a/libraries/physics/src/MeshInfo.cpp b/libraries/physics/src/MeshInfo.cpp index 8df5ff914d..29ddc74a98 100644 --- a/libraries/physics/src/MeshInfo.cpp +++ b/libraries/physics/src/MeshInfo.cpp @@ -17,9 +17,11 @@ using namespace meshinfo; //origin is the default reference point for generating the tetrahedron from each triangle of the mesh. MeshInfo::MeshInfo(vector *vertices, vector *triangles) :\ -_vertices(vertices), -_triangles(triangles), -_centerOfMass(Vertex(0.0, 0.0, 0.0)){ + _vertices(vertices), + _centerOfMass(Vertex(0.0, 0.0, 0.0)), + _triangles(triangles) +{ + } MeshInfo::~MeshInfo(){