From ef1e426273b3412e63ee511495f8013375250245 Mon Sep 17 00:00:00 2001 From: LaShonda Hopper Date: Tue, 25 Jul 2017 14:09:46 -0400 Subject: [PATCH] [WL21389] Some code and todo cleanup in prep for PR1. Changes to be committed: modified: libraries/entities-renderer/src/RenderableShapeEntityItem.cpp modified: libraries/entities/src/ShapeEntityItem.cpp modified: libraries/physics/src/ShapeFactory.cpp modified: libraries/shared/src/ShapeInfo.cpp --- .../src/RenderableShapeEntityItem.cpp | 29 +++++-------------- libraries/entities/src/ShapeEntityItem.cpp | 7 ++--- libraries/physics/src/ShapeFactory.cpp | 2 ++ libraries/shared/src/ShapeInfo.cpp | 20 ++++++------- 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp b/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp index a305b201d6..5fa2354bd5 100644 --- a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp @@ -114,26 +114,13 @@ void RenderableShapeEntityItem::computeShapeInfo(ShapeInfo& info) { break; case entity::Shape::Cylinder: { _collisionShapeType = SHAPE_TYPE_CYLINDER_Y; - // TODO_CUSACK: determine if rotation is axis-aligned + // TODO WL21389: determine if rotation is axis-aligned //const Transform::Quat & rot = _transform.getRotation(); -#if 0 - // TODO: some way to tell apart SHAPE_TYPE_CYLINDER_Y - // TODO_CUSACK: Should allow for minor variance along axes? - if ((entityDimensions.y >= entityDimensions.x) && (entityDimensions.y >= entityDimensions.z)) { - } - else if (entityDimensions.x >= entityDimensions.z) { - _collisionShapeType = SHAPE_TYPE_CYLINDER_X; - } - else if (entityDimensions.z >= entityDimensions.x) { - _collisionShapeType = SHAPE_TYPE_CYLINDER_Z; - } - else //...there was no major axis, treat as a hull - { - _collisionShapeType = SHAPE_TYPE_SIMPLE_HULL; - //TODO_CUSACK: pointCollection - } -#endif + // TODO WL21389: some way to tell apart SHAPE_TYPE_CYLINDER_Y, _X, _Z based on rotation and + // hull ( or dimensions, need circular cross section) + // Should allow for minor variance along axes? + } break; case entity::Shape::Triangle: @@ -145,15 +132,15 @@ void RenderableShapeEntityItem::computeShapeInfo(ShapeInfo& info) { case entity::Shape::Dodecahedron: case entity::Shape::Icosahedron: case entity::Shape::Cone: { - _collisionShapeType = SHAPE_TYPE_SIMPLE_HULL; - //TODO_CUSACK: pointCollection + //TODO WL21389: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later) + //_collisionShapeType = SHAPE_TYPE_SIMPLE_HULL; } break; case entity::Shape::Torus: { // Not in GeometryCache::buildShapes, unsupported. _collisionShapeType = SHAPE_TYPE_NONE; - //TODO_CUSACK: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later) + //TODO WL21389: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later if desired support) } break; default:{ diff --git a/libraries/entities/src/ShapeEntityItem.cpp b/libraries/entities/src/ShapeEntityItem.cpp index 58799de807..157e3afab3 100644 --- a/libraries/entities/src/ShapeEntityItem.cpp +++ b/libraries/entities/src/ShapeEntityItem.cpp @@ -88,14 +88,14 @@ EntityItemProperties ShapeEntityItem::getProperties(EntityPropertyFlags desiredP void ShapeEntityItem::setShape(const entity::Shape& shape) { _shape = shape; - switch (_shape) { // TODO_CUSACK fill out? + switch (_shape) { // TODO WL21389: fill out with other shapes? case entity::Shape::Cube: _type = EntityTypes::Box; _collisionShapeType = ShapeType::SHAPE_TYPE_BOX; break; case entity::Shape::Sphere: _type = EntityTypes::Sphere; - _collisionShapeType = ShapeType::SHAPE_TYPE_ELLIPSOID; // TODO_CUSACK defer? Check to see if sphere is more appropriate? + _collisionShapeType = ShapeType::SHAPE_TYPE_ELLIPSOID; break; default: _type = EntityTypes::Shape; @@ -162,8 +162,7 @@ void ShapeEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBit APPEND_ENTITY_PROPERTY(PROP_ALPHA, getAlpha()); } -// This value specifes how the shape should be treated by physics calculations. -// For now, all polys will act as spheres +// This value specifes how the shape should be treated by physics calculations. ShapeType ShapeEntityItem::getShapeType() const { return _collisionShapeType; } diff --git a/libraries/physics/src/ShapeFactory.cpp b/libraries/physics/src/ShapeFactory.cpp index 87580c6726..97174b2216 100644 --- a/libraries/physics/src/ShapeFactory.cpp +++ b/libraries/physics/src/ShapeFactory.cpp @@ -307,11 +307,13 @@ const btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info) const btVector3 btHalfExtents(halfExtents.x, halfExtents.y, halfExtents.z); shape = new btCylinderShapeX(btHalfExtents); } + break; case SHAPE_TYPE_CYLINDER_Z: { const glm::vec3 halfExtents = info.getHalfExtents(); const btVector3 btHalfExtents(halfExtents.x, halfExtents.y, halfExtents.z); shape = new btCylinderShapeZ(btHalfExtents); } + break; case SHAPE_TYPE_CYLINDER_Y: { const glm::vec3 halfExtents = info.getHalfExtents(); const btVector3 btHalfExtents(halfExtents.x, halfExtents.y, halfExtents.z); diff --git a/libraries/shared/src/ShapeInfo.cpp b/libraries/shared/src/ShapeInfo.cpp index 5d6b4ece7a..5930880859 100644 --- a/libraries/shared/src/ShapeInfo.cpp +++ b/libraries/shared/src/ShapeInfo.cpp @@ -29,7 +29,7 @@ void ShapeInfo::clear() { } void ShapeInfo::setParams(ShapeType type, const glm::vec3& halfExtents, QString url) { - //TODO_CUSACK: Does this need additional cases and handling added? + //TODO WL21389: Does this need additional cases and handling added? _url = ""; _type = type; setHalfExtents(halfExtents); @@ -56,8 +56,9 @@ void ShapeInfo::setParams(ShapeType type, const glm::vec3& halfExtents, QString } void ShapeInfo::setBox(const glm::vec3& halfExtents) { - //TODO_CUSACK: Should this pointlist clearance added in case + //TODO WL21389: Should this pointlist clearance added in case // this is a re-purposed instance? + // See https://github.com/highfidelity/hifi/pull/11024#discussion_r128885491 _url = ""; _type = SHAPE_TYPE_BOX; setHalfExtents(halfExtents); @@ -65,8 +66,7 @@ void ShapeInfo::setBox(const glm::vec3& halfExtents) { } void ShapeInfo::setSphere(float radius) { - //TODO_CUSACK: Should this pointlist clearance added in case - // this is a re-purposed instance? + //TODO WL21389: See comment in setBox regarding clearance _url = ""; _type = SHAPE_TYPE_SPHERE; radius = glm::max(radius, MIN_HALF_EXTENT); @@ -75,17 +75,14 @@ void ShapeInfo::setSphere(float radius) { } void ShapeInfo::setPointCollection(const ShapeInfo::PointCollection& pointCollection) { - //TODO_CUSACK: Should this have protection against inadvertant clearance and type - // resetting? If for some reason this was called and point list was and is emtpy - // would we still wish to clear out everything? + //TODO WL21389: May need to skip resetting type here. _pointCollection = pointCollection; _type = (_pointCollection.size() > 0) ? SHAPE_TYPE_COMPOUND : SHAPE_TYPE_NONE; _doubleHashKey.clear(); } void ShapeInfo::setCapsuleY(float radius, float halfHeight) { - //TODO_CUSACK: Should this pointlist clearance added in case - // this is a re-purposed instance? + //TODO WL21389: See comment in setBox regarding clearance _url = ""; _type = SHAPE_TYPE_CAPSULE_Y; radius = glm::max(radius, MIN_HALF_EXTENT); @@ -127,7 +124,7 @@ int ShapeInfo::getLargestSubshapePointCount() const { } float ShapeInfo::computeVolume() const { - //TODO_CUSACK: Add support for other ShapeTypes. + //TODO WL21389: Add support for other ShapeTypes. const float DEFAULT_VOLUME = 1.0f; float volume = DEFAULT_VOLUME; switch(_type) { @@ -161,7 +158,7 @@ float ShapeInfo::computeVolume() const { } bool ShapeInfo::contains(const glm::vec3& point) const { - //TODO_CUSACK: Add support for other ShapeTypes like Ellipsoid/Compound. + //TODO WL21389: Add support for other ShapeTypes like Ellipsoid/Compound. switch(_type) { case SHAPE_TYPE_SPHERE: return glm::length(point) <= _halfExtents.x; @@ -206,6 +203,7 @@ bool ShapeInfo::contains(const glm::vec3& point) const { } const DoubleHashKey& ShapeInfo::getHash() const { + //TODO WL21389: Need to include the pointlist for SIMPLE_HULL in hash // NOTE: we cache the key so we only ever need to compute it once for any valid ShapeInfo instance. if (_doubleHashKey.isNull() && _type != SHAPE_TYPE_NONE) { bool useOffset = glm::length2(_offset) > MIN_SHAPE_OFFSET * MIN_SHAPE_OFFSET;