From 2f134657b46d6a04c41fee20c7a8c570af35058a Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 24 Feb 2012 16:34:39 -0800 Subject: [PATCH] speed up xml loading by avoiding repeated grammar creation - refs #1028 (TODO - still need to handle text/shield expressions) --- include/mapnik/color_factory.hpp | 37 +++++++++++++++++ include/mapnik/expression.hpp | 10 +++++ src/expression.cpp | 44 +++++++++++--------- src/load_map.cpp | 70 ++++++++++++++++++++++++++------ 4 files changed, 128 insertions(+), 33 deletions(-) diff --git a/include/mapnik/color_factory.hpp b/include/mapnik/color_factory.hpp index 181bffb2e..37794890c 100644 --- a/include/mapnik/color_factory.hpp +++ b/include/mapnik/color_factory.hpp @@ -63,6 +63,20 @@ public: } } + static bool parse_from_string(color & c, std::string const& css_color, + mapnik::css_color_grammar const& g) + { + std::string::const_iterator first = css_color.begin(); + std::string::const_iterator last = css_color.end(); + bool result = + boost::spirit::qi::phrase_parse(first, + last, + g, + boost::spirit::ascii::space, + c); + return result && (first == last); + } + static color from_string(std::string const& css_color) { color c; @@ -81,6 +95,29 @@ class MAPNIK_DECL color_factory : boost::noncopyable { public: + static void parse_from_string(color & c, std::string const& css_color, + mapnik::css_color_grammar g) + { + std::string::const_iterator first = css_color.begin(); + std::string::const_iterator last = css_color.end(); + mapnik::css css_; + bool result = + boost::spirit::qi::phrase_parse(first, + last, + g, + boost::spirit::ascii::space, + css_); + if (!result) + { + throw config_error(std::string("Failed to parse color value: ") + + "Expected a CSS color, but got '" + css_color + "'"); + } + c.set_red(css_.r); + c.set_green(css_.g); + c.set_blue(css_.b); + c.set_alpha(css_.a); + } + static void init_from_string(color & c, std::string const& css_color) { typedef std::string::const_iterator iterator_type; diff --git a/include/mapnik/expression.hpp b/include/mapnik/expression.hpp index 9ec1222dc..4192c2a81 100644 --- a/include/mapnik/expression.hpp +++ b/include/mapnik/expression.hpp @@ -26,6 +26,7 @@ // mapnik #include #include +#include // stl #include @@ -35,6 +36,15 @@ namespace mapnik typedef boost::shared_ptr expression_ptr; +class expression_factory +{ +public: + static expression_ptr compile(std::string const& str,transcoder const& tr); + static bool parse_from_string(expression_ptr const& expr, + std::string const& str, + mapnik::expression_grammar const& g); +}; + MAPNIK_DECL expression_ptr parse_expression (std::string const& wkt, std::string const& encoding); MAPNIK_DECL expression_ptr parse_expression (std::string const& wkt); diff --git a/src/expression.cpp b/src/expression.cpp index b4b8719d0..9223021bc 100644 --- a/src/expression.cpp +++ b/src/expression.cpp @@ -22,38 +22,42 @@ //$Id$ #include -#include #include #include // boost #include +#include namespace mapnik { -class expression_factory +expression_ptr expression_factory::compile(std::string const& str,transcoder const& tr) { -public: - static expression_ptr compile(std::string const& str,transcoder const& tr) + expression_ptr expr(boost::make_shared(true)); + std::string::const_iterator itr = str.begin(); + std::string::const_iterator end = str.end(); + mapnik::expression_grammar g(tr); + bool r = boost::spirit::qi::phrase_parse(itr,end,g, boost::spirit::standard_wide::space,*expr); + if (r && itr==end) { - expression_ptr expr(new expr_node(true)); - - std::string::const_iterator itr = str.begin(); - std::string::const_iterator end = str.end(); - mapnik::expression_grammar g(tr); - - bool r = boost::spirit::qi::phrase_parse(itr,end,g, boost::spirit::standard_wide::space,*expr); - if (r && itr==end) - { - return expr; - } - else - { - throw config_error( "Failed to parse expression: \"" + str + "\"" ); - } + return expr; } -}; + else + { + throw config_error( "Failed to parse expression: \"" + str + "\"" ); + } +} + +bool expression_factory::parse_from_string(expression_ptr const& expr, + std::string const& str, + mapnik::expression_grammar const& g) +{ + std::string::const_iterator itr = str.begin(); + std::string::const_iterator end = str.end(); + bool r = boost::spirit::qi::phrase_parse(itr,end,g, boost::spirit::standard_wide::space,*expr); + return (r && itr==end); +} expression_ptr parse_expression (std::string const& wkt,std::string const& encoding) { diff --git a/src/load_map.cpp b/src/load_map.cpp index 6fc3f1af0..a222e12de 100644 --- a/src/load_map.cpp +++ b/src/load_map.cpp @@ -87,7 +87,12 @@ public: strict_( strict ), filename_( filename ), relative_to_xml_(true), - font_manager_(font_engine_) {} + font_manager_(font_engine_), + color_grammar_(), + // TODO - use xml encoding? + tr_("utf8"), + expr_grammar_(tr_) + {} void parse_map(Map & map, ptree const & sty, std::string const& base_path=""); private: @@ -115,11 +120,14 @@ private: void parse_raster_colorizer(raster_colorizer_ptr const& rc, ptree const& node ); void parse_stroke(stroke & strk, ptree const & sym); + expression_ptr parse_expr(std::string const& expr); void ensure_font_face( const std::string & face_name ); std::string ensure_relative_to_xml( boost::optional opt_path ); void ensure_attrs( ptree const& sym, std::string name, std::string attrs); + boost::optional get_opt_color_attr(boost::property_tree::ptree const& node, + std::string const& name); bool strict_; std::string filename_; @@ -129,6 +137,9 @@ private: face_manager font_manager_; std::map file_sources_; std::map fontsets_; + mapnik::css_color_grammar color_grammar_; + mapnik::transcoder tr_; + mapnik::expression_grammar expr_grammar_; }; @@ -199,6 +210,40 @@ void load_map_string(Map & map, std::string const& str, bool strict, std::string parser.parse_map(map, pt, base_path); } +expression_ptr map_parser::parse_expr(std::string const& str) +{ + expression_ptr expr(boost::make_shared(true)); + if (!expression_factory::parse_from_string(expr,str,expr_grammar_)) + { + throw mapnik::config_error( "Failed to parse expression: '" + str + "'" ); + } + return expr; + +} + +boost::optional map_parser::get_opt_color_attr(boost::property_tree::ptree const& node, + std::string const& name) +{ + + boost::optional str = node.get_optional( std::string(".") + name); + boost::optional result; + if (str && !str->empty()) + { + mapnik::color c; + if (mapnik::color_factory::parse_from_string(c,*str,color_grammar_)) + { + result.reset(c); + } + else + { + throw config_error(std::string("Failed to parse attribute ") + + name + "'. Expected color" + + " but got '" + *str + "'"); + } + } + return result; +} + void map_parser::parse_map( Map & map, ptree const & pt, std::string const& base_path ) { try @@ -251,7 +296,7 @@ void map_parser::parse_map( Map & map, ptree const & pt, std::string const& base map.set_base_path( base ); } - optional bgcolor = get_opt_attr(map_node, "background-color"); + optional bgcolor = get_opt_color_attr(map_node, "background-color"); if (bgcolor) { map.set_background( * bgcolor ); @@ -793,8 +838,7 @@ void map_parser::parse_rule( feature_type_style & style, ptree const & r ) get_opt_child( r, "Filter"); if (filter_expr) { - // TODO - can we use encoding defined for XML document for filter expressions? - rule.set_filter(parse_expression(*filter_expr,"utf8")); + rule.set_filter(parse_expr(*filter_expr)); } optional else_filter = @@ -1106,7 +1150,7 @@ void map_parser::parse_markers_symbolizer( rule & rule, ptree const & sym ) symbol.set_transform(matrix); } - optional c = get_opt_attr(sym, "fill"); + optional c = get_opt_color_attr(sym, "fill"); if (c) symbol.set_fill(*c); optional spacing = get_opt_attr(sym, "spacing"); if (spacing) symbol.set_spacing(*spacing); @@ -1290,7 +1334,7 @@ void map_parser::parse_text_symbolizer( rule & rule, ptree const & sym ) if (placement_type) { placement_finder = placements::registry::instance()->from_xml(*placement_type, sym, fontsets_); } else { - placement_finder = text_placements_ptr(new text_placements_dummy()); + placement_finder = text_placements_ptr(boost::make_shared()); placement_finder->defaults.from_xml(sym, fontsets_); } if (strict_ && @@ -1333,7 +1377,7 @@ void map_parser::parse_shield_symbolizer( rule & rule, ptree const & sym ) if (placement_type) { placement_finder = placements::registry::instance()->from_xml(*placement_type, sym, fontsets_); } else { - placement_finder = text_placements_ptr(new text_placements_dummy()); + placement_finder = text_placements_ptr(boost::make_shared()); } placement_finder->defaults.from_xml(sym, fontsets_); if (strict_ && @@ -1432,7 +1476,7 @@ void map_parser::parse_shield_symbolizer( rule & rule, ptree const & sym ) void map_parser::parse_stroke(stroke & strk, ptree const & sym) { // stroke color - optional c = get_opt_attr(sym, "stroke"); + optional c = get_opt_color_attr(sym, "stroke"); if (c) strk.set_color(*c); // stroke-width @@ -1542,7 +1586,7 @@ void map_parser::parse_polygon_symbolizer( rule & rule, ptree const & sym ) { polygon_symbolizer poly_sym; // fill - optional fill = get_opt_attr(sym, "fill"); + optional fill = get_opt_color_attr(sym, "fill"); if (fill) poly_sym.set_fill(*fill); // fill-opacity optional opacity = get_opt_attr(sym, "fill-opacity"); @@ -1573,14 +1617,14 @@ void map_parser::parse_building_symbolizer( rule & rule, ptree const & sym ) building_symbolizer building_sym; // fill - optional fill = get_opt_attr(sym, "fill"); + optional fill = get_opt_color_attr(sym, "fill"); if (fill) building_sym.set_fill(*fill); // fill-opacity optional opacity = get_opt_attr(sym, "fill-opacity"); if (opacity) building_sym.set_opacity(*opacity); // height optional height = get_opt_attr(sym, "height"); - if (height) building_sym.set_height(parse_expression(*height, "utf8")); + if (height) building_sym.set_height(parse_expr(*height)); parse_metawriter_in_symbolizer(building_sym, sym); rule.append(building_sym); @@ -1667,7 +1711,7 @@ void map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc, rc->set_default_mode( default_mode ); // default colour - optional default_color = get_opt_attr(node, "default-color"); + optional default_color = get_opt_color_attr(node, "default-color"); if (default_color) { rc->set_default_color( *default_color ); @@ -1698,7 +1742,7 @@ void map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc, { ensure_attrs(stop_tag.second, "stop", "color,mode,value,label"); // colour is optional. - optional stopcolor = get_opt_attr(stop, "color"); + optional stopcolor = get_opt_color_attr(stop, "color"); if (!stopcolor) { *stopcolor = *default_color; }