Fix for possible normalization issue for line corner points
Description
If points in PolylineVolume or Corridor are collinear, computation fails with error "Normalized result is not a number" in the line below, because sum of normalized forward and backward vectors is zero.
https://github.com/CesiumGS/cesium/blob/0e9a425b475cd3cfdd90f35e9cdbdda453e448d8/packages/engine/Source/Core/PolylineVolumeGeometryLibrary.js#L438
The cornerDirection variable is only used within the doCorner === true execution branch, so I moved it inside the branch for better scoping. From my understanding, the forward and backward projection checks within the doCorner condition exclude the computation of corners for collinear points. As a result, normalizing cornerDirection in this execution branch should not cause any errors.
Issue number and link
Fixes #12254
Testing plan
This issue is difficult to replicate. I encountered it by chance, with the data shown in Sandcastle example attached to related issue.
I haven't yet found a way to generate synthetic data to reproduce the issue, mainly due to the scaleToSurface call, which modifies coordinates based on a specific location.
https://github.com/CesiumGS/cesium/blob/0e9a425b475cd3cfdd90f35e9cdbdda453e448d8/packages/engine/Source/Core/PolylineVolumeGeometryLibrary.js#L35
To test it, I followed this approach:
- I created a simplified version of
computePositionsthat omits the scaleToSurface call and other irrelevant operations. This resulted in a "Normalized result is not a number" error when passing a simple collinear line like[[0,0,0], [1,1,1], [2,2,2], [3,3,3]]as thepositionsinput. - I also tested the problematic data locally, both before and after the changes. The error was resolved after applying the changes.
Author checklist
- [x] I have submitted a Contributor License Agreement
- [x] I have added my name to
CONTRIBUTORS.md - [x] 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, @p-skakun!
:white_check_mark: We can confirm we have a CLA on file for you.
I added a few tests, but the release-tests check is failing for an unrelated (at least, it appears that way) spec.
Thanks @p-skakun! The CI test failure is due to https://github.com/CesiumGS/cesium/issues/11958, not this PR.