From 5ff0bc409b8f0def2cc3084b87ae9628a473c94c Mon Sep 17 00:00:00 2001 From: NeetBhagat Date: Fri, 30 Jun 2017 15:40:38 +0530 Subject: [PATCH 01/19] solve the bug : Avatar scale value should be 3.0 ,if user has set scale value > 3.0 previously . --- interface/src/avatar/MyAvatar.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index e5c4f4b972..13a3c56e0f 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -67,6 +67,7 @@ const float MAX_WALKING_SPEED = 2.6f; // human walking speed const float MAX_BOOST_SPEED = 0.5f * MAX_WALKING_SPEED; // action motor gets additive boost below this speed const float MIN_AVATAR_SPEED = 0.05f; const float MIN_AVATAR_SPEED_SQUARED = MIN_AVATAR_SPEED * MIN_AVATAR_SPEED; // speed is set to zero below this +const float DEFAULT_MAX_AVATAR_SCALE = 3.0f; const float YAW_SPEED_DEFAULT = 120.0f; // degrees/sec const float PITCH_SPEED_DEFAULT = 90.0f; // degrees/sec @@ -1075,6 +1076,7 @@ void MyAvatar::loadData() { getHead()->setBasePitch(loadSetting(settings, "headPitch", 0.0f)); _targetScale = loadSetting(settings, "scale", 1.0f); + _targetScale = (_targetScale > DEFAULT_MAX_AVATAR_SCALE) ? DEFAULT_MAX_AVATAR_SCALE : _targetScale; setScale(glm::vec3(_targetScale)); _prefOverrideAnimGraphUrl.set(QUrl(settings.value("animGraphURL", "").toString())); From dd2dbd9b39ab3eb5a45e0d754ea3c938ccce63b1 Mon Sep 17 00:00:00 2001 From: NeetBhagat Date: Fri, 7 Jul 2017 18:14:21 +0530 Subject: [PATCH 02/19] Initial Commit to resolve problem :> When Max Avatar scale is set in Sandbox -> setting, take that value as default max value --- interface/src/avatar/MyAvatar.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 42ed32072c..442ded8156 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -68,7 +68,6 @@ const float MAX_WALKING_SPEED = 2.6f; // human walking speed const float MAX_BOOST_SPEED = 0.5f * MAX_WALKING_SPEED; // action motor gets additive boost below this speed const float MIN_AVATAR_SPEED = 0.05f; const float MIN_AVATAR_SPEED_SQUARED = MIN_AVATAR_SPEED * MIN_AVATAR_SPEED; // speed is set to zero below this -const float DEFAULT_MAX_AVATAR_SCALE = 3.0f; const float YAW_SPEED_DEFAULT = 120.0f; // degrees/sec const float PITCH_SPEED_DEFAULT = 90.0f; // degrees/sec @@ -1076,10 +1075,6 @@ void MyAvatar::loadData() { getHead()->setBasePitch(loadSetting(settings, "headPitch", 0.0f)); - _targetScale = loadSetting(settings, "scale", 1.0f); - _targetScale = (_targetScale > DEFAULT_MAX_AVATAR_SCALE) ? DEFAULT_MAX_AVATAR_SCALE : _targetScale; - setScale(glm::vec3(_targetScale)); - _prefOverrideAnimGraphUrl.set(QUrl(settings.value("animGraphURL", "").toString())); _fullAvatarURLFromPreferences = settings.value("fullAvatarURL", AvatarData::defaultFullAvatarModelUrl()).toUrl(); _fullAvatarModelName = settings.value("fullAvatarModelName", DEFAULT_FULL_AVATAR_MODEL_NAME).toString(); @@ -2270,7 +2265,16 @@ void MyAvatar::restrictScaleFromDomainSettings(const QJsonObject& domainSettings static const QString MAX_SCALE_OPTION = "max_avatar_scale"; float settingMaxScale = avatarsObject[MAX_SCALE_OPTION].toDouble(MAX_AVATAR_SCALE); setDomainMaximumScale(settingMaxScale); - + + // Set avatar scale. The current avatar scale should not be more than domain's max_avatar_scale. + Settings settings; + settings.beginGroup("Avatar"); + _targetScale = loadSetting(settings, "scale", 1.0f); + qCDebug(interfaceapp, "Current avatar scale is %f. Receive max_avatar_scale %f from this domain settings. Selecting the minimum value to set the scale of an avatar.", _targetScale, _domainMaximumScale); + _targetScale = (_targetScale > _domainMaximumScale) ? _domainMaximumScale : _targetScale; + qCDebug(interfaceapp) << "Avatar scale " << _targetScale; + setScale(glm::vec3(_targetScale)); + settings.endGroup(); // make sure that the domain owner didn't flip min and max if (_domainMinimumScale > _domainMaximumScale) { std::swap(_domainMinimumScale, _domainMaximumScale); From e08e9d68216316a92dee03a1e6d6c0624368d683 Mon Sep 17 00:00:00 2001 From: NeetBhagat Date: Tue, 11 Jul 2017 20:39:51 +0530 Subject: [PATCH 03/19] The current scale of avatar should not be more than domain's max_avatar_scale and not less than domain's min_avatar_scale. --- interface/src/avatar/MyAvatar.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index fc86bfd959..41ae07f54f 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2266,31 +2266,31 @@ void MyAvatar::restrictScaleFromDomainSettings(const QJsonObject& domainSettings static const QString MAX_SCALE_OPTION = "max_avatar_scale"; float settingMaxScale = avatarsObject[MAX_SCALE_OPTION].toDouble(MAX_AVATAR_SCALE); setDomainMaximumScale(settingMaxScale); - - // Set avatar scale. The current avatar scale should not be more than domain's max_avatar_scale. - Settings settings; - settings.beginGroup("Avatar"); - _targetScale = loadSetting(settings, "scale", 1.0f); - qCDebug(interfaceapp, "Current avatar scale is %f. Receive max_avatar_scale %f from this domain settings. Selecting the minimum value to set the scale of an avatar.", _targetScale, _domainMaximumScale); - _targetScale = (_targetScale > _domainMaximumScale) ? _domainMaximumScale : _targetScale; - qCDebug(interfaceapp) << "Avatar scale " << _targetScale; - setScale(glm::vec3(_targetScale)); - settings.endGroup(); + // make sure that the domain owner didn't flip min and max if (_domainMinimumScale > _domainMaximumScale) { std::swap(_domainMinimumScale, _domainMaximumScale); } + // Set avatar current scale + Settings settings; + settings.beginGroup("Avatar"); + _targetScale = loadSetting(settings, "scale", 1.0f); - qCDebug(interfaceapp, "This domain requires a minimum avatar scale of %f and a maximum avatar scale of %f", - (double)_domainMinimumScale, (double)_domainMaximumScale); + qCDebug(interfaceapp, "This domain requires a minimum avatar scale of %f and a maximum avatar scale of %f. Current avatar scale is %f", + (double)_domainMinimumScale, (double)_domainMaximumScale, (double)_targetScale); // debug to log if this avatar's scale in this domain will be clamped - auto clampedScale = glm::clamp(_targetScale, _domainMinimumScale, _domainMaximumScale); + float clampedScale = glm::clamp(_targetScale, _domainMinimumScale, _domainMaximumScale); if (_targetScale != clampedScale) { - qCDebug(interfaceapp, "Avatar scale will be clamped to %f because %f is not allowed by current domain", - (double)clampedScale, (double)_targetScale); + qCDebug(interfaceapp, "Current avatar scale is clamped to %f because %f is not allowed by current domain", + clampedScale, _targetScale); + // The current scale of avatar should not be more than domain's max_avatar_scale and not less than domain's min_avatar_scale . + _targetScale = clampedScale; } + + setScale(glm::vec3(_targetScale)); + settings.endGroup(); } void MyAvatar::clearScaleRestriction() { From 6b969bc39d784cb142f7ddb2ebbda9df1f12c562 Mon Sep 17 00:00:00 2001 From: utkarshgautamnyu Date: Tue, 11 Jul 2017 13:05:07 -0700 Subject: [PATCH 04/19] added code to record total number of ATP/HTTP/File bytes downloaded during session Update FileResourceRequest.cpp Update AssetResourceRequest.cpp Update FileResourceRequest.cpp Update HTTPResourceRequest.cpp Update AssetResourceRequest.cpp Update AssetResourceRequest.cpp Update FileResourceRequest.cpp Update AssetResourceRequest.cpp Update AssetResourceRequest.cpp Update FileResourceRequest.cpp Update FileResourceRequest.cpp Update HTTPResourceRequest.cpp Update FileResourceRequest.cpp Update FileResourceRequest.cpp Update Application.cpp Update Application.cpp --- interface/src/Application.cpp | 8 ++++++++ libraries/networking/src/AssetResourceRequest.cpp | 4 +++- libraries/networking/src/FileResourceRequest.cpp | 5 ++++- libraries/networking/src/HTTPResourceRequest.cpp | 5 +++++ libraries/networking/src/ResourceRequest.h | 3 +++ 5 files changed, 23 insertions(+), 2 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ddd1870723..07c8a928c5 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1466,6 +1466,14 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo properties["atp_mapping_requests"] = atpMappingRequests; properties["throttled"] = _displayPlugin ? _displayPlugin->isThrottled() : false; + + QJsonObject bytesDownloaded; + bytesDownloaded["atp"] = statTracker->getStat(STAT_ATP_RESOURCE_TOTAL_BYTES).toInt(); + bytesDownloaded["http"] = statTracker->getStat(STAT_HTTP_RESOURCE_TOTAL_BYTES).toInt(); + bytesDownloaded["file"] = statTracker->getStat(STAT_FILE_RESOURCE_TOTAL_BYTES).toInt(); + bytesDownloaded["total"] = bytesDownloaded["atp"].toInt() + bytesDownloaded["http"].toInt() + + bytesDownloaded["file"].toInt(); + properties["bytesDownloaded"] = bytesDownloaded; auto myAvatar = getMyAvatar(); glm::vec3 avatarPosition = myAvatar->getPosition(); diff --git a/libraries/networking/src/AssetResourceRequest.cpp b/libraries/networking/src/AssetResourceRequest.cpp index f4a3b20fd5..a41283cc0d 100644 --- a/libraries/networking/src/AssetResourceRequest.cpp +++ b/libraries/networking/src/AssetResourceRequest.cpp @@ -187,6 +187,9 @@ void AssetResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytes emit progress(bytesReceived, bytesTotal); auto now = p_high_resolution_clock::now(); + + // Recording ATP bytes downloaded in stats + DependencyManager::get()->updateStat(STAT_ATP_RESOURCE_TOTAL_BYTES, bytesReceived); // if we haven't received the full asset check if it is time to output progress to log // we do so every X seconds to assist with ATP download tracking @@ -201,6 +204,5 @@ void AssetResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytes _lastProgressDebug = now; } - } diff --git a/libraries/networking/src/FileResourceRequest.cpp b/libraries/networking/src/FileResourceRequest.cpp index d0e2721679..26857716e1 100644 --- a/libraries/networking/src/FileResourceRequest.cpp +++ b/libraries/networking/src/FileResourceRequest.cpp @@ -20,7 +20,7 @@ void FileResourceRequest::doSend() { auto statTracker = DependencyManager::get(); statTracker->incrementStat(STAT_FILE_REQUEST_STARTED); - + int fileSize = 0; QString filename = _url.toLocalFile(); // sometimes on windows, we see the toLocalFile() return null, @@ -53,6 +53,7 @@ void FileResourceRequest::doSend() { } _result = ResourceRequest::Success; + fileSize = file.size(); } } else { @@ -68,6 +69,8 @@ void FileResourceRequest::doSend() { if (_result == ResourceRequest::Success) { statTracker->incrementStat(STAT_FILE_REQUEST_SUCCESS); + // Recording FILE bytes downloaded in stats + statTracker->updateStat(STAT_FILE_RESOURCE_TOTAL_BYTES,fileSize); } else { statTracker->incrementStat(STAT_FILE_REQUEST_FAILED); } diff --git a/libraries/networking/src/HTTPResourceRequest.cpp b/libraries/networking/src/HTTPResourceRequest.cpp index 266ea429a0..c6d0370a70 100644 --- a/libraries/networking/src/HTTPResourceRequest.cpp +++ b/libraries/networking/src/HTTPResourceRequest.cpp @@ -201,6 +201,11 @@ void HTTPResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytesT _sendTimer->start(); emit progress(bytesReceived, bytesTotal); + + // Recording HTTP bytes downloaded in stats + DependencyManager::get()->updateStat(STAT_HTTP_RESOURCE_TOTAL_BYTES, bytesReceived); + + } void HTTPResourceRequest::onTimeout() { diff --git a/libraries/networking/src/ResourceRequest.h b/libraries/networking/src/ResourceRequest.h index 39bcb3fe93..3ee86025a2 100644 --- a/libraries/networking/src/ResourceRequest.h +++ b/libraries/networking/src/ResourceRequest.h @@ -33,6 +33,9 @@ const QString STAT_HTTP_REQUEST_CACHE = "CacheHTTPRequest"; const QString STAT_ATP_MAPPING_REQUEST_STARTED = "StartedATPMappingRequest"; const QString STAT_ATP_MAPPING_REQUEST_FAILED = "FailedATPMappingRequest"; const QString STAT_ATP_MAPPING_REQUEST_SUCCESS = "SuccessfulATPMappingRequest"; +const QString STAT_HTTP_RESOURCE_TOTAL_BYTES = "HTTPBytesDownloaded"; +const QString STAT_ATP_RESOURCE_TOTAL_BYTES = "ATPBytesDownloaded"; +const QString STAT_FILE_RESOURCE_TOTAL_BYTES = "FILEBytesDownloaded"; class ResourceRequest : public QObject { Q_OBJECT From 11f8f5cffccb6a6409fcaf77c13bd07275780cda Mon Sep 17 00:00:00 2001 From: NeetBhagat Date: Thu, 13 Jul 2017 18:26:52 +0530 Subject: [PATCH 05/19] =?UTF-8?q?Solve=20:=20implicit=20conversion=20from?= =?UTF-8?q?=20=E2=80=98float=E2=80=99=20to=20=E2=80=98double=E2=80=99=20is?= =?UTF-8?q?sue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- interface/src/avatar/MyAvatar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 41ae07f54f..6ccc0ab516 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2277,7 +2277,7 @@ void MyAvatar::restrictScaleFromDomainSettings(const QJsonObject& domainSettings _targetScale = loadSetting(settings, "scale", 1.0f); qCDebug(interfaceapp, "This domain requires a minimum avatar scale of %f and a maximum avatar scale of %f. Current avatar scale is %f", - (double)_domainMinimumScale, (double)_domainMaximumScale, (double)_targetScale); + _domainMinimumScale, _domainMaximumScale, _targetScale); // debug to log if this avatar's scale in this domain will be clamped float clampedScale = glm::clamp(_targetScale, _domainMinimumScale, _domainMaximumScale); From 7ea1e7285f0e6697309d34db73e37e48a6fb6af1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 28 Jun 2017 14:59:46 -0700 Subject: [PATCH 06/19] map EntityItemID to EntityItemPointer --- libraries/entities/src/AddEntityOperator.cpp | 4 +- .../entities/src/DeleteEntityOperator.cpp | 2 +- libraries/entities/src/EntityTree.cpp | 82 ++++++++++--------- libraries/entities/src/EntityTree.h | 15 ++-- libraries/entities/src/EntityTreeElement.cpp | 3 +- .../entities/src/MovingEntitiesOperator.cpp | 3 +- .../entities/src/UpdateEntityOperator.cpp | 10 +-- 7 files changed, 59 insertions(+), 60 deletions(-) diff --git a/libraries/entities/src/AddEntityOperator.cpp b/libraries/entities/src/AddEntityOperator.cpp index e86e70dd80..78d986f538 100644 --- a/libraries/entities/src/AddEntityOperator.cpp +++ b/libraries/entities/src/AddEntityOperator.cpp @@ -46,10 +46,8 @@ bool AddEntityOperator::preRecursion(const OctreeElementPointer& element) { // If this element is the best fit for the new entity properties, then add/or update it if (entityTreeElement->bestFitBounds(_newEntityBox)) { - + _tree->addEntityMapEntry(_newEntity); entityTreeElement->addEntityItem(_newEntity); - _tree->setContainingElement(_newEntity->getEntityItemID(), entityTreeElement); - _foundNew = true; keepSearching = false; } else { diff --git a/libraries/entities/src/DeleteEntityOperator.cpp b/libraries/entities/src/DeleteEntityOperator.cpp index 709c281341..cbd0ad7839 100644 --- a/libraries/entities/src/DeleteEntityOperator.cpp +++ b/libraries/entities/src/DeleteEntityOperator.cpp @@ -96,7 +96,7 @@ bool DeleteEntityOperator::preRecursion(const OctreeElementPointer& element) { bool entityDeleted = entityTreeElement->removeEntityItem(theEntity); // remove it from the element assert(entityDeleted); (void)entityDeleted; // quite warning - _tree->setContainingElement(details.entity->getEntityItemID(), NULL); // update or id to element lookup + _tree->clearEntityMapEntry(details.entity->getEntityItemID()); _foundCount++; } } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4773f45af7..31b96ebfc4 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -91,11 +91,14 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) { _simulation->clearEntities(); } { - QWriteLocker locker(&_entityToElementLock); - foreach(EntityTreeElementPointer element, _entityToElementMap) { - element->cleanupEntities(); + QWriteLocker locker(&_entityMapLock); + foreach(EntityItemPointer entity, _entityMap) { + EntityTreeElementPointer element = entity->getElement(); + if (element) { + element->cleanupEntities(); + } } - _entityToElementMap.clear(); + _entityMap.clear(); } Octree::eraseAllOctreeElements(createNewRoot); @@ -136,29 +139,24 @@ void EntityTree::postAddEntity(EntityItemPointer entity) { } bool EntityTree::updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode) { - EntityTreeElementPointer containingElement = getContainingElement(entityID); + EntityItemPointer entity; + { + QReadLocker locker(&_entityMapLock); + entity = _entityMap.value(entityID); + } + if (!entity) { + return false; + } + return updateEntity(entity, properties, senderNode); +} + +bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperties& origProperties, + const SharedNodePointer& senderNode) { + EntityTreeElementPointer containingElement = entity->getElement(); if (!containingElement) { return false; } - EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); - if (!existingEntity) { - return false; - } - - return updateEntityWithElement(existingEntity, properties, containingElement, senderNode); -} - -bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode) { - EntityTreeElementPointer containingElement = getContainingElement(entity->getEntityItemID()); - if (!containingElement) { - return false; - } - return updateEntityWithElement(entity, properties, containingElement, senderNode); -} - -bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& origProperties, - EntityTreeElementPointer containingElement, const SharedNodePointer& senderNode) { EntityItemProperties properties = origProperties; bool allowLockChange; @@ -331,8 +329,7 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI } // TODO: this final containingElement check should eventually be removed (or wrapped in an #ifdef DEBUG). - containingElement = getContainingElement(entity->getEntityItemID()); - if (!containingElement) { + if (!entity->getElement()) { qCDebug(entities) << "UNEXPECTED!!!! after updateEntity() we no longer have a containing element??? entityID=" << entity->getEntityItemID(); return false; @@ -1502,27 +1499,38 @@ int EntityTree::processEraseMessageDetails(const QByteArray& dataByteArray, cons } EntityTreeElementPointer EntityTree::getContainingElement(const EntityItemID& entityItemID) /*const*/ { - QReadLocker locker(&_entityToElementLock); - EntityTreeElementPointer element = _entityToElementMap.value(entityItemID); - return element; + QReadLocker locker(&_entityMapLock); + EntityItemPointer entity = _entityMap.value(entityItemID); + if (entity) { + return entity->getElement(); + } + return EntityTreeElementPointer(nullptr); } -void EntityTree::setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element) { - QWriteLocker locker(&_entityToElementLock); - if (element) { - _entityToElementMap[entityItemID] = element; - } else { - _entityToElementMap.remove(entityItemID); +void EntityTree::addEntityMapEntry(EntityItemPointer entity) { + EntityItemID id = entity->getEntityItemID(); + QWriteLocker locker(&_entityMapLock); + EntityItemPointer otherEntity = _entityMap.value(id); + if (otherEntity) { + qCDebug(entities) << "EntityTree::addEntityMapEntry() found pre-existing id " << id; + assert(false); + return; } + _entityMap.insert(id, entity); +} + +void EntityTree::clearEntityMapEntry(const EntityItemID& id) { + QWriteLocker locker(&_entityMapLock); + _entityMap.remove(id); } void EntityTree::debugDumpMap() { qCDebug(entities) << "EntityTree::debugDumpMap() --------------------------"; - QReadLocker locker(&_entityToElementLock); - QHashIterator i(_entityToElementMap); + QReadLocker locker(&_entityMapLock); + QHashIterator i(_entityMap); while (i.hasNext()) { i.next(); - qCDebug(entities) << i.key() << ": " << i.value().get(); + qCDebug(entities) << i.key() << ": " << i.value()->getElement().get(); } qCDebug(entities) << "-----------------------------------------------------"; } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 24e6c364b1..efb8cf81ba 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -119,9 +119,6 @@ public: // use this method if you only know the entityID bool updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); - // use this method if you have a pointer to the entity (avoid an extra entity lookup) - bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); - // check if the avatar is a child of this entity, If so set the avatar parentID to null void unhookChildAvatar(const EntityItemID entityID); void deleteEntity(const EntityItemID& entityID, bool force = false, bool ignoreWarnings = true); @@ -183,7 +180,8 @@ public: int processEraseMessageDetails(const QByteArray& buffer, const SharedNodePointer& sourceNode); EntityTreeElementPointer getContainingElement(const EntityItemID& entityItemID) /*const*/; - void setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element); + void addEntityMapEntry(EntityItemPointer entity); + void clearEntityMapEntry(const EntityItemID& id); void debugDumpMap(); virtual void dumpTree() override; virtual void pruneTree() override; @@ -275,9 +273,8 @@ signals: protected: void processRemovedEntities(const DeleteEntityOperator& theOperator); - bool updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& properties, - EntityTreeElementPointer containingElement, - const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); + bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, + const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); static bool findNearPointOperation(const OctreeElementPointer& element, void* extraData); static bool findInSphereOperation(const OctreeElementPointer& element, void* extraData); static bool findInCubeOperation(const OctreeElementPointer& element, void* extraData); @@ -309,8 +306,8 @@ protected: _deletedEntityItemIDs << id; } - mutable QReadWriteLock _entityToElementLock; - QHash _entityToElementMap; + mutable QReadWriteLock _entityMapLock; + QHash _entityMap; EntitySimulationPointer _simulation; diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index cce7ee006f..4177724d1e 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -1001,7 +1001,6 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int if (currentContainingElement.get() != this) { currentContainingElement->removeEntityItem(entityItem); addEntityItem(entityItem); - _myTree->setContainingElement(entityItemID, getThisPointer()); } } } @@ -1032,9 +1031,9 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int // don't add if we've recently deleted.... if (!_myTree->isDeletedEntity(entityItem->getID())) { + _myTree->addEntityMapEntry(entityItem); addEntityItem(entityItem); // add this new entity to this elements entities entityItemID = entityItem->getEntityItemID(); - _myTree->setContainingElement(entityItemID, getThisPointer()); _myTree->postAddEntity(entityItem); if (entityItem->getCreated() == UNKNOWN_CREATED_TIME) { entityItem->recordCreationTime(); diff --git a/libraries/entities/src/MovingEntitiesOperator.cpp b/libraries/entities/src/MovingEntitiesOperator.cpp index ab97c67aa2..b9d5b681f6 100644 --- a/libraries/entities/src/MovingEntitiesOperator.cpp +++ b/libraries/entities/src/MovingEntitiesOperator.cpp @@ -51,7 +51,7 @@ MovingEntitiesOperator::~MovingEntitiesOperator() { void MovingEntitiesOperator::addEntityToMoveList(EntityItemPointer entity, const AACube& newCube) { - EntityTreeElementPointer oldContainingElement = _tree->getContainingElement(entity->getEntityItemID()); + EntityTreeElementPointer oldContainingElement = entity->getElement(); AABox newCubeClamped = newCube.clamp((float)-HALF_TREE_SCALE, (float)HALF_TREE_SCALE); if (_wantDebug) { @@ -201,7 +201,6 @@ bool MovingEntitiesOperator::preRecursion(const OctreeElementPointer& element) { oldElement->removeEntityItem(details.entity); } entityTreeElement->addEntityItem(details.entity); - _tree->setContainingElement(entityItemID, entityTreeElement); } _foundNewCount++; //details.newFound = true; // TODO: would be nice to add this optimization diff --git a/libraries/entities/src/UpdateEntityOperator.cpp b/libraries/entities/src/UpdateEntityOperator.cpp index ec6051af04..7a5c87187a 100644 --- a/libraries/entities/src/UpdateEntityOperator.cpp +++ b/libraries/entities/src/UpdateEntityOperator.cpp @@ -138,8 +138,8 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { qCDebug(entities) << " _foundNew=" << _foundNew; } - // If we haven't yet found the old entity, and this subTreeContains our old - // entity, then we need to keep searching. + // If we haven't yet found the old element, and this subTreeContains our old element, + // then we need to keep searching. if (!_foundOld && subtreeContainsOld) { if (_wantDebug) { @@ -169,7 +169,6 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { // NOTE: we know we haven't yet added it to its new element because _removeOld is true EntityTreeElementPointer oldElement = _existingEntity->getElement(); oldElement->removeEntityItem(_existingEntity); - _tree->setContainingElement(_entityItemID, NULL); if (oldElement != _containingElement) { qCDebug(entities) << "WARNING entity moved during UpdateEntityOperator recursion"; @@ -187,8 +186,8 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { } } - // If we haven't yet found the new entity, and this subTreeContains our new - // entity, then we need to keep searching. + // If we haven't yet found the new element, and this subTreeContains our new element, + // then we need to keep searching. if (!_foundNew && subtreeContainsNew) { if (_wantDebug) { @@ -221,7 +220,6 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { } } entityTreeElement->addEntityItem(_existingEntity); - _tree->setContainingElement(_entityItemID, entityTreeElement); } _foundNew = true; // we found the new element _removeOld = false; // and it has already been removed from the old From 39e5259e034e1251a857778baef83efcf75222a4 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 28 Jun 2017 16:01:12 -0700 Subject: [PATCH 07/19] remove unused variable --- libraries/entities/src/MovingEntitiesOperator.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/entities/src/MovingEntitiesOperator.cpp b/libraries/entities/src/MovingEntitiesOperator.cpp index b9d5b681f6..42e5a2ece5 100644 --- a/libraries/entities/src/MovingEntitiesOperator.cpp +++ b/libraries/entities/src/MovingEntitiesOperator.cpp @@ -193,7 +193,6 @@ bool MovingEntitiesOperator::preRecursion(const OctreeElementPointer& element) { // If this element is the best fit for the new bounds of this entity then add the entity to the element if (!details.newFound && entityTreeElement->bestFitBounds(details.newCube)) { - EntityItemID entityItemID = details.entity->getEntityItemID(); // remove from the old before adding EntityTreeElementPointer oldElement = details.entity->getElement(); if (oldElement != entityTreeElement) { From a0c6c49360373d0d0509498c7299069168ce7044 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 29 Jun 2017 10:20:47 -0700 Subject: [PATCH 08/19] bump some "debug" messages to "warning" status --- libraries/entities/src/EntityTree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 31b96ebfc4..5cb47d7a1e 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -188,7 +188,7 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti bool success; AACube queryCube = entity->getQueryAACube(success); if (!success) { - qCDebug(entities) << "Warning -- failed to get query-cube for" << entity->getID(); + qCWarning(entities) << "failed to get query-cube for" << entity->getID(); } UpdateEntityOperator theOperator(getThisPointer(), containingElement, entity, queryCube); recurseTreeWithOperator(&theOperator); @@ -1512,7 +1512,7 @@ void EntityTree::addEntityMapEntry(EntityItemPointer entity) { QWriteLocker locker(&_entityMapLock); EntityItemPointer otherEntity = _entityMap.value(id); if (otherEntity) { - qCDebug(entities) << "EntityTree::addEntityMapEntry() found pre-existing id " << id; + qCWarning(entities) << "EntityTree::addEntityMapEntry() found pre-existing id " << id; assert(false); return; } From fe79514b08eeeef0333abdefe7da8b058c342632 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 30 Jun 2017 10:22:16 -0700 Subject: [PATCH 09/19] lock when changing EntityTree elements --- libraries/entities/src/EntityTree.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 5cb47d7a1e..26dc678347 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -90,16 +90,17 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) { if (_simulation) { _simulation->clearEntities(); } - { - QWriteLocker locker(&_entityMapLock); - foreach(EntityItemPointer entity, _entityMap) { + QHash localMap; + localMap.swap(_entityMap); + this->withWriteLock([&] { + foreach(EntityItemPointer entity, localMap) { EntityTreeElementPointer element = entity->getElement(); if (element) { element->cleanupEntities(); } } - _entityMap.clear(); - } + }); + localMap.clear(); Octree::eraseAllOctreeElements(createNewRoot); resetClientEditStats(); From f71ef554b731eb2bcf701dffa2b6eae4d2d0aef1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 30 Jun 2017 10:23:33 -0700 Subject: [PATCH 10/19] use copy of _entityMap for debug logging --- libraries/entities/src/EntityTree.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 26dc678347..f0e94246b5 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1526,9 +1526,11 @@ void EntityTree::clearEntityMapEntry(const EntityItemID& id) { } void EntityTree::debugDumpMap() { + // QHash's are implicitly shared, so we make a shared copy and use that instead. + // This way we might be able to avoid both a lock and a true copy. + QHash localMap(_entityMap); qCDebug(entities) << "EntityTree::debugDumpMap() --------------------------"; - QReadLocker locker(&_entityMapLock); - QHashIterator i(_entityMap); + QHashIterator i(localMap); while (i.hasNext()) { i.next(); qCDebug(entities) << i.key() << ": " << i.value()->getElement().get(); From 02bc9b9610e9f9fab7243579019cb35b2167b1ac Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 30 Jun 2017 10:28:43 -0700 Subject: [PATCH 11/19] minimize the lock context --- libraries/entities/src/EntityTree.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index f0e94246b5..e359340a5f 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1500,8 +1500,11 @@ int EntityTree::processEraseMessageDetails(const QByteArray& dataByteArray, cons } EntityTreeElementPointer EntityTree::getContainingElement(const EntityItemID& entityItemID) /*const*/ { - QReadLocker locker(&_entityMapLock); - EntityItemPointer entity = _entityMap.value(entityItemID); + EntityItemPointer entity; + { + QReadLocker locker(&_entityMapLock); + entity = _entityMap.value(entityItemID); + } if (entity) { return entity->getElement(); } From 8fc4d1f43e7e6369223b94d8cf96ad1b71e83a57 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 3 Jul 2017 09:20:44 -0700 Subject: [PATCH 12/19] make UNEXPECTED logs warnings --- libraries/entities/src/EntityTree.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index e359340a5f..5e58736477 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -331,7 +331,7 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti // TODO: this final containingElement check should eventually be removed (or wrapped in an #ifdef DEBUG). if (!entity->getElement()) { - qCDebug(entities) << "UNEXPECTED!!!! after updateEntity() we no longer have a containing element??? entityID=" + qCWarning(entities) << "EntityTree::updateEntity() we no longer have a containing element for entityID=" << entity->getEntityItemID(); return false; } @@ -364,7 +364,7 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti // You should not call this on existing entities that are already part of the tree! Call updateEntity() EntityTreeElementPointer containingElement = getContainingElement(entityID); if (containingElement) { - qCDebug(entities) << "UNEXPECTED!!! ----- don't call addEntity() on existing entity items. entityID=" << entityID + qCWarning(entities) << "EntityTree::addEntity() on existing entity item with entityID=" << entityID << "containingElement=" << containingElement.get(); return result; } @@ -420,7 +420,7 @@ void EntityTree::deleteEntity(const EntityItemID& entityID, bool force, bool ign EntityTreeElementPointer containingElement = getContainingElement(entityID); if (!containingElement) { if (!ignoreWarnings) { - qCDebug(entities) << "UNEXPECTED!!!! EntityTree::deleteEntity() entityID doesn't exist!!! entityID=" << entityID; + qCWarning(entities) << "EntityTree::deleteEntity() on non-existent entityID=" << entityID; } return; } @@ -428,8 +428,7 @@ void EntityTree::deleteEntity(const EntityItemID& entityID, bool force, bool ign EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); if (!existingEntity) { if (!ignoreWarnings) { - qCDebug(entities) << "UNEXPECTED!!!! don't call EntityTree::deleteEntity() on entity items that don't exist. " - "entityID=" << entityID; + qCWarning(entities) << "EntityTree::deleteEntity() on non-existant entity item with entityID=" << entityID; } return; } @@ -476,7 +475,7 @@ void EntityTree::deleteEntities(QSet entityIDs, bool force, bool i EntityTreeElementPointer containingElement = getContainingElement(entityID); if (!containingElement) { if (!ignoreWarnings) { - qCDebug(entities) << "UNEXPECTED!!!! EntityTree::deleteEntities() entityID doesn't exist!!! entityID=" << entityID; + qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entityID=" << entityID; } continue; } @@ -484,8 +483,7 @@ void EntityTree::deleteEntities(QSet entityIDs, bool force, bool i EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); if (!existingEntity) { if (!ignoreWarnings) { - qCDebug(entities) << "UNEXPECTED!!!! don't call EntityTree::deleteEntities() on entity items that don't exist. " - "entityID=" << entityID; + qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entity item with entityID=" << entityID; } continue; } @@ -973,7 +971,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c const SharedNodePointer& senderNode) { if (!getIsServer()) { - qCDebug(entities) << "UNEXPECTED!!! processEditPacketData() should only be called on a server tree."; + qCWarning(entities) << "EntityTree::processEditPacketData() should only be called on a server tree."; return 0; } From c7ec82f98ab31e81a1224af5fe9d23684ec13fa3 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 3 Jul 2017 10:59:41 -0700 Subject: [PATCH 13/19] use local copy of _element for thread safety --- libraries/entities/src/EntityItem.cpp | 11 +++++++---- libraries/entities/src/EntityTreeElement.cpp | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 23ce097cc2..5996327e87 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -89,7 +89,8 @@ EntityItem::EntityItem(const EntityItemID& entityItemID) : EntityItem::~EntityItem() { // clear out any left-over actions - EntityTreePointer entityTree = _element ? _element->getTree() : nullptr; + EntityTreeElementPointer element = _element; // use local copy of _element for logic below + EntityTreePointer entityTree = element ? element->getTree() : nullptr; EntitySimulationPointer simulation = entityTree ? entityTree->getSimulation() : nullptr; if (simulation) { clearActions(simulation); @@ -880,8 +881,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef // Tracking for editing roundtrips here. We will tell our EntityTree that we just got incoming data about // and entity that was edited at some time in the past. The tree will determine how it wants to track this // information. - if (_element && _element->getTree()) { - _element->getTree()->trackIncomingEntityLastEdited(lastEditedFromBufferAdjusted, bytesRead); + EntityTreeElementPointer element = _element; // use local copy of _element for logic below + if (element && element->getTree()) { + element->getTree()->trackIncomingEntityLastEdited(lastEditedFromBufferAdjusted, bytesRead); } @@ -2056,7 +2058,8 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulationPoi _previouslyDeletedActions.insert(actionID, usecTimestampNow()); if (_objectActions.contains(actionID)) { if (!simulation) { - EntityTreePointer entityTree = _element ? _element->getTree() : nullptr; + EntityTreeElementPointer element = _element; // use local copy of _element for logic below + EntityTreePointer entityTree = element ? element->getTree() : nullptr; simulation = entityTree ? entityTree->getSimulation() : nullptr; } diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 4177724d1e..108cb39222 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -885,10 +885,10 @@ EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemI void EntityTreeElement::cleanupEntities() { withWriteLock([&] { foreach(EntityItemPointer entity, _entityItems) { + // NOTE: only EntityTreeElement should ever be changing the value of entity->_element // NOTE: We explicitly don't delete the EntityItem here because since we only // access it by smart pointers, when we remove it from the _entityItems // we know that it will be deleted. - //delete entity; entity->_element = NULL; } _entityItems.clear(); @@ -903,6 +903,7 @@ bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) { EntityItemPointer& entity = _entityItems[i]; if (entity->getEntityItemID() == id) { foundEntity = true; + // NOTE: only EntityTreeElement should ever be changing the value of entity->_element entity->_element = NULL; _entityItems.removeAt(i); break; @@ -918,6 +919,7 @@ bool EntityTreeElement::removeEntityItem(EntityItemPointer entity) { numEntries = _entityItems.removeAll(entity); }); if (numEntries > 0) { + // NOTE: only EntityTreeElement should ever be changing the value of entity->_element assert(entity->_element.get() == this); entity->_element = NULL; return true; From de2139106245304cf0703d12df63c8b91d89ec61 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Jul 2017 09:59:13 -0700 Subject: [PATCH 14/19] add script to measure cost of entity lookup by id --- .../tests/entityLookupCostMeasurement.js | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 scripts/developer/tests/entityLookupCostMeasurement.js diff --git a/scripts/developer/tests/entityLookupCostMeasurement.js b/scripts/developer/tests/entityLookupCostMeasurement.js new file mode 100644 index 0000000000..15697fe13d --- /dev/null +++ b/scripts/developer/tests/entityLookupCostMeasurement.js @@ -0,0 +1,104 @@ +// Creates a large number of entities on the cardinal planes of the octree (all +// objects will live in the root octree element). Measures how long it takes +// to update the properties of the first and last entity. The difference +// between the two measurements shows how the cost of lookup changes as a +// function of the number of entities. For best results run this in an +// otherwise empty domain. + +var firstId; +var lastId; +var NUM_ENTITIES_ON_SIDE = 25; + +// create the objects +createObjects = function () { + var STRIDE = 0.75; + var WIDTH = 0.5; + var DIMENSIONS = { x: WIDTH, y: WIDTH, z: WIDTH }; + var LIFETIME = 20; + + var properties = { + name: "", + type : "Box", + dimensions : DIMENSIONS, + position : { x: 0, y: 0, z: 0}, + lifetime : LIFETIME, + color : { red:255, green: 64, blue: 255 } + }; + + // xy + var planeName = "xy"; + for (var i = 0; i < NUM_ENTITIES_ON_SIDE; ++i) { + for (var j = 0; j < NUM_ENTITIES_ON_SIDE; ++j) { + properties.name = "Box-" + planeName + "-" + i + "." + j; + properties.position = { x: i * STRIDE, y: j * STRIDE, z: 0 }; + var red = i * 255 / NUM_ENTITIES_ON_SIDE; + var green = j * 255 / NUM_ENTITIES_ON_SIDE; + var blue = 0; + properties.color = { red: red, green: green, blue: blue }; + if (i == 0 && j == 0) { + firstId = Entities.addEntity(properties); + } else { + Entities.addEntity(properties); + } + } + } + + // yz + var planeName = "yz"; + for (var i = 0; i < NUM_ENTITIES_ON_SIDE; ++i) { + for (var j = 0; j < NUM_ENTITIES_ON_SIDE; ++j) { + properties.name = "Box-" + planeName + "-" + i + "." + j; + properties.position = { x: 0, y: i * STRIDE, z: j * STRIDE }; + var red = 0; + var green = i * 255 / NUM_ENTITIES_ON_SIDE; + var blue = j * 255 / NUM_ENTITIES_ON_SIDE; + properties.color = { red: red, green: green, blue: blue }; + Entities.addEntity(properties); + } + } + + // zx + var planeName = "zx"; + for (var i = 0; i < NUM_ENTITIES_ON_SIDE; ++i) { + for (var j = 0; j < NUM_ENTITIES_ON_SIDE; ++j) { + properties.name = "Box-" + planeName + "-" + i + "." + j; + properties.position = { x: j * STRIDE, y: 0, z: i * STRIDE }; + var red = j * 255 / NUM_ENTITIES_ON_SIDE; + var green = 0; + var blue = i * 255 / NUM_ENTITIES_ON_SIDE; + properties.color = { red: red, green: green, blue: blue }; + lastId = Entities.addEntity(properties); + } + } +}; + +createObjects(); + +// measure the time it takes to edit the first and last entities many times +// (requires a lookup by entityId each time) +changeProperties = function (id) { + var newProperties = { color : { red: 255, green: 255, blue: 255 } }; + Entities.editEntity(id, newProperties); +} + +// first +var NUM_CHANGES = 10000; +var firstStart = Date.now(); +for (var k = 0; k < NUM_CHANGES; ++k) { + changeProperties(firstId); +} +var firstEnd = Date.now(); +var firstDt = firstEnd - firstStart; + +// last +var lastStart = Date.now(); +for (var k = 0; k < NUM_CHANGES; ++k) { + changeProperties(lastId); +} +var lastEnd = Date.now(); +var lastDt = lastEnd - lastStart; + +// print the results +var numEntities = 3 * NUM_ENTITIES_ON_SIDE * NUM_ENTITIES_ON_SIDE; +print("numEntities = " + numEntities + " numEdits = " + NUM_CHANGES + " firstDt = " + firstDt + " lastDt = " + lastDt); + From 93d1725b3c87333ce3055cd4f748ff8086aa05a7 Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Fri, 14 Jul 2017 19:30:35 +0100 Subject: [PATCH 15/19] make cursor position change based on mouse click position --- interface/resources/qml/hifi/tablet/NewModelDialog.qml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/resources/qml/hifi/tablet/NewModelDialog.qml b/interface/resources/qml/hifi/tablet/NewModelDialog.qml index 5040c043f4..d47c981440 100644 --- a/interface/resources/qml/hifi/tablet/NewModelDialog.qml +++ b/interface/resources/qml/hifi/tablet/NewModelDialog.qml @@ -65,7 +65,8 @@ Rectangle { onClicked: { newModelDialog.keyboardEnabled = HMD.active parent.focus = true; - parent.forceActiveFocus() + parent.forceActiveFocus(); + modelURL.cursorPosition = modelURL.positionAt(mouseX, mouseY, TextInput.CursorBetweenCharaters); } } } From 974dff36b0ec6ecf0c5de1a34440a6a37bbe55a0 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Mon, 17 Jul 2017 13:03:56 -0700 Subject: [PATCH 16/19] Make hover overlays work for hand controller lasers --- interface/src/Application.cpp | 2 ++ .../entities-renderer/src/EntityTreeRenderer.cpp | 4 ++-- libraries/entities/src/HoverOverlayInterface.cpp | 8 ++++++++ libraries/entities/src/HoverOverlayInterface.h | 5 +++++ scripts/system/controllers/handControllerGrab.js | 11 +++++++++++ 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 707fc7b98b..8e487a8196 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2116,6 +2116,7 @@ void Application::initializeUi() { surfaceContext->setContextProperty("ApplicationCompositor", &getApplicationCompositor()); surfaceContext->setContextProperty("AvatarInputs", AvatarInputs::getInstance()); + surfaceContext->setContextProperty("HoverOverlay", DependencyManager::get().data()); if (auto steamClient = PluginManager::getInstance()->getSteamClientPlugin()) { surfaceContext->setContextProperty("Steam", new SteamScriptingInterface(engine, steamClient.get())); @@ -5818,6 +5819,7 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri auto entityScriptServerLog = DependencyManager::get(); scriptEngine->registerGlobalObject("EntityScriptServerLog", entityScriptServerLog.data()); scriptEngine->registerGlobalObject("AvatarInputs", AvatarInputs::getInstance()); + scriptEngine->registerGlobalObject("HoverOverlay", DependencyManager::get().data()); qScriptRegisterMetaType(scriptEngine, OverlayIDtoScriptValue, OverlayIDfromScriptValue); diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 475471b0fb..987d3118f7 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -464,12 +464,12 @@ void EntityTreeRenderer::connectSignalsToSlots(EntityScriptingInterface* entityS connect(this, &EntityTreeRenderer::clickReleaseOnEntity, entityScriptingInterface, &EntityScriptingInterface::clickReleaseOnEntity); connect(this, &EntityTreeRenderer::hoverEnterEntity, entityScriptingInterface, &EntityScriptingInterface::hoverEnterEntity); - connect(this, &EntityTreeRenderer::hoverEnterEntity, hoverOverlayInterface, &HoverOverlayInterface::createHoverOverlay); + connect(this, SIGNAL(hoverEnterEntity(const EntityItemID&, const PointerEvent&)), hoverOverlayInterface, SLOT(createHoverOverlay(const EntityItemID&, const PointerEvent&))); connect(this, &EntityTreeRenderer::hoverOverEntity, entityScriptingInterface, &EntityScriptingInterface::hoverOverEntity); connect(this, &EntityTreeRenderer::hoverLeaveEntity, entityScriptingInterface, &EntityScriptingInterface::hoverLeaveEntity); - connect(this, &EntityTreeRenderer::hoverLeaveEntity, hoverOverlayInterface, &HoverOverlayInterface::destroyHoverOverlay); + connect(this, SIGNAL(hoverLeaveEntity(const EntityItemID&, const PointerEvent&)), hoverOverlayInterface, SLOT(destroyHoverOverlay(const EntityItemID&, const PointerEvent&))); connect(this, &EntityTreeRenderer::enterEntity, entityScriptingInterface, &EntityScriptingInterface::enterEntity); connect(this, &EntityTreeRenderer::leaveEntity, entityScriptingInterface, &EntityScriptingInterface::leaveEntity); diff --git a/libraries/entities/src/HoverOverlayInterface.cpp b/libraries/entities/src/HoverOverlayInterface.cpp index 85b2738fc4..dcfde41e39 100644 --- a/libraries/entities/src/HoverOverlayInterface.cpp +++ b/libraries/entities/src/HoverOverlayInterface.cpp @@ -24,7 +24,15 @@ void HoverOverlayInterface::createHoverOverlay(const EntityItemID& entityItemID, setCurrentHoveredEntity(entityItemID); } +void HoverOverlayInterface::createHoverOverlay(const EntityItemID& entityItemID) { + HoverOverlayInterface::createHoverOverlay(entityItemID, PointerEvent()); +} + void HoverOverlayInterface::destroyHoverOverlay(const EntityItemID& entityItemID, const PointerEvent& event) { qCDebug(hover_overlay) << "Destroying Hover Overlay on top of entity with ID: " << entityItemID; setCurrentHoveredEntity(QUuid()); } + +void HoverOverlayInterface::destroyHoverOverlay(const EntityItemID& entityItemID) { + HoverOverlayInterface::destroyHoverOverlay(entityItemID, PointerEvent()); +} diff --git a/libraries/entities/src/HoverOverlayInterface.h b/libraries/entities/src/HoverOverlayInterface.h index 1e56cc03dd..a39faab819 100644 --- a/libraries/entities/src/HoverOverlayInterface.h +++ b/libraries/entities/src/HoverOverlayInterface.h @@ -22,6 +22,9 @@ #include "EntityTree.h" #include "HoverOverlayLogging.h" +/**jsdoc +* @namespace HoverOverlay +*/ class HoverOverlayInterface : public QObject, public Dependency { Q_OBJECT @@ -34,7 +37,9 @@ public: public slots: void createHoverOverlay(const EntityItemID& entityItemID, const PointerEvent& event); + void createHoverOverlay(const EntityItemID& entityItemID); void destroyHoverOverlay(const EntityItemID& entityItemID, const PointerEvent& event); + void destroyHoverOverlay(const EntityItemID& entityItemID); private: bool _verboseLogging { true }; diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index 04921fe14d..fae3e91c01 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -187,6 +187,8 @@ var DEFAULT_GRABBABLE_DATA = { var USE_BLACKLIST = true; var blacklist = []; +var entitiesWithHoverOverlays = []; + var FORBIDDEN_GRAB_NAMES = ["Grab Debug Entity", "grab pointer"]; var FORBIDDEN_GRAB_TYPES = ["Unknown", "Light", "PolyLine", "Zone"]; @@ -2201,6 +2203,15 @@ function MyController(hand) { entityPropertiesCache.addEntity(rayPickInfo.entityID); } + if (rayPickInfo.entityID && entitiesWithHoverOverlays.indexOf(rayPickInfo.entityID) == -1) { + entitiesWithHoverOverlays.forEach(function (element) { + HoverOverlay.destroyHoverOverlay(element); + }); + entitiesWithHoverOverlays = []; + HoverOverlay.createHoverOverlay(rayPickInfo.entityID); + entitiesWithHoverOverlays.push(rayPickInfo.entityID); + } + var candidateHotSpotEntities = Entities.findEntities(handPosition, MAX_EQUIP_HOTSPOT_RADIUS); entityPropertiesCache.addEntities(candidateHotSpotEntities); From a37f0137e7529cc31e38e14bcadec5eb5bc3a7d0 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Mon, 17 Jul 2017 14:29:43 -0700 Subject: [PATCH 17/19] Destroy overlays when trigger released --- scripts/system/controllers/handControllerGrab.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index fae3e91c01..78c4b2960e 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -3774,6 +3774,11 @@ function MyController(hand) { this.release = function() { this.turnOffVisualizations(); + entitiesWithHoverOverlays.forEach(function (element) { + HoverOverlay.destroyHoverOverlay(element); + }); + entitiesWithHoverOverlays = []; + if (this.grabbedThingID !== null) { Messages.sendMessage('Hifi-Teleport-Ignore-Remove', this.grabbedThingID); From f5abb4a090f2b413ca36a1e2599096d18bb19e85 Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Mon, 17 Jul 2017 23:18:16 +0100 Subject: [PATCH 18/19] trying new tablet position spwan --- scripts/system/libraries/WebTablet.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/libraries/WebTablet.js b/scripts/system/libraries/WebTablet.js index 142ed6e7b6..53f88ea62d 100644 --- a/scripts/system/libraries/WebTablet.js +++ b/scripts/system/libraries/WebTablet.js @@ -76,7 +76,7 @@ function calcSpawnInfo(hand, tabletHeight) { var TABLET_RAKE_ANGLE = 30; rotation = Quat.multiply(Quat.angleAxis(TABLET_RAKE_ANGLE, Vec3.multiplyQbyV(lookAt, Vec3.UNIT_X)), lookAt); - var RELATIVE_SPAWN_OFFSET = { x: 0, y: 0.4, z: 0.05 }; + var RELATIVE_SPAWN_OFFSET = { x: 0, y: 0.6, z: 0.1 }; position = Vec3.sum(position, Vec3.multiplyQbyV(rotation, Vec3.multiply(tabletHeight, RELATIVE_SPAWN_OFFSET))); return { From ff51fa71990153733ca61eeed396a8ed5e08ba36 Mon Sep 17 00:00:00 2001 From: NeetBhagat Date: Tue, 18 Jul 2017 10:42:01 +0530 Subject: [PATCH 19/19] Resolved review comments. --- interface/src/avatar/MyAvatar.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 6ccc0ab516..5c56d2607c 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2276,15 +2276,16 @@ void MyAvatar::restrictScaleFromDomainSettings(const QJsonObject& domainSettings settings.beginGroup("Avatar"); _targetScale = loadSetting(settings, "scale", 1.0f); - qCDebug(interfaceapp, "This domain requires a minimum avatar scale of %f and a maximum avatar scale of %f. Current avatar scale is %f", - _domainMinimumScale, _domainMaximumScale, _targetScale); + qCDebug(interfaceapp) << "This domain requires a minimum avatar scale of " << _domainMinimumScale + << " and a maximum avatar scale of " << _domainMaximumScale + << ". Current avatar scale is " << _targetScale; // debug to log if this avatar's scale in this domain will be clamped float clampedScale = glm::clamp(_targetScale, _domainMinimumScale, _domainMaximumScale); if (_targetScale != clampedScale) { - qCDebug(interfaceapp, "Current avatar scale is clamped to %f because %f is not allowed by current domain", - clampedScale, _targetScale); + qCDebug(interfaceapp) << "Current avatar scale is clamped to " << clampedScale + << " because " << _targetScale << " is not allowed by current domain"; // The current scale of avatar should not be more than domain's max_avatar_scale and not less than domain's min_avatar_scale . _targetScale = clampedScale; }