[WL21389] PR1 Update based on code review discussion & feedback (details below).

* Removed some left overs from prior approach.
* Moved _collisionShapeType & getShapeType override from ShapeEntityItem to
  RenderableShapeEntityItem (see thread: https://github.com/highfidelity/hifi/pull/11048#discussion_r130154903)
* Switched _collisionShapeType default from SHAPE_TYPE_NONE to SHAPE_TYPE_ELLIPSOID
** see thread: https://github.com/highfidelity/hifi/pull/11048#discussion_r129982909

Note(s):
* Retested and the cylinder behaves as expected along with the Box & Sphere shapes
  save from the previously mentioned caveats in the PR notes (https://github.com/highfidelity/hifi/pull/11048)
* Confirmed that currently unsupported shapes (hedrons, polygons, & cone) fallback to
  ellipsoid behavior given default change.

Changes Committed:
	modified:   libraries/entities-renderer/src/RenderableShapeEntityItem.cpp
	modified:   libraries/entities-renderer/src/RenderableShapeEntityItem.h
	modified:   libraries/entities/src/ShapeEntityItem.cpp
	modified:   libraries/entities/src/ShapeEntityItem.h
	modified:   libraries/shared/src/ShapeInfo.cpp
This commit is contained in:
LaShonda Hopper 2017-07-28 15:59:17 -04:00
parent 6b5cabf00a
commit cc4fbc97cd
5 changed files with 67 additions and 65 deletions

View file

@ -16,7 +16,6 @@
#include <StencilMaskPass.h>
#include <GeometryCache.h>
#include <PerfStat.h>
#include <ShapeFactory.h>
#include <render-utils/simple_vert.h>
#include <render-utils/simple_frag.h>
@ -91,67 +90,72 @@ void RenderableShapeEntityItem::computeShapeInfo(ShapeInfo& info) {
const glm::vec3 entityDimensions = getDimensions();
switch (_shape){
case entity::Shape::Quad:
case entity::Shape::Cube: {
_collisionShapeType = SHAPE_TYPE_BOX;
}
break;
case entity::Shape::Sphere: {
float diameter = entityDimensions.x;
const float MIN_DIAMETER = 0.001f;
const float MIN_RELATIVE_SPHERICAL_ERROR = 0.001f;
if (diameter > MIN_DIAMETER
&& fabsf(diameter - entityDimensions.y) / diameter < MIN_RELATIVE_SPHERICAL_ERROR
&& fabsf(diameter - entityDimensions.z) / diameter < MIN_RELATIVE_SPHERICAL_ERROR) {
_collisionShapeType = SHAPE_TYPE_SPHERE;
case entity::Shape::Quad:
case entity::Shape::Cube: {
_collisionShapeType = SHAPE_TYPE_BOX;
}
else {
break;
case entity::Shape::Sphere: {
float diameter = entityDimensions.x;
const float MIN_DIAMETER = 0.001f;
const float MIN_RELATIVE_SPHERICAL_ERROR = 0.001f;
if (diameter > MIN_DIAMETER
&& fabsf(diameter - entityDimensions.y) / diameter < MIN_RELATIVE_SPHERICAL_ERROR
&& fabsf(diameter - entityDimensions.z) / diameter < MIN_RELATIVE_SPHERICAL_ERROR) {
_collisionShapeType = SHAPE_TYPE_SPHERE;
}
else {
_collisionShapeType = SHAPE_TYPE_ELLIPSOID;
}
}
break;
case entity::Shape::Cylinder: {
_collisionShapeType = SHAPE_TYPE_CYLINDER_Y;
// TODO WL21389: determine if rotation is axis-aligned
//const Transform::Quat & rot = _transform.getRotation();
// 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:
case entity::Shape::Hexagon:
case entity::Shape::Octagon:
case entity::Shape::Circle:
case entity::Shape::Tetrahedron:
case entity::Shape::Octahedron:
case entity::Shape::Dodecahedron:
case entity::Shape::Icosahedron:
case entity::Shape::Cone: {
//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_ELLIPSOID;
//TODO WL21389: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later if desired support)
}
break;
default:{
_collisionShapeType = SHAPE_TYPE_ELLIPSOID;
}
}
break;
case entity::Shape::Cylinder: {
_collisionShapeType = SHAPE_TYPE_CYLINDER_Y;
// TODO WL21389: determine if rotation is axis-aligned
//const Transform::Quat & rot = _transform.getRotation();
// 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:
case entity::Shape::Hexagon:
case entity::Shape::Octagon:
case entity::Shape::Circle:
case entity::Shape::Tetrahedron:
case entity::Shape::Octahedron:
case entity::Shape::Dodecahedron:
case entity::Shape::Icosahedron:
case entity::Shape::Cone: {
//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 WL21389: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later if desired support)
}
break;
default:{
//_collisionShapeType = SHAPE_TYPE_NONE; // Remains SHAPE_TYPE_NONE.
}
break;
break;
}
EntityItem::computeShapeInfo(info);
}
// This value specifes how the shape should be treated by physics calculations.
ShapeType RenderableShapeEntityItem::getShapeType() const {
return _collisionShapeType;
}
void RenderableShapeEntityItem::render(RenderArgs* args) {
PerformanceTimer perfTimer("RenderableShapeEntityItem::render");
//Q_ASSERT(getType() == EntityTypes::Shape);

View file

@ -29,10 +29,17 @@ public:
bool isTransparent() override;
virtual void computeShapeInfo(ShapeInfo& info) override;
ShapeType getShapeType() const override;
private:
std::unique_ptr<Procedural> _procedural { nullptr };
//! This is SHAPE_TYPE_ELLIPSOID rather than SHAPE_TYPE_NONE to maintain
//! prior functionality where new or unsupported shapes are treated as
//! ellipsoids.
ShapeType _collisionShapeType{ ShapeType::SHAPE_TYPE_ELLIPSOID };
SIMPLE_RENDERABLE();
};

View file

@ -88,14 +88,12 @@ EntityItemProperties ShapeEntityItem::getProperties(EntityPropertyFlags desiredP
void ShapeEntityItem::setShape(const entity::Shape& shape) {
_shape = shape;
switch (_shape) { // TODO WL21389: fill out with other shapes?
switch (_shape) {
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;
break;
default:
_type = EntityTypes::Shape;
@ -162,11 +160,6 @@ void ShapeEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBit
APPEND_ENTITY_PROPERTY(PROP_ALPHA, getAlpha());
}
// This value specifes how the shape should be treated by physics calculations.
ShapeType ShapeEntityItem::getShapeType() const {
return _collisionShapeType;
}
void ShapeEntityItem::setColor(const rgbColor& value) {
memcpy(_color, value, sizeof(rgbColor));
}

View file

@ -84,7 +84,6 @@ public:
QColor getQColor() const;
void setColor(const QColor& value);
ShapeType getShapeType() const override;
bool shouldBePhysical() const override { return !isDead(); }
bool supportsDetailedRayIntersection() const override;
@ -100,7 +99,6 @@ protected:
float _alpha { 1 };
rgbColor _color;
entity::Shape _shape { entity::Shape::Sphere };
ShapeType _collisionShapeType { ShapeType::SHAPE_TYPE_NONE };
};
#endif // hifi_ShapeEntityItem_h

View file

@ -124,7 +124,7 @@ int ShapeInfo::getLargestSubshapePointCount() const {
}
float ShapeInfo::computeVolume() const {
//TODO WL21389: Add support for other ShapeTypes.
//TODO WL21389: Add support for other ShapeTypes( CYLINDER_X, CYLINDER_Y, etc).
const float DEFAULT_VOLUME = 1.0f;
float volume = DEFAULT_VOLUME;
switch(_type) {