Recent bug fixes

I put in


   public boolean intersects(BoundingVolume bv) {
        if (bv==null)
            return false;
        else
          return bv.intersectsOrientedBoundingBox(this);
   }



for null bounds checking, and with my milkshape loader I was saving Matrix3f objects as Quaternions, which causes some info loss for a few models, so I checked in a bug fix that saves them as Matrix3f objects. Extreamly small changes (which shouldn't need a review(??) being bug fixes), but thought you may be curious so this is what they were.


PS: I've written a very extendable OBBTree interface I'll be submitting a Code Review for soon when I get back tonight.

I personally feel that we should try and stay way from null checking unless it is absolutely necessary or null is an expiated parameter because of the speed drain. I know that it is not that slow but it would it add up? I think that programers should be able to wouch out for null pointers when thay call a function.

perhaps throw a NullPointerException from that method?



DP

The reason I had it in, was because sometimes a Spatial in a scene doesn’t have a bounding volume, but may get caught up in a recursive OBBTree intersect call, which made it through an exception. You do make good points about it, though, so I’m open either way.

If volume.instersects(null) is ever a valid call, then we need the checks. If sending null to the intersects method is never valid, then we don’t need the check, and we should just document that a nullpointexception will occur.

"Cep21" wrote:
The reason I had it in, was because sometimes a Spatial in a scene doesn't have a bounding volume, but may get caught up in a recursive OBBTree intersect call, which made it through an exception.

IMHO, everything which goes into scenegraph should have a bounding volume. Maybe just make it reference to static, immutable bounds which contains everything or nothing (depending on what you want) ?
"Cep21" wrote:
The reason I had it in, was because sometimes a Spatial in a scene doesn't have a bounding volume, but may get caught up in a recursive OBBTree intersect call, which made it through an exception. You do make good points about it, though, so I'm open either way.

This is what I was referring to when I said "null is an expiated parameter".