spatial_index/quad_tree - remove `operator>>' requirement, instead value_type stored must have standard layout for correct (de)serialisation + update unit test

include/mapnik/quad_tree.hpp

@@ -30,6 +30,7 @@
 // stl
 #include <algorithm>
 #include <vector>
+#include <type_traits>
 
 namespace mapnik
 {
@@ -172,6 +173,8 @@ public:
     template <typename OutputStream>
     void write(OutputStream & out)
     {
+        static_assert(std::is_standard_layout<value_type>::value,
+                      "Values stored in quad-tree must be standard layout types to allow serialisation");
         char header[16];
         std::memset(header,0,16);
         header[0]='m';

include/mapnik/util/spatial_index.hpp

@@ -23,10 +23,13 @@
 #ifndef MAPNIK_UTIL_SPATIAL_INDEX_HPP
 #define MAPNIK_UTIL_SPATIAL_INDEX_HPP
 
+//mapnik
 #include <mapnik/coord.hpp>
 #include <mapnik/box2d.hpp>
 #include <mapnik/query.hpp>
 #include <mapnik/geom_util.hpp>
+// stl
+#include <type_traits>
 
 using mapnik::box2d;
 using mapnik::query;
@@ -43,16 +46,17 @@ private:
 
     spatial_index();
     ~spatial_index();
-    spatial_index(const spatial_index&);
-    spatial_index& operator=(const spatial_index&);
+    spatial_index(spatial_index const&);
+    spatial_index& operator=(spatial_index const&);
     static int read_ndr_integer(InputStream& in);
     static void read_envelope(InputStream& in, box2d<double>& envelope);
-    static void query_node(const Filter& filter, InputStream& in, std::vector<Value> & results);
+    static void query_node(Filter const& filter, InputStream& in, std::vector<Value> & results);
 };
 
 template <typename Value, typename Filter, typename InputStream>
 box2d<double> spatial_index<Value, Filter, InputStream>::bounding_box(InputStream& in)
 {
+    static_assert(std::is_standard_layout<Value>::value, "Values stored in quad-tree must be standard layout type");
     in.seekg(16 + 4, std::ios::beg);
     box2d<double> box;
     read_envelope(in, box);
@@ -63,6 +67,7 @@ box2d<double> spatial_index<Value, Filter, InputStream>::bounding_box(InputStrea
 template <typename Value, typename Filter, typename InputStream>
 void spatial_index<Value, Filter, InputStream>::query(Filter const& filter, InputStream& in, std::vector<Value>& results)
 {
+    static_assert(std::is_standard_layout<Value>::value, "Values stored in quad-tree must be standard layout types");
     in.seekg(16, std::ios::beg);
     query_node(filter, in, results);
 }
@@ -83,7 +88,7 @@ void spatial_index<Value, Filter, InputStream>::query_node(Filter const& filter,
     for (int i = 0; i < num_shapes; ++i)
     {
         Value item;
-        in >> item;
+        in.read(reinterpret_cast<char*>(&item), sizeof(Value));
         results.push_back(std::move(item));
     }
 

plugins/input/csv/csv_datasource.cpp

@@ -610,6 +610,7 @@ mapnik::featureset_ptr csv_datasource::features(mapnik::query const& q) const
         }
         else if (has_disk_index_)
         {
+            std::cerr << "DISK_INDEX" << std::endl;
             mapnik::filter_in_box filter(q.get_bbox());
             return std::make_shared<csv_index_featureset>(filename_, filter, locator_, separator_, headers_, ctx_);
         }

plugins/input/csv/csv_index_featureset.hpp

@@ -35,18 +35,9 @@
 #include <mapnik/mapped_memory_cache.hpp>
 #endif
 
-using value_type = std::pair<std::size_t, std::size_t>;
-namespace std {
-template <typename InputStream>
-InputStream & operator>>(InputStream & in, value_type & value)
-{
-    in.read(reinterpret_cast<char*>(&value), sizeof(value_type));
-    return in;
-}
-}
-
 class csv_index_featureset : public mapnik::Featureset
 {
+    using value_type = std::pair<std::size_t, std::size_t>;
     using locator_type = detail::geometry_column_locator;
 public:
 

test/unit/datasource/spatial_index.cpp

@@ -20,17 +20,21 @@
  *
  *****************************************************************************/
 
+#include <iostream>
+#include <fstream>
+
 #include "catch.hpp"
 
-#include <mapnik/util/spatial_index.hpp>
 #include <mapnik/quad_tree.hpp>
+#include <mapnik/util/spatial_index.hpp>
 
 TEST_CASE("spatial_index")
 {
     SECTION("mapnik::quad_tree<T>")
     {
-        // value type to store inside index (must provide operator<< + operator>>)
-        using value_type = std::size_t;
+        // value_type must have standard layout (http://en.cppreference.com/w/cpp/types/is_standard_layout)
+        using value_type = std::int32_t;
+        using mapnik::filter_in_box;
         mapnik::box2d<double> extent(0,0,100,100);
         mapnik::quad_tree<value_type> tree(extent);
         REQUIRE(tree.extent() == extent);
@@ -38,7 +42,33 @@ TEST_CASE("spatial_index")
         tree.insert(1, mapnik::box2d<double>(10,10,20,20));
         tree.insert(2, mapnik::box2d<double>(30,30,40,40));
         tree.insert(3, mapnik::box2d<double>(30,10,40,20));
-        REQUIRE(tree.count() == 3);
-        REQUIRE(tree.count_items() == 3);
+        tree.insert(4, mapnik::box2d<double>(1,1,2,2));
+        tree.trim();
+
+        REQUIRE(tree.count() == 10);
+        REQUIRE(tree.count_items() == 4);
+
+        // serialise
+        std::ostringstream out(std::ios::binary);
+        tree.write(out);
+        out.flush();
+
+        REQUIRE(out.str().length() == 472);
+        REQUIRE(out.str().at(0) == 'm');
+
+        // read bounding box
+        std::istringstream in(out.str(), std::ios::binary);
+        auto box = mapnik::util::spatial_index<value_type, filter_in_box, std::istringstream>::bounding_box(in);
+        REQUIRE(box == tree.extent());
+        // bounding box query
+        std::vector<value_type> results;
+        filter_in_box filter(box);
+        mapnik::util::spatial_index<value_type, filter_in_box, std::istringstream>::query(filter, in, results);
+
+        REQUIRE(results[0] == 1);
+        REQUIRE(results[1] == 4);
+        REQUIRE(results[2] == 3);
+        REQUIRE(results[3] == 2);
+        REQUIRE(results.size() == 4);
     }
 }