Improve SVG text parsing capabilities

Prior to this change all <tspan> elements without x, and y attributes would
just inherit the parent elements such attributes and be placed in the same
location. This didn't respect whether these attributes were consumed already
by the parent.

Having multiple x and y elements, or having a different number of x and y
elements was also not handled in line with the rules for SVG.

modules/juce_gui_basics/drawables/juce_SVGParser.cpp

@@ -245,7 +245,7 @@ public:
 
             case 'H':
             case 'h':
-                if (parseCoord (d, p1.x, false, true))
+                if (parseCoord (d, p1.x, false, Axis::x))
                 {
                     if (isRelative)
                         p1.x += last.x;
@@ -263,7 +263,7 @@ public:
 
             case 'V':
             case 'v':
-                if (parseCoord (d, p1.y, false, false))
+                if (parseCoord (d, p1.y, false, Axis::y))
                 {
                     if (isRelative)
                         p1.y += last.y;
@@ -491,7 +491,7 @@ private:
 
         if (tag == "g")         return parseGroupElement (xml, true);
         if (tag == "svg")       return parseSVGElement (xml);
-        if (tag == "text")      return parseText (xml, true);
+        if (tag == "text")      return parseText (xml, true, nullptr);
         if (tag == "image")     return parseImage (xml, true);
         if (tag == "switch")    return parseSwitch (xml);
         if (tag == "a")         return parseLinkElement (xml);
@@ -666,7 +666,7 @@ private:
 
     Drawable* parseUseOther (const XmlPath& xml) const
     {
-        if (auto* drawableText  = parseText (xml, false))    return drawableText;
+        if (auto* drawableText  = parseText (xml, false, nullptr))    return drawableText;
         if (auto* drawableImage = parseImage (xml, false))   return drawableImage;
 
         return nullptr;
@@ -750,7 +750,7 @@ private:
         for (auto t = dashList.getCharPointer();;)
         {
             float value;
-            if (! parseCoord (t, value, true, true))
+            if (! parseCoord (t, value, true, Axis::x))
                 break;
 
             dashLengths.add (value);
@@ -1050,8 +1050,83 @@ private:
         return op.target;
     }
 
+    /*  Handling the stateful consumption of x and y coordinates added to <text> and <tspan> elements.
+
+        <text> elements must have their own x and y attributes, or be positioned at (0, 0) since groups
+        enclosing <text> elements can't have x and y attributes.
+
+        <tspan> elements can be embedded inside <text> elements, and <tspan> elements. <text> elements
+        can't be embedded inside <text> or <tspan> elements.
+
+        A <tspan> element can have its own x, y attributes, which it will consume at the same time as
+        it consumes its parent's attributes. Its own elements will take precedence, but parent elements
+        will be consumed regardless.
+    */
+    class StringLayoutState
+    {
+    public:
+        StringLayoutState (StringLayoutState* parentIn, Array<float> xIn, Array<float> yIn)
+            : parent (parentIn),
+              xCoords (std::move (xIn)),
+              yCoords (std::move (yIn))
+        {
+        }
+
+        Point<float> getNextStartingPos() const
+        {
+            if (parent != nullptr)
+                return parent->getNextStartingPos();
+
+            return nextStartingPos;
+        }
+
+        void setNextStartingPos (Point<float> newPos)
+        {
+            nextStartingPos = newPos;
+
+            if (parent != nullptr)
+                parent->setNextStartingPos (newPos);
+        }
+
+        std::pair<std::optional<float>, std::optional<float>> popCoords()
+        {
+            auto x = xCoords.isEmpty() ? std::optional<float>{} : std::make_optional (xCoords.removeAndReturn (0));
+            auto y = yCoords.isEmpty() ? std::optional<float>{} : std::make_optional (yCoords.removeAndReturn (0));
+
+            if (parent != nullptr)
+            {
+                auto [parentX, parentY] = parent->popCoords();
+
+                if (! x.has_value())
+                    x = parentX;
+
+                if (! y.has_value())
+                    y = parentY;
+            }
+
+            return { x, y };
+        }
+
+        bool hasMoreCoords() const
+        {
+            if (! xCoords.isEmpty() || ! yCoords.isEmpty())
+                return true;
+
+            if (parent != nullptr)
+                return parent->hasMoreCoords();
+
+            return false;
+        }
+
+    private:
+        StringLayoutState* parent = nullptr;
+        Point<float> nextStartingPos;
+        Array<float> xCoords, yCoords;
+    };
+
     Drawable* parseText (const XmlPath& xml, bool shouldParseTransform,
-                         AffineTransform* additonalTransform = nullptr) const
+                         AffineTransform* additonalTransform,
+                         StringLayoutState* parentLayoutState = nullptr) const
     {
         if (shouldParseTransform && xml->hasAttribute ("transform"))
         {
@@ -1067,10 +1142,11 @@ private:
         if (! xml->hasTagName ("text") && ! xml->hasTagNameIgnoringNamespace ("tspan"))
             return nullptr;
 
-        Array<float> xCoords, yCoords;
-
-        getCoordList (xCoords,  getInheritedAttribute (xml, "x"),  true, true);
-        getCoordList (yCoords,  getInheritedAttribute (xml, "y"),  true, false);
+        // If a <tspan> element has no x, or y attributes of its own, it can still use the
+        // parent's yet unconsumed such attributes.
+        StringLayoutState layoutState { parentLayoutState,
+                                        getCoordList (*xml, Axis::x),
+                                        getCoordList (*xml, Axis::y) };
 
         auto font = getFont (xml);
         auto anchorStr = getStyleAttribute (xml, "text-anchor");
@@ -1086,28 +1162,20 @@ private:
 
                 const auto subtextElements = [&]
                 {
-                    std::vector<std::tuple<String, float, float>> result;
-
-                    if (xCoords.size() == 1)
-                    {
-                        result.emplace_back (fullText, xCoords[0], yCoords[0]);
-                        return result;
-                    }
+                    std::vector<std::tuple<String, std::optional<float>, std::optional<float>>> result;
 
-                    if (xCoords.size() != yCoords.size() || fullText.length() != yCoords.size())
+                    for (auto it = fullText.begin(), end = fullText.end(); it != end;)
                     {
-                        jassertfalse;
-                        result.emplace_back (fullText, xCoords[0], yCoords[0]);
-                        return result;
+                        const auto pos = layoutState.popCoords();
+                        const auto next = layoutState.hasMoreCoords() ? it + 1 : end;
+                        result.emplace_back (String (it, next), pos.first, pos.second);
+                        it = next;
                     }
 
-                    for (int i = 0; i < xCoords.size(); ++i)
-                        result.emplace_back (fullText.substring (i, i + 1), xCoords[i], yCoords[i]);
-
                     return result;
                 }();
 
-                for (const auto& [text, x, y] : subtextElements)
+                for (const auto& [text, optX, optY] : subtextElements)
                 {
                     auto dt = new DrawableText();
                     dc->addAndMakeVisible (dt);
@@ -1123,6 +1191,9 @@ private:
                     dt->setColour (parseColour (xml, "fill", Colours::black)
                                        .withMultipliedAlpha (parseSafeFloat (getStyleAttribute (xml, "fill-opacity", "1"))));
 
+                    const auto x = optX.value_or (layoutState.getNextStartingPos().getX());
+                    const auto y = optY.value_or (layoutState.getNextStartingPos().getY());
+
                     Rectangle<float> bounds (x, y - font.getAscent(),
                                              font.getStringWidthFloat (text), font.getHeight());
 
@@ -1130,11 +1201,13 @@ private:
                     else if (anchorStr == "end") bounds.setX (bounds.getX() - bounds.getWidth());
 
                     dt->setBoundingBox (bounds);
+
+                    layoutState.setNextStartingPos ({ bounds.getRight(), y });
                 }
             }
             else if (e->hasTagNameIgnoringNamespace ("tspan"))
             {
-                dc->addAndMakeVisible (parseText (xml.getChild (e), true));
+                dc->addAndMakeVisible (parseText (xml.getChild (e), true, nullptr, &layoutState));
             }
         }
 
@@ -1263,7 +1336,9 @@ private:
     }
 
     //==============================================================================
-    bool parseCoord (String::CharPointerType& s, float& value, bool allowUnits, bool isX) const
+    enum class Axis { x, y };
+
+    bool parseCoord (String::CharPointerType& s, float& value, bool allowUnits, Axis axis) const
     {
         String number;
 
@@ -1273,14 +1348,14 @@ private:
             return false;
         }
 
-        value = getCoordLength (number, isX ? viewBoxW : viewBoxH);
+        value = getCoordLength (number, axis == Axis::x ? viewBoxW : viewBoxH);
         return true;
     }
 
     bool parseCoords (String::CharPointerType& s, Point<float>& p, bool allowUnits) const
     {
-        return parseCoord (s, p.x, allowUnits, true)
-            && parseCoord (s, p.y, allowUnits, false);
+        return parseCoord (s, p.x, allowUnits, Axis::x)
+            && parseCoord (s, p.y, allowUnits, Axis::y);
     }
 
     bool parseCoordsOrSkip (String::CharPointerType& s, Point<float>& p, bool allowUnits) const
@@ -1319,13 +1394,26 @@ private:
         return getCoordLength (xml->getStringAttribute (attName), sizeForProportions);
     }
 
-    void getCoordList (Array<float>& coords, const String& list, bool allowUnits, bool isX) const
+    Array<float> getCoordList (const XmlElement& xml, Axis axis) const
+    {
+        const String attributeName { axis == Axis::x ? "x" : "y" };
+
+        if (! xml.hasAttribute (attributeName))
+            return {};
+
+        return getCoordList (xml.getStringAttribute (attributeName), true, axis);
+    }
+
+    Array<float> getCoordList (const String& list, bool allowUnits, Axis axis) const
     {
         auto text = list.getCharPointer();
         float value;
+        Array<float> coords;
 
-        while (parseCoord (text, value, allowUnits, isX))
+        while (parseCoord (text, value, allowUnits, axis))
             coords.add (value);
+
+        return coords;
     }
 
     static float parseSafeFloat (const String& s)