Additional OrientedBoundingBox functionality
Description
Added a few functions to the OrientedBoundingBox class:
-
fromMinMaxcomputes an OBB from given min/max points -
transformtransforms an OBB with a matrix -
intersecttests for intersection between two OBBs
The first ones are rather simple "convenience" functions. I used these functions, for example, in https://github.com/CesiumGS/cesium/pull/12112 , but there, only 'drafted' them, still outside of the OrientedBoundingBox class.
The intersect function is a bit more involved. It implements the "Separating Axis Theorem", as described in https://gamma.cs.unc.edu/users/gottschalk/main.pdf . Strangely, this function already did exist, back when there was still a class called ObjectOrientedBoundingBox. This class was deprecated in https://github.com/CesiumGS/cesium/pull/2782 and eventually removed, without a replacement for the intersect function. So I'm not even 100% sure whether it should be added again 😬
Issue number and link
No specific issue, but ...
- useful for https://github.com/CesiumGS/cesium/pull/12112
- vaguely related to https://github.com/CesiumGS/cesium/issues/2861
Testing plan
In addition to the Specs/UnitTests, the following is a sandcastle that can be used for testing the intersect function this when running this branch on localhost:
http://localhost:8080/Apps/Sandcastle/#c=3Vhtb9s2EP4rhL9ExlzZWpqt0FxvdZJlBtq4SJMtMgwUlETbQijJkKjEbpH/viMpipL8kqSxjWFFapA88l6eO94d5cVRytB9QB5Igt6jiDygU5IGWWj+LdaMccMT89M4YjiISDJutND3cYTQlMYusdEE05S0xtFj87dxNI4mWeSxII6QlxDMyOckCAMW3BMjdt0W8mIaJ0153hOipyQOCUuWVeH9eDHMGAV5FzndEGcQCoMoCLPQLu8+xQmDEY6OjTeW2Wkh/dts5cfwYtsxsV/9yDPCHqVlGPuEfsIsCRagaH5ezt+akyQOr2KGudnXCY5SKoaGlAx2mzNMJx8WJG3pJY9EDMCEeVlOAD848kgVDYXBIKcqLBR2djFS1mp17fIkJ2MGEzcD422Us0LSNXaBDZ/V5X5Q54TJYoshPZozfiwjlxCWJVHZDh0MNQOUANBHIaBUnc8JTvhCxXWfSaLOCDU+FNuMwiAmXJFxoHWUStIEPGTDhkwtPZZ8/sjDWLrj7Lx/c/H19uvwanAxuASn/HL867vjTmddpJ8RN5sOXffJiJeer3q4FIr1EDEX6KeaIithZC5Xl77VY8vPFaxKHiYB3+/34yzyg2gKN8+QHFqV0F3HqTAVWNavu9qjECiHxMp5hXoBKVxyFmD6J11exxzH/xmAbr73y3xGEkBPyi5yX5korto6GUobyVdmcDMFycT0cEgSbE44elVuRlVyS11/P0tEzrJRp3oP1gU5t6YS5f8d71RUAx0qsMwVITWx7+ealLQUxn3S2XIlXdUS64vKQK5+p14LROqV4aIXSkrl3I2mojaLfZREUzazkdXpQOVSqw+BLxeLtcemxqp8AytgbXA4BJgwIA2+kduWGjnFaCRGAArDt3ro6KHcgKMpheCZ3lZmTmU2GkcvCaRcphKYSytHhEK6XrGPTY/GETFqi4Oz88vrwbUjWdSIYUZZMKfL3Js1J9Y2lyPhtiRmZrL4CvugfmoUiDSVQxVP7axd6eA8oYNzAB1GT+gw2qMOXzxMibE+jGRg51EtQ3qLJjKw4ufUAHmgXAHgjBoWtCLI5aB8R4GsLuU4arff7/Af54c+QTuPAAa4aZDVbCQWT8WtR0c8VR3B1fTF0DoyBfWfvwgcETULw39vBt4DuwKxiG4GLZTN/dL5h4DNxEFO5njdY5qBp9Q2vqyzsuIjk3ZLHOQawI6IIYxCkqZ4StDDjHAdxN5hv8/PAXQp8Zju3CScnY3ZAzp9/sedVE0WndVsoUOIe2fDA4KnW/Sz+D3uqDfESh4/5nlcC+UoPbebAMaFjvKpVbbUerml1oEttZ5rqaUttaSllAgORbHiDXh9zcrXcK2qKQ14VIniDmp8L0oaL5S6rNlgly5tNthXKm/QG5UqnI1O3paqnCKKdAZb357oqVOd5nsfhW45GHdR7N3FGTPh0eLdGYWuZUezOKYu5rnCj70sBKjMKWHnlPBhfzmAjqaR7xk3+ME6b3hL0WU/EGinWkZLca494OUdNSo1WWa+amuA0GUWuiTRHE0Brcqia6nOVupoI1W6YjvZ2U7ezFw6bzvZ2U4elXqtSrsHsJVrgd5QVABdDoQfik8C+pquuTJmkft4I94RTw5L16qYEpPGU4iMgdqHxg3ouINmLmRTh5yQMJbdvb51hVl6qYiG1edv0bKrPjRAv1c+L5hX52eo+sXB7H+8OdcIbmvfVzR7oTXWqjXWFms4rq2qrhdX5+eXL1PUeqailSQmReDa02bbg6xTvN/qOQAyxtBNSXKPXcjypRwwbohLC5nDTDM39ZLAJYZMAetyyRN8nB3xGe2Aj0wZu2Lk7IrRLkyTCWtXjJxdMdpo2jhSRQUm9W88ImgbrUY3ZUtKejzm/wjCeZwwlCXUMM02I+EcHtUkbbuZd0eY6aXis0q3rY50/eAeBf77NV+tkUdxmgJlklH6BWJr3Oh127C/cozGmGfW4T1JKF7yLTOr91EumqbZbcN09VRRdoXWXcZx6cmk12Vu7C976nXeZUlPv/O7zO+Ja9dtw6i6rmcwD6J5Bh3Ack5AWsK7bjAoDCKYQQvGx3gBY/ElAGYpI3M+hSHAjd+4UPRhLrpvO3/Ly178RvjDRkdCwlFuwXq5jCwYZw7HYXayhXmFTcU2mCRPgOEcHAxnn2A4rwJjdHAwRvsEY/TDYMgsflA01Bem/cChytJr8HAOj4ezVzyc1+ExOjweo73i8eP3RbYGu8Lj3dNoSIH7QkN1Oq9Bwzk0Gs5e0XBeh8bo0GiM9orG824KDIt2DMZ5n6Yaun8B
It uses one fixed bounding box, and allows editing the size, position, and orientation of the other one, showing the other one in red when they intersect (and printing infos to the console)
Author checklist
- [x] I have submitted a Contributor License Agreement
- [x] I have added my name to
CONTRIBUTORS.md - [ ] I have updated
CHANGES.mdwith a short summary of my change - [x] I have added or updated unit tests to ensure consistent code coverage
- [x] I have updated the inline documentation, and included code examples where relevant
- [x] I have performed a self-review of my code
Thank you for the pull request, @javagl!
:white_check_mark: We can confirm we have a CLA on file for you.
@lukemckinstry Could you please give this a first review pass?
@javagl can you help connect the dots for me on how these functions are helping solve the core issue? https://github.com/CesiumGS/cesium/issues/12108
@javagl To clarify what we spoke about offline, you mentioned that the intersect functionality is not strictly needed in support of https://github.com/CesiumGS/cesium/pull/12112, is that correct?
If so, what do you think of removing that for the time being in the interest of getting this merged?
@lukemckinstry Sorry, I seem to have overlooked that. question/notification.
The intersect function is not required to solve any of the linked issues. I thought that doing intersection checks would be one of the more fundamental things that one might do with bounding volumes. But in CesiumJS, this was apparently "emulated"/replaced with intersectPlane.
The other functions, fromMinMax and transform are useful for fixing the linked issues, because...
- bounding volumes have to be created from the
min/maxproperties of thePOSITIONaccessor of the glTF - they have to be transformed with the transform of the node that the respective mesh is attached to (which is currently not done at all, and at the core, the main reason for the issue)
Both of these functions could also be implemented locally, without adding them to the public API of OrientedBoundingBox. (In fact, they are implemented like that in the current state of the corresponding PR).
So, the bottom line: I think that the functions could be useful. But they are not "required" for anything. So if this is preferred, then this PR could be closed as it is.
@javagl What you are saying about implementing fromMinMax and transform on the OrientedBoundingBox class as done here makes sense. We agree these functions are better implemented on this class than locally to the glTF specific code.
We think it is best to remove intersect from this PR (since as you point out the fuctionality is already implemented elsewhere) and then move forward with just fromMinMax and transform.
Let us know when this is ready for review again. Thanks!
@javagl What is the status of this PR? If it is paused, can we close it for the time being to keep things tidy?
This is still on my TODO list. When removing intersect from this PR, there will ... not be sooo much left. And outside of the context of https://github.com/CesiumGS/cesium/pull/12112 , it may not be obvious wha fromMinMax and transform are useful. In doubt, this one can be closed, and I'll add these functions ("privately/locally") in https://github.com/CesiumGS/cesium/pull/12112 , when I'm able to continue with that. Promoting these functions into the public API could then be a second step.