From 26c9732b4e308691006a0c1ae577f2c6da2eedbb Mon Sep 17 00:00:00 2001
From: Benjamin Doherty <bendoherty@google.com>
Date: Fri, 9 Jul 2021 12:16:38 -0700
Subject: [PATCH] Fix, MSVC's std::sort fails on Zip2Iterator

---
 libs/utils/include/utils/Zip2Iterator.h |  7 +---
 libs/utils/test/test_Zip2Iterator.cpp   | 49 ++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/libs/utils/include/utils/Zip2Iterator.h b/libs/utils/include/utils/Zip2Iterator.h
index a85a33663c..1bdb423b90 100644
--- a/libs/utils/include/utils/Zip2Iterator.h
+++ b/libs/utils/include/utils/Zip2Iterator.h
@@ -51,7 +51,6 @@ public:
 
     using value_type = std::pair<Val1, Val2>;
     using reference = Ref;
-    using pointer = value_type*;
     using difference_type = ptrdiff_t;
     using iterator_category = std::random_access_iterator_tag;
 
@@ -60,11 +59,9 @@ public:
     Zip2Iterator(Zip2Iterator const& rhs) noexcept = default;
     Zip2Iterator& operator=(Zip2Iterator const& rhs) = default;
 
-    value_type operator*() const { return { *mIt.first, *mIt.second }; }
-    reference  operator*()       { return { *mIt.first, *mIt.second }; }
+    reference operator*() const { return { *mIt.first, *mIt.second }; }
 
-    const value_type operator[](size_t n) const { return *(*this + n); }
-          reference  operator[](size_t n)       { return *(*this + n); }
+    reference operator[](size_t n) const { return *(*this + n); }
 
     Zip2Iterator& operator++() {
         ++mIt.first;
diff --git a/libs/utils/test/test_Zip2Iterator.cpp b/libs/utils/test/test_Zip2Iterator.cpp
index d76d0ce635..f7329206e8 100644
--- a/libs/utils/test/test_Zip2Iterator.cpp
+++ b/libs/utils/test/test_Zip2Iterator.cpp
@@ -17,6 +17,7 @@
 #include <gtest/gtest.h>
 
 #include <utils/Zip2Iterator.h>
+#include <utils/StructureOfArrays.h>
 
 #include <algorithm>
 #include <array>
@@ -37,12 +38,56 @@ TEST(Zip2IteratorTest, sort) {
     EXPECT_FALSE(std::is_sorted(keys.begin(), keys.end()));
     EXPECT_FALSE(std::is_sorted(values.begin(), values.end()));
 
-#if !defined(WIN32)
     Zip2Iterator<Array::iterator, Array::iterator> b = { values.begin(), keys.begin() };
     Zip2Iterator<Array::iterator, Array::iterator> e = b + values.size();
     std::sort(b, e, [](auto const& lhs, auto const& rhs) { return lhs.second < rhs.second; });
 
     EXPECT_TRUE(std::is_sorted(keys.begin(), keys.end()));
     EXPECT_TRUE(std::is_sorted(values.begin(), values.end()));
-#endif
+}
+
+enum {
+    LIGHT_INSTANCE,
+};
+
+using LightSoa = utils::StructureOfArrays<
+    int
+>;
+
+TEST(Zip2IteratorTest, soa) {
+    LightSoa lightData;
+
+    lightData.push_back(3);
+    lightData.push_back(2);
+    lightData.push_back(1);
+
+    float distances[3] = {1.00f, 0.5f, 0.25f};
+
+    EXPECT_EQ(lightData.size(), 3);
+
+    Zip2Iterator<LightSoa::iterator, float*> z = { lightData.begin(), distances };
+
+    // To sort lightData by distance, we just need to swap the first and third elements.
+    // MSVC's std::sort does something akin to the following:
+
+    // using _BidIt = Zip2Iterator<LightSoa::iterator, float*>;
+    // const _BidIt _First = z;
+    // const _BidIt _Last = z + 2;
+
+    // std::iter_value_t<_BidIt> _Val = std::move(*_First);
+    // *_First = std::move(*_Last);
+    // *_Last = std::move(_Val);
+
+    std::sort(z, z + lightData.size(),
+          [](auto const& lhs, auto const& rhs) { return lhs.second < rhs.second; });
+
+    auto const* instances = lightData.data<LIGHT_INSTANCE>();
+
+    EXPECT_EQ(distances[0], 0.25f);
+    EXPECT_EQ(distances[1], 0.5f);
+    EXPECT_EQ(distances[2], 1.0f);
+
+    EXPECT_EQ(instances[0], 1);
+    EXPECT_EQ(instances[1], 2);
+    EXPECT_EQ(instances[2], 3);
 }