Allow resetting model matrix when tiles have regions #12409
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When loading a tileset where the tiles contained bounding regions, changing the
modelMatrix
of the tileset, and then setting themodelMatrix
back to its initial value, the update caused aA detailed description of what went wrong is in #12002 (comment)
While creating a (pretty strict, dedicated) spec, I noticed that the suggested fix does not fully cover it. There are basically four cases to consider:
result
is aTileOrientedBoundingBox
, then reuse itTileOrientedBoundingBox
, then create a new oneresult
is aTileBoundingRegion
, then reuse itTileBoundingRegion
, then create a new one(It's hard to be 100% sure that all cases can happen in practice, but easy to handle them generically)
Issue number and link
Fixes #12002
Fixes #12403
Testing plan
The issue descriptions contain examples that caused the crash (basically the same example, though).
Also, I added a dedicated spec for this.
NOTE: The whole spec is currently wrapped into a
expect ... not.toThrow
. This is the actual change that is done here. One could consider this as the "baseline", though, and remove this check, and only use the expect's for the respective bounding volume types.Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change