From 688ee68cf9309a8c8733450e9ed5969f3906d9a3 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 13 Sep 2017 21:38:54 -0700 Subject: [PATCH] Ensure thread-safety for model pointer access --- .../src/RenderableModelEntityItem.cpp | 201 ++++++++++-------- .../src/RenderableModelEntityItem.h | 21 +- 2 files changed, 131 insertions(+), 91 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index c6bad008e4..2508b598af 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -36,6 +36,29 @@ static CollisionRenderMeshCache collisionMeshCache; +void ModelEntityWrapper::setModel(const ModelPointer& model) { + withWriteLock([&] { + if (_model != model) { + _model = model; + if (_model) { + _needsInitialSimulation = true; + } + } + }); +} + +ModelPointer ModelEntityWrapper::getModel() const { + return resultWithReadLock([&] { + return _model; + }); +} + +bool ModelEntityWrapper::isModelLoaded() const { + return resultWithReadLock([&] { + return _model.operator bool() && _model->isLoaded(); + }); +} + EntityItemPointer RenderableModelEntityItem::factory(const EntityItemID& entityID, const EntityItemProperties& properties) { EntityItemPointer entity{ new RenderableModelEntityItem(entityID, properties.getDimensionsInitialized()) }; entity->setProperties(properties); @@ -43,7 +66,7 @@ EntityItemPointer RenderableModelEntityItem::factory(const EntityItemID& entityI } RenderableModelEntityItem::RenderableModelEntityItem(const EntityItemID& entityItemID, bool dimensionsInitialized) : - ModelEntityItem(entityItemID), + ModelEntityWrapper(entityItemID), _dimensionsInitialized(dimensionsInitialized) { } @@ -83,41 +106,47 @@ QVariantMap parseTexturesToMap(QString textures, const QVariantMap& defaultTextu } void RenderableModelEntityItem::doInitialModelSimulation() { + ModelPointer model = getModel(); + if (!model) { + return; + } // The machinery for updateModelBounds will give existing models the opportunity to fix their // translation/rotation/scale/registration. The first two are straightforward, but the latter two have guards to // make sure they don't happen after they've already been set. Here we reset those guards. This doesn't cause the // entity values to change -- it just allows the model to match once it comes in. - _model->setScaleToFit(false, getDimensions()); - _model->setSnapModelToRegistrationPoint(false, getRegistrationPoint()); + model->setScaleToFit(false, getDimensions()); + model->setSnapModelToRegistrationPoint(false, getRegistrationPoint()); // now recalculate the bounds and registration - _model->setScaleToFit(true, getDimensions()); - _model->setSnapModelToRegistrationPoint(true, getRegistrationPoint()); - _model->setRotation(getRotation()); - _model->setTranslation(getPosition()); + model->setScaleToFit(true, getDimensions()); + model->setSnapModelToRegistrationPoint(true, getRegistrationPoint()); + model->setRotation(getRotation()); + model->setTranslation(getPosition()); { - PerformanceTimer perfTimer("_model->simulate"); - _model->simulate(0.0f); + PerformanceTimer perfTimer("model->simulate"); + model->simulate(0.0f); } _needsInitialSimulation = false; } void RenderableModelEntityItem::autoResizeJointArrays() { - if (_model && _model->isLoaded() && !_needsInitialSimulation) { - resizeJointArrays(_model->getJointStateCount()); + ModelPointer model = getModel(); + if (model && model->isLoaded() && !_needsInitialSimulation) { + resizeJointArrays(model->getJointStateCount()); } } bool RenderableModelEntityItem::needsUpdateModelBounds() const { - if (!hasModel() || !_model) { + ModelPointer model = getModel(); + if (!hasModel() || !model) { return false; } - if (!_dimensionsInitialized || !_model->isActive()) { + if (!_dimensionsInitialized || !model->isActive()) { return false; } - if (_model->needsReload()) { + if (model->needsReload()) { return true; } @@ -129,21 +158,21 @@ bool RenderableModelEntityItem::needsUpdateModelBounds() const { return true; } - if (_model->getScaleToFitDimensions() != getDimensions()) { + if (model->getScaleToFitDimensions() != getDimensions()) { return true; } - if (_model->getRegistrationPoint() != getRegistrationPoint()) { + if (model->getRegistrationPoint() != getRegistrationPoint()) { return true; } bool success; auto transform = getTransform(success); if (success) { - if (_model->getTranslation() != transform.getTranslation()) { + if (model->getTranslation() != transform.getTranslation()) { return true; } - if (_model->getRotation() != transform.getRotation()) { + if (model->getRotation() != transform.getRotation()) { return true; } } @@ -158,16 +187,6 @@ void RenderableModelEntityItem::updateModelBounds() { } } -void RenderableModelEntityItem::setModel(const ModelPointer& model) { - withWriteLock([&] { - if (_model != model) { - _model = model; - if (_model) { - _needsInitialSimulation = true; - } - } - }); -} EntityItemProperties RenderableModelEntityItem::getProperties(EntityPropertyFlags desiredProperties) const { EntityItemProperties properties = ModelEntityItem::getProperties(desiredProperties); // get the properties from our base class @@ -175,43 +194,44 @@ EntityItemProperties RenderableModelEntityItem::getProperties(EntityPropertyFlag properties.setTextureNames(_originalTextures); } - if (_model) { - properties.setRenderInfoVertexCount(_model->getRenderInfoVertexCount()); - properties.setRenderInfoTextureCount(_model->getRenderInfoTextureCount()); - properties.setRenderInfoTextureSize(_model->getRenderInfoTextureSize()); - properties.setRenderInfoDrawCalls(_model->getRenderInfoDrawCalls()); - properties.setRenderInfoHasTransparent(_model->getRenderInfoHasTransparent()); + ModelPointer model = getModel(); + if (model) { + properties.setRenderInfoVertexCount(model->getRenderInfoVertexCount()); + properties.setRenderInfoTextureCount(model->getRenderInfoTextureCount()); + properties.setRenderInfoTextureSize(model->getRenderInfoTextureSize()); + properties.setRenderInfoDrawCalls(model->getRenderInfoDrawCalls()); + properties.setRenderInfoHasTransparent(model->getRenderInfoHasTransparent()); + + if (model->isLoaded()) { + // TODO: improve naturalDimensions in the future, + // for now we've added this hack for setting natural dimensions of models + Extents meshExtents = model->getFBXGeometry().getUnscaledMeshExtents(); + properties.setNaturalDimensions(meshExtents.maximum - meshExtents.minimum); + properties.calculateNaturalPosition(meshExtents.minimum, meshExtents.maximum); + } } - if (_model && _model->isLoaded()) { - // TODO: improve naturalDimensions in the future, - // for now we've added this hack for setting natural dimensions of models - Extents meshExtents = _model->getFBXGeometry().getUnscaledMeshExtents(); - properties.setNaturalDimensions(meshExtents.maximum - meshExtents.minimum); - properties.calculateNaturalPosition(meshExtents.minimum, meshExtents.maximum); - } return properties; } bool RenderableModelEntityItem::supportsDetailedRayIntersection() const { - return resultWithReadLock([&] { - return _model && _model->isLoaded(); - }); + return isModelLoaded(); } bool RenderableModelEntityItem::findDetailedRayIntersection(const glm::vec3& origin, const glm::vec3& direction, bool& keepSearching, OctreeElementPointer& element, float& distance, BoxFace& face, glm::vec3& surfaceNormal, void** intersectedObject, bool precisionPicking) const { - if (!_model) { + auto model = getModel(); + if (!model) { return true; } // qCDebug(entitiesrenderer) << "RenderableModelEntityItem::findDetailedRayIntersection() precisionPicking:" // << precisionPicking; QString extraInfo; - return _model->findRayIntersectionAgainstSubMeshes(origin, direction, distance, + return model->findRayIntersectionAgainstSubMeshes(origin, direction, distance, face, surfaceNormal, extraInfo, precisionPicking, false); } @@ -242,7 +262,7 @@ void RenderableModelEntityItem::setCompoundShapeURL(const QString& url) { // parse it twice. auto currentCompoundShapeURL = getCompoundShapeURL(); ModelEntityItem::setCompoundShapeURL(url); - if (getCompoundShapeURL() != currentCompoundShapeURL || !_model) { + if (getCompoundShapeURL() != currentCompoundShapeURL || !getModel()) { if (getShapeType() == SHAPE_TYPE_COMPOUND) { getCollisionGeometryResource(); } @@ -252,17 +272,18 @@ void RenderableModelEntityItem::setCompoundShapeURL(const QString& url) { bool RenderableModelEntityItem::isReadyToComputeShape() const { ShapeType type = getShapeType(); + auto model = getModel(); if (type == SHAPE_TYPE_COMPOUND) { - if (!_model || getCompoundShapeURL().isEmpty()) { + if (!model || getCompoundShapeURL().isEmpty()) { return false; } - if (_model->getURL().isEmpty()) { + if (model->getURL().isEmpty()) { // we need a render geometry with a scale to proceed, so give up. return false; } - if (_model->isLoaded()) { + if (model->isLoaded()) { if (!getCompoundShapeURL().isEmpty() && !_compoundShapeResource) { const_cast(this)->getCollisionGeometryResource(); } @@ -281,7 +302,7 @@ bool RenderableModelEntityItem::isReadyToComputeShape() const { // the model is still being downloaded. return false; } else if (type >= SHAPE_TYPE_SIMPLE_HULL && type <= SHAPE_TYPE_STATIC_MESH) { - return (_model && _model->isLoaded()); + return isModelLoaded(); } return true; } @@ -292,6 +313,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) { ShapeType type = getShapeType(); glm::vec3 dimensions = getDimensions(); + auto model = getModel(); if (type == SHAPE_TYPE_COMPOUND) { updateModelBounds(); @@ -373,14 +395,14 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) { // to the visual model and apply them to the collision model (without regard for the // collision model's extents). - glm::vec3 scaleToFit = dimensions / _model->getFBXGeometry().getUnscaledMeshExtents().size(); + glm::vec3 scaleToFit = dimensions / model->getFBXGeometry().getUnscaledMeshExtents().size(); // multiply each point by scale before handing the point-set off to the physics engine. // also determine the extents of the collision model. glm::vec3 registrationOffset = dimensions * (ENTITY_ITEM_DEFAULT_REGISTRATION_POINT - getRegistrationPoint()); for (int32_t i = 0; i < pointCollection.size(); i++) { for (int32_t j = 0; j < pointCollection[i].size(); j++) { // back compensate for registration so we can apply that offset to the shapeInfo later - pointCollection[i][j] = scaleToFit * (pointCollection[i][j] + _model->getOffset()) - registrationOffset; + pointCollection[i][j] = scaleToFit * (pointCollection[i][j] + model->getOffset()) - registrationOffset; } } shapeInfo.setParams(type, dimensions, getCompoundShapeURL()); @@ -389,11 +411,11 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) { assert(_model && _model->isLoaded()); updateModelBounds(); - _model->updateGeometry(); + model->updateGeometry(); // compute meshPart local transforms QVector localTransforms; - const FBXGeometry& fbxGeometry = _model->getFBXGeometry(); + const FBXGeometry& fbxGeometry = model->getFBXGeometry(); int numFbxMeshes = fbxGeometry.meshes.size(); int totalNumVertices = 0; glm::mat4 invRegistraionOffset = glm::translate(dimensions * (getRegistrationPoint() - ENTITY_ITEM_DEFAULT_REGISTRATION_POINT)); @@ -401,7 +423,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) { const FBXMesh& mesh = fbxGeometry.meshes.at(i); if (mesh.clusters.size() > 0) { const FBXCluster& cluster = mesh.clusters.at(0); - auto jointMatrix = _model->getRig().getJointTransform(cluster.jointIndex); + auto jointMatrix = model->getRig().getJointTransform(cluster.jointIndex); // we backtranslate by the registration offset so we can apply that offset to the shapeInfo later localTransforms.push_back(invRegistraionOffset * jointMatrix * cluster.inverseBindMatrix); } else { @@ -417,7 +439,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) { return; } - auto& meshes = _model->getGeometry()->getMeshes(); + auto& meshes = model->getGeometry()->getMeshes(); int32_t numMeshes = (int32_t)(meshes.size()); const int MAX_ALLOWED_MESH_COUNT = 1000; @@ -631,7 +653,8 @@ void RenderableModelEntityItem::setCollisionShape(const btCollisionShape* shape) } bool RenderableModelEntityItem::contains(const glm::vec3& point) const { - if (EntityItem::contains(point) && _model && _compoundShapeResource && _compoundShapeResource->isLoaded()) { + auto model = getModel(); + if (EntityItem::contains(point) && model && _compoundShapeResource && _compoundShapeResource->isLoaded()) { return _compoundShapeResource->getFBXGeometry().convexHullContains(worldToEntity(point)); } @@ -639,11 +662,12 @@ bool RenderableModelEntityItem::contains(const glm::vec3& point) const { } bool RenderableModelEntityItem::shouldBePhysical() const { + auto model = getModel(); // If we have a model, make sure it hasn't failed to download. // If it has, we'll report back that we shouldn't be physical so that physics aren't held waiting for us to be ready. - if (_model && getShapeType() == SHAPE_TYPE_COMPOUND && _model->didCollisionGeometryRequestFail()) { + if (model && getShapeType() == SHAPE_TYPE_COMPOUND && model->didCollisionGeometryRequestFail()) { return false; - } else if (_model && getShapeType() != SHAPE_TYPE_NONE && _model->didVisualGeometryRequestFail()) { + } else if (model && getShapeType() != SHAPE_TYPE_NONE && model->didVisualGeometryRequestFail()) { return false; } else { return ModelEntityItem::shouldBePhysical(); @@ -651,9 +675,10 @@ bool RenderableModelEntityItem::shouldBePhysical() const { } glm::quat RenderableModelEntityItem::getAbsoluteJointRotationInObjectFrame(int index) const { - if (_model) { + auto model = getModel(); + if (model) { glm::quat result; - if (_model->getAbsoluteJointRotationInRigFrame(index, result)) { + if (model->getAbsoluteJointRotationInRigFrame(index, result)) { return result; } } @@ -661,9 +686,10 @@ glm::quat RenderableModelEntityItem::getAbsoluteJointRotationInObjectFrame(int i } glm::vec3 RenderableModelEntityItem::getAbsoluteJointTranslationInObjectFrame(int index) const { - if (_model) { + auto model = getModel(); + if (model) { glm::vec3 result; - if (_model->getAbsoluteJointTranslationInRigFrame(index, result)) { + if (model->getAbsoluteJointTranslationInRigFrame(index, result)) { return result; } } @@ -671,10 +697,11 @@ glm::vec3 RenderableModelEntityItem::getAbsoluteJointTranslationInObjectFrame(in } bool RenderableModelEntityItem::setAbsoluteJointRotationInObjectFrame(int index, const glm::quat& rotation) { - if (!_model) { + auto model = getModel(); + if (!model) { return false; } - const Rig& rig = _model->getRig(); + const Rig& rig = model->getRig(); int jointParentIndex = rig.getJointParentIndex(index); if (jointParentIndex == -1) { return setLocalJointRotation(index, rotation); @@ -700,10 +727,11 @@ bool RenderableModelEntityItem::setAbsoluteJointRotationInObjectFrame(int index, } bool RenderableModelEntityItem::setAbsoluteJointTranslationInObjectFrame(int index, const glm::vec3& translation) { - if (!_model) { + auto model = getModel(); + if (!model) { return false; } - const Rig& rig = _model->getRig(); + const Rig& rig = model->getRig(); int jointParentIndex = rig.getJointParentIndex(index); if (jointParentIndex == -1) { @@ -730,9 +758,10 @@ bool RenderableModelEntityItem::setAbsoluteJointTranslationInObjectFrame(int ind } glm::quat RenderableModelEntityItem::getLocalJointRotation(int index) const { - if (_model) { + auto model = getModel(); + if (model) { glm::quat result; - if (_model->getJointRotation(index, result)) { + if (model->getJointRotation(index, result)) { return result; } } @@ -740,9 +769,10 @@ glm::quat RenderableModelEntityItem::getLocalJointRotation(int index) const { } glm::vec3 RenderableModelEntityItem::getLocalJointTranslation(int index) const { - if (_model) { + auto model = getModel(); + if (model) { glm::vec3 result; - if (_model->getJointTranslation(index, result)) { + if (model->getJointTranslation(index, result)) { return result; } } @@ -810,19 +840,22 @@ void RenderableModelEntityItem::setJointTranslationsSet(const QVector& tra void RenderableModelEntityItem::locationChanged(bool tellPhysics) { PerformanceTimer pertTimer("locationChanged"); EntityItem::locationChanged(tellPhysics); - if (_model && _model->isLoaded()) { - _model->updateRenderItems(); + auto model = getModel(); + if (model && model->isLoaded()) { + model->updateRenderItems(); } } int RenderableModelEntityItem::getJointIndex(const QString& name) const { - return (_model && _model->isActive()) ? _model->getRig().indexOfJoint(name) : -1; + auto model = getModel(); + return (model && model->isActive()) ? model->getRig().indexOfJoint(name) : -1; } QStringList RenderableModelEntityItem::getJointNames() const { QStringList result; - if (_model && _model->isActive()) { - const Rig& rig = _model->getRig(); + auto model = getModel(); + if (model && model->isActive()) { + const Rig& rig = model->getRig(); int jointCount = rig.getJointStateCount(); for (int jointIndex = 0; jointIndex < jointCount; jointIndex++) { result << rig.nameOfJoint(jointIndex); @@ -832,19 +865,16 @@ QStringList RenderableModelEntityItem::getJointNames() const { } bool RenderableModelEntityItem::getMeshes(MeshProxyList& result) { - if (!_model || !_model->isLoaded()) { + auto model = getModel(); + if (!model || !model->isLoaded()) { return false; } - BLOCKING_INVOKE_METHOD(_model.get(), "getMeshes", Q_RETURN_ARG(MeshProxyList, result)); + BLOCKING_INVOKE_METHOD(model.get(), "getMeshes", Q_RETURN_ARG(MeshProxyList, result)); return !result.isEmpty(); } void RenderableModelEntityItem::copyAnimationJointDataToModel() { - ModelPointer model; - withReadLock([&] { - model = _model; - }); - + auto model = getModel(); if (!model || !model->isLoaded()) { return; } @@ -859,7 +889,7 @@ void RenderableModelEntityItem::copyAnimationJointDataToModel() { jointData.rotationDirty = false; } if (jointData.translationDirty) { - _model->setJointTranslation(index, true, jointData.joint.translation, 1.0f); + model->setJointTranslation(index, true, jointData.joint.translation, 1.0f); jointData.translationDirty = false; } } @@ -1192,7 +1222,6 @@ void ModelEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& sce // When the individual mesh parts of a model finish fading, they will mark their Model as needing updating // we will watch for that and ask the model to update it's render items if (model->getRenderItemsNeedUpdate()) { - qDebug() << "QQQ" << __FUNCTION__ << "Update model render items" << model->getURL(); model->updateRenderItems(); } diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.h b/libraries/entities-renderer/src/RenderableModelEntityItem.h index d8ecbeb282..b9c751761d 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.h +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.h @@ -34,10 +34,24 @@ class ModelEntityRenderer; } } //#define MODEL_ENTITY_USE_FADE_EFFECT - -class RenderableModelEntityItem : public ModelEntityItem { +class ModelEntityWrapper : public ModelEntityItem { + using Parent = ModelEntityItem; friend class render::entities::ModelEntityRenderer; +protected: + ModelEntityWrapper(const EntityItemID& entityItemID) : Parent(entityItemID) {} + void setModel(const ModelPointer& model); + ModelPointer getModel() const; + bool isModelLoaded() const; + + bool _needsInitialSimulation{ true }; +private: + ModelPointer _model; +}; + +class RenderableModelEntityItem : public ModelEntityWrapper { + friend class render::entities::ModelEntityRenderer; + using Parent = ModelEntityWrapper; public: static EntityItemPointer factory(const EntityItemID& entityID, const EntityItemProperties& properties); @@ -97,14 +111,11 @@ private: bool isAnimatingSomething() const; void autoResizeJointArrays(); void copyAnimationJointDataToModel(); - void setModel(const ModelPointer& model); void getCollisionGeometryResource(); GeometryResource::Pointer _compoundShapeResource; bool _originalTexturesRead { false }; QVariantMap _originalTextures; - ModelPointer _model; - bool _needsInitialSimulation { true }; bool _dimensionsInitialized { true }; bool _needsJointSimulation { false }; bool _showCollisionGeometry { false };