Skip to content

Commit

Permalink
macOS fixes, especially to losslessNarrow.
Browse files Browse the repository at this point in the history
  • Loading branch information
kring committed Jan 21, 2025
1 parent c11a48d commit fc8f124
Show file tree
Hide file tree
Showing 14 changed files with 389 additions and 148 deletions.
6 changes: 6 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

### v0.44.0 - 2025-02-03

##### Breaking Changes :mega:

- Removed `Math::rotation`. Use `glm::rotation` from `<glm/gtx/quaternion.hpp>` instead.
- 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`.

##### Additions :tada:

- Added conversion of I3dm batch table metadata to `EXT_structural_metadata` and `EXT_instance_features` extensions.
Expand Down
2 changes: 0 additions & 2 deletions Cesium3DTilesContent/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,3 @@ target_link_libraries(Cesium3DTilesContent
PRIVATE
libmorton::libmorton
)

target_compile_definitions(Cesium3DTilesContent PRIVATE GLM_ENABLE_EXPERIMENTAL)
2 changes: 0 additions & 2 deletions CesiumGeometry/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,3 @@ cesium_target_include_directories(
target_link_libraries(CesiumGeometry PUBLIC
CesiumUtility
)

target_compile_definitions(CesiumGeometry PRIVATE GLM_ENABLE_EXPERIMENTAL)
2 changes: 0 additions & 2 deletions CesiumGeospatial/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ PRIVATE
s2::s2
)

target_compile_definitions(CesiumGeospatial PRIVATE GLM_ENABLE_EXPERIMENTAL)

if(CESIUM_DISABLE_DEFAULT_ELLIPSOID)
target_compile_definitions(CesiumGeospatial PUBLIC CESIUM_DISABLE_DEFAULT_ELLIPSOID=1)
endif()
2 changes: 0 additions & 2 deletions CesiumGltf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,3 @@ target_link_libraries(CesiumGltf
PUBLIC
CesiumUtility
)

target_compile_definitions(CesiumGltf PRIVATE GLM_ENABLE_EXPERIMENTAL)
14 changes: 1 addition & 13 deletions CesiumGltf/include/CesiumGltf/MetadataConversions.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,14 @@
#include <CesiumUtility/JsonValue.h>

#include <glm/common.hpp>
#include <glm/gtx/string_cast.hpp>

#include <cerrno>
#include <cstdint>
#include <optional>
#include <string>
#include <string_view>

#ifndef GLM_ENABLE_EXPERIMENTAL
// If we define GLM_ENABLE_EXPERIMENTAL here, we undefine it at the end of this
// header file.
#define GLM_ENABLE_EXPERIMENTAL
#define GLM_ENABLE_EXPERIMENTAL_defined_locally
#endif
#include <glm/gtx/string_cast.hpp>

namespace CesiumGltf {
/**
* @brief Default conversion between two types. No actual conversion is defined.
Expand Down Expand Up @@ -857,8 +850,3 @@ struct MetadataConversions<
#pragma endregion

} // namespace CesiumGltf

#ifdef GLM_ENABLE_EXPERIMENTAL_defined_locally
#undef GLM_ENABLE_EXPERIMENTAL
#undef GLM_ENABLE_EXPERIMENTAL_defined_locally
#endif
121 changes: 118 additions & 3 deletions CesiumGltf/test/TestJsonValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <doctest/doctest.h>

#include <cmath>
#include <cstdint>
#include <limits>

Expand Down Expand Up @@ -41,9 +42,13 @@ TEST_CASE(

TEST_CASE("JsonValue::getSafeNumber() returns std::nullopt if narrowing "
"conversion error would occur") {
SUBCASE("2^64 - 1 cannot be converted back to a double") {
auto value = JsonValue(std::numeric_limits<std::uint64_t>::max());
REQUIRE(!value.getSafeNumber<double>().has_value());
SUBCASE("At least one of 2^64 - 1 and 2^64 - 2 cannot be converted back to a "
"double") {
auto value1 = JsonValue(std::numeric_limits<std::uint64_t>::max());
auto value2 = JsonValue(std::numeric_limits<std::uint64_t>::max() - 1);
bool oneCantConvert = !value1.getSafeNumber<double>().has_value() ||
!value2.getSafeNumber<double>().has_value();
REQUIRE(oneCantConvert);
}

SUBCASE("-2^64 cannot be converted back to a double") {
Expand Down Expand Up @@ -100,3 +105,113 @@ TEST_CASE("JsonValue Equality operator") {

CHECK(booleanValueTrue2 == booleanValueTrue);
}

TEST_CASE("losslessNarrow") {
SUBCASE("identity casts") {
CHECK(losslessNarrow<double, double>(1.0) == 1.0);
CHECK(losslessNarrow<double, double>(-1.0) == -1.0);
}

SUBCASE("float-to-double") {
CHECK(losslessNarrow<double, float>(1.0f) == 1.0);
CHECK(losslessNarrow<double, float>(-1.0f) == -1.0);

std::optional<double> result =
losslessNarrow<double, float>(std::numeric_limits<float>::quiet_NaN());
bool nanny = result.has_value() && std::isnan(*result);
CHECK(nanny);
CHECK(
losslessNarrow<double, float>(std::numeric_limits<float>::infinity()) ==
std::numeric_limits<double>::infinity());
}

SUBCASE("double-to-float") {
CHECK(losslessNarrow<float, double>(1.0) == 1.0f);
CHECK(losslessNarrow<float, double>(-1.0) == -1.0f);
CHECK(losslessNarrow<float, double>(1e300) == std::nullopt);
CHECK(losslessNarrow<float, double>(-1e300) == std::nullopt);
CHECK(losslessNarrow<float, double>(1.2345678901234) == std::nullopt);

std::optional<float> result =
losslessNarrow<float, double>(std::numeric_limits<double>::quiet_NaN());
bool nanny = result.has_value() && std::isnan(*result);
CHECK(nanny);
CHECK(
losslessNarrow<float, double>(
std::numeric_limits<double>::infinity()) ==
std::numeric_limits<float>::infinity());
}

SUBCASE("double-to-integer") {
CHECK(losslessNarrow<int8_t, double>(1.0) == 1);
CHECK(losslessNarrow<int8_t, double>(-1.0) == -1);
CHECK(losslessNarrow<int8_t, double>(1.1) == std::nullopt);
CHECK(losslessNarrow<int8_t, double>(127.0) == 127);
CHECK(losslessNarrow<int8_t, double>(128.0) == std::nullopt);
CHECK(losslessNarrow<uint8_t, double>(1.0) == 1);
CHECK(losslessNarrow<uint8_t, double>(-1.0) == std::nullopt);
CHECK(losslessNarrow<uint8_t, double>(255.0) == 255);
CHECK(losslessNarrow<uint8_t, double>(256.0) == std::nullopt);
CHECK(
losslessNarrow<uint8_t, double>(
std::numeric_limits<double>::quiet_NaN()) == std::nullopt);
CHECK(
losslessNarrow<uint8_t, double>(
std::numeric_limits<double>::infinity()) == std::nullopt);
}

SUBCASE("integer-to-double") {
auto isCorrectOrNullOpt = [](std::optional<double> result,
int64_t expected) {
if (result.has_value()) {
CHECK(expected == static_cast<int64_t>(result.value()));
}
};

CHECK(losslessNarrow<double, int64_t>(1) == 1.0);
CHECK(
losslessNarrow<double, int64_t>(4294967296LL) == 4294967296.0); // 2^32
isCorrectOrNullOpt(
losslessNarrow<double, int64_t>(9223372036854775807LL),
9223372036854775807LL); // 2^63 - 1
isCorrectOrNullOpt(
losslessNarrow<double, int64_t>(9223372036854775806LL),
9223372036854775806LL); // 2^63 - 2
isCorrectOrNullOpt(
losslessNarrow<double, int64_t>(9223372036854775805LL),
9223372036854775805LL); // 2^63 - 3
}

SUBCASE("signed integers") {
CHECK(losslessNarrow<int8_t, int64_t>(1) == 1);
CHECK(losslessNarrow<int8_t, int64_t>(127) == 127);
CHECK(losslessNarrow<int8_t, int64_t>(128) == std::nullopt);
CHECK(losslessNarrow<int8_t, int64_t>(-1) == -1);
CHECK(losslessNarrow<int8_t, int64_t>(-127) == -127);
CHECK(losslessNarrow<int8_t, int64_t>(-128) == -128);
CHECK(losslessNarrow<int8_t, int64_t>(-129) == std::nullopt);
}

SUBCASE("unsigned integers") {
CHECK(losslessNarrow<uint8_t, uint64_t>(1) == 1);
CHECK(losslessNarrow<uint8_t, uint64_t>(255) == 255);
CHECK(losslessNarrow<uint8_t, uint64_t>(256) == std::nullopt);
}

SUBCASE("signed integers to unsigned integers") {
CHECK(losslessNarrow<uint8_t, int8_t>(1) == 1.0);
CHECK(losslessNarrow<uint8_t, int8_t>(-1) == std::nullopt);
CHECK(losslessNarrow<uint8_t, int8_t>(127) == 127);
CHECK(losslessNarrow<uint8_t, int64_t>(127) == 127);
CHECK(losslessNarrow<uint8_t, int64_t>(128) == 128);
CHECK(losslessNarrow<uint8_t, int64_t>(127) == 127);
CHECK(losslessNarrow<uint8_t, int64_t>(255) == 255);
CHECK(losslessNarrow<uint8_t, int64_t>(256) == std::nullopt);
}

SUBCASE("unsigned integers to signed integers") {
CHECK(losslessNarrow<int8_t, uint8_t>(127) == 127);
CHECK(losslessNarrow<int8_t, uint8_t>(128) == std::nullopt);
CHECK(losslessNarrow<int16_t, uint8_t>(128) == 128);
}
}
2 changes: 0 additions & 2 deletions CesiumGltfContent/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ set_target_properties(CesiumGltfContent
PUBLIC_HEADER "${CESIUM_GLTF_CONTENT_PUBLIC_HEADERS}"
)

target_compile_definitions(CesiumGltfContent PRIVATE GLM_ENABLE_EXPERIMENTAL)

target_sources(
CesiumGltfContent
PRIVATE
Expand Down
1 change: 0 additions & 1 deletion CesiumNativeTests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ PRIVATE

target_compile_definitions(cesium-native-tests
PRIVATE
GLM_ENABLE_EXPERIMENTAL
CESIUM_NATIVE_DATA_DIR=\"${CMAKE_CURRENT_LIST_DIR}/../data\"
)

Expand Down
28 changes: 3 additions & 25 deletions CesiumUtility/include/CesiumUtility/JsonValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,8 @@ namespace CesiumUtility {
* @tparam T The type to convert to.
* @param u The value to perform the conversion on.
*/
template <typename T, typename U>
constexpr std::optional<T> losslessNarrow(U u) noexcept {
constexpr const bool is_different_signedness =
(std::is_signed<T>::value != std::is_signed<U>::value);

const T t = static_cast<T>(u);

if (static_cast<U>(t) != u ||
(is_different_signedness && ((t < T{}) != (u < U{})))) {
return std::nullopt;
}

return t;
}
template <typename TTo, typename TFrom>
extern std::optional<TTo> losslessNarrow(TFrom from) noexcept;

/**
* @brief Attempts a narrowing conversion of `U` into `T` without losing
Expand All @@ -53,17 +41,7 @@ constexpr std::optional<T> losslessNarrow(U u) noexcept {
*/
template <typename T, typename U>
constexpr T losslessNarrowOrDefault(U u, T defaultValue) noexcept {
constexpr const bool is_different_signedness =
(std::is_signed<T>::value != std::is_signed<U>::value);

const T t = static_cast<T>(u);

if (static_cast<U>(t) != u ||
(is_different_signedness && ((t < T{}) != (u < U{})))) {
return defaultValue;
}

return t;
return losslessNarrow<T, U>(u).value_or(defaultValue);
}

/**
Expand Down
47 changes: 0 additions & 47 deletions CesiumUtility/include/CesiumUtility/Math.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,53 +456,6 @@ class CESIUMUTILITY_API Math final {
return down;
}
}

/**
* @brief Construct a vector perpendicular to the argument.
* @param v The input vector
* @return A vector perpendicular to the input vector
*/
template <typename T, glm::qualifier Q>
static glm::vec<3, T, Q> perpVec(const glm::vec<3, T, Q>& v) {
// This constructs a vector whose dot product with v will be 0, hence
// perpendicular to v. As seen in the "Physically Based Rendering".
if (std::abs(v.x) > std::abs(v.y)) {
return glm::vec<3, T, Q>(-v.z, 0, v.x) / std::sqrt(v.x * v.x + v.z * v.z);
}
return glm::vec<3, T, Q>(0, v.z, -v.y) / std::sqrt(v.y * v.y + v.z * v.z);
}

/** @brief Compute the rotation between two unit vectors.
* @param vec1 The first vector.
* @param vec2 The second vector.
* @return A quaternion representing the rotation of vec1 to vec2.
*/
template <typename T, glm::qualifier Q>
static glm::qua<T, Q>
rotation(const glm::vec<3, T, Q>& vec1, const glm::vec<3, T, Q>& vec2) {
// If we take the dot and cross products of the two vectors and store
// them in a quaternion, that quaternion represents twice the required
// rotation. We get the correct quaternion by "averaging" with the zero
// rotation quaternion, in a way analagous to finding the half vector
// between two 3D vectors.
auto cosRot = dot(vec1, vec2);
auto rotAxis = cross(vec1, vec2);
auto rotAxisLen2 = dot(rotAxis, rotAxis);
// Not using epsilon for these tests. If abs(cosRot) < 1.0, we can still
// create a sensible rotation.
if (cosRot >= 1 || (rotAxisLen2 == 0 && cosRot > 0)) {
// zero rotation
return glm::qua<T, Q>(1, 0, 0, 0);
}
if (cosRot <= -1 || (rotAxisLen2 == 0 && cosRot < 0)) {
auto perpAxis = CesiumUtility::Math::perpVec(vec1);
// rotation by pi radians
return glm::qua<T, Q>(0, perpAxis);
}

glm::qua<T, Q> sumQuat(cosRot + 1, rotAxis);
return normalize(sumQuat);
}
};

} // namespace CesiumUtility
Loading

0 comments on commit fc8f124

Please sign in to comment.