Original report (archived issue) by Jennifer Buehler (Bitbucket: JenniferBuehler).
I stumbled upon a problem with Box when using the default constructor, and subsequently setting the box min/max with the Min() and Max() functions, e.g.
ignition::math::Box b;
b.Min() = something;
b.Max() = somethingElse;
The default constructor doesn't initialize BoxPrivate::extent to EXTENT_FINITE, as the other constructors would, e.g. the constructor taking two vectors:
//////////////////////////////////////////////////
Box::Box(const Vector3d &_vec1, const Vector3d &_vec2)
: dataPtr(new BoxPrivate)
{
this->dataPtr->extent = BoxPrivate::EXTENT_FINITE;
...
} So in my opinion it should be documented that to have a valid Box, one needs to use any constructor but the default one, or we should add a method Box::Set(const ignition::Vector3d &min, ignition::Vector3d &max) which also sets the extent to EXTENT_FINITE, and document at Min() and Max() that if these functions are used to initialize the box, one would have to additionally ensure the box was previously correctly initialized with a suitable constructor, or that the new Set() function should be used instead.
Problems arising if the extent is not initialized become clear with the Box::operator+=:
//////////////////////////////////////////////////
const Box &Box::operator+=(const Box &_b)
{
if (this->dataPtr->extent != BoxPrivate::EXTENT_NULL)
{
this->dataPtr->min.Min(_b.dataPtr->min);
this->dataPtr->max.Max(_b.dataPtr->max);
}
else
{
this->dataPtr->min = _b.dataPtr->min;
this->dataPtr->max = _b.dataPtr->max;
this->dataPtr->extent = _b.dataPtr->extent;
}
return *this;
} An invalid box will never be added correctly to other boxes.
I noticed this in Gazebo, where gazebo::physics::Link::BoundingBox() uses this operator.
I would propose to update the documentation and add a Set() function as suggested. If you agree, or have other suggestions, let me know and I can send a PR for this.
Original report (archived issue) by Jennifer Buehler (Bitbucket: JenniferBuehler).
I stumbled upon a problem with
Boxwhen using the default constructor, and subsequently setting the box min/max with theMin()andMax()functions, e.g.The default constructor doesn't initialize
BoxPrivate::extenttoEXTENT_FINITE, as the other constructors would, e.g. the constructor taking two vectors:So in my opinion it should be documented that to have a valid
Box, one needs to use any constructor but the default one, or we should add a methodBox::Set(const ignition::Vector3d &min, ignition::Vector3d &max)which also sets the extent to EXTENT_FINITE, and document atMin()andMax()that if these functions are used to initialize the box, one would have to additionally ensure the box was previously correctly initialized with a suitable constructor, or that the newSet()function should be used instead.Problems arising if the extent is not initialized become clear with the
Box::operator+=:An invalid box will never be added correctly to other boxes.
I noticed this in Gazebo, where
gazebo::physics::Link::BoundingBox()uses this operator.I would propose to update the documentation and add a
Set()function as suggested. If you agree, or have other suggestions, let me know and I can send a PR for this.