MengeCrowdSim / Menge

The source code for the Menge crowd simulation framework
Apache License 2.0
138 stars 64 forks source link

Probably a bug in OBBShape::getCentroid() #99

Closed alafi closed 6 years ago

alafi commented 6 years ago

After modifying the orientation of an OBB goal in one of the examples, I started receiving the exception: Can't compute a path to a goal outside of the navigation mesh. Bad NavMeshVelComponent!

I believe there is a bug in calculating the centroid of the OBB shape. The current implementation is: return _pivot + getXBasis() * (_size.x() * 0.5f) + getYBasis() * (_size.y() * 0.5f);

while it should be: Vector2 centerPoint(_size.x() * 0.5f, _size.y() * 0.5f); return _pivot + Vector2(getXBasis() * centerPoint, getYBasis() * centerPoint);

SeanCurtis-TRI commented 6 years ago

I have two thoughts:

  1. I'm 99% sure that the current implementation is correct and the implementation you propose does the opposite. I'll draw up a diagram with the math in a bit. The short gist is the operation you define rotates the center in the opposite direction.

  2. The real issue is, I believe, orthogonal to this. The OBB wasn't placed on the navigation mesh. Would I be correct in assuming that the goal is near the edge of the navigation mesh? It seems like the rotation is taking the pivot off the mesh (NavMesh uses a naive mechanism for placing the goal on the nav mesh -- it's completely based on the pivot).

alafi commented 6 years ago

Thank you for your reply. You are right about the goal being adjusant to the navemesh border, but it is completely located inside it. I will relook at the issue and try to visualize it.

curds01 commented 6 years ago

I've revisited, and I've become persuaded that there is a bug. However, I would assert the bug is in getXBasis() and getYBasis().

The code says:

/*!
   @brief    Returns the x-axis of the OBB's local frame

   @returns  The direction of the obb's x-axis.
   */
  Vector2 getXBasis() const { return Vector2(_cosTheta, -_sinTheta); }

  /*!
   @brief    Returns the y-axis of the OBB's local frame

   @returns  The direction of the obb's y-axis.
   */
  Vector2 getYBasis() const { return Vector2(_sinTheta, _cosTheta); }

As I read this, it suggests (loosely), that getXBasis() returns a vector in the direction of the OBB's frame's x-direction, expressed in the world frame. So, if theta were 45 degrees, one would expect the vector: [cos(theta), sin(theta)]. And similarly, the y-basis is incorrectly positioned

/*!
   @brief    Returns the x-axis of the OBB's local frame

   @returns  The direction of the obb's x-axis.
   */
  Vector2 getXBasis() const { return Vector2(_cosTheta, _sinTheta); }

  /*!
   @brief    Returns the y-axis of the OBB's local frame

   @returns  The direction of the obb's y-axis.
   */
  Vector2 getYBasis() const { return Vector2(-_sinTheta, _cosTheta); }

Note the reversal of the sign of the _sinTheta term.

However, it's important to make sure that all usages of the bases are correct and consistent -- i.e., we may end up with a better pivot, but a worse "nearest point". So, definitely a bug, definitely needs to be fixed.

curds01 commented 6 years ago

See the referenced PR -- I believe I've fixed the problem.