From b974a4a683cc2fe52c8ff4c5ca697626ba351b34 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 6 Apr 2021 10:30:12 +0100 Subject: [PATCH] Simplify proj_transform_cache implementation (ref https://github.com/mapnik/mapnik/pull/4191#issuecomment-813377082) --- .../mapnik/feature_style_processor_impl.hpp | 4 +- include/mapnik/map.hpp | 7 +-- include/mapnik/proj_transform_cache.hpp | 52 +++---------------- src/map.cpp | 32 ++++-------- src/proj_transform_cache.cpp | 44 ++++++++++++++-- 5 files changed, 60 insertions(+), 79 deletions(-) diff --git a/include/mapnik/feature_style_processor_impl.hpp b/include/mapnik/feature_style_processor_impl.hpp index 96c4da3d1..775d8a379 100644 --- a/include/mapnik/feature_style_processor_impl.hpp +++ b/include/mapnik/feature_style_processor_impl.hpp @@ -254,7 +254,7 @@ void feature_style_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 query_ext = extent; // unbuffered box2d buffered_query_ext(query_ext); // buffered @@ -493,7 +493,7 @@ void feature_style_processor::render_material(layer_rendering_materia layer const& lay = mat.lay_; std::vector 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(); diff --git a/include/mapnik/map.hpp b/include/mapnik/map.hpp index 8cb81f5db..4f9e3d689 100644 --- a/include/mapnik/map.hpp +++ b/include/mapnik/map.hpp @@ -33,7 +33,6 @@ #include #include #include -#include #include MAPNIK_DISABLE_WARNING_PUSH #include @@ -53,7 +52,6 @@ using featureset_ptr = std::shared_ptr; class feature_type_style; class view_transform; class layer; -struct proj_transform_cache; class MAPNIK_DECL Map : boost::equality_comparable { @@ -103,7 +101,7 @@ private: boost::optional font_directory_; freetype_engine::font_file_mapping_type font_file_mapping_; freetype_engine::font_memory_cache_type font_memory_cache_; - std::unique_ptr proj_cache_; + public: using const_style_iterator = std::map::const_iterator; using style_iterator = std::map::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(); }; diff --git a/include/mapnik/proj_transform_cache.hpp b/include/mapnik/proj_transform_cache.hpp index b080081aa..6bd8581fe 100644 --- a/include/mapnik/proj_transform_cache.hpp +++ b/include/mapnik/proj_transform_cache.hpp @@ -24,53 +24,17 @@ #ifndef MAPNIK_PROJ_TRANSFORM_CACHE_HPP #define MAPNIK_PROJ_TRANSFORM_CACHE_HPP -#include -#include - -MAPNIK_DISABLE_WARNING_PUSH -#include -#include -#include -#include -MAPNIK_DISABLE_WARNING_POP +#include +#include namespace mapnik { +class proj_transform; // fwd decl +namespace proj_transform_cache { -struct proj_transform_cache : util::noncopyable -{ - using key_type = std::pair; - using compatible_key_type = std::pair; +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); - struct compatible_hash - { - template - std::size_t operator() (KeyType const& key) const - { - using hash_type = boost::hash; - 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, 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; -}; - -} +} // namespace proj_transform_cache +} // mamespace mapnik #endif // MAPNIK_PROJ_TRANSFORM_CACHE_HPP diff --git a/src/map.cpp b/src/map.cpp index bc7d4d2c4..e9146e4c5 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -76,8 +76,7 @@ Map::Map() extra_params_(), font_directory_(), font_file_mapping_(), - font_memory_cache_(), - proj_cache_(std::make_unique()) {} + font_memory_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_(), - proj_cache_(std::make_unique()) {} + font_memory_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_(), - proj_cache_(std::make_unique()) + font_memory_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_)), - proj_cache_(std::move(rhs.proj_cache_)) {} + font_memory_cache_(std::move(rhs.font_memory_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 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()); }); } diff --git a/src/proj_transform_cache.cpp b/src/proj_transform_cache.cpp index 1e051bd8a..88720c1d5 100644 --- a/src/proj_transform_cache.cpp +++ b/src/proj_transform_cache.cpp @@ -20,13 +20,49 @@ * *****************************************************************************/ + #include +#include +MAPNIK_DISABLE_WARNING_PUSH +#include +#include +#include +#include +MAPNIK_DISABLE_WARNING_POP namespace mapnik { +namespace proj_transform_cache { +namespace { +using key_type = std::pair; +using compatible_key_type = std::pair; -thread_local proj_transform_cache::cache_type proj_transform_cache::cache_ = cache_type(); +struct compatible_hash +{ + template + std::size_t operator() (KeyType const& key) const + { + using hash_type = boost::hash; + 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, 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(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(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