Floor is an order of magnitude faster!

Ekumen-OS · Nov 5, 2022 · e528efa · e528efa
1 parent 6b51fca
commit e528efa
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 8 deletions.
diff --git a/beluga/core/include/beluga/voxel_hash.h b/beluga/core/include/beluga/voxel_hash.h
@@ -25,12 +25,17 @@ struct voxel_hash<
   template <std::size_t... Ids>
   static constexpr std::size_t hash_impl(const Tuple<Types...>& tuple, double voxel_size, std::index_sequence<Ids...>) {
     constexpr auto kBits = std::numeric_limits<std::size_t>::digits / sizeof...(Ids);
-    return (round_and_shift<kBits, Ids>(std::get<Ids>(tuple) / voxel_size) | ...);
+    return (floor_and_shift<kBits, Ids>(std::get<Ids>(tuple) / voxel_size) | ...);
   }
 
   template <std::size_t N, std::size_t I, class T, typename = std::enable_if_t<std::is_arithmetic_v<T>>>
-  static constexpr std::size_t round_and_shift(T value) {
-    return std::bitset<N>(std::llround(value)).to_ullong() << (I * N);
+  static constexpr std::size_t floor_and_shift(T value) {
+    // Signed to unsigned conversion was "implementation defined" until C++20.
+    // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0907r4.html
+    // The following line was tested on clang, msvc and gcc x86-64 to produce the 
+    // expected two’s complement representation with -std=c++17.
+    std::size_t unsigned_value = static_cast<long long>(std::floor(value));
+    return std::bitset<N>{unsigned_value}.to_ullong() << (I * N);
   }
 };
 

diff --git a/beluga/core/test/voxel_hash_test.cpp b/beluga/core/test/voxel_hash_test.cpp
@@ -25,7 +25,7 @@ TEST(VoxelHash, Round) {
 TEST(VoxelHash, VoxelSize) {
   using Tuple = std::tuple<double, double>;
   constexpr double kVoxelSize = 0.1;
-  auto hash1 = beluga::voxel_hash<Tuple>{}(std::make_tuple(10.3, 5.1), kVoxelSize);
+  auto hash1 = beluga::voxel_hash<Tuple>{}(std::make_tuple(10.3, 5.13), kVoxelSize);
   auto hash2 = beluga::voxel_hash<Tuple>{}(std::make_tuple(10.33, 5.14), kVoxelSize);
   EXPECT_EQ(hash1, hash2);
   auto hash3 = beluga::voxel_hash<Tuple>{}(std::make_tuple(10.0, 5.0), kVoxelSize);
@@ -35,8 +35,8 @@ TEST(VoxelHash, VoxelSize) {
 TEST(VoxelHash, Negative) {
   using Tuple = std::tuple<double, double>;
   constexpr double kVoxelSize = 0.1;
-  auto hash1 = beluga::voxel_hash<Tuple>{}(std::make_tuple(-10.3, -2.1), kVoxelSize);
-  auto hash2 = beluga::voxel_hash<Tuple>{}(std::make_tuple(-10.33, -2.14), kVoxelSize);
+  auto hash1 = beluga::voxel_hash<Tuple>{}(std::make_tuple(-10.3, -2.13), kVoxelSize);
+  auto hash2 = beluga::voxel_hash<Tuple>{}(std::make_tuple(-10.27, -2.14), kVoxelSize);
   EXPECT_EQ(hash1, hash2);
   auto hash3 = beluga::voxel_hash<Tuple>{}(std::make_tuple(-10.0, -2.0), kVoxelSize);
   EXPECT_NE(hash1, hash3);
@@ -46,8 +46,8 @@ TEST(VoxelHash, NoCollisions) {
   using Tuple = std::tuple<double, double, double>;
   constexpr int kLimit = 100;
 
-  // Brute-force search for collisions
-  // With kLimit = 100 and a 3-element tuple, we test 8'120'601 combinations
+  // Brute-force search for collisions.
+  // With kLimit = 100 and a 3-element tuple, we test 8'120'601 combinations.
   auto hashes = ranges::views::cartesian_product(
                     ranges::views::closed_iota(-kLimit, kLimit), ranges::views::closed_iota(-kLimit, kLimit),
                     ranges::views::closed_iota(-kLimit, kLimit)) |