Simplify proj_transform_cache implementation (ref https://github.com/mapnik/mapnik/pull/4191#issuecomment-813377082)

include/mapnik/feature_style_processor_impl.hpp

@@ -254,7 +254,7 @@ void feature_style_processor<Processor>::prepare_layer(layer_rendering_material
     }
 
     processor_context_ptr current_ctx = ds->get_context(ctx_map);
-    proj_transform const* proj_trans_ptr = m_.get_proj_transform(mat.proj0_.params(), mat.proj1_.params());
+    proj_transform const* proj_trans_ptr = proj_transform_cache::get(mat.proj0_.params(), mat.proj1_.params());
     box2d<double> query_ext = extent; // unbuffered
     box2d<double> buffered_query_ext(query_ext);  // buffered
 
@@ -493,7 +493,7 @@ void feature_style_processor<Processor>::render_material(layer_rendering_materia
     layer const& lay = mat.lay_;
 
     std::vector<rule_cache> const & rule_caches = mat.rule_caches_;
-    proj_transform const* proj_trans_ptr = m_.get_proj_transform(mat.proj0_.params(), mat.proj1_.params());
+    proj_transform const* proj_trans_ptr = proj_transform_cache::get(mat.proj0_.params(), mat.proj1_.params());
     bool cache_features = lay.cache_features() && active_styles.size() > 1;
 
     datasource_ptr ds = lay.datasource();

include/mapnik/map.hpp

@@ -33,7 +33,6 @@
 #include <mapnik/well_known_srs.hpp>
 #include <mapnik/image_compositing.hpp>
 #include <mapnik/font_engine_freetype.hpp>
-#include <mapnik/proj_transform.hpp>
 #include <mapnik/warning.hpp>
 MAPNIK_DISABLE_WARNING_PUSH
 #include <mapnik/warning_ignore.hpp>
@@ -53,7 +52,6 @@ using featureset_ptr = std::shared_ptr<Featureset>;
 class feature_type_style;
 class view_transform;
 class layer;
-struct proj_transform_cache;
 
 class MAPNIK_DECL Map : boost::equality_comparable<Map>
 {
@@ -103,7 +101,7 @@ private:
     boost::optional<std::string> font_directory_;
     freetype_engine::font_file_mapping_type font_file_mapping_;
     freetype_engine::font_memory_cache_type font_memory_cache_;
-    std::unique_ptr<proj_transform_cache> proj_cache_;
+
 public:
     using const_style_iterator = std::map<std::string,feature_type_style>::const_iterator;
     using style_iterator = std::map<std::string,feature_type_style>::iterator;
@@ -501,12 +499,9 @@ public:
         return font_memory_cache_;
     }
 
-    proj_transform const* get_proj_transform(std::string const& source, std::string const& dest) const;
-
 private:
     friend void swap(Map & rhs, Map & lhs);
     void fixAspectRatio();
-    void init_proj_transform(std::string const& source, std::string const& dest);
     void init_proj_transforms();
 };
 

include/mapnik/proj_transform_cache.hpp

@@ -24,53 +24,17 @@
 #ifndef MAPNIK_PROJ_TRANSFORM_CACHE_HPP
 #define MAPNIK_PROJ_TRANSFORM_CACHE_HPP
 
-#include <mapnik/util/noncopyable.hpp>
+#include <mapnik/config.hpp>
-#include <mapnik/proj_transform.hpp>
+#include <string>
-
-MAPNIK_DISABLE_WARNING_PUSH
-#include <mapnik/warning_ignore.hpp>
-#include <boost/functional/hash.hpp>
-#include <boost/unordered_map.hpp>
-#include <boost/utility/string_view.hpp>
-MAPNIK_DISABLE_WARNING_POP
 
 namespace mapnik  {
+class proj_transform; // fwd decl
+namespace proj_transform_cache {
 
-struct proj_transform_cache : util::noncopyable
+void MAPNIK_DECL init(std::string const& source, std::string const& dest);
-{
+proj_transform const* MAPNIK_DECL get(std::string const& source, std::string const& dest);
-    using key_type = std::pair<std::string, std::string>;
-    using compatible_key_type = std::pair<boost::string_view, boost::string_view>;
 
-    struct compatible_hash
+} // namespace proj_transform_cache
-    {
+} // mamespace mapnik
-        template <typename KeyType>
-        std::size_t operator() (KeyType const& key) const
-        {
-            using hash_type = boost::hash<typename KeyType::first_type>;
-            std::size_t seed = hash_type{}(key.first);
-            seed ^= hash_type{}(key.second) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
-            return seed;
-        }
-    };
-
-    struct compatible_predicate
-    {
-        bool operator()(compatible_key_type const& k1,
-                        compatible_key_type const& k2) const
-        {
-            return k1 == k2;
-        }
-    };
-
-    using cache_type = boost::unordered_map<key_type, std::unique_ptr<proj_transform>, compatible_hash>;
-
-    proj_transform_cache() = default;
-
-    thread_local static cache_type cache_;
-    void init(std::string const& source, std::string const& dest) const;
-    proj_transform const* get(std::string const& source, std::string const& dest) const;
-};
-
-}
 
 #endif // MAPNIK_PROJ_TRANSFORM_CACHE_HPP

src/map.cpp

@@ -76,8 +76,7 @@ Map::Map()
     extra_params_(),
     font_directory_(),
     font_file_mapping_(),
-    font_memory_cache_(),
+    font_memory_cache_() {}
-    proj_cache_(std::make_unique<proj_transform_cache>()) {}
 
 Map::Map(int width,int height, std::string const& srs)
     : width_(width),
@@ -91,8 +90,7 @@ Map::Map(int width,int height, std::string const& srs)
       extra_params_(),
       font_directory_(),
       font_file_mapping_(),
-      font_memory_cache_(),
+      font_memory_cache_() {}
-      proj_cache_(std::make_unique<proj_transform_cache>()) {}
 
 Map::Map(Map const& rhs)
     : width_(rhs.width_),
@@ -114,8 +112,7 @@ Map::Map(Map const& rhs)
       font_directory_(rhs.font_directory_),
       font_file_mapping_(rhs.font_file_mapping_),
       // on copy discard memory caches
-      font_memory_cache_(),
+      font_memory_cache_()
-      proj_cache_(std::make_unique<proj_transform_cache>())
 {
     init_proj_transforms();
 }
@@ -140,8 +137,7 @@ Map::Map(Map && rhs)
       extra_params_(std::move(rhs.extra_params_)),
       font_directory_(std::move(rhs.font_directory_)),
       font_file_mapping_(std::move(rhs.font_file_mapping_)),
-      font_memory_cache_(std::move(rhs.font_memory_cache_)),
+      font_memory_cache_(std::move(rhs.font_memory_cache_)) {}
-      proj_cache_(std::move(rhs.proj_cache_)) {}
 
 Map::~Map() {}
 
@@ -333,13 +329,13 @@ size_t Map::layer_count() const
 
 void Map::add_layer(layer const& l)
 {
-    init_proj_transform(srs_, l.srs());
+    proj_transform_cache::init(srs_, l.srs());
     layers_.emplace_back(l);
 }
 
 void Map::add_layer(layer && l)
 {
-    init_proj_transform(srs_, l.srs());
+    proj_transform_cache::init(srs_, l.srs());
     layers_.push_back(std::move(l));
 }
 
@@ -539,7 +535,7 @@ void Map::zoom_all()
             if (layer.active())
             {
                 std::string const& layer_srs = layer.srs();
-                proj_transform const* proj_trans_ptr = proj_cache_->get(srs_, layer_srs);;
+                proj_transform const* proj_trans_ptr = proj_transform_cache::get(srs_, layer_srs);
                 box2d<double> layer_ext = layer.envelope();
                 if (proj_trans_ptr->backward(layer_ext, PROJ_ENVELOPE_POINTS))
                 {
@@ -719,7 +715,7 @@ featureset_ptr Map::query_point(unsigned index, double x, double y) const
         mapnik::datasource_ptr ds = layer.datasource();
         if (ds)
         {
-            proj_transform const* proj_trans_ptr = proj_cache_->get(layer.srs(), srs_);
+            proj_transform const* proj_trans_ptr = proj_transform_cache::get(layer.srs(), srs_);
             double z = 0;
             if (!proj_trans_ptr->equal() && !proj_trans_ptr->backward(x,y,z))
             {
@@ -782,23 +778,13 @@ void Map::set_extra_parameters(parameters& params)
     extra_params_ = params;
 }
 
-mapnik::proj_transform const* Map::get_proj_transform(std::string const& source, std::string const& dest) const
-{
-    return proj_cache_->get(source, dest);
-}
-
-void Map::init_proj_transform(std::string const& source, std::string const& dest)
-{
-    proj_cache_->init(source, dest);
-}
-
 void Map::init_proj_transforms()
 {
     std::for_each(layers_.begin(),
                   layers_.end(),
                   [this] (auto const& l)
                   {
-                      init_proj_transform(srs_, l.srs());
+                      proj_transform_cache::init(srs_, l.srs());
                   });
 }
 

src/proj_transform_cache.cpp

@@ -20,13 +20,49 @@
  *
  *****************************************************************************/
 
+
 #include <mapnik/proj_transform_cache.hpp>
+#include <mapnik/proj_transform.hpp>
+MAPNIK_DISABLE_WARNING_PUSH
+#include <mapnik/warning_ignore.hpp>
+#include <boost/functional/hash.hpp>
+#include <boost/unordered_map.hpp>
+#include <boost/utility/string_view.hpp>
+MAPNIK_DISABLE_WARNING_POP
 
 namespace mapnik {
+namespace proj_transform_cache {
+namespace {
+using key_type = std::pair<std::string, std::string>;
+using compatible_key_type = std::pair<boost::string_view, boost::string_view>;
 
-thread_local proj_transform_cache::cache_type proj_transform_cache::cache_ = cache_type();
+struct compatible_hash
+{
+    template <typename KeyType>
+    std::size_t operator() (KeyType const& key) const
+    {
+        using hash_type = boost::hash<typename KeyType::first_type>;
+        std::size_t seed = hash_type{}(key.first);
+        seed ^= hash_type{}(key.second) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
+        return seed;
+    }
+};
 
-void proj_transform_cache::init(std::string const& source, std::string const& dest) const
+struct compatible_predicate
+{
+    bool operator()(compatible_key_type const& k1,
+                    compatible_key_type const& k2) const
+    {
+        return k1 == k2;
+    }
+};
+
+using cache_type = boost::unordered_map<key_type, std::unique_ptr<proj_transform>, compatible_hash>;
+thread_local static cache_type cache_ = {};
+
+} // namespace
+
+void init(std::string const& source, std::string const& dest)
 {
     compatible_key_type key = std::make_pair<boost::string_view, boost::string_view>(source, dest);
     auto itr = cache_.find(key, compatible_hash{}, compatible_predicate{});
@@ -39,7 +75,7 @@ void proj_transform_cache::init(std::string const& source, std::string const& de
     }
 }
 
-proj_transform const* proj_transform_cache::get(std::string const& source, std::string const& dest) const
+proj_transform const* get(std::string const& source, std::string const& dest)
 {
 
     compatible_key_type key = std::make_pair<boost::string_view, boost::string_view>(source, dest);
@@ -54,5 +90,5 @@ proj_transform const* proj_transform_cache::get(std::string const& source, std::
     return itr->second.get();
 }
 
-
+} // namespace proj_transform_cache
 } // namespace mapnik