Merge pull request #1450 from rundel/master

path_parse simplification

demo/c++/rundemo.cpp

@@ -61,7 +61,7 @@ int main ( int argc , char** argv)
         freetype_engine::register_font(mapnik_dir + "/fonts/DejaVuSans.ttf");
 
         Map m(800,600);
-        m.set_background(color_factory::from_string("white"));
+        m.set_background(parse_color("white"));
 
         // create styles
 

include/mapnik/color.hpp

@@ -60,8 +60,6 @@ public:
         alpha_(alpha)
         {}
 
-    color( std::string const& css_string);
-
     color(const color& rhs)
       : red_(rhs.red_),
         green_(rhs.green_),
@@ -69,16 +67,32 @@ public:
         alpha_(rhs.alpha_)
         {}
 
-    color& operator=(const color& rhs)
+    color( std::string const& str);
+
+    std::string to_string() const;
+    std::string to_hex_string() const;
+
+    color& operator=(color const& rhs)
     {
-            if (this==&rhs) return *this;
+        if (this==&rhs) 
+            return *this;
+        
         red_   = rhs.red_;
         green_ = rhs.green_;
         blue_  = rhs.blue_;
         alpha_ = rhs.alpha_;
+        
         return *this;
     }
 
+    inline bool operator==(color const& rhs) const
+    {
+        return (red_== rhs.red()) &&
+               (green_ == rhs.green()) &&
+               (blue_  == rhs.blue()) &&
+               (alpha_ == rhs.alpha());
+    }
+
     inline unsigned red() const
     {
         return red_;
@@ -123,18 +137,6 @@ public:
         return (alpha_ << 24) | (blue_ << 16) | (green_ << 8) | (red_) ;
 #endif
     }
-
-    inline bool operator==(color const& rhs) const
-    {
-        return (red_== rhs.red()) &&
-            (green_ == rhs.green()) &&
-            (blue_  == rhs.blue()) &&
-            (alpha_ == rhs.alpha());
-
-    }
-
-    std::string to_string() const;
-    std::string to_hex_string() const;
 };
 
 template <typename charT, typename traits>

include/mapnik/color_factory.hpp

@@ -24,27 +24,16 @@
 #define MAPNIK_COLOR_FACTORY_HPP
 
 // mapnik
-#include <mapnik/config.hpp>
+#include <mapnik/color.hpp>
+#include <mapnik/css_color_grammar.hpp>
 
-// boost
+#include <string>
-#include <boost/utility.hpp>
 
 namespace mapnik {
 
-class color;
+MAPNIK_DECL mapnik::color parse_color(std::string const& str);
+MAPNIK_DECL mapnik::color parse_color(std::string const& str, mapnik::css_color_grammar<std::string::const_iterator> const& g);
 
-template <typename Iterator> struct css_color_grammar;
-class MAPNIK_DECL color_factory : boost::noncopyable
-{
-public:
-
-    static void init_from_string(color & c, std::string const& css_color);
-
-    static bool parse_from_string(color & c, std::string const& css_color,
-                                  mapnik::css_color_grammar<std::string::const_iterator> const& g);
-
-    static color from_string(std::string const& css_color);
-};
 }
 
 #endif // MAPNIK_COLOR_FACTORY_HPP

include/mapnik/expression.hpp

@@ -26,6 +26,7 @@
 // mapnik
 #include <mapnik/config.hpp>
 #include <mapnik/expression_node.hpp>
+#include <mapnik/expression_grammar.hpp>
 
 // stl
 #include <string>
@@ -34,19 +35,11 @@ namespace mapnik
 {
 
 typedef boost::shared_ptr<expr_node> expression_ptr;
-template <typename Iterator> struct expression_grammar;
 
-class expression_factory
+
-{
+MAPNIK_DECL expression_ptr parse_expression (std::string const& wkt, std::string const& encoding = "UTF8");
-public:
+MAPNIK_DECL expression_ptr parse_expression (std::string const& wkt,
-    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<std::string::const_iterator> 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);
 
 }
 

include/mapnik/parse_path.hpp

@@ -28,6 +28,7 @@
 #include <mapnik/attribute.hpp>
 #include <mapnik/feature.hpp>
 #include <mapnik/value.hpp>
+#include <mapnik/path_expression_grammar.hpp>
 
 // boost
 #include <boost/shared_ptr.hpp>
@@ -40,17 +41,12 @@
 
 namespace mapnik {
 
-typedef boost::variant<std::string, attribute> path_component;
-typedef std::vector<path_component> path_expression;
 typedef boost::shared_ptr<path_expression> path_expression_ptr;
-template <typename Iterator> struct path_expression_grammar;
 
 MAPNIK_DECL path_expression_ptr parse_path(std::string const & str);
-MAPNIK_DECL bool parse_path_from_string(path_expression_ptr const& path,
+MAPNIK_DECL path_expression_ptr parse_path(std::string const & str,
-                                        std::string const & str,
                                            path_expression_grammar<std::string::const_iterator> const& g);
 
-
 template <typename T>
 struct path_processor
 {

include/mapnik/path_expression_grammar.hpp

@@ -39,14 +39,16 @@
 
 namespace mapnik
 {
+	
 using namespace boost;
 namespace qi = boost::spirit::qi;
 namespace phoenix = boost::phoenix;
 namespace standard_wide =  boost::spirit::standard_wide;
 
 using standard_wide::space_type;
-using standard_wide::space;
+
 typedef boost::variant<std::string, attribute> path_component;
+typedef std::vector<path_component> path_expression;
 
 template <typename Iterator>
 struct path_expression_grammar : qi::grammar<Iterator, std::vector<path_component>(), space_type>

include/mapnik/rule.hpp

@@ -135,27 +135,6 @@ private:
 
     struct deepcopy_symbolizer : public boost::static_visitor<>
     {
-
-        void operator () (markers_symbolizer & sym) const
-        {
-            copy_path_ptr(sym);
-        }
-
-        void operator () (point_symbolizer & sym) const
-        {
-            copy_path_ptr(sym);
-        }
-
-        void operator () (polygon_pattern_symbolizer & sym) const
-        {
-            copy_path_ptr(sym);
-        }
-
-        void operator () (line_pattern_symbolizer & sym) const
-        {
-            copy_path_ptr(sym);
-        }
-
         void operator () (raster_symbolizer & sym) const
         {
             raster_colorizer_ptr old_colorizer = sym.get_colorizer();
@@ -174,7 +153,6 @@ private:
 
         void operator () (shield_symbolizer & sym) const
         {
-            copy_path_ptr(sym);
             copy_text_ptr(sym);
         }
 
@@ -183,19 +161,12 @@ private:
             copy_height_ptr(sym);
         }
 
-
         template <typename T> void operator () (T &sym) const
         {
             boost::ignore_unused_variable_warning(sym);
         }
 
     private:
-        template <class T>
-        void copy_path_ptr(T & sym) const
-        {
-            std::string path = path_processor_type::to_string(*sym.get_filename());
-            sym.set_filename( parse_path(path) );
-        }
 
         template <class T>
         void copy_text_ptr(T & sym) const

src/build.py

@@ -187,6 +187,7 @@ source = Split(
     text_properties.cpp
     xml_tree.cpp
     config_error.cpp
+    color_factory.cpp
     """
     )
 

src/color.cpp

@@ -27,22 +27,15 @@
 
 // boost
 #include <boost/format.hpp>
-#include <boost/version.hpp>
 
 // stl
 #include <sstream>
 
-#include <mapnik/css_color_grammar.hpp>
-
 namespace mapnik {
 
-color::color( std::string const& css_string)
+color::color(std::string const& str)
-    : red_(0),
-      green_(0),
-      blue_(0),
-      alpha_(0xff)
 {
-    color_factory::init_from_string(*this,css_string);
+    *this = parse_color(str);
 }
 
 std::string color::to_string() const
@@ -85,90 +78,5 @@ std::string color::to_hex_string() const
     }
 }
 
-
-/****************************************************************************/
-void color_factory::init_from_string(color & c, std::string const& css_color)
-{
-    typedef std::string::const_iterator iterator_type;
-    typedef mapnik::css_color_grammar<iterator_type> css_color_grammar;
-
-    css_color_grammar g;
-    iterator_type first = css_color.begin();
-    iterator_type last =  css_color.end();
-    // boost 1.41 -> 1.44 compatibility, to be removed in mapnik 2.1 (dane)
-#if BOOST_VERSION >= 104500
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        c);
-    if (!result)
-    {
-        throw config_error(std::string("Failed to parse color value: ") +
-                           "Expected a CSS color, but got '" + css_color + "'");
-    }
-#else
-    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);
-#endif
-}
-
-bool color_factory::parse_from_string(color & c, std::string const& css_color,
-                                      mapnik::css_color_grammar<std::string::const_iterator> const& g)
-{
-    std::string::const_iterator first = css_color.begin();
-    std::string::const_iterator last =  css_color.end();
-    // boost 1.41 -> 1.44 compatibility, to be removed in mapnik 2.1 (dane)
-#if BOOST_VERSION >= 104500
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        c);
-    return result && (first == last);
-#else
-    mapnik::css css_;
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        css_);
-    if (result && (first == last))
-    {
-        c.set_red(css_.r);
-        c.set_green(css_.g);
-        c.set_blue(css_.b);
-        c.set_alpha(css_.a);
-        return true;
-    }
-    return false;
-#endif
-}
-
-
-color color_factory::from_string(std::string const& css_color)
-{
-    color c;
-    init_from_string(c, css_color);
-    return c;
-}
-
 }
 

src/color_factory.cpp

@@ -0,0 +1,76 @@
+/*****************************************************************************
+ *
+ * This file is part of Mapnik (c++ mapping toolkit)
+ *
+ * Copyright (C) 2011 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
+ *
+ *****************************************************************************/
+
+// mapnik
+#include <mapnik/color.hpp>
+#include <mapnik/color_factory.hpp>
+#include <mapnik/config_error.hpp>
+#include <mapnik/css_color_grammar.hpp>
+
+// boost
+#include <boost/format.hpp>
+#include <boost/version.hpp>
+
+// stl
+#include <sstream>
+
+
+namespace mapnik {
+
+color parse_color(std::string const& str)
+{
+    css_color_grammar<std::string::const_iterator> g;
+    return parse_color(str, g);
+}
+
+color parse_color(std::string const& str,
+                  css_color_grammar<std::string::const_iterator> const& g)
+{
+    color c;
+
+    std::string::const_iterator first = str.begin();
+    std::string::const_iterator last =  str.end();
+    
+    // boost 1.41 -> 1.44 compatibility, to be removed in mapnik 2.1 (dane)
+#if BOOST_VERSION >= 104500
+    bool result = boost::spirit::qi::phrase_parse(first, last, g,
+                                                  boost::spirit::ascii::space,
+                                                  c);
+#else
+    mapnik::css css_;
+    bool result = boost::spirit::qi::phrase_parse(first, last, g,
+                                                  boost::spirit::ascii::space, 
+                                                  css_);
+    c.set_red(css_.r);
+    c.set_green(css_.g);
+    c.set_blue(css_.b);
+    c.set_alpha(css_.a);
+    
+#endif
+
+    if (result && (first == last))
+        return c;
+    else
+        throw config_error( "Failed to parse color: \"" + str + "\"" );
+}
+
+}

src/expression.cpp

@@ -33,16 +33,26 @@
 namespace mapnik
 {
 
-expression_ptr expression_factory::compile(std::string const& str,transcoder const& tr)
+expression_ptr parse_expression(std::string const& str, std::string const& encoding)
 {
-    expression_ptr expr(boost::make_shared<expr_node>(true));
+    transcoder tr(encoding);
+    expression_grammar<std::string::const_iterator> g(tr);
+
+    return parse_expression(str, g);
+}
+
+expression_ptr parse_expression(std::string const& str,
+                                mapnik::expression_grammar<std::string::const_iterator> const& g)
+{
+    expr_node node;
+
     std::string::const_iterator itr = str.begin();
     std::string::const_iterator end = str.end();
-    mapnik::expression_grammar<std::string::const_iterator> g(tr);
+    
-    bool r = boost::spirit::qi::phrase_parse(itr,end,g, boost::spirit::standard_wide::space,*expr);
+    bool r = boost::spirit::qi::phrase_parse(itr, end, g, boost::spirit::standard_wide::space, node);
     if (r && itr==end)
     {
-        return expr;
+        return boost::make_shared<expr_node>(node);
     }
     else
     {
@@ -50,24 +60,5 @@ expression_ptr expression_factory::compile(std::string const& str,transcoder con
     }
 }
 
-bool expression_factory::parse_from_string(expression_ptr const& expr,
-                                           std::string const& str,
-                                           mapnik::expression_grammar<std::string::const_iterator> 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)
-{
-    transcoder tr(encoding);
-    return expression_factory::compile(wkt,tr);
-}
-
-expression_ptr parse_expression (std::string const& wkt)
-{
-    return parse_expression(wkt,"utf8");
-}
 }

src/load_map.cpp

@@ -899,13 +899,7 @@ void map_parser::parse_point_symbolizer(rule & rule, xml_node const & sym)
             *file = ensure_relative_to_xml(file);
             std::string filename = *file;
             ensure_exists(filename);
-            path_expression_ptr expr(boost::make_shared<path_expression>());
+            symbol.set_filename( parse_path(filename, sym.get_tree().path_expr_grammar) );
-            if (!parse_path_from_string(expr, filename, sym.get_tree().path_expr_grammar))
-            {
-                throw mapnik::config_error("Failed to parse path_expression '" + filename + "'");
-            }
-
-            symbol.set_filename(expr);
 
             optional<std::string> image_transform_wkt = sym.get_opt_attr<std::string>("transform");
             if (image_transform_wkt)
@@ -928,14 +922,13 @@ void map_parser::parse_point_symbolizer(rule & rule, xml_node const & sym)
     }
 }
 
-
+void map_parser::parse_markers_symbolizer(rule & rule, xml_node const& sym)
-void map_parser::parse_markers_symbolizer(rule & rule, xml_node const& node)
 {
     try
     {
         std::string filename("");
-        optional<std::string> file = node.get_opt_attr<std::string>("file");
+        optional<std::string> file = sym.get_opt_attr<std::string>("file");
-        optional<std::string> base = node.get_opt_attr<std::string>("base");
+        optional<std::string> base = sym.get_opt_attr<std::string>("base");
 
         if (file && !file->empty())
         {
@@ -951,7 +944,7 @@ void map_parser::parse_markers_symbolizer(rule & rule, xml_node const& node)
             filename = ensure_relative_to_xml(file);
         }
 
-        optional<std::string> marker_type = node.get_opt_attr<std::string>("marker-type");
+        optional<std::string> marker_type = sym.get_opt_attr<std::string>("marker-type");
         if (marker_type)
         {
             // TODO - revisit whether to officially deprecate marker-type
@@ -971,68 +964,67 @@ void map_parser::parse_markers_symbolizer(rule & rule, xml_node const& node)
             }
         }
 
-        markers_symbolizer sym;
+        markers_symbolizer symbol;
 
         if (!filename.empty())
         {
             ensure_exists(filename);
-            path_expression_ptr expr(boost::make_shared<path_expression>());
+            symbol.set_filename( parse_path(filename, sym.get_tree().path_expr_grammar) );
-            if (!parse_path_from_string(expr, filename, node.get_tree().path_expr_grammar))
-            {
-                throw mapnik::config_error("Failed to parse path_expression '" + filename + "'");
-            }
-            sym.set_filename(expr);
         }
 
         // overall opacity to be applied to all paths
-        optional<float> opacity = node.get_opt_attr<float>("opacity");
+        optional<float> opacity = sym.get_opt_attr<float>("opacity");
-        if (opacity) sym.set_opacity(*opacity);
+        if (opacity) symbol.set_opacity(*opacity);
 
-        optional<float> fill_opacity = node.get_opt_attr<float>("fill-opacity");
+        optional<float> fill_opacity = sym.get_opt_attr<float>("fill-opacity");
-        if (fill_opacity) sym.set_fill_opacity(*fill_opacity);
+        if (fill_opacity) symbol.set_fill_opacity(*fill_opacity);
 
-        optional<std::string> image_transform_wkt = node.get_opt_attr<std::string>("transform");
+        optional<std::string> image_transform_wkt = sym.get_opt_attr<std::string>("transform");
         if (image_transform_wkt)
         {
             mapnik::transform_list_ptr tl = boost::make_shared<mapnik::transform_list>();
-            if (!mapnik::parse_transform(*tl, *image_transform_wkt, node.get_tree().transform_expr_grammar))
+            if (!mapnik::parse_transform(*tl, *image_transform_wkt, sym.get_tree().transform_expr_grammar))
             {
                 throw mapnik::config_error("Failed to parse transform: '" + *image_transform_wkt + "'");
             }
-            sym.set_image_transform(tl);
+            symbol.set_image_transform(tl);
         }
 
-        optional<color> c = node.get_opt_attr<color>("fill");
+        optional<color> c = sym.get_opt_attr<color>("fill");
-        if (c) sym.set_fill(*c);
+        if (c) symbol.set_fill(*c);
-        optional<double> spacing = node.get_opt_attr<double>("spacing");
-        if (spacing) sym.set_spacing(*spacing);
-        optional<double> max_error = node.get_opt_attr<double>("max-error");
-        if (max_error) sym.set_max_error(*max_error);
-        optional<boolean> allow_overlap = node.get_opt_attr<boolean>("allow-overlap");
-        optional<boolean> ignore_placement = node.get_opt_attr<boolean>("ignore-placement");
-        if (allow_overlap) sym.set_allow_overlap(*allow_overlap);
-        if (ignore_placement) sym.set_ignore_placement(*ignore_placement);
         
-        optional<expression_ptr> width = node.get_opt_attr<expression_ptr>("width");
+        optional<double> spacing = sym.get_opt_attr<double>("spacing");
-        if (width) sym.set_width(*width);
+        if (spacing) symbol.set_spacing(*spacing);
         
-        optional<expression_ptr> height = node.get_opt_attr<expression_ptr>("height");
+        optional<double> max_error = sym.get_opt_attr<double>("max-error");
-        if (height) sym.set_height(*height);
+        if (max_error) symbol.set_max_error(*max_error);
+        
+        optional<boolean> allow_overlap = sym.get_opt_attr<boolean>("allow-overlap");
+        if (allow_overlap) symbol.set_allow_overlap(*allow_overlap);
+        
+        optional<boolean> ignore_placement = sym.get_opt_attr<boolean>("ignore-placement");
+        if (ignore_placement) symbol.set_ignore_placement(*ignore_placement);
+
+        optional<expression_ptr> width = sym.get_opt_attr<expression_ptr>("width");
+        if (width) symbol.set_width(*width);
+
+        optional<expression_ptr> height = sym.get_opt_attr<expression_ptr>("height");
+        if (height) symbol.set_height(*height);
 
         stroke strk;
-        if (parse_stroke(strk,node))
+        if (parse_stroke(strk,sym))
         {
-            sym.set_stroke(strk);
+            symbol.set_stroke(strk);
         }
 
-        marker_placement_e placement = node.get_attr<marker_placement_e>("placement",sym.get_marker_placement());
+        marker_placement_e placement = sym.get_attr<marker_placement_e>("placement",symbol.get_marker_placement());
-        sym.set_marker_placement(placement);
+        symbol.set_marker_placement(placement);
-        parse_symbolizer_base(sym, node);
+        parse_symbolizer_base(symbol, sym);
-        rule.append(sym);
+        rule.append(symbol);
     }
     catch (config_error const& ex)
     {
-        ex.append_context(node);
+        ex.append_context(sym);
         throw;
     }
 }
@@ -1060,12 +1052,7 @@ void map_parser::parse_line_pattern_symbolizer(rule & rule, xml_node const & sym
 
         file = ensure_relative_to_xml(file);
         ensure_exists(file);
-        path_expression_ptr expr(boost::make_shared<path_expression>());
+        line_pattern_symbolizer symbol( parse_path(file, sym.get_tree().path_expr_grammar) );
-        if (!parse_path_from_string(expr, file, sym.get_tree().path_expr_grammar))
-        {
-            throw mapnik::config_error("Failed to parse path_expression '" + file + "'");
-        }
-        line_pattern_symbolizer symbol(expr);
 
         parse_symbolizer_base(symbol, sym);
         rule.append(symbol);
@@ -1102,12 +1089,7 @@ void map_parser::parse_polygon_pattern_symbolizer(rule & rule,
 
         file = ensure_relative_to_xml(file);
         ensure_exists(file);
-        path_expression_ptr expr(boost::make_shared<path_expression>());
+        polygon_pattern_symbolizer symbol( parse_path(file, sym.get_tree().path_expr_grammar) );
-        if (!parse_path_from_string(expr, file, sym.get_tree().path_expr_grammar))
-        {
-            throw mapnik::config_error("Failed to parse path_expression '" + file + "'");
-        }
-        polygon_pattern_symbolizer symbol(expr);
 
         // pattern alignment
         pattern_alignment_e p_alignment = sym.get_attr<pattern_alignment_e>("alignment",LOCAL_ALIGNMENT);
@@ -1254,12 +1236,7 @@ void map_parser::parse_shield_symbolizer(rule & rule, xml_node const& sym)
 
         file = ensure_relative_to_xml(file);
         ensure_exists(file);
-        path_expression_ptr expr(boost::make_shared<path_expression>());
+        shield_symbol.set_filename( parse_path(file, sym.get_tree().path_expr_grammar) );
-        if (!parse_path_from_string(expr, file, sym.get_tree().path_expr_grammar))
-        {
-            throw mapnik::config_error("Failed to parse path_expression '" + file + "'");
-        }
-        shield_symbol.set_filename(expr);
         parse_symbolizer_base(shield_symbol, sym);
         rule.append(shield_symbol);
     }

src/parse_path.cpp

@@ -29,30 +29,26 @@ namespace mapnik {
 
 path_expression_ptr parse_path(std::string const& str)
 {
-    path_expression_ptr path =  boost::make_shared<path_expression>();
     path_expression_grammar<std::string::const_iterator> g;
+    return parse_path(str,g);
+}
+
+path_expression_ptr parse_path(std::string const& str,
+                               path_expression_grammar<std::string::const_iterator> const& g)
+{
+    path_expression path;  
     
     std::string::const_iterator itr = str.begin();
     std::string::const_iterator end = str.end();
-    bool r = qi::phrase_parse(itr, end, g, space, *path);
+    bool r = qi::phrase_parse(itr, end, g, boost::spirit::standard_wide::space, path);
     if (r  && itr == end)
     {
-        return path;
+        return boost::make_shared<path_expression>(path); //path;
     }
     else
     {
-        throw std::runtime_error("Failed to parse path expression");
+        throw std::runtime_error("Failed to parse path expression: \"" + str + "\"");
     }
 }
 
-bool parse_path_from_string(path_expression_ptr const& path,
-                                           std::string const & str,
-                                           path_expression_grammar<std::string::const_iterator> const& g)
-{
-    std::string::const_iterator itr = str.begin();
-    std::string::const_iterator end = str.end();
-    bool r = qi::phrase_parse(itr, end, g, space, *path);
-    return (r && itr==end);
-}
-
 }

src/svg/svg_parser.cpp

@@ -74,7 +74,7 @@ agg::rgba8 parse_color(const char* str)
     mapnik::color c(100,100,100);
     try
     {
-        mapnik::color_factory::init_from_string(c,str);
+        c = mapnik::parse_color(str);
     }
     catch (mapnik::config_error & ex)
     {
@@ -784,7 +784,7 @@ void svg_parser::parse_gradient_stop(xmlTextReaderPtr reader)
             {
                 try
                 {
-                    mapnik::color_factory::init_from_string(stop_color,kv.second.c_str());
+                    stop_color = mapnik::parse_color(kv.second.c_str());
                 }
                 catch (mapnik::config_error & ex)
                 {
@@ -804,7 +804,7 @@ void svg_parser::parse_gradient_stop(xmlTextReaderPtr reader)
     {
         try
         {
-            mapnik::color_factory::init_from_string(stop_color,(const char *) value);
+            stop_color = mapnik::parse_color((const char *) value);
         }
         catch (mapnik::config_error & ex)
         {

src/xml_tree.cpp

@@ -86,29 +86,13 @@ inline boost::optional<std::string> fast_cast(xml_tree const& tree, std::string
 template <>
 inline boost::optional<color> fast_cast(xml_tree const& tree, std::string const& value)
 {
-    mapnik::color c;
+    return parse_color(value, tree.color_grammar);
-    if (mapnik::color_factory::parse_from_string(c, value, tree.color_grammar))
-    {
-        return c;
-    }
-    else
-    {
-        throw config_error("Failed to parse color '"+value+"'");
-    }
 }
 
 template <>
 inline boost::optional<expression_ptr> fast_cast(xml_tree const& tree, std::string const& value)
 {
-    expression_ptr expr(boost::make_shared<expr_node>(true));
+    return parse_expression(value, tree.expr_grammar);
-    if (expression_factory::parse_from_string(expr, value, tree.expr_grammar))
-    {
-        return expr;
-    }
-    else
-    {
-        throw mapnik::config_error("Failed to parse expression '" + value + "'");
-    }
 }
 
 /****************************************************************************/

tests/cpp_tests/svg_renderer_tests/combined_test.cpp

@@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(combined_test_case)
     typedef svg_renderer<std::ostream_iterator<char> > svg_ren;
 
     Map map(800, 600);
-    map.set_background(color_factory::from_string("white"));
+    map.set_background(parse_color("white"));
 
     std::ostringstream output_stream;
     std::ostream_iterator<char> output_stream_iterator(output_stream);

tests/cpp_tests/svg_renderer_tests/file_output_test.cpp

@@ -43,7 +43,7 @@ BOOST_AUTO_TEST_CASE(file_output_test_case)
     typedef svg_renderer<std::ostream_iterator<char> > svg_ren;
 
     Map map(800, 600);
-    map.set_background(color_factory::from_string("blue"));
+    map.set_background(parse_color("blue"));
 
     std::string output_filename = "file_output_test_case.svg";
     std::ofstream output_stream(output_filename.c_str());

tests/cpp_tests/svg_renderer_tests/path_element_test.cpp

@@ -220,7 +220,7 @@ void render_to_file(Map const& m, const std::string output_filename)
 BOOST_AUTO_TEST_CASE(path_element_test_case_1)
 {
     Map m(800,600);
-    m.set_background(color_factory::from_string("steelblue"));
+    m.set_background(parse_color("steelblue"));
 
     prepare_map(m);