From e412e125995a72ecbe2cbc685c5ba820021e929b Mon Sep 17 00:00:00 2001
From: Artem Pavlenko <artem@mapnik.org>
Date: Tue, 30 Apr 2024 10:01:02 +0100
Subject: [PATCH] svg_parser - cleanup + consistent push/pop attributes

---
 src/svg/svg_parser.cpp | 216 +++++++++++++++++++----------------------
 1 file changed, 98 insertions(+), 118 deletions(-)

diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp
index c67cf4466..853438fd3 100644
--- a/src/svg/svg_parser.cpp
+++ b/src/svg/svg_parser.cpp
@@ -500,117 +500,102 @@ void traverse_tree(svg_parser& parser, rapidxml::xml_node<char> const* node)
 
     switch (node->type())
     {
-        case rapidxml::node_element: {
-            parser.font_sizes_.push_back(parser.font_sizes_.back());
-            switch (name)
+    case rapidxml::node_element:
+        parser.font_sizes_.push_back(parser.font_sizes_.back());
+        switch (name)
+        {
+        case "defs"_case: {
+            if (node->first_node() != nullptr)
             {
-                case "defs"_case: {
-                    if (node->first_node() != nullptr)
-                    {
-                        parser.is_defs_ = true;
-                    }
-                    break;
-                }
-                case "clipPath"_case:
-                case "symbol"_case:
-                case "pattern"_case: {
-                    parser.ignore_ = true;
-                    break;
-                }
-                // the gradient tags *should* be in defs, but illustrator seems not to put them in there so
-                // accept them anywhere
-                case "linearGradient"_case:
-                    parse_linear_gradient(parser, node);
-                    break;
-                case "radialGradient"_case:
-                    parse_radial_gradient(parser, node);
-                    break;
+                parser.is_defs_ = true;
             }
-
-            if (!parser.is_defs_) // FIXME
-            {
-                switch (name)
-                {
-                    case "g"_case:
-                        if (node->first_node() != nullptr)
-                        {
-                            parser.path_.push_attr();
-                            parse_id(parser, node);
-                            if (parser.css_style_)
-                                process_css(parser, node);
-                            parse_attr(parser, node);
-                            parser.path_.begin_group();
-                        }
-                        break;
-                    case "use"_case:
-                        parser.path_.push_attr();
-                        parse_id(parser, node);
-                        if (parser.css_style_)
-                            process_css(parser, node);
-                        parse_attr(parser, node);
-                        if (parser.path_.cur_attr().opacity < 1.0)
-                            parser.path_.begin_group();
-                        parse_use(parser, node);
-                        if (parser.path_.cur_attr().opacity < 1.0)
-                            parser.path_.end_group();
-                        parser.path_.pop_attr();
-                        break;
-                    default:
-                        parser.path_.push_attr();
-                        parse_id(parser, node);
-                        if (parser.css_style_)
-                            process_css(parser, node);
-                        parse_attr(parser, node);
-                        if (parser.path_.display())
-                        {
-                            if (parser.path_.cur_attr().opacity < 1.0)
-                                parser.path_.begin_group();
-                            parse_element(parser, node->name(), node);
-                            if (parser.path_.cur_attr().opacity < 1.0)
-                                parser.path_.end_group();
-                        }
-                        parser.path_.pop_attr();
-                }
-            }
-            else
-            {
-                // save node for later
-                parse_id(parser, node);
-            }
-
-            if ("style"_case == name)
-            {
-                // <style> element is not expected to have nested elements
-                // we're only interested in DATA or CDATA
-                for (auto const* child = node->first_node(); child; child = child->next_sibling())
-                {
-                    if (child->type() == rapidxml::node_data || child->type() == rapidxml::node_cdata)
-                    {
-                        auto const grammar = mapnik::grammar();
-                        auto const skipper = mapnik::skipper();
-                        char const* first = child->value();
-                        char const* last = first + child->value_size();
-                        bool result = boost::spirit::x3::phrase_parse(first, last, grammar, skipper, parser.css_data_);
-                        if (result && first == last && !parser.css_data_.empty())
-                        {
-                            parser.css_style_ = true;
-                        }
-                    }
-                }
-            }
-            else
-            {
-                for (auto const* child = node->first_node(); child; child = child->next_sibling())
-                {
-                    traverse_tree(parser, child);
-                }
-            }
-
-            end_element(parser, node);
-        }
-        break;
-        default:
             break;
+        }
+        case "clipPath"_case:
+        case "symbol"_case:
+        case "pattern"_case: {
+            parser.ignore_ = true;
+            break;
+        }
+        // the gradient tags *should* be in defs, but illustrator seems not to put them in there so
+        // accept them anywhere
+        case "linearGradient"_case:
+            parse_linear_gradient(parser, node);
+            break;
+        case "radialGradient"_case:
+            parse_radial_gradient(parser, node);
+            break;
+        case "g"_case:
+            parser.path_.push_attr();
+            parse_id(parser, node);
+            if (parser.css_style_)
+                process_css(parser, node);
+            parse_attr(parser, node);
+            parser.path_.begin_group();
+            break;
+        case "use"_case:
+            parser.path_.push_attr();
+            parse_id(parser, node);
+            if (parser.css_style_)
+                process_css(parser, node);
+            parse_attr(parser, node);
+            if (parser.path_.cur_attr().opacity < 1.0)
+                parser.path_.begin_group();
+            parse_use(parser, node);
+            if (parser.path_.cur_attr().opacity < 1.0)
+                parser.path_.end_group();
+            parser.path_.pop_attr();
+            break;
+        default:
+            parser.path_.push_attr();
+            parse_id(parser, node);
+            if (parser.css_style_)
+                process_css(parser, node);
+            parse_attr(parser, node);
+            if (parser.path_.display())
+            {
+                if (parser.path_.cur_attr().opacity < 1.0)
+                    parser.path_.begin_group();
+                parse_element(parser, node->name(), node);
+                if (parser.path_.cur_attr().opacity < 1.0)
+                    parser.path_.end_group();
+            }
+            parser.path_.pop_attr();
+            break;
+        }
+
+        if ("style"_case == name)
+        {
+            // <style> element is not expected to have nested elements
+            // we're only interested in DATA or CDATA
+            for (auto const* child = node->first_node(); child; child = child->next_sibling())
+            {
+                if (child->type() == rapidxml::node_data || child->type() == rapidxml::node_cdata)
+                {
+                    auto const grammar = mapnik::grammar();
+                    auto const skipper = mapnik::skipper();
+                    char const* first = child->value();
+                    char const* last = first + child->value_size();
+                    bool result = boost::spirit::x3::phrase_parse(first, last, grammar, skipper, parser.css_data_);
+                    if (result && first == last && !parser.css_data_.empty())
+                    {
+                        parser.css_style_ = true;
+                    }
+                }
+            }
+        }
+        else
+        {
+            for (auto const* child = node->first_node(); child; child = child->next_sibling())
+            {
+                traverse_tree(parser, child);
+            }
+        }
+
+        end_element(parser, node);
+        break;
+    default:
+        break;
     }
 }
 
@@ -618,20 +603,15 @@ void end_element(svg_parser& parser, rapidxml::xml_node<char> const* node)
 {
     parser.font_sizes_.pop_back();
     auto name = name_to_int(node->name());
-    if (!parser.is_defs_ && (name == "g"_case))
+    if (name == "g"_case)
     {
         parser.path_.end_group();
-        if (node->first_node() != nullptr)
-        {
-            parser.path_.pop_attr();
-        }
+        parser.path_.pop_attr();
+
     }
     else if (name == "svg"_case)
     {
-        // if (node->first_node() != nullptr)
-        //{
-        // parser.path_.pop_attr();
-        //}
+        parser.path_.pop_attr();
     }
     else if (name == "defs"_case)
     {
@@ -680,7 +660,7 @@ void parse_element(svg_parser& parser, char const* name, rapidxml::xml_node<char
             parse_ellipse(parser, node);
             break;
         case "svg"_case:
-            // parser.path_.push_attr();
+            parser.path_.push_attr();
             parse_dimensions(parser, node);
             parse_attr(parser, node);
             parser.path_.set_opacity(parser.path_.cur_attr().opacity);