CesiumGS / CesiumGS/cesium-native

Consider to not throw invalid_argument exceptions

Open
#205 3 comments 0 reactions 0 assignees View on GitHub
quality
Dominant language
C++
Stars
623
Forks
277
PR merge metrics
No merged PRs in 30d

Description

Right now, there are few places where an `invalid_argument` exception can be thrown:

- In the `Plane` constructor, when the given normal is not normalized ( https://github.com/CesiumGS/cesium-native/blob/8a2957d32f433438b6b91ad37059c1ca0c8665c5/CesiumGeometry/src/Plane.cpp#L13 )
- In the `Ray` constructor, when the ray direction is not normalized ( https://github.com/CesiumGS/cesium-native/blob/8a2957d32f433438b6b91ad37059c1ca0c8665c5/CesiumGeometry/src/Ray.cpp#L13 )
- In the `EllipsoidTangentPlane` class, when the origin is near the center ( https://github.com/CesiumGS/cesium-native/blob/b824699d3dd9503a5943007247f683b6adc132d4/CesiumGeospatial/src/EllipsoidTangentPlane.cpp#L51 )

As much as I'm a fan of early, clear error checks and responses, we should consider handling this differently: Recent crashes in Unreal ( https://github.com/CesiumGS/cesium-unreal/issues/290 and https://github.com/CesiumGS/cesium-unreal/issues/336 ) have been caused by this, and these crashes are hard, painful, and non-recoverable.

I'm not sure about the best solution for this.

- Simple solutions (like filling stuff with `NaN` or so) are certainly not appropriate, becauses these will have the effect of "Nothing is rendered", **without** any clue of what's happening at all.
- Enforcing to wrap the creation of these objects into `try-catch` will be hard (and inconvenient)
- More strict, invasive solutions could be (brainstorming) to make the constructors of these classes `private`, and only allow them to be created with factory methods that return an `std::optional`. We could also consider separate `createThrowing`/`createUncheckedWithNaN`/`createOptional` factory methods.

I just did a websearch about what e.g. GLM does for a `normalize` call on zero-length vectors, but... surprisingly there is no obvious information for that (have to try it out, I guess). The point is that we should consider "following conventions" for such cases ... if there *are* conventions. Otherwise, it largely depends on the desired and expected behavior, and the crucial question of "What should happen when the precondition does not hold?".

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.