Skip to content

Commit

Permalink
Merge pull request #1083 from CesiumGS/culling-volume-crash
Browse files Browse the repository at this point in the history
Fix crash while creating culling volume when the camera is very far from the globe
  • Loading branch information
kring authored Jan 29, 2025
2 parents c14d37d + 344f386 commit 56ceceb
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Removed `Math::perpVector`. Use `glm::perp` from `<glm/gtx/perpendicular.hpp>` instead.
- Using Cesium Native in non-cmake projects now requires manually defining `GLM_ENABLE_EXPERIMENTAL`.
- cesium-native no longer uses the `GLM_FORCE_SIZE_T_LENGTH` option with the `glm` library
- `CullingVolume` has been moved from the `Cesium3DTilesSelection` namespace to the `CesiumGeometry` namespace.

##### Additions :tada:

Expand All @@ -24,6 +25,7 @@
- Fixed a bug in `SharedAssetDepot` that could cause assertion failures in debug builds, and could rarely cause premature deletion of shared assets even in release builds.
- Fixed a bug that could cause `Tileset::sampleHeightMostDetailed` to return a height that is not the highest one when the sampled tileset contained multiple heights at the given location.
- `LayerJsonTerrainLoader` will now log errors and warnings when failing to load a `.terrain` file referenced in the layer.json, instead of silently ignoring them.
- Fixed a crash in `CullingVolume` when the camera was very far away from the globe.

### v0.43.0 - 2025-01-02

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class CESIUM3DTILESSELECTION_API ViewState final {
const double _sseDenominator;
const std::optional<CesiumGeospatial::Cartographic> _positionCartographic;

const CullingVolume _cullingVolume;
const CesiumGeometry::CullingVolume _cullingVolume;
};

} // namespace Cesium3DTilesSelection
4 changes: 2 additions & 2 deletions CesiumGeometry/include/CesiumGeometry/CullingVolume.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include <CesiumGeometry/Plane.h>

namespace Cesium3DTilesSelection {
namespace CesiumGeometry {

/**
* @brief A culling volume, defined by four planes.
Expand Down Expand Up @@ -58,4 +58,4 @@ CullingVolume createCullingVolume(
const glm::dvec3& up,
double fovx,
double fovy) noexcept;
} // namespace Cesium3DTilesSelection
} // namespace CesiumGeometry
14 changes: 11 additions & 3 deletions CesiumGeometry/src/CullingVolume.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
#include <glm/geometric.hpp>
#include <glm/trigonometric.hpp>

namespace Cesium3DTilesSelection {
#include <algorithm>
#include <cmath>
#include <limits>

namespace CesiumGeometry {

CullingVolume createCullingVolume(
const glm::dvec3& position,
Expand All @@ -18,7 +22,11 @@ CullingVolume createCullingVolume(
const double r = glm::tan(0.5 * fovx);
const double l = -r;

const double n = 1.0;
const double positionLen = glm::length(position);
const double n = std::max(
1.0,
std::nextafter(positionLen, std::numeric_limits<double>::max()) -
positionLen);

// TODO: this is all ported directly from CesiumJS, can probably be refactored
// to be more efficient with GLM.
Expand Down Expand Up @@ -67,4 +75,4 @@ CullingVolume createCullingVolume(

return {leftPlane, rightPlane, topPlane, bottomPlane};
}
} // namespace Cesium3DTilesSelection
} // namespace CesiumGeometry
34 changes: 34 additions & 0 deletions CesiumGeometry/test/TestCullingVolume.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#include <CesiumGeometry/CullingVolume.h>
#include <CesiumUtility/Math.h>

#include <doctest/doctest.h>
#include <glm/ext/matrix_transform.hpp>
#include <glm/ext/vector_double3.hpp>
#include <glm/fwd.hpp>
#include <glm/gtx/euler_angles.hpp>

#include <cmath>

using namespace doctest;
using namespace CesiumGeometry;
using namespace CesiumUtility;

TEST_CASE("CullingVolume::createCullingVolume") {
SUBCASE("Shouldn't crash when too far from the globe") {
CHECK_NOTHROW(createCullingVolume(
glm::dvec3(1e20, 1e20, 1e20),
glm::dvec3(0, 0, 1),
glm::dvec3(0, 1, 0),
Math::PiOverTwo,
Math::PiOverTwo));
}

SUBCASE("Shouldn't crash at the center of the globe") {
CHECK_NOTHROW(createCullingVolume(
glm::dvec3(0, 0, 0),
glm::dvec3(0, 0, 1),
glm::dvec3(0, 1, 0),
Math::PiOverTwo,
Math::PiOverTwo));
}
}

0 comments on commit 56ceceb

Please sign in to comment.