fix: Crash when 3D Tiles bounding region has 0 volume

[verybigzhouhai]

No description provided.

[cesium-concierge]

Thanks for the pull request @verybigzhouhai!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[verybigzhouhai]

fixed #7933

[verybigzhouhai]

@OmarShehata @lilleyse To solve this problem, I added validation when creating shpere, box.

[OmarShehata]

Thanks for opening this PR @verybigzhouhai ! I'll try to take a deeper look soon. For now, can you add a unit test as well to ensure that it does not crash for this type of data in the future?

[verybigzhouhai]

@OmarShehata Okay, there are still some problems with the validation of creating regions, and I'll add a unit test after solving them.

[verybigzhouhai]

this is ready,eventually I added validation for box and shpere. @OmarShehata

[verybigzhouhai]

@OmarShehata Will you review this?

[OmarShehata]

I'll try to take a look sometime this week.

[cesium-concierge]

Thanks again for your contribution @verybigzhouhai!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[verybigzhouhai]

@OmarShehata Is this hopefully merged in the next release version?

[OmarShehata]

@verybigzhouhai I think this looks good. The only thing I would change is the name of the test:

can`t have an oriented bounding box with 0 volume

To maybe something like:

does not crash for bounding box with 0 volume

Other than that, @lilleyse do you want to confirm that this was the fix you had in mind for:

I would also make sure that the axes are still orthogonal to each other when one or two of the half-axes are non-zero length.

[verybigzhouhai]

@OmarShehata I have modified the name.

[cesium-concierge]

Thanks again for your contribution @verybigzhouhai!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[hpinkos]

Thanks @verybigzhouhai! I pushed a change to the halfAxes check in TileOrientedBoundingBox because the axes need to be orthogonal.

@OmarShehata or @IanLilleyT can you review?

[OmarShehata]

Thanks for making this more robust Hannah. I updated the spec which had pretty low coverage for all the new combinations that checkHalfAxes was checking. It should now be 100% covered.

I spoke to @lilleyse briefly about this last night - I'd still like to get your blessing before merging that this is both necessary and appropriate since it was your original suggestion. From my side, this looks good, and since it exits early from the common case (and seems to only run on tile creation) I think it shouldn't cause any performance dips.

[lilleyse]

@OmarShehata it looks like the test if failing.

I'm good with the approach here. Just want to add that the check will also happen when the tile transform changes but that's not much of a concern either.

[lilleyse]

Also CHANGES.md needs to be updated.

[OmarShehata]

Thanks Sean! I had a typo in the test, my bad. This should be all good now.

Thanks @verybigzhouhai for finding the issue and contributing this fix!