From cb708d7f5a0f978f179a0782c286d4fb61454c69 Mon Sep 17 00:00:00 2001
From: artemp <artem@mapnik.org>
Date: Fri, 5 Feb 2016 16:28:12 +0100
Subject: [PATCH] transform_grammar: correct and simplify rules per
 https://github.com/mapnik/mapnik/commit/bcc7495a8728e71efbfa6ee397af3da58ea1958d#commitcomment-15915607
 image_filter: relax `agg-stack-blur` rule to allow
 `agg-stack-blur,agg-stack-blur(),agg-stack-blur(1),agg-stack-blur(1,1)`
 syntax + add image filter parsing tests

---
 include/mapnik/image_filter_grammar_impl.hpp  | 13 +++---
 .../transform_expression_grammar_impl.hpp     |  4 +-
 test/unit/imaging/image_filter.cpp            | 41 +++++++++++++++++++
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/include/mapnik/image_filter_grammar_impl.hpp b/include/mapnik/image_filter_grammar_impl.hpp
index 7b8212ec7..168d9a112 100644
--- a/include/mapnik/image_filter_grammar_impl.hpp
+++ b/include/mapnik/image_filter_grammar_impl.hpp
@@ -57,14 +57,12 @@ image_filter_grammar<Iterator,ContType>::image_filter_grammar()
     qi::_g_type _g;
     qi::_h_type _h;
     qi::_r1_type _r1;
-    qi::eps_type eps;
-    qi::char_type char_;
     qi::double_type double_;
-    qi::no_skip_type no_skip;
     using phoenix::push_back;
     using phoenix::construct;
     using phoenix::at_c;
-    start = -(filter % no_skip[*char_(", ")])
+
+    start = -(filter % *lit(','))
         ;
 
     filter =
@@ -101,11 +99,10 @@ image_filter_grammar<Iterator,ContType>::image_filter_grammar()
         color_to_alpha_filter(_val)
         ;
 
-    agg_blur_filter = lit("agg-stack-blur")[_a = 1, _b = 1]
+    agg_blur_filter = (lit("agg-stack-blur")[_a = 1, _b = 1]
         >> -( lit('(') >> -( radius_[_a = _1][_b = _1]
-                             >> -(lit(',') >> radius_[_b = _1]))
-              >> lit(')'))
-        [push_back(_r1,construct<mapnik::filter::agg_stack_blur>(_a,_b))]
+                             >> -(lit(',') >> radius_[_b = _1])) >> lit(')')))
+        [push_back(_r1, construct<mapnik::filter::agg_stack_blur>(_a,_b))]
         ;
 
     scale_hsla_filter = lit("scale-hsla")
diff --git a/include/mapnik/transform_expression_grammar_impl.hpp b/include/mapnik/transform_expression_grammar_impl.hpp
index 7f5317e69..4b8c2b92a 100644
--- a/include/mapnik/transform_expression_grammar_impl.hpp
+++ b/include/mapnik/transform_expression_grammar_impl.hpp
@@ -45,7 +45,6 @@ transform_expression_grammar<Iterator>::transform_expression_grammar()
     qi::_3_type _3;
     qi::_6_type _6;
     qi::_val_type _val;
-    qi::char_type char_;
     qi::double_type double_;
     qi::lit_type lit;
     qi::no_case_type no_case;
@@ -57,8 +56,7 @@ transform_expression_grammar<Iterator>::transform_expression_grammar()
     // the order provided.  The individual transform definitions are
     // separated by whitespace and/or a comma.
 
-    qi::no_skip_type no_skip;
-    start = transform_ % no_skip[*char_(", ")] ;
+    start = transform_ % *lit(',') ;
 
     transform_ = matrix | translate | scale | rotate | skewX | skewY ;
 
diff --git a/test/unit/imaging/image_filter.cpp b/test/unit/imaging/image_filter.cpp
index f2805b068..4ee5b1a72 100644
--- a/test/unit/imaging/image_filter.cpp
+++ b/test/unit/imaging/image_filter.cpp
@@ -7,6 +7,9 @@
 #include <mapnik/image_filter.hpp>
 #include <mapnik/image_util.hpp>
 #include <mapnik/image_filter_types.hpp>
+// stl
+#include <sstream>
+#include <array>
 
 TEST_CASE("image filter") {
 
@@ -389,6 +392,44 @@ SECTION("test colorize-alpha - two color with transparency") {
 
 } // END SECTION
 
+SECTION("test parsing image-filters") {
+
+    std::string str = ""; // empty string
+    std::vector<mapnik::filter::filter_type> filters;
+    CHECK(parse_image_filters(str, filters));
+    CHECK(filters.size() == 0);
+
+    std::array<std::string,17> expected = {{ "emboss",
+                                             "emboss",
+                                             "blur",
+                                             "gray",
+                                             "edge-detect",
+                                             "sobel",
+                                             "sharpen",
+                                             "x-gradient",
+                                             "y-gradient",
+                                             "invert",
+                                             "color-blind-protanope",
+                                             "color-blind-deuteranope",
+                                             "color-blind-tritanope",
+                                             "agg-stack-blur(1,1)",
+                                             "agg-stack-blur(1,1)",
+                                             "agg-stack-blur(2,2)",
+                                             "agg-stack-blur(2,3)"}};
+
+    str += "emboss emboss() blur,gray ,edge-detect, sobel , , sharpen,,,x-gradient y-gradientinvert";
+    str += "color-blind-protanope color-blind-deuteranope color-blind-tritanope agg-stack-blur,agg-stack-blur(),agg-stack-blur(2),agg-stack-blur(2,3)" ;
+    CHECK(parse_image_filters(str, filters));
+    CHECK(filters.size() == expected.size());
+    std::size_t count = 0;
+    for (auto const& filter : filters)
+    {
+        std::stringstream ss;
+        ss << filter;
+        CHECK (expected[count++] == ss.str());
+    }
+}
+
 SECTION("test colorize-alpha - parsing correct input") {
 
     std::string s("colorize-alpha(#0000ff 0%, #00ff00 100%)");