aabb ctor treats 2 params as extrema in any order #812
Conversation
The original constructor expected that the first parameter was the minimum point, and the second was the maximum. The new constructor now selects the minimums of all coordinates in the two parameters, and the maximums of all the coordinates. In this way, the two parameters are just two extreme points of the bounds, so is significantly more robust. This change was done because the original code had bugs when handling spheres of negative radius (used to model glass spheres with voids). Resolves #733
|
Not sure how I feel about this change. |
|
|
This is defo overkill, but this is a good place where a templated constructor could exist to create a bounding box that can accept 1 to N vec3 and 1 to N aabb |
|
I'm trying to keep this change surgical to specifically address the weakness exposed with negative radius spheres. Personally, I don't like the negative radius hack to implement spheres of air — I feel like you should be able to express all of this with the index of refraction, “a sphere of glass in air“, or “a sphere of glass in water”, or “a sphere of air in glass”. Still the attempts that one reader tried to model this failed for various mysterious reasons, where the negative radius trick work (and was suggested by Peter). The one catch was that the AABB failed because it passed inverted min/max points for the constructor. I suggested this fix because there could be other situations where this effect happens, and mostly because there's no good reason to be this picky about the two extrema. We really shouldn't care the all coordinates of the first parameter should be less than all corresponding coordinates of the second parameter, when that's really a requirement for the current AABB implementation. As an example, if I know the two extrema for a given object, and then apply a chain of transforms, I should be able to just pass the two transformed extrema, without requiring the modeling/transform code to fix things up. That way you don't require a fixup block for translate, and a fixup block for rotate, and a fixup block for scale and so forth (and applied at each step of the chain). This felt like a case of “be conservative in what you send, be liberal in what you accept”. |
|
Also note that AABB is due for rework per other issues in v4. |
The original constructor expected that the first parameter was the
minimum point, and the second was the maximum. The new constructor now
selects the minimums of all coordinates in the two parameters, and the
maximums of all the coordinates. In this way, the two parameters are
just two extreme points of the bounds, so is significantly more robust.
This change was done because the original code had bugs when handling
spheres of negative radius (used to model glass spheres with voids).
Resolves #733