From f25626f6783c1b220c692ef4c7f41e39bd3651f9 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 7 Dec 2012 02:31:05 -0800 Subject: [PATCH 1/7] also clear out stale object files than scons my no longer know about --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 118082a4f..8f800e055 100755 --- a/Makefile +++ b/Makefile @@ -16,6 +16,8 @@ mapnik: clean: @python scons/scons.py -c --config=cache --implicit-cache --max-drift=1 @if test -e ".sconsign.dblite"; then rm ".sconsign.dblite"; fi + @find ./ -name "*.os" -exec rm {} \; + @find ./ -name "*.o" -exec rm {} \; distclean: if test -e ".sconf_temp/"; then rm -r ".sconf_temp/"; fi From 7c58bf9fcb77a2306561ceb52e212b93873a693f Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 7 Dec 2012 14:06:13 -0800 Subject: [PATCH 2/7] replace boost::trim with faster custom trim - closes #1633 --- include/mapnik/util/trim.hpp | 76 +++++++++++++++++++ plugins/input/csv/csv_datasource.cpp | 17 +++-- plugins/input/postgis/postgis_featureset.cpp | 5 +- src/box2d.cpp | 11 ++- src/libxml2_loader.cpp | 18 +---- src/load_map.cpp | 5 +- src/projection.cpp | 9 +-- src/rapidxml_loader.cpp | 7 +- .../data/broken_maps/in_valid_whitespace.xml | 4 + 9 files changed, 109 insertions(+), 43 deletions(-) create mode 100644 include/mapnik/util/trim.hpp create mode 100644 tests/data/broken_maps/in_valid_whitespace.xml diff --git a/include/mapnik/util/trim.hpp b/include/mapnik/util/trim.hpp new file mode 100644 index 000000000..938956fca --- /dev/null +++ b/include/mapnik/util/trim.hpp @@ -0,0 +1,76 @@ +/***************************************************************************** + * + * 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 + * + *****************************************************************************/ + +#ifndef MAPNIK_TRIM_HPP +#define MAPNIK_TRIM_HPP + +// boost +//#include + +// stl +#include +#include + +namespace mapnik { namespace util { + +/* + faster trim (than boost::trim) + that intentionally does not respect + std::locale to avoid overhead in cases + where the locale is not critical +*/ + +static inline bool not_whitespace(int ch) +{ + if (ch == ' ' || ch == '\n' || ch == '\r' || ch == '\t') return false; + return true; +} + +// trim from start +static inline std::string & ltrim(std::string & s) +{ + s.erase(s.begin(), std::find_if(s.begin(), s.end(), not_whitespace)); + return s; +} + +// trim from end +static inline std::string & rtrim(std::string & s) +{ + s.erase(std::find_if(s.rbegin(), s.rend(), not_whitespace).base(), s.end()); + return s; +} + +// trim from both ends +static inline void trim(std::string & s) +{ + ltrim(rtrim(s)); +} + +static inline std::string trim_copy(std::string s) +{ + return ltrim(rtrim(s)); +} + + +}} // end of namespace mapnik + +#endif // MAPNIK_TRIM_HPP diff --git a/plugins/input/csv/csv_datasource.cpp b/plugins/input/csv/csv_datasource.cpp index 2723488de..042c19448 100644 --- a/plugins/input/csv/csv_datasource.cpp +++ b/plugins/input/csv/csv_datasource.cpp @@ -41,6 +41,7 @@ #include #include #include +#include // stl #include @@ -68,7 +69,7 @@ csv_datasource::csv_datasource(parameters const& params, bool bind) separator_(*params_.get("separator", "")), quote_(*params_.get("quote", "")), headers_(), - manual_headers_(boost::trim_copy(*params_.get("headers", ""))), + manual_headers_(mapnik::util::trim_copy(*params_.get("headers", ""))), strict_(*params_.get("strict", false)), quiet_(*params_.get("quiet", false)), filesize_max_(*params_.get("filesize_max", 20.0)), // MB @@ -196,7 +197,7 @@ void csv_datasource::parse_csv(T & stream, // if user has not passed a separator manually // then attempt to detect by reading first line - std::string sep = boost::trim_copy(separator); + std::string sep = mapnik::util::trim_copy(separator); if (sep.empty()) { // default to ',' @@ -240,10 +241,10 @@ void csv_datasource::parse_csv(T & stream, typedef boost::escaped_list_separator escape_type; - std::string esc = boost::trim_copy(escape); + std::string esc = mapnik::util::trim_copy(escape); if (esc.empty()) esc = "\\"; - std::string quo = boost::trim_copy(quote); + std::string quo = mapnik::util::trim_copy(quote); if (quo.empty()) quo = "\""; MAPNIK_LOG_DEBUG(csv) << "csv_datasource: csv grammar: sep: '" << sep @@ -281,7 +282,7 @@ void csv_datasource::parse_csv(T & stream, unsigned idx(0); for (; beg != tok.end(); ++beg) { - std::string val = boost::trim_copy(*beg); + std::string val = mapnik::util::trim_copy(*beg); std::string lower_val = boost::algorithm::to_lower_copy(val); if (lower_val == "wkt" || (lower_val.find("geom") != std::string::npos)) @@ -324,7 +325,7 @@ void csv_datasource::parse_csv(T & stream, Tokenizer::iterator beg = tok.begin(); std::string val; if (beg != tok.end()) - val = boost::trim_copy(*beg); + val = mapnik::util::trim_copy(*beg); // skip blank lines if (val.empty()) @@ -338,7 +339,7 @@ void csv_datasource::parse_csv(T & stream, for (; beg != tok.end(); ++beg) { ++idx; - val = boost::trim_copy(*beg); + val = mapnik::util::trim_copy(*beg); if (val.empty()) { if (strict_) @@ -522,7 +523,7 @@ void csv_datasource::parse_csv(T & stream, } else { - value = boost::trim_copy(*beg); + value = mapnik::util::trim_copy(*beg); ++beg; } diff --git a/plugins/input/postgis/postgis_featureset.cpp b/plugins/input/postgis/postgis_featureset.cpp index 44e62a0b8..25d516bc8 100644 --- a/plugins/input/postgis/postgis_featureset.cpp +++ b/plugins/input/postgis/postgis_featureset.cpp @@ -32,9 +32,9 @@ #include #include #include +#include // boost -#include #include // stl @@ -182,8 +182,7 @@ feature_ptr postgis_featureset::next() case 1042: //bpchar { - std::string str(buf); - boost::trim(str); + std::string str = mapnik::util::trim_copy(buf); feature->put(name, tr_->transcode(str.c_str())); break; } diff --git a/src/box2d.cpp b/src/box2d.cpp index 384809cc2..7a64a10e5 100644 --- a/src/box2d.cpp +++ b/src/box2d.cpp @@ -22,13 +22,13 @@ // mapnik #include +#include // stl #include // boost #include -#include #include // agg @@ -359,14 +359,17 @@ bool box2d::from_string(std::string const& s) for (boost::tokenizer >::iterator beg = tok.begin(); beg != tok.end(); ++beg) { - std::string item(*beg); - boost::trim(item); + std::string item = mapnik::util::trim_copy(*beg); // note: we intentionally do not use mapnik::util::conversions::string2double // here to ensure that shapeindex can statically compile mapnik::box2d without // needing to link to libmapnik std::string::const_iterator str_beg = item.begin(); std::string::const_iterator str_end = item.end(); - bool r = boost::spirit::qi::phrase_parse(str_beg,str_end,boost::spirit::qi::double_,boost::spirit::ascii::space,d[i]); + bool r = boost::spirit::qi::phrase_parse(str_beg, + str_end, + boost::spirit::qi::double_, + boost::spirit::ascii::space, + d[i]); if (!(r && (str_beg == str_end))) { break; diff --git a/src/libxml2_loader.cpp b/src/libxml2_loader.cpp index e0f034a98..90f6e5cc3 100644 --- a/src/libxml2_loader.cpp +++ b/src/libxml2_loader.cpp @@ -23,15 +23,14 @@ #ifdef HAVE_LIBXML2 // mapnik -#include #include #include #include +#include // boost #include #include -#include // libxml #include @@ -39,8 +38,6 @@ #include #include -using namespace std; - #define DEFAULT_OPTIONS (XML_PARSE_NOERROR | XML_PARSE_NOENT | XML_PARSE_NOBLANKS | XML_PARSE_DTDLOAD | XML_PARSE_NOCDATA) namespace mapnik @@ -75,7 +72,7 @@ public: boost::filesystem::path path(filename); if (!boost::filesystem::exists(path)) { - throw config_error(string("Could not load map file: File does not exist"), 0, filename); + throw config_error(std::string("Could not load map file: File does not exist"), 0, filename); } xmlDocPtr doc = xmlCtxtReadFile(ctx_, filename.c_str(), encoding_, options_); @@ -92,13 +89,6 @@ public: throw config_error(msg, error->line, error->file); } } - - /* - if ( ! ctx->valid ) - { - MAPNIK_LOG_WARN(libxml2_loader) << "libxml2_loader: Failed to validate DTD."; - } - */ load(doc, node); } @@ -114,7 +104,7 @@ public: { boost::filesystem::path path(base_path); if (!boost::filesystem::exists(path)) { - throw config_error(string("Could not locate base_path '") + + throw config_error(std::string("Could not locate base_path '") + base_path + "': file or directory does not exist"); } } @@ -186,7 +176,7 @@ private: case XML_TEXT_NODE: { std::string trimmed((const char*)cur_node->content); - boost::algorithm::trim(trimmed); + mapnik::util::trim(trimmed); if (trimmed.empty()) break; //Don't add empty text nodes node.add_child(trimmed, cur_node->line, true); } diff --git a/src/load_map.cpp b/src/load_map.cpp index 89aa84d39..4d4e1b978 100644 --- a/src/load_map.cpp +++ b/src/load_map.cpp @@ -48,12 +48,12 @@ #include #include #include +#include #include // boost #include #include -#include #include #include #include @@ -255,8 +255,7 @@ void map_parser::parse_map(Map & map, xml_node const& pt, std::string const& bas for (boost::tokenizer >::iterator beg = tokens.begin(); beg != tokens.end(); ++beg) { - std::string item(*beg); - boost::trim(item); + std::string item = mapnik::util::trim_copy(*beg); if (!mapnik::util::string2int(item,n[i])) { throw config_error(std::string("Invalid version string encountered: '") diff --git a/src/projection.cpp b/src/projection.cpp index 1776a22db..900b5addd 100644 --- a/src/projection.cpp +++ b/src/projection.cpp @@ -23,9 +23,7 @@ // mapnik #include #include - -// boost -#include +#include // proj4 #include @@ -151,10 +149,7 @@ void projection::init() std::string projection::expanded() const { if (proj_) { - std::string def(pj_get_def( proj_, 0 )); - //boost::algorithm::ireplace_first(def,params_,""); - boost::trim(def); - return def; + return mapnik::util::trim_copy(pj_get_def( proj_, 0 )); } return std::string(""); } diff --git a/src/rapidxml_loader.cpp b/src/rapidxml_loader.cpp index 6a32e318f..4c0898bee 100644 --- a/src/rapidxml_loader.cpp +++ b/src/rapidxml_loader.cpp @@ -31,16 +31,15 @@ #include #include #include +#include // boost #include -#include // stl #include #include -using namespace std; namespace rapidxml = boost::property_tree::detail::rapidxml; namespace mapnik { @@ -108,7 +107,7 @@ public: // { // boost::filesystem::path path(base_path); // if (!boost::filesystem::exists(path)) { -// throw config_error(string("Could not locate base_path '") + +// throw config_error(std::string("Could not locate base_path '") + // base_path + "': file or directory does not exist"); // } // } @@ -145,7 +144,7 @@ private: case rapidxml::node_cdata: { std::string trimmed(cur_node->value()); - boost::trim(trimmed); + mapnik::util::trim(trimmed); if (trimmed.empty()) break; //Don't add empty text nodes node.add_child(trimmed, 0, true); } diff --git a/tests/data/broken_maps/in_valid_whitespace.xml b/tests/data/broken_maps/in_valid_whitespace.xml new file mode 100644 index 000000000..6ce5452b6 --- /dev/null +++ b/tests/data/broken_maps/in_valid_whitespace.xml @@ -0,0 +1,4 @@ + + + + \ No newline at end of file From 574fec840319be6465d7e6825c1b9401a5158141 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 7 Dec 2012 16:30:36 -0800 Subject: [PATCH 3/7] fix includes in new trim header to allow linux compile --- include/mapnik/util/trim.hpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/mapnik/util/trim.hpp b/include/mapnik/util/trim.hpp index 938956fca..4803f2f31 100644 --- a/include/mapnik/util/trim.hpp +++ b/include/mapnik/util/trim.hpp @@ -23,16 +23,14 @@ #ifndef MAPNIK_TRIM_HPP #define MAPNIK_TRIM_HPP -// boost -//#include - // stl #include -#include +#include namespace mapnik { namespace util { /* + https://github.com/mapnik/mapnik/issues/1633 faster trim (than boost::trim) that intentionally does not respect std::locale to avoid overhead in cases From 7fd96359dd19dbfb5470bd26b7b29393ab4c0db7 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 10 Dec 2012 22:06:55 +0000 Subject: [PATCH 4/7] + more tests --- utils/shapefile/shapefile_reader.py | 37 +++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/utils/shapefile/shapefile_reader.py b/utils/shapefile/shapefile_reader.py index 318ec90cb..27f74875b 100755 --- a/utils/shapefile/shapefile_reader.py +++ b/utils/shapefile/shapefile_reader.py @@ -5,6 +5,36 @@ import os import sys import struct +ShapeType = { 0 : "NullShape", + 1 : "Point", + 3 : "PolyLine", + 5 : "Polygon", + 8 : "MultiPoint", + 11: "PointZ", + 13: "PolyLineZ", + 15: "PolygonZ", + 18: "MultiPointZ", + 21: "PointM", + 23: "PolyLineM", + 25: "PolygonM", + 28: "MultiPointM", + 31: "MultiPatch"} + +def test_record(_type, record) : + if _type == 0: + print "NULL shape" + elif _type == 11: #PointZ + test_pointz(record) + +def test_pointz(record): + if len(record) != 36 : + print>>sys.stderr,"BAD SHAPE FILE: expected 36 bytes got",len(record) + sys.exit(1) + _type,x,y,z,m = struct.unpack(">sys.stderr,"BAD SHAPE FILE: expected PointZ or NullShape got",_type + sys.exit(1) + if __name__ == "__main__" : if len(sys.argv) !=2: @@ -24,11 +54,12 @@ if __name__ == "__main__" : # SHP header _,_,_,_,_,_,shp_file_length = header[0].unpack_from(shp.read(28)) - _,_,lox,loy,hix,hiy,_,_,_,_ = header[1].unpack_from(shp.read(72)) + version,_type,lox,loy,hix,hiy,_,_,_,_ = header[1].unpack_from(shp.read(72)) print "SHX FILE_LENGTH=",shx_file_length,"bytes" print "SHP FILE_LENGTH=",shp_file_length,"bytes" + print "TYPE", ShapeType[_type] print "BBOX(",lox,loy,hix,hiy,")" record_header = struct.Struct(">II") record = struct.Struct(">II") @@ -38,10 +69,12 @@ if __name__ == "__main__" : offset,shx_content_length = record.unpack_from(shx.read(8)) shp.seek(offset*2, os.SEEK_SET) record_number,content_length = record_header.unpack_from(shp.read(8)) - #print (2*offset),record_number,content_length + if shx_content_length <> content_length: print "BAD SHAPE FILE: content_lenght mismatch in SHP and SHX",shx_content_length,content_length sys.exit(1) + ## + test_record(_type, shp.read(2*content_length)) calc_total_size +=(4 + content_length) count+=1 From 18a052167682f11bbe29a5a8347cea2777235eb7 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Mon, 10 Dec 2012 15:55:13 -0800 Subject: [PATCH 5/7] add a visual test for marker middle point placement on lines - refs #1604 --- tests/visual_tests/data/marker-on-line.csv | 2 + .../grids/marker-on-line-600-reference.json | 109 ++++++++++++++++++ .../images/marker-on-line-600-reference.png | Bin 0 -> 3795 bytes tests/visual_tests/styles/marker-on-line.xml | 21 ++++ tests/visual_tests/test.py | 2 + 5 files changed, 134 insertions(+) create mode 100644 tests/visual_tests/data/marker-on-line.csv create mode 100644 tests/visual_tests/grids/marker-on-line-600-reference.json create mode 100644 tests/visual_tests/images/marker-on-line-600-reference.png create mode 100644 tests/visual_tests/styles/marker-on-line.xml diff --git a/tests/visual_tests/data/marker-on-line.csv b/tests/visual_tests/data/marker-on-line.csv new file mode 100644 index 000000000..4835d65c5 --- /dev/null +++ b/tests/visual_tests/data/marker-on-line.csv @@ -0,0 +1,2 @@ +i|wkt +1|LINESTRING(-10 0, 0 20, 10 0, 15 5) diff --git a/tests/visual_tests/grids/marker-on-line-600-reference.json b/tests/visual_tests/grids/marker-on-line-600-reference.json new file mode 100644 index 000000000..16697474c --- /dev/null +++ b/tests/visual_tests/grids/marker-on-line-600-reference.json @@ -0,0 +1,109 @@ +{ + "keys": [ + "", + "1" + ], + "data": {}, + "grid": [ + " ! ", + " !! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! !!! ", + " ! !!! ", + " ! !! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ", + " ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! ! ! ", + " ! ! ", + " ! !! " + ] +} \ No newline at end of file diff --git a/tests/visual_tests/images/marker-on-line-600-reference.png b/tests/visual_tests/images/marker-on-line-600-reference.png new file mode 100644 index 0000000000000000000000000000000000000000..b9893f53776a76194704c134122ff07429069648 GIT binary patch literal 3795 zcmW-kX@Mk>$Fa#qO6Xwu|-Dx4`1 znaT{!YcfQO0?K@aMFuu|-(Q7-y1fEy@7iPAYp^0wxunckqruMBiA}Ay!+*XRd3z*w zdB^g+5!u77mMo7MNADka`R3c-2C4p4rq^#Le;X;eatDQK^S#YpMxQN{e4s`*v?f>G z{Cps{8wC@6Ta`CA2LU5pPYjpaf?A_U@8dZDS-&j$oobf(z0;;(ya>F=~`VbQJ-KG4V>Z9uXFsBME-Tr2Y>wzRlZrnLbaNDUYKn1R5_ zJVgzYQ~l1@^e}V8pYM>~XYQdtmE}Bj!Gs%OHdC?Tr!ExJMkS47^O-S($2PVvGuEJu zR8%qT)vK!LdraHmLI0;FCcbVA|8Odn5UHq~Dqq2ni_G-6EUh|R-ot{F?zqlRwFWy- zx7Elm$E7;~S>wXG9H(L62bZq?vU&z~)fD%abvl8irAE=;V`adAA)np{3`Pmk1 z3Lm)yn_{pcfO`4D!%_^myij8dM+-!>OIN~q6=S%_nqzPs5J-(8;l3J!fZcwE7{@Xo z4!~G;{ zLEdoW$ogjvg*nfzpZ?djCs#%eCja-}{~f$h>N7Se2Vv>VUypA&IsSO{>G2C4>b#e4 z?pz-UIdLWvf%W3tj)IXxgI~QE>9YRs!K>HTM}~dJCzC+<1Je`o{y&y=)Q3D{YvJmN ztdaL^AtNNWMA0X)u(xVV-&$(pZ1a!3AoNEq-QHoN_5FX@L zs?{<-M)#3MpJ+zq;*=^Q2QV8i=M=;ckxqUo4*~8BEGJfNOEqYzXV?i)Rf91S;X`@b z#Oa{(DB;Kd=PR335Sy>vsggQ?ZOUJ7+}0vO*`==v!)d&1%5JQ;!xav z_?~MeLK6Hfw0{61dL8;JO_C^MvJ9%Q8!__6HKz zow0^n6b0*2KC6%q*$HhOc3#CMK%4zdsAs(|H6RIF5cn6%xi;`Zv$hcA?X=)q6o`mg z_e0B2z`E~>I7*_KxyqE(>&7@h-b_fv%Q_8U!#|kB?-6apn%&14*+XWoIM0G}sGT%yP}cU~fq44`XyXq@5ROy0}ij z|31bd9cEEViF3qa1R+1T#9FdU4t3pksdZcznDPB=(lg73ikIMZv3`g}atVEqm;e#I z4OAA_4Jz9MNRMF~s#t=VDvD5=Rm)0S5RX`7d{&Dh5wUz;i?%3I5sSS8GT3MY(u3~P z^QpTMbVzp>Rn-N;P1PzWZwGP(2PdL+^K%w%N5mH_rC*A}phWQ6=#<4aU@YJ9#sT1x zLtW?ep45H3;E|#YiR-T}jnaEjhw=PJ3pi>OYIU!1?ru#rj?o*7A%z|JBRIUHA-!n4a3$D@LZPFyBV09L5p z&o0ZJbY_R$Z;&E5E!CyzXHZx0jGp+7=%XHI-F|%#wFD=77HiN)^k~T~y_ove<3Flm zk*(2R;r^Mb2{va~qG zNS9lL(wscI1{Ctkx!2%DhZ3*Jb`*uF+PrK7lefHiLDN3sO@^a1XN2AeL&p|6wd4rdP_ zk>bt7XFBhJ;#|AKSu_$EoKGa^yavABWp{Xu5%E|3NqvmZA#M76s&yK6MU0h|B<-ha z#9tEmY!9iZ+X=hu%vx}G0!jBE=^id24CrzYeB+8~*QgN}aSN&Ty$A8M3bler%OqFpJgTk~n4+W)ykk*dTAPez-xHI5-`iTgQm}xS zk>Gy>OMYq=qG3tivKUjwT%B`u%^y8qnt5kKDXO0vw}C15jUr*oL_f|*YYQp1YbI)1 z7Mn7*=p0p=u+%;3?&r9-!L_8_$67uM_cFLPw)3aR(;(_@XIslR;2s9iaM=7Q5*Wg4 z!{4`-x8m-Gu)y%3DdK}N+{;@29Opq+xZ4zPsTG{;FU&lJbP;R_{9Y!nqvOhBOx}*eDE+g(aw+hOwh6IUaz2~SED!gF+GR;^ znCW{vK`m3}t!7q+b79tzmgljijC?jq7jOrk*(YmB0kp)fyIreRVr+zd&HeRrELoe5 zLCeyuD-Wub0yfZ_xkF=&dG`e?CwBYFQnga-6Ge}6f4jt(_pY*XmVUExiAm(`j|@nc^d(C-pa9YD3I_(8%jVb(LEycKIIrQEYGI zB9myglb2z>^3_%|&${&&cpH57G!uMjKrYmea*_kAp{$BS1&?{HarLhiY(e7nyCG>c z3$DlF()RMd za9>3UgN+qcJmK-2#wpBk;u(r0-3)#wsp1-T;lopiJ8=!idTdioqEkIpZiX|>fm7J* zAhJiObs9X8UhlnDX-#N#{yJ@-K{Ws5K9`YAfftx&yV;9Suw~70qL!5f9T`+*>U^O!SIu`Gt}Y z(?uqF)RC8Kbx)e=3eVXms|T|8J>kL2Tl$xC_o}=5CY>5Q4?I{j S?|_>5S%rAzl9*OGS@eHLD%W%X literal 0 HcmV?d00001 diff --git a/tests/visual_tests/styles/marker-on-line.xml b/tests/visual_tests/styles/marker-on-line.xml new file mode 100644 index 000000000..1ca8ada6b --- /dev/null +++ b/tests/visual_tests/styles/marker-on-line.xml @@ -0,0 +1,21 @@ + + + + + line + point-placement + + csv + ../data/marker-on-line.csv + | + + + diff --git a/tests/visual_tests/test.py b/tests/visual_tests/test.py index b064c0294..3d79c00fc 100755 --- a/tests/visual_tests/test.py +++ b/tests/visual_tests/test.py @@ -38,6 +38,8 @@ files = [ {'name': "lines-3", 'sizes': sizes_few_square,'bbox':default_text_box}, {'name': "lines-shield", 'sizes': sizes_few_square,'bbox':default_text_box}, {'name': "marker-multi-policy", 'sizes':[(600,400)]}, + {'name': "marker-on-line", 'sizes':[(600,400)], + 'bbox': mapnik.Box2d(-10, 0, 15, 20)}, {'name': "marker_line_placement_on_points"}, {'name': "whole-centroid", 'sizes':[(600,400)], 'bbox': mapnik.Box2d(736908, 4390316, 2060771, 5942346)}, From 02bb955b59c6f7f4206e43ac909d1105c0c588a3 Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 11 Dec 2012 09:37:53 +0000 Subject: [PATCH 6/7] + simplify and refactor shape featuresets to be more resilient to various quirky shapefiles. + fix io navigation functions to use std::streampos --- plugins/input/shape/shape_featureset.cpp | 222 +++++------------- plugins/input/shape/shape_featureset.hpp | 1 + .../input/shape/shape_index_featureset.cpp | 173 ++++++-------- .../input/shape/shape_index_featureset.hpp | 5 +- plugins/input/shape/shape_io.cpp | 68 +----- plugins/input/shape/shape_io.hpp | 10 +- plugins/input/shape/shp_index.hpp | 14 +- 7 files changed, 154 insertions(+), 339 deletions(-) diff --git a/plugins/input/shape/shape_featureset.cpp b/plugins/input/shape/shape_featureset.cpp index 3cdaec46d..0ccf8aef6 100644 --- a/plugins/input/shape/shape_featureset.cpp +++ b/plugins/input/shape/shape_featureset.cpp @@ -44,6 +44,7 @@ shape_featureset::shape_featureset(filterT const& filter, : filter_(filter), shape_(shape_name, false), query_ext_(), + feature_bbox_(), tr_(new transcoder(encoding)), file_length_(file_length), row_limit_(row_limit), @@ -62,176 +63,74 @@ feature_ptr shape_featureset::next() return feature_ptr(); } - std::streampos pos = shape_.shp().pos(); - // skip null shapes - while (pos > 0 && pos < std::streampos(file_length_ * 2)) + while (shape_.shp().pos() < std::streampos(file_length_ * 2)) { - shape_.move_to(pos); - if (shape_.type() == shape_io::shape_null) + shape_.move_to(shape_.shp().pos()); + shape_file::record_type record(shape_.reclength_ * 2); + shape_.shp().read_record(record); + int type = record.read_ndr_integer(); + + // skip null shapes + if (type == shape_io::shape_null) continue; + + feature_ptr feature(feature_factory::create(ctx_, shape_.id_)); + switch (type) { - pos += std::streampos(12); - } - else + case shape_io::shape_point: + case shape_io::shape_pointm: + case shape_io::shape_pointz: { + double x = record.read_double(); + double y = record.read_double(); + if (!filter_.pass(mapnik::box2d(x,y,x,y))) + continue; + std::auto_ptr point(new geometry_type(mapnik::Point)); + point->move_to(x, y); + feature->paths().push_back(point); break; } - } - - if (pos < std::streampos(file_length_ * 2)) - { - int type = shape_.type(); - feature_ptr feature(feature_factory::create(ctx_, shape_.id_)); - - if (type == shape_io::shape_point) + case shape_io::shape_multipoint: + case shape_io::shape_multipointm: + case shape_io::shape_multipointz: { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - ++count_; + shape_io::read_bbox(record, feature_bbox_); + if (!filter_.pass(feature_bbox_)) continue; + int num_points = record.read_ndr_integer(); + for (int i = 0; i < num_points; ++i) + { + double x = record.read_double(); + double y = record.read_double(); + std::auto_ptr point(new geometry_type(mapnik::Point)); + point->move_to(x, y); + feature->paths().push_back(point); + } + break; } - else if (type == shape_io::shape_pointm) + + case shape_io::shape_polyline: + case shape_io::shape_polylinem: + case shape_io::shape_polylinez: { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - // skip m - shape_.shp().skip(8); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - ++count_; + shape_io::read_bbox(record, feature_bbox_); + if (!filter_.pass(feature_bbox_)) continue; + shape_io::read_polyline(record, feature->paths()); + break; } - else if (type == shape_io::shape_pointz) + case shape_io::shape_polygon: + case shape_io::shape_polygonm: + case shape_io::shape_polygonz: { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - // skip z - shape_.shp().skip(8); - // skip m if exists: OGR bug - if (shape_.reclength_ == 18) - { - shape_.shp().skip(8); - } - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - ++count_; + shape_io::read_bbox(record, feature_bbox_); + if (!filter_.pass(feature_bbox_)) continue; + shape_io::read_polygon(record, feature->paths()); + break; } - else - { - // skip shapes - for (;;) - { - std::streampos pos = shape_.shp().pos(); - if (shape_.type() == shape_io::shape_null) - { - pos += std::streampos(12); - - // TODO handle the shapes - MAPNIK_LOG_WARN(shape) << "shape_featureset: NULL SHAPE len=" << shape_.reclength_; - } - else if (filter_.pass(shape_.current_extent())) - { - break; - } - else - { - pos += std::streampos(2 * shape_.reclength_ - 36); - } - - if (pos > 0 && pos < std::streampos(file_length_ * 2)) - { - shape_.move_to(pos); - } - else - { - MAPNIK_LOG_DEBUG(shape) << "shape_featureset: Total shapes read=" << count_; - - return feature_ptr(); - } - } - - switch (type) - { - case shape_io::shape_multipoint: - { - int num_points = shape_.shp().read_ndr_integer(); - for (int i = 0; i < num_points; ++i) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - } - ++count_; - break; - } - - case shape_io::shape_multipointm: - { - int num_points = shape_.shp().read_ndr_integer(); - for (int i = 0; i < num_points; ++i) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - } - - // skip m - shape_.shp().skip(2 * 8 + 8 * num_points); - ++count_; - break; - } - - case shape_io::shape_multipointz: - { - int num_points = shape_.shp().read_ndr_integer(); - for (int i = 0; i < num_points; ++i) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - } - - // skip z - shape_.shp().skip(2 * 8 + 8 * num_points); - - // check if we have measure data - if (shape_.reclength_ == (unsigned)(num_points * 16 + 36)) - { - // skip m - shape_.shp().skip(2 * 8 + 8 * num_points); - } - ++count_; - break; - } - - case shape_io::shape_polyline: - case shape_io::shape_polylinem: - case shape_io::shape_polylinez: - { - shape_.read_polyline(feature->paths()); - ++count_; - break; - } - - case shape_io::shape_polygon: - case shape_io::shape_polygonm: - case shape_io::shape_polygonz: - { - shape_.read_polygon(feature->paths()); - ++count_; - break; - } - } + default : + MAPNIK_LOG_DEBUG(shape) << "shape_featureset: Unsupported type" << type; + return feature_ptr(); } + // FIXME: https://github.com/mapnik/mapnik/issues/1020 feature->set_id(shape_.id_); if (attr_ids_.size()) @@ -251,15 +150,12 @@ feature_ptr shape_featureset::next() MAPNIK_LOG_ERROR(shape) << "Shape Plugin: error processing attributes"; } } - + ++count_; return feature; } - else - { - MAPNIK_LOG_DEBUG(shape) << "shape_featureset: Total shapes read=" << count_; - return feature_ptr(); - } + MAPNIK_LOG_DEBUG(shape) << "shape_featureset: Total shapes read=" << count_; + return feature_ptr(); } template diff --git a/plugins/input/shape/shape_featureset.hpp b/plugins/input/shape/shape_featureset.hpp index 3b77bbbdc..9a140e099 100644 --- a/plugins/input/shape/shape_featureset.hpp +++ b/plugins/input/shape/shape_featureset.hpp @@ -56,6 +56,7 @@ private: filterT filter_; shape_io shape_; box2d query_ext_; + mutable box2d feature_bbox_; boost::scoped_ptr tr_; long file_length_; std::vector attr_ids_; diff --git a/plugins/input/shape/shape_index_featureset.cpp b/plugins/input/shape/shape_index_featureset.cpp index bff1a1e5c..39aef2469 100644 --- a/plugins/input/shape/shape_index_featureset.cpp +++ b/plugins/input/shape/shape_index_featureset.cpp @@ -49,7 +49,8 @@ shape_index_featureset::shape_index_featureset(filterT const& filter, shape_(shape), tr_(new transcoder(encoding)), row_limit_(row_limit), - count_(0) + count_(0), + feature_bbox_() { shape_.shp().skip(100); setup_attributes(ctx_, attribute_names, shape_name, shape_,attr_ids_); @@ -58,18 +59,18 @@ shape_index_featureset::shape_index_featureset(filterT const& filter, if (index) { #ifdef SHAPE_MEMORY_MAPPED_FILE - //shp_index >::query(filter, index->file(), ids_); - shp_index::query(filter, index->file(), ids_); + //shp_index >::query(filter, index->file(), offsets_); + shp_index::query(filter, index->file(), offsets_); #else - shp_index::query(filter, index->file(), ids_); + shp_index::query(filter, index->file(), offsets_); #endif } - std::sort(ids_.begin(), ids_.end()); + std::sort(offsets_.begin(), offsets_.end()); - MAPNIK_LOG_DEBUG(shape) << "shape_index_featureset: Query size=" << ids_.size(); + MAPNIK_LOG_DEBUG(shape) << "shape_index_featureset: Query size=" << offsets_.size(); - itr_ = ids_.begin(); + itr_ = offsets_.begin(); } template @@ -80,103 +81,67 @@ feature_ptr shape_index_featureset::next() return feature_ptr(); } - if (itr_ != ids_.end()) + while ( itr_ != offsets_.end()) { - int pos = *itr_++; - shape_.move_to(pos); - - int type = shape_.type(); + shape_.move_to(*itr_++); + shape_file::record_type record(shape_.reclength_ * 2); + shape_.shp().read_record(record); + int type = record.read_ndr_integer(); feature_ptr feature(feature_factory::create(ctx_,shape_.id_)); - if (type == shape_io::shape_point) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - ++count_; - } - else if (type == shape_io::shape_pointm) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - // skip m - shape_.shp().skip(8); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - ++count_; - } - else if (type == shape_io::shape_pointz) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - // skip z - shape_.shp().skip(8); - // skip m if exists - if (shape_.reclength_ == 8 + 36) - { - shape_.shp().skip(8); - } - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - ++count_; - } - else - { - while(! filter_.pass(shape_.current_extent()) && - itr_ != ids_.end()) - { - if (shape_.type() != shape_io::shape_null) - { - pos = *itr_++; - shape_.move_to(pos); - } - else - { - return feature_ptr(); - } - } - switch (type) - { - case shape_io::shape_multipoint: - case shape_io::shape_multipointm: - case shape_io::shape_multipointz: - { - int num_points = shape_.shp().read_ndr_integer(); - for (int i = 0; i < num_points; ++i) - { - double x = shape_.shp().read_double(); - double y = shape_.shp().read_double(); - geometry_type* point = new geometry_type(mapnik::Point); - point->move_to(x, y); - feature->add_geometry(point); - } - // ignore m and z for now - ++count_; - break; - } - - case shape_io::shape_polyline: - case shape_io::shape_polylinem: - case shape_io::shape_polylinez: - { - shape_.read_polyline(feature->paths()); - ++count_; - break; - } - case shape_io::shape_polygon: - case shape_io::shape_polygonm: - case shape_io::shape_polygonz: - { - shape_.read_polygon(feature->paths()); - ++count_; - break; - } - } + switch (type) + { + case shape_io::shape_point: + case shape_io::shape_pointm: + case shape_io::shape_pointz: + { + double x = record.read_double(); + double y = record.read_double(); + std::auto_ptr point(new geometry_type(mapnik::Point)); + point->move_to(x, y); + feature->paths().push_back(point); + break; } + case shape_io::shape_multipoint: + case shape_io::shape_multipointm: + case shape_io::shape_multipointz: + { + shape_io::read_bbox(record, feature_bbox_); + if (!filter_.pass(feature_bbox_)) continue; + int num_points = record.read_ndr_integer(); + for (int i = 0; i < num_points; ++i) + { + double x = record.read_double(); + double y = record.read_double(); + std::auto_ptr point(new geometry_type(mapnik::Point)); + point->move_to(x, y); + feature->paths().push_back(point); + } + break; + } + case shape_io::shape_polyline: + case shape_io::shape_polylinem: + case shape_io::shape_polylinez: + { + shape_io::read_bbox(record, feature_bbox_); + if (!filter_.pass(feature_bbox_)) continue; + shape_io::read_polyline(record,feature->paths()); + break; + } + case shape_io::shape_polygon: + case shape_io::shape_polygonm: + case shape_io::shape_polygonz: + { + shape_io::read_bbox(record, feature_bbox_); + if (!filter_.pass(feature_bbox_)) continue; + shape_io::read_polygon(record,feature->paths()); + break; + } + default : + MAPNIK_LOG_DEBUG(shape) << "shape_index_featureset: Unsupported type" << type; + return feature_ptr(); + } + // FIXME feature->set_id(shape_.id_); if (attr_ids_.size()) @@ -196,14 +161,12 @@ feature_ptr shape_index_featureset::next() MAPNIK_LOG_ERROR(shape) << "Shape Plugin: error processing attributes"; } } + ++count_; return feature; } - else - { - MAPNIK_LOG_DEBUG(shape) << "shape_index_featureset: " << count_ << " features"; - return feature_ptr(); - } + MAPNIK_LOG_DEBUG(shape) << "shape_index_featureset: " << count_ << " features"; + return feature_ptr(); } diff --git a/plugins/input/shape/shape_index_featureset.hpp b/plugins/input/shape/shape_index_featureset.hpp index 410a10358..9aeb0b0de 100644 --- a/plugins/input/shape/shape_index_featureset.hpp +++ b/plugins/input/shape/shape_index_featureset.hpp @@ -60,11 +60,12 @@ private: context_ptr ctx_; shape_io & shape_; boost::scoped_ptr tr_; - std::vector ids_; - std::vector::iterator itr_; + std::vector offsets_; + std::vector::iterator itr_; std::vector attr_ids_; const int row_limit_; mutable int count_; + mutable box2d feature_bbox_; }; #endif // SHAPE_INDEX_FEATURESET_HPP diff --git a/plugins/input/shape/shape_io.cpp b/plugins/input/shape/shape_io.cpp index 95db509d8..51c53e9df 100644 --- a/plugins/input/shape/shape_io.cpp +++ b/plugins/input/shape/shape_io.cpp @@ -65,27 +65,11 @@ shape_io::shape_io(std::string const& shape_name, bool open_index) shape_io::~shape_io() {} -void shape_io::move_to(int pos) +void shape_io::move_to(std::streampos pos) { shp_.seek(pos); id_ = shp_.read_xdr_integer(); reclength_ = shp_.read_xdr_integer(); - type_ = static_cast(shp_.read_ndr_integer()); - - if (type_ != shape_null && type_ != shape_point && type_ != shape_pointm && type_ != shape_pointz) - { - shp_.read_envelope(cur_extent_); - } -} - -shape_io::shapeType shape_io::type() const -{ - return type_; -} - -const box2d& shape_io::current_extent() const -{ - return cur_extent_; } shape_file& shape_io::shp() @@ -98,11 +82,17 @@ dbf_file& shape_io::dbf() return dbf_; } -void shape_io::read_polyline(mapnik::geometry_container & geom) +void shape_io::read_bbox(shape_file::record_type & record, mapnik::box2d & bbox) { - shape_file::record_type record(reclength_ * 2 - 36); - shp_.read_record(record); + double lox = record.read_double(); + double loy = record.read_double(); + double hix = record.read_double(); + double hiy = record.read_double(); + bbox.init(lox, loy, hix, hiy); +} +void shape_io::read_polyline( shape_file::record_type & record, mapnik::geometry_container & geom) +{ int num_parts = record.read_ndr_integer(); int num_points = record.read_ndr_integer(); if (num_parts == 1) @@ -155,30 +145,10 @@ void shape_io::read_polyline(mapnik::geometry_container & geom) geom.push_back(line); } } - // z-range - //double z0=record.read_double(); - //double z1=record.read_double(); - //for (int i=0;i parts(num_parts); @@ -218,20 +188,4 @@ void shape_io::read_polygon(mapnik::geometry_container & geom) geom.push_back(poly); } - // z-range - //double z0=record.read_double(); - //double z1=record.read_double(); - //for (int i=0;iis_open()); } - void move_to(int id); - shapeType type() const; - const box2d& current_extent() const; - void read_polyline(mapnik::geometry_container & geom); - void read_polygon(mapnik::geometry_container & geom); + void move_to(std::streampos pos); + static void read_bbox(shape_file::record_type & record, mapnik::box2d & bbox); + static void read_polyline(shape_file::record_type & record,mapnik::geometry_container & geom); + static void read_polygon(shape_file::record_type & record,mapnik::geometry_container & geom); + shapeType type_; shape_file shp_; dbf_file dbf_; diff --git a/plugins/input/shape/shp_index.hpp b/plugins/input/shape/shp_index.hpp index c944affa1..1d69b5d7a 100644 --- a/plugins/input/shape/shp_index.hpp +++ b/plugins/input/shape/shp_index.hpp @@ -20,8 +20,8 @@ * *****************************************************************************/ -#ifndef SHP_INDEX_HH -#define SHP_INDEX_HH +#ifndef SHP_INDEX_HPP +#define SHP_INDEX_HPP // stl #include @@ -38,7 +38,7 @@ template class shp_index { public: - static void query(const filterT& filter, IStream& file,std::vector& pos); + static void query(filterT const& filter, IStream& file,std::vector& pos); private: shp_index(); ~shp_index(); @@ -46,18 +46,18 @@ private: shp_index& operator=(const shp_index&); static int read_ndr_integer(IStream& in); static void read_envelope(IStream& in, box2d& envelope); - static void query_node(const filterT& filter, IStream& in, std::vector& pos); + static void query_node(const filterT& filter, IStream& in, std::vector& pos); }; template -void shp_index::query(const filterT& filter, IStream& file, std::vector& pos) +void shp_index::query(const filterT& filter, IStream& file, std::vector& pos) { file.seekg(16, std::ios::beg); query_node(filter, file, pos); } template -void shp_index::query_node(const filterT& filter, IStream& file, std::vector& ids) +void shp_index::query_node(const filterT& filter, IStream& file, std::vector& ids) { int offset = read_ndr_integer(file); @@ -100,4 +100,4 @@ void shp_index::read_envelope(IStream& file, box2d& en file.read(reinterpret_cast(&envelope), sizeof(envelope)); } -#endif // SHP_INDEX_HH +#endif // SHP_INDEX_HPP From 562fada9d0f680f59b2d9f396c95320a0d753479 Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 11 Dec 2012 15:44:19 +0000 Subject: [PATCH 7/7] + return default (value_null) if attribute is not present --- include/mapnik/feature.hpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/include/mapnik/feature.hpp b/include/mapnik/feature.hpp index a86aa768b..30e3f4a2e 100644 --- a/include/mapnik/feature.hpp +++ b/include/mapnik/feature.hpp @@ -93,6 +93,8 @@ private: typedef MAPNIK_DECL context > context_type; typedef MAPNIK_DECL boost::shared_ptr context_ptr; +static const value default_value; + class MAPNIK_DECL feature_impl : private boost::noncopyable { friend class feature_kv_iterator; @@ -126,7 +128,6 @@ public: put_new(key,value(val)); } - void put(context_type::key_type const& key, value const& val) { context_type::map_type::const_iterator itr = ctx_->mapping_.find(key); @@ -141,7 +142,6 @@ public: } } - void put_new(context_type::key_type const& key, value const& val) { context_type::map_type::const_iterator itr = ctx_->mapping_.find(key); @@ -158,7 +158,6 @@ public: } } - bool has_key(context_type::key_type const& key) const { return (ctx_->mapping_.find(key) != ctx_->mapping_.end()); @@ -170,14 +169,14 @@ public: if (itr != ctx_->mapping_.end()) return get(itr->second); else - throw std::out_of_range(std::string("Key does not exist: '") + key + "'"); + return default_value; } value_type const& get(std::size_t index) const { if (index < data_.size()) return data_[index]; - throw std::out_of_range("Index out of range"); + return default_value; } boost::optional get_optional(std::size_t index) const