Merge pull request #15880 from AndrewMeadows/fix-safe-landing-redux

BUGZ-863: wait for objects to be added to physics before completing SafeLanding

interface/src/avatar/OtherAvatar.cpp

@@ -201,6 +201,7 @@ void OtherAvatar::computeShapeLOD() {
         break;
     case workload::Region::UNKNOWN:
     case workload::Region::INVALID:
+    case workload::Region::R4:
     case workload::Region::R3:
     default:
         newLOD = BodyLOD::Sphere;

interface/src/octree/SafeLanding.cpp

@@ -203,8 +203,22 @@ bool SafeLanding::isEntityPhysicsReady(const EntityItemPointer& entity) {
             if (hasAABox && downloadedCollisionTypes.count(modelEntity->getShapeType()) != 0) {
                 auto space = _entityTreeRenderer->getWorkloadSpace();
                 uint8_t region = space ? space->getRegion(entity->getSpaceIndex()) : (uint8_t)workload::Region::INVALID;
-                bool shouldBePhysical = region < workload::Region::R3 && entity->shouldBePhysical();
+
-                return (!shouldBePhysical || entity->isInPhysicsSimulation() || modelEntity->computeShapeFailedToLoad());
+                // Note: the meanings of the workload regions are:
+                //   R1 = in physics simulation and willing to own simulation
+                //   R2 = in physics simulation but does NOT want to own simulation
+                //   R3 = not in physics simulation but kinematically animated when velocities are non-zero
+                //   R4 = sorted by workload and found to be outside R3
+                //   UNKNOWN = known to workload but not yet sorted
+                //   INVALID = not known to workload
+                // So any entity sorted into R3 or R4 is definitelyNotPhysical
+
+                bool definitelyNotPhysical = region == workload::Region::R3 ||
+                    region == workload::Region::R4 ||
+                    !entity->shouldBePhysical() ||
+                    modelEntity->unableToLoadCollisionShape();
+                bool definitelyPhysical = entity->isInPhysicsSimulation();
+                return definitelyNotPhysical || definitelyPhysical;
             }
         }
     }

libraries/entities-renderer/src/RenderableModelEntityItem.cpp

@@ -282,27 +282,26 @@ bool RenderableModelEntityItem::findDetailedParabolaIntersection(const glm::vec3
 }
 
 void RenderableModelEntityItem::fetchCollisionGeometryResource() {
-    _compoundShapeResource = DependencyManager::get<ModelCache>()->getCollisionGeometryResource(getCollisionShapeURL());
+    _collisionGeometryResource = DependencyManager::get<ModelCache>()->getCollisionGeometryResource(getCollisionShapeURL());
 }
 
-bool RenderableModelEntityItem::computeShapeFailedToLoad() {
+bool RenderableModelEntityItem::unableToLoadCollisionShape() {
-    if (!_compoundShapeResource) {
+    if (!_collisionGeometryResource) {
         fetchCollisionGeometryResource();
     }
-
+    return (_collisionGeometryResource && _collisionGeometryResource->isFailed());
-    return (_compoundShapeResource && _compoundShapeResource->isFailed());
 }
 
 void RenderableModelEntityItem::setShapeType(ShapeType type) {
     ModelEntityItem::setShapeType(type);
     auto shapeType = getShapeType();
     if (shapeType == SHAPE_TYPE_COMPOUND || shapeType == SHAPE_TYPE_SIMPLE_COMPOUND) {
-        if (!_compoundShapeResource && !getCollisionShapeURL().isEmpty()) {
+        if (!_collisionGeometryResource && !getCollisionShapeURL().isEmpty()) {
             fetchCollisionGeometryResource();
         }
-    } else if (_compoundShapeResource && !getCompoundShapeURL().isEmpty()) {
+    } else if (_collisionGeometryResource && !getCompoundShapeURL().isEmpty()) {
         // the compoundURL has been set but the shapeType does not agree
-        _compoundShapeResource.reset();
+        _collisionGeometryResource.reset();
     }
 }
 
@@ -333,11 +332,11 @@ bool RenderableModelEntityItem::isReadyToComputeShape() const {
         }
 
         if (model->isLoaded()) {
-            if (!shapeURL.isEmpty() && !_compoundShapeResource) {
+            if (!shapeURL.isEmpty() && !_collisionGeometryResource) {
                 const_cast<RenderableModelEntityItem*>(this)->fetchCollisionGeometryResource();
             }
 
-            if (_compoundShapeResource && _compoundShapeResource->isLoaded()) {
+            if (_collisionGeometryResource && _collisionGeometryResource->isLoaded()) {
                 // we have both URLs AND both geometries AND they are both fully loaded.
                 if (_needsInitialSimulation) {
                     // the _model's offset will be wrong until _needsInitialSimulation is false
@@ -368,7 +367,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) {
     }
 
     if (type == SHAPE_TYPE_COMPOUND) {
-        if (!_compoundShapeResource || !_compoundShapeResource->isLoaded()) {
+        if (!_collisionGeometryResource || !_collisionGeometryResource->isLoaded()) {
             return;
         }
 
@@ -376,8 +375,8 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) {
 
         // should never fall in here when collision model not fully loaded
         // TODO: assert that all geometries exist and are loaded
-        //assert(_model && _model->isLoaded() && _compoundShapeResource && _compoundShapeResource->isLoaded());
+        //assert(_model && _model->isLoaded() && _collisionGeometryResource && _collisionGeometryResource->isLoaded());
-        const HFMModel& collisionGeometry = _compoundShapeResource->getHFMModel();
+        const HFMModel& collisionGeometry = _collisionGeometryResource->getHFMModel();
 
         ShapeInfo::PointCollection& pointCollection = shapeInfo.getPointCollection();
         pointCollection.clear();
@@ -499,7 +498,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) {
 
         std::vector<std::shared_ptr<const graphics::Mesh>> meshes;
         if (type == SHAPE_TYPE_SIMPLE_COMPOUND) {
-            auto& hfmMeshes = _compoundShapeResource->getHFMModel().meshes;
+            auto& hfmMeshes = _collisionGeometryResource->getHFMModel().meshes;
             meshes.reserve(hfmMeshes.size());
             for (auto& hfmMesh : hfmMeshes) {
                 meshes.push_back(hfmMesh._mesh);
@@ -727,10 +726,10 @@ int RenderableModelEntityItem::avatarJointIndex(int modelJointIndex) {
 
 bool RenderableModelEntityItem::contains(const glm::vec3& point) const {
     auto model = getModel();
-    if (EntityItem::contains(point) && model && _compoundShapeResource && _compoundShapeResource->isLoaded()) {
+    if (EntityItem::contains(point) && model && _collisionGeometryResource && _collisionGeometryResource->isLoaded()) {
         glm::mat4 worldToHFMMatrix = model->getWorldToHFMMatrix();
         glm::vec3 hfmPoint = worldToHFMMatrix * glm::vec4(point, 1.0f);
-        return _compoundShapeResource->getHFMModel().convexHullContains(hfmPoint);
+        return _collisionGeometryResource->getHFMModel().convexHullContains(hfmPoint);
     }
 
     return false;

libraries/entities-renderer/src/RenderableModelEntityItem.h

@@ -79,7 +79,7 @@ public:
 
     virtual bool isReadyToComputeShape() const override;
     virtual void computeShapeInfo(ShapeInfo& shapeInfo) override;
-    bool computeShapeFailedToLoad();
+    bool unableToLoadCollisionShape();
 
     virtual bool contains(const glm::vec3& point) const override;
     void stopModelOverrideIfNoParent();
@@ -120,7 +120,7 @@ private:
     bool readyToAnimate() const;
     void fetchCollisionGeometryResource();
 
-    GeometryResource::Pointer _compoundShapeResource;
+    GeometryResource::Pointer _collisionGeometryResource;
     std::vector<int> _jointMap;
     QVariantMap _originalTextures;
     bool _jointMapCompleted { false };

libraries/physics/src/PhysicalEntitySimulation.cpp

@@ -49,9 +49,9 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) {
     assert(entity);
     assert(!entity->isDead());
     uint8_t region = _space->getRegion(entity->getSpaceIndex());
-    bool shouldBePhysical = region < workload::Region::R3 && entity->shouldBePhysical();
+    bool maybeShouldBePhysical = (region < workload::Region::R3 || region == workload::Region::UNKNOWN) && entity->shouldBePhysical();
     bool canBeKinematic = region <= workload::Region::R3;
-    if (shouldBePhysical) {
+    if (maybeShouldBePhysical) {
         EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
         if (motionState) {
             motionState->setRegion(region);
@@ -330,6 +330,18 @@ void PhysicalEntitySimulation::buildMotionStatesForEntitiesThatNeedThem() {
             continue;
         }
 
+        uint8_t region = _space->getRegion(entity->getSpaceIndex());
+        if (region == workload::Region::UNKNOWN) {
+            // the workload hasn't categorized it yet --> skip for later
+            ++entityItr;
+            continue;
+        }
+        if (region > workload::Region::R2) {
+            // not in physical zone --> remove from list
+            entityItr = _entitiesToAddToPhysics.erase(entityItr);
+            continue;
+        }
+
         if (entity->isReadyToComputeShape()) {
             ShapeRequest shapeRequest(entity);
             ShapeRequests::iterator  requestItr = _shapeRequests.find(shapeRequest);

libraries/workload/src/workload/Region.h

@@ -18,17 +18,17 @@ public:
     using Type = uint8_t;
 
     enum Name : uint8_t {
-        R1 = 0,
+        R1 = 0,   // R1 = in physics simulation and client will bid for simulation ownership
-        R2,
+        R2,       // R2 = in physics simulation but client prefers to NOT have simulation ownership
-        R3,
+        R3,       // R3 = are NOT in physics simulation but yes kinematically animated when velocities are non-zero
-        UNKNOWN,
+        R4,       // R4 = known to workload but outside R3, not in physics, not animated if moving
-        INVALID,
+        UNKNOWN,  // UNKNOWN = known to workload but unsorted
+        INVALID,  // INVALID = not known to workload
     };
 
-    static const uint8_t NUM_CLASSIFICATIONS = 4;
+    static constexpr uint32_t NUM_KNOWN_REGIONS = uint32_t(Region::R4 - Region::R1 + 1); // R1 through R4 inclusive
-    static const uint8_t NUM_TRANSITIONS = NUM_CLASSIFICATIONS * (NUM_CLASSIFICATIONS - 1);
+    static constexpr uint32_t NUM_TRACKED_REGIONS = uint32_t(Region::R3 - Region::R1 + 1); // R1 through R3 inclusive
-
+    static const uint8_t NUM_REGION_TRANSITIONS = NUM_KNOWN_REGIONS * (NUM_KNOWN_REGIONS - 1);
-    static const uint8_t NUM_VIEW_REGIONS = (NUM_CLASSIFICATIONS - 1);
 
     static uint8_t computeTransitionIndex(uint8_t prevIndex, uint8_t newIndex);
 
@@ -62,13 +62,13 @@ inline uint8_t Region::computeTransitionIndex(uint8_t prevIndex, uint8_t newInde
     // 3  |       |       |       |       |
     //    |     9 |    10 |    11 |    -1 |
     //    +-------+-------+-------+-------+
-    uint8_t p = prevIndex + Region::NUM_CLASSIFICATIONS * newIndex;
+    uint8_t p = prevIndex + Region::NUM_KNOWN_REGIONS * newIndex;
-    if (0 == (p % (Region::NUM_CLASSIFICATIONS + 1))) {
+    if (0 == (p % (Region::NUM_KNOWN_REGIONS + 1))) {
         return -1;
     }
-    return p - (1 + p / (Region::NUM_CLASSIFICATIONS + 1));
+    return p - (1 + p / (Region::NUM_KNOWN_REGIONS + 1));
 }
 
 } // namespace workload
 
-#endif // hifi_workload_Region_h
+#endif // hifi_workload_Region_h

libraries/workload/src/workload/RegionState.cpp

@@ -28,7 +28,9 @@ void RegionState::run(const workload::WorkloadContextPointer& renderContext, con
     // ...
     // inputs[2N] = vector of ids exiting region N
     // inputs[2N + 1] = vector of ids entering region N
-    assert(inputs.size() == 2 * Region::UNKNOWN);
+    //
+    // But we only pass inputs for R1 through R3
+    assert(inputs.size() == 2 * RegionState::NUM_REGIONS_TRACKED);
 
     // The id's in each vector are sorted in ascending order
     // because the source vectors are scanned in ascending order.

libraries/workload/src/workload/RegionState.h

@@ -53,9 +53,7 @@ namespace workload {
         using Inputs = IndexVectors;
         using JobModel = workload::Job::ModelI<RegionState, Inputs, Config>;
 
-        RegionState() {
+        RegionState() { _state.resize(workload::Region::NUM_TRACKED_REGIONS); }
-            _state.resize(Region::UNKNOWN);
-        }
 
         void configure(const Config& config);
         void run(const workload::WorkloadContextPointer& renderContext, const Inputs& inputs);

libraries/workload/src/workload/RegionTracker.cpp

@@ -33,15 +33,15 @@ void RegionTracker::run(const WorkloadContextPointer& context, Outputs& outputs)
         //Changes changes;
         space->categorizeAndGetChanges(outChanges);
 
-        // use exit/enter lists for each region less than Region::UNKNOWN
+        // use exit/enter lists for each region less than Region::R4
-        outRegionChanges.resize(2 * (workload::Region::NUM_CLASSIFICATIONS - 1));
+        outRegionChanges.resize(2 * workload::Region::NUM_TRACKED_REGIONS);
         for (uint32_t i = 0; i < outChanges.size(); ++i) {
             Space::Change& change = outChanges[i];
-            if (change.prevRegion < Region::UNKNOWN) {
+            if (change.prevRegion < Region::R4) {
                 // EXIT list index = 2 * regionIndex
                 outRegionChanges[2 * change.prevRegion].push_back(change.proxyId);
             }
-            if (change.region < Region::UNKNOWN) {
+            if (change.region < Region::R4) {
                 // ENTER list index = 2 * regionIndex + 1
                 outRegionChanges[2 * change.region + 1].push_back(change.proxyId);
             }

libraries/workload/src/workload/Space.cpp

@@ -99,7 +99,7 @@ void Space::categorizeAndGetChanges(std::vector<Space::Change>& changes) {
         if (proxy.region < Region::INVALID) {
             glm::vec3 proxyCenter = glm::vec3(proxy.sphere);
             float proxyRadius = proxy.sphere.w;
-            uint8_t region = Region::UNKNOWN;
+            uint8_t region = Region::R4;
             for (uint32_t j = 0; j < numViews; ++j) {
                 auto& view = _views[j];
                 // for each 'view' we need only increment 'k' below the current value of 'region'

libraries/workload/src/workload/View.cpp

@@ -42,11 +42,11 @@ Sphere View::evalRegionSphere(const View& view, float originRadius, float maxDis
 }
 
 void View::updateRegionsDefault(View& view) {
-    std::vector<float> config(Region::NUM_VIEW_REGIONS * 2, 0.0f);
+    std::vector<float> config(Region::NUM_TRACKED_REGIONS * 2, 0.0f);
 
     float refFar = 10.0f;
     float refClose = 2.0f;
-    for (int i = 0; i < Region::NUM_VIEW_REGIONS; i++) {
+    for (int i = 0; i < (int)Region::NUM_TRACKED_REGIONS; i++) {
         float weight = i + 1.0f;
         config[i * 2] = refClose;
         config[i * 2 + 1] = refFar * weight;
@@ -56,13 +56,13 @@ void View::updateRegionsDefault(View& view) {
 }
 
 void View::updateRegionsFromBackFronts(View& view) {
-    for (int i = 0; i < Region::NUM_VIEW_REGIONS; i++) {
+    for (int i = 0; i < (int)Region::NUM_TRACKED_REGIONS; i++) {
         view.regions[i] = evalRegionSphere(view, view.regionBackFronts[i].x, view.regionBackFronts[i].y);
     }
 }
 
 void View::updateRegionsFromBackFrontDistances(View& view, const float* configDistances) {
-    for (int i = 0; i < Region::NUM_VIEW_REGIONS; i++) {
+    for (int i = 0; i < (int)Region::NUM_TRACKED_REGIONS; i++) {
         view.regionBackFronts[i] = glm::vec2(configDistances[i * 2], configDistances[i * 2 + 1]);
     }
     updateRegionsFromBackFronts(view);

libraries/workload/src/workload/View.h

@@ -48,10 +48,10 @@ public:
     float originRadius{ 0.5f };
 
     // N regions distances
-    glm::vec2 regionBackFronts[Region::NUM_VIEW_REGIONS + 1];
+    glm::vec2 regionBackFronts[Region::NUM_TRACKED_REGIONS];
 
     // N regions spheres
-    Sphere regions[Region::NUM_VIEW_REGIONS];
+    Sphere regions[Region::NUM_TRACKED_REGIONS];
 
     // Set fov properties from angle
     void setFov(float angleRad);

libraries/workload/src/workload/ViewTask.cpp

@@ -82,7 +82,7 @@ void SetupViews::run(const WorkloadContextPointer& renderContext, const Input& i
 
 
 ControlViews::ControlViews() {
-    for (int32_t i = 0; i < workload::Region::NUM_VIEW_REGIONS; i++) {
+    for (uint32_t i = 0; i < workload::Region::NUM_TRACKED_REGIONS; i++) {
         regionBackFronts[i] = MIN_VIEW_BACK_FRONTS[i];
         regionRegulators[i] = Regulator(std::chrono::milliseconds(2), MIN_VIEW_BACK_FRONTS[i], MAX_VIEW_BACK_FRONTS[i], glm::vec2(RELATIVE_STEP_DOWN), glm::vec2(RELATIVE_STEP_UP));
     }
@@ -166,7 +166,7 @@ glm::vec2 Regulator::clamp(const glm::vec2& backFront) const {
 
 void ControlViews::regulateViews(workload::Views& outViews, const workload::Timings& timings) {
     for (auto& outView : outViews) {
-        for (int32_t r = 0; r < workload::Region::NUM_VIEW_REGIONS; r++) {
+        for (uint32_t r = 0; r < workload::Region::NUM_TRACKED_REGIONS; r++) {
             outView.regionBackFronts[r] = regionBackFronts[r];
         }
     }
@@ -198,13 +198,13 @@ void ControlViews::enforceRegionContainment() {
     // and each region should never exceed its min/max limits
     const glm::vec2 MIN_REGION_GAP = { 1.0f, 2.0f };
     // enforce outside --> in
-    for (int32_t i = workload::Region::NUM_VIEW_REGIONS - 2; i >= 0; --i) {
+    for (int32_t i = (int32_t)workload::Region::NUM_TRACKED_REGIONS - 2; i >= 0; --i) {
         int32_t j = i + 1;
         regionBackFronts[i] = regionRegulators[i].clamp(glm::min(regionBackFronts[i], regionBackFronts[j] - MIN_REGION_GAP));
     }
     // enforce inside --> out
-    for (int32_t i = 1; i < workload::Region::NUM_VIEW_REGIONS; ++i) {
+    for (uint32_t i = 1; i < workload::Region::NUM_TRACKED_REGIONS; ++i) {
-        int32_t j = i - 1;
+        uint32_t j = i - 1;
         regionBackFronts[i] = regionRegulators[i].clamp(glm::max(regionBackFronts[i], regionBackFronts[j] + MIN_REGION_GAP));
     }
 }

libraries/workload/src/workload/ViewTask.h

@@ -196,7 +196,7 @@ namespace workload {
         } data;
 
         struct DataExport {
-            static const int SIZE{ workload::Region::NUM_VIEW_REGIONS };
+            static const int SIZE{ workload::Region::NUM_TRACKED_REGIONS };
             float timings[SIZE];
             glm::vec2 ranges[SIZE];
             QList<qreal> _timings { 6, 2.0 };
@@ -252,8 +252,8 @@ namespace workload {
         void configure(const Config& config);
         void run(const workload::WorkloadContextPointer& runContext, const Input& inputs, Output& outputs);
 
-        std::array<glm::vec2, workload::Region::NUM_VIEW_REGIONS> regionBackFronts;
+        std::array<glm::vec2, workload::Region::NUM_TRACKED_REGIONS> regionBackFronts;
-        std::array<Regulator, workload::Region::NUM_VIEW_REGIONS> regionRegulators;
+        std::array<Regulator, workload::Region::NUM_TRACKED_REGIONS> regionRegulators;
 
         void regulateViews(workload::Views& views, const workload::Timings& timings);
         void enforceRegionContainment();