From 726724b3c7dcf8a534548c6294c41dbffd591bd4 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 1 Mar 2012 18:36:13 +0000 Subject: [PATCH 1/5] pass by ref move conversions impl into .cpp --- include/mapnik/ptree_helpers.hpp | 6 +- include/mapnik/util/conversions.hpp | 80 ++------------ plugins/input/postgis/connection.hpp | 2 - plugins/input/postgis/postgis_datasource.cpp | 12 +-- plugins/input/postgis/postgis_featureset.cpp | 2 +- src/build.py | 1 + src/conversions.cpp | 106 +++++++++++++++++++ src/image_util.cpp | 10 +- 8 files changed, 130 insertions(+), 89 deletions(-) create mode 100644 src/conversions.cpp diff --git a/include/mapnik/ptree_helpers.hpp b/include/mapnik/ptree_helpers.hpp index aaee81256..51708d0f7 100644 --- a/include/mapnik/ptree_helpers.hpp +++ b/include/mapnik/ptree_helpers.hpp @@ -218,7 +218,7 @@ template <> inline boost::optional fast_cast(std::string const& value) { int result; - if (mapnik::conversions::string2int(value,&result)) + if (mapnik::conversions::string2int(value,result)) return boost::optional(result); return boost::optional(); } @@ -227,7 +227,7 @@ template <> inline boost::optional fast_cast(std::string const& value) { double result; - if (mapnik::conversions::string2double(value,&result)) + if (mapnik::conversions::string2double(value,result)) return boost::optional(result); return boost::optional(); } @@ -236,7 +236,7 @@ template <> inline boost::optional fast_cast(std::string const& value) { float result; - if (mapnik::conversions::string2float(value,&result)) + if (mapnik::conversions::string2float(value,result)) return boost::optional(result); return boost::optional(); } diff --git a/include/mapnik/util/conversions.hpp b/include/mapnik/util/conversions.hpp index a5defe261..1b38dee3d 100644 --- a/include/mapnik/util/conversions.hpp +++ b/include/mapnik/util/conversions.hpp @@ -25,83 +25,19 @@ // mapnik -// boost -#include - // stl #include namespace mapnik { namespace conversions { -using namespace boost::spirit; - -// TODO - convert to templates - -static bool string2int(const char * value, int * result) -{ - size_t length = strlen(value); - if (length < 1 || value == NULL) - return false; - const char *begin = value; - const char *iter = begin; - const char *end = value + length; - bool r = qi::phrase_parse(iter,end,qi::int_,ascii::space,*result); - return r && (iter == end); -} - -static bool string2int(std::string const& value, int * result) -{ - if (value.empty()) - return false; - std::string::const_iterator str_beg = value.begin(); - std::string::const_iterator str_end = value.end(); - bool r = qi::phrase_parse(str_beg,str_end,qi::int_,ascii::space,*result); - return r && (str_beg == str_end); -} - -static bool string2double(std::string const& value, double * result) -{ - if (value.empty()) - return false; - std::string::const_iterator str_beg = value.begin(); - std::string::const_iterator str_end = value.end(); - bool r = qi::phrase_parse(str_beg,str_end,qi::double_,ascii::space,*result); - return r && (str_beg == str_end); -} - -static bool string2double(const char * value, double * result) -{ - size_t length = strlen(value); - if (length < 1 || value == NULL) - return false; - const char *begin = value; - const char *iter = begin; - const char *end = value + length; - bool r = qi::phrase_parse(iter,end,qi::double_,ascii::space,*result); - return r && (iter == end); -} - -static bool string2float(std::string const& value, float * result) -{ - if (value.empty()) - return false; - std::string::const_iterator str_beg = value.begin(); - std::string::const_iterator str_end = value.end(); - bool r = qi::phrase_parse(str_beg,str_end,qi::float_,ascii::space,*result); - return r && (str_beg == str_end); -} - -static bool string2float(const char * value, float * result) -{ - size_t length = strlen(value); - if (length < 1 || value == NULL) - return false; - const char *begin = value; - const char *iter = begin; - const char *end = value + length; - bool r = qi::phrase_parse(iter,end,qi::float_,ascii::space,*result); - return r && (iter == end); -} +bool string2int(const char * value, int & result); +bool string2int(std::string const& value, int & result); + +bool string2double(std::string const& value, double & result); +bool string2double(const char * value, double & result); + +bool string2float(std::string const& value, float & result); +bool string2float(const char * value, float & result); } } diff --git a/plugins/input/postgis/connection.hpp b/plugins/input/postgis/connection.hpp index a774a9b46..a0325fce3 100644 --- a/plugins/input/postgis/connection.hpp +++ b/plugins/input/postgis/connection.hpp @@ -50,8 +50,6 @@ public: conn_ = PQconnectdb(connection_str.c_str()); if (PQstatus(conn_) != CONNECTION_OK) { - // note: empty ctor is intentional here - // as somehow constructor string can dissapear std::ostringstream s; s << "Postgis Plugin: "; if (conn_ ) diff --git a/plugins/input/postgis/postgis_datasource.cpp b/plugins/input/postgis/postgis_datasource.cpp index 732fbf647..0c5f083e8 100644 --- a/plugins/input/postgis/postgis_datasource.cpp +++ b/plugins/input/postgis/postgis_datasource.cpp @@ -175,7 +175,7 @@ void postgis_datasource::bind() const if (srid_c != NULL) { int result; - if (mapnik::conversions::string2int(srid_c,&result)) + if (mapnik::conversions::string2int(srid_c,result)) srid_ = result; } } @@ -205,7 +205,7 @@ void postgis_datasource::bind() const if (srid_c != NULL) { int result; - if (mapnik::conversions::string2int(srid_c,&result)) + if (mapnik::conversions::string2int(srid_c,result)) srid_ = result; } } @@ -685,10 +685,10 @@ box2d postgis_datasource::envelope() const double loy; double hix; double hiy; - if (mapnik::conversions::string2double(rs->getValue(0),&lox) && - mapnik::conversions::string2double(rs->getValue(1),&loy) && - mapnik::conversions::string2double(rs->getValue(2),&hix) && - mapnik::conversions::string2double(rs->getValue(3),&hiy)) + if (mapnik::conversions::string2double(rs->getValue(0),lox) && + mapnik::conversions::string2double(rs->getValue(1),loy) && + mapnik::conversions::string2double(rs->getValue(2),hix) && + mapnik::conversions::string2double(rs->getValue(3),hiy)) { extent_.init(lox,loy,hix,hiy); extent_initialized_ = true; diff --git a/plugins/input/postgis/postgis_featureset.cpp b/plugins/input/postgis/postgis_featureset.cpp index 052782f85..f4bbfe4b2 100644 --- a/plugins/input/postgis/postgis_featureset.cpp +++ b/plugins/input/postgis/postgis_featureset.cpp @@ -162,7 +162,7 @@ feature_ptr postgis_featureset::next() { std::string str = mapnik::sql_utils::numeric2string(buf); double val; - if (mapnik::conversions::string2double(str,&val)) + if (mapnik::conversions::string2double(str,val)) feature->put(name,val); } else diff --git a/src/build.py b/src/build.py index cde069d0f..c25bc2fb5 100644 --- a/src/build.py +++ b/src/build.py @@ -104,6 +104,7 @@ else: # Linux and others source = Split( """ color.cpp + conversions.cpp box2d.cpp building_symbolizer.cpp datasource_cache.cpp diff --git a/src/conversions.cpp b/src/conversions.cpp new file mode 100644 index 000000000..7debd7a6c --- /dev/null +++ b/src/conversions.cpp @@ -0,0 +1,106 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2012 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +// boost +#include + +#define BOOST_SPIRIT_AUTO(domain_, name, expr) \ + typedef boost::proto::result_of:: \ + deep_copy::type name##_expr_type; \ + BOOST_SPIRIT_ASSERT_MATCH( \ + boost::spirit::domain_::domain, name##_expr_type); \ + BOOST_AUTO(name, boost::proto::deep_copy(expr)); \ + + +namespace mapnik { namespace conversions { + +using namespace boost::spirit; + +BOOST_SPIRIT_AUTO(qi, INTEGER, qi::int_); +BOOST_SPIRIT_AUTO(qi, FLOAT, qi::float_); +BOOST_SPIRIT_AUTO(qi, DOUBLE, qi::double_); + +bool string2int(const char * value, int & result) +{ + size_t length = strlen(value); + if (length < 1 || value == NULL) + return false; + const char *iter = value; + const char *end = value + length; + bool r = qi::phrase_parse(iter,end,INTEGER,ascii::space,result); + return r && (iter == end); +} + +bool string2int(std::string const& value, int & result) +{ + if (value.empty()) + return false; + std::string::const_iterator str_beg = value.begin(); + std::string::const_iterator str_end = value.end(); + bool r = qi::phrase_parse(str_beg,str_end,INTEGER,ascii::space,result); + return r && (str_beg == str_end); +} + +bool string2double(std::string const& value, double & result) +{ + if (value.empty()) + return false; + std::string::const_iterator str_beg = value.begin(); + std::string::const_iterator str_end = value.end(); + bool r = qi::phrase_parse(str_beg,str_end,DOUBLE,ascii::space,result); + return r && (str_beg == str_end); +} + +bool string2double(const char * value, double & result) +{ + size_t length = strlen(value); + if (length < 1 || value == NULL) + return false; + const char *iter = value; + const char *end = value + length; + bool r = qi::phrase_parse(iter,end,DOUBLE,ascii::space,result); + return r && (iter == end); +} + +bool string2float(std::string const& value, float & result) +{ + if (value.empty()) + return false; + std::string::const_iterator str_beg = value.begin(); + std::string::const_iterator str_end = value.end(); + bool r = qi::phrase_parse(str_beg,str_end,FLOAT,ascii::space,result); + return r && (str_beg == str_end); +} + +bool string2float(const char * value, float & result) +{ + size_t length = strlen(value); + if (length < 1 || value == NULL) + return false; + const char *iter = value; + const char *end = value + length; + bool r = qi::phrase_parse(iter,end,FLOAT,ascii::space,result); + return r && (iter == end); +} + +} +} diff --git a/src/image_util.cpp b/src/image_util.cpp index 011166e55..c3cee7610 100644 --- a/src/image_util.cpp +++ b/src/image_util.cpp @@ -158,7 +158,7 @@ void handle_png_options(std::string const& type, if (*colors < 0) throw ImageWriterException("invalid color parameter: unavailable for true color images"); - if (!mapnik::conversions::string2int(t.substr(2),colors) || *colors < 0 || *colors > 256) + if (!mapnik::conversions::string2int(t.substr(2),*colors) || *colors < 0 || *colors > 256) throw ImageWriterException("invalid color parameter: " + t.substr(2)); } else if (boost::algorithm::starts_with(t, "t=")) @@ -166,14 +166,14 @@ void handle_png_options(std::string const& type, if (*colors < 0) throw ImageWriterException("invalid trans_mode parameter: unavailable for true color images"); - if (!mapnik::conversions::string2int(t.substr(2),trans_mode) || *trans_mode < 0 || *trans_mode > 2) + if (!mapnik::conversions::string2int(t.substr(2),*trans_mode) || *trans_mode < 0 || *trans_mode > 2) throw ImageWriterException("invalid trans_mode parameter: " + t.substr(2)); } else if (boost::algorithm::starts_with(t, "g=")) { if (*colors < 0) throw ImageWriterException("invalid gamma parameter: unavailable for true color images"); - if (!mapnik::conversions::string2double(t.substr(2),gamma) || *gamma < 0) + if (!mapnik::conversions::string2double(t.substr(2),*gamma) || *gamma < 0) { throw ImageWriterException("invalid gamma parameter: " + t.substr(2)); } @@ -186,7 +186,7 @@ void handle_png_options(std::string const& type, #define Z_BEST_COMPRESSION 9 #define Z_DEFAULT_COMPRESSION (-1) */ - if (!mapnik::conversions::string2int(t.substr(2),compression) + if (!mapnik::conversions::string2int(t.substr(2),*compression) || *compression < Z_DEFAULT_COMPRESSION || *compression > Z_BEST_COMPRESSION) { @@ -317,7 +317,7 @@ void save_to_stream(T const& image, std::string const& val = t.substr(4); if (!val.empty()) { - if (!mapnik::conversions::string2int(val,&quality) || quality < 0 || quality > 100) + if (!mapnik::conversions::string2int(val,quality) || quality < 0 || quality > 100) { throw ImageWriterException("invalid jpeg quality: '" + val + "'"); } From 9eec4eb1be49664ff3d5f8abe81ebf1b28639357 Mon Sep 17 00:00:00 2001 From: Hermann Kraus Date: Fri, 2 Mar 2012 00:51:44 +0100 Subject: [PATCH 2/5] Fix OSM plugin. --- plugins/input/osm/osm_featureset.cpp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/plugins/input/osm/osm_featureset.cpp b/plugins/input/osm/osm_featureset.cpp index 9d45c033f..21f09aea3 100644 --- a/plugins/input/osm/osm_featureset.cpp +++ b/plugins/input/osm/osm_featureset.cpp @@ -56,7 +56,6 @@ template feature_ptr osm_featureset::next() { feature_ptr feature; - bool success = false; osm_item* cur_item = dataset_->next_item(); if (cur_item != NULL) @@ -70,7 +69,6 @@ feature_ptr osm_featureset::next() geometry_type* point = new geometry_type(mapnik::Point); point->move_to(lon, lat); feature->add_geometry(point); - success = true; } else if (dataset_->current_item_is_way()) { @@ -114,29 +112,26 @@ feature_ptr osm_featureset::next() static_cast(cur_item)->nodes[count]->lat); } feature->add_geometry(geom); - success = true; } } } // can feature_ptr be compared to NULL? - no - if (success) + if (feature) { - std::map::iterator i = cur_item->keyvals.begin(); + std::set::const_iterator itr = attribute_names_.begin(); + std::set::const_iterator end = attribute_names_.end(); + std::map::iterator end_keyvals = cur_item->keyvals.end(); - // add the keyvals to the feature. the feature seems to be a map - // of some sort so this should work - see dbf_file::add_attribute() - while (i != cur_item->keyvals.end()) + for (; itr != end; itr++) { - // only add if in the specified set of attribute names - if (attribute_names_.find(i->first) != attribute_names_.end()) - { - feature->put(i->first,tr_->transcode(i->second.c_str())); + std::map::iterator i = cur_item->keyvals.find(*itr); + if (i != end_keyvals) { + feature->put_new(i->first,tr_->transcode(i->second.c_str())); + } else { + feature->put_new(*itr, ""); } - - i++; } - return feature; } } From 69824314ed5e3047b55099b7ecb25f49c82d5ce1 Mon Sep 17 00:00:00 2001 From: Hermann Kraus Date: Fri, 2 Mar 2012 01:01:13 +0100 Subject: [PATCH 3/5] Simplify OSM plugin. --- plugins/input/osm/osm_featureset.cpp | 120 ++++++++++++--------------- 1 file changed, 53 insertions(+), 67 deletions(-) diff --git a/plugins/input/osm/osm_featureset.cpp b/plugins/input/osm/osm_featureset.cpp index 21f09aea3..25fcacdc4 100644 --- a/plugins/input/osm/osm_featureset.cpp +++ b/plugins/input/osm/osm_featureset.cpp @@ -58,84 +58,70 @@ feature_ptr osm_featureset::next() feature_ptr feature; osm_item* cur_item = dataset_->next_item(); - if (cur_item != NULL) + if (!cur_item) return feature_ptr(); + if (dataset_->current_item_is_node()) { - if (dataset_->current_item_is_node()) + feature = feature_factory::create(ctx_,feature_id_); + ++feature_id_; + double lat = static_cast(cur_item)->lat; + double lon = static_cast(cur_item)->lon; + geometry_type* point = new geometry_type(mapnik::Point); + point->move_to(lon, lat); + feature->add_geometry(point); + } + else if (dataset_->current_item_is_way()) + { + // Loop until we find a feature which passes the filter + while (cur_item) + { + bounds b = static_cast(cur_item)->get_bounds(); + if (filter_.pass(box2d(b.w, b.s, b.e, b.n))) break; + cur_item = dataset_->next_item(); + } + + if (!cur_item) return feature_ptr(); + if (static_cast(cur_item)->nodes.size()) { feature = feature_factory::create(ctx_,feature_id_); ++feature_id_; - double lat = static_cast(cur_item)->lat; - double lon = static_cast(cur_item)->lon; - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(lon, lat); - feature->add_geometry(point); - } - else if (dataset_->current_item_is_way()) - { - bounds b = static_cast(cur_item)->get_bounds(); - - // Loop until we find a feature which passes the filter - while (cur_item != NULL && - ! filter_.pass(box2d(b.w, b.s, b.e, b.n))) + geometry_type* geom; + if (static_cast(cur_item)->is_polygon()) { - cur_item = dataset_->next_item(); - if (cur_item != NULL) - { - b = static_cast(cur_item)->get_bounds(); - } + geom = new geometry_type(mapnik::Polygon); + } + else + { + geom = new geometry_type(mapnik::LineString); } - if (cur_item != NULL) + geom->move_to(static_cast(cur_item)->nodes[0]->lon, + static_cast(cur_item)->nodes[0]->lat); + + for (unsigned int count = 1; + count < static_cast(cur_item)->nodes.size(); + count++) { - if (static_cast(cur_item)->nodes.size()) - { - feature = feature_factory::create(ctx_,feature_id_); - ++feature_id_; - geometry_type* geom; - if (static_cast(cur_item)->is_polygon()) - { - geom = new geometry_type(mapnik::Polygon); - } - else - { - geom = new geometry_type(mapnik::LineString); - } - - geom->move_to(static_cast(cur_item)->nodes[0]->lon, - static_cast(cur_item)->nodes[0]->lat); - - for (unsigned int count = 1; - count < static_cast(cur_item)->nodes.size(); - count++) - { - geom->line_to(static_cast(cur_item)->nodes[count]->lon, - static_cast(cur_item)->nodes[count]->lat); - } - feature->add_geometry(geom); - } + geom->line_to(static_cast(cur_item)->nodes[count]->lon, + static_cast(cur_item)->nodes[count]->lat); } - } - - // can feature_ptr be compared to NULL? - no - if (feature) - { - std::set::const_iterator itr = attribute_names_.begin(); - std::set::const_iterator end = attribute_names_.end(); - std::map::iterator end_keyvals = cur_item->keyvals.end(); - - for (; itr != end; itr++) - { - std::map::iterator i = cur_item->keyvals.find(*itr); - if (i != end_keyvals) { - feature->put_new(i->first,tr_->transcode(i->second.c_str())); - } else { - feature->put_new(*itr, ""); - } - } - return feature; + feature->add_geometry(geom); } } - return feature_ptr(); + + std::set::const_iterator itr = attribute_names_.begin(); + std::set::const_iterator end = attribute_names_.end(); + std::map::iterator end_keyvals = cur_item->keyvals.end(); + + for (; itr != end; itr++) + { + std::map::iterator i = cur_item->keyvals.find(*itr); + if (i != end_keyvals) { + feature->put_new(i->first,tr_->transcode(i->second.c_str())); + } else { + feature->put_new(*itr, ""); + } + } + return feature; } template From 1fcfbaac5248c4f25d7ac66b85b5e3ba3d4f1128 Mon Sep 17 00:00:00 2001 From: Hermann Kraus Date: Fri, 2 Mar 2012 01:45:44 +0100 Subject: [PATCH 4/5] Add test for TextSymbolizer line placement. --- tests/visual_tests/lines-1.xml | 20 +++++++ tests/visual_tests/lines.osm | 101 +++++++++++++++++++++++++++++++++ tests/visual_tests/test.py | 23 +++++--- 3 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 tests/visual_tests/lines-1.xml create mode 100644 tests/visual_tests/lines.osm diff --git a/tests/visual_tests/lines-1.xml b/tests/visual_tests/lines-1.xml new file mode 100644 index 000000000..e90947932 --- /dev/null +++ b/tests/visual_tests/lines-1.xml @@ -0,0 +1,20 @@ + + + + + + My Style + + osm + lines.osm + + + + + + diff --git a/tests/visual_tests/lines.osm b/tests/visual_tests/lines.osm new file mode 100644 index 000000000..0bd8f1bb0 --- /dev/null +++ b/tests/visual_tests/lines.osm @@ -0,0 +1,101 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/visual_tests/test.py b/tests/visual_tests/test.py index c0d49195c..dd068783f 100755 --- a/tests/visual_tests/test.py +++ b/tests/visual_tests/test.py @@ -11,6 +11,7 @@ dirname = os.path.dirname(sys.argv[0]) files = [ ("list", 800, 600, 400, 300, 250, 200, 150, 100), ("simple", 800, 600, 400, 300, 250, 200, 150, 100), + ("lines-1", (800, 800), (600, 600), (400, 400), (200, 200)), ("simple-E", 500), ("simple-NE", 500), ("simple-NW", 500), @@ -26,21 +27,24 @@ files = [ ("shieldsymbolizer-1", 490, 495, 497, 498, 499, 500, 501, 502, 505, 510), ("expressionformat", 500)] -def render(filename, width): +def render(filename, width, height=100): + print "-"*80 + print "Rendering style \"%s\" with size %dx%d ... " % (filename, width, height) + print "-"*80 width = int(width) - m = mapnik.Map(width, 100) + height = int(height) + m = mapnik.Map(width, height) mapnik.load_map(m, os.path.join(dirname, "%s.xml" % filename), False) bbox = mapnik.Box2d(-0.05, -0.01, 0.95, 0.01) m.zoom_to_box(bbox) basefn = '%s-%d' % (filename, width) mapnik.render_to_file(m, basefn+'-agg.png') diff = compare(basefn + '-agg.png', basefn + '-reference.png') - if diff == 0: - rms = 'ok' - else: - rms = 'error: %u different pixels' % diff + if diff > 0: + print "-"*80 + print 'Error: %u different pixels' % diff + print "-"*80 - print "Rendering style \"%s\" with width %d ... %s" % (filename, width, rms) return m if len(sys.argv) == 2: @@ -50,7 +54,10 @@ elif len(sys.argv) > 2: for f in files: for width in f[1:]: - m = render(f[0], width) + if isinstance(width, tuple): + m = render(f[0], width[0], width[1]) + else: + m = render(f[0], width) mapnik.save_map(m, "%s-out.xml" % f[0]) summary() From cc547385e6140306cb6702b9c8d4b3d94f6a8a26 Mon Sep 17 00:00:00 2001 From: Hermann Kraus Date: Fri, 2 Mar 2012 12:38:38 +0100 Subject: [PATCH 5/5] Avoid adding the same text to detector twice. --- src/symbolizer_helpers.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/symbolizer_helpers.cpp b/src/symbolizer_helpers.cpp index d71c10514..4be456195 100644 --- a/src/symbolizer_helpers.cpp +++ b/src/symbolizer_helpers.cpp @@ -62,7 +62,10 @@ text_placement_info_ptr text_symbolizer_helper::get_lin if (!placement_->placements.empty()) { //Found a placement - finder.update_detector(); + if (points_on_line_) + { + finder.update_detector(); + } geo_itr_ = geometries_to_process_.erase(geo_itr_); if (writer_.first) writer_.first->add_text( *placement_, font_manager_,