Fix #1039 (<span foreground=> tags) (#1058)

* refactor svg def to store element references instead of mobjects

* Refactor svg defaults to parsing instead of cascading

* Break up style attribute before cascading

* add tag inheritance SVG test

* black styling

* MarkupText: Deprecate <color> tag and use <span foreground> instead

* Typo in docstring

* Update examples and docs for MarkupText

* Fix errors in MarkupText examples

* Fixed one more error in the docs

* Improved documentation for MarkupText

* underline_color is now supported
* overline and overline_color are now working
* strikethrough and strikethrough_color are now working
* additional examples for those features

* Fix for docs build

* Use existing logger.

* revisions from feedback set 1

* delete style from cascading attributes again (it is parsed but not cascaded)

* Reinterpret stroke=none as stroke-width=0 and opacity=1

* Update use tag test to handle two more cases

Co-authored-by: Philipp Imhof <[email protected]>

Author: markromanmiller

manim/mobject/svg/style_utils.py

@@ -9,15 +9,26 @@
 
 from typing import Dict, List
 
-SUPPORTED_STYLING_ATTRIBUTES: List[str] = [
+
+CASCADING_STYLING_ATTRIBUTES: List[str] = [
     "fill",
     "stroke",
-    "style",
     "fill-opacity",
     "stroke-opacity",
 ]
 
 
+# The default styling specifications for SVG images,
+# according to https://www.w3.org/TR/SVG/painting.html
+# (ctrl-F for "initial")
+SVG_DEFAULT_ATTRIBUTES: Dict[str, str] = {
+    "fill": "black",
+    "fill-opacity": "1",
+    "stroke": "none",
+    "stroke-opacity": "1",
+}
+
+
 def cascade_element_style(
     element: MinidomElement, inherited: Dict[str, str]
 ) -> Dict[str, str]:
@@ -46,11 +57,29 @@ def cascade_element_style(
 
     style = inherited.copy()
 
-    for attr in SUPPORTED_STYLING_ATTRIBUTES:
+    # cascade the regular elements.
+    for attr in CASCADING_STYLING_ATTRIBUTES:
         entry = element.getAttribute(attr)
         if entry:
             style[attr] = entry
 
+    # the style attribute should be handled separately in order to
+    # break it up nicely. furthermore, style takes priority over other
+    # attributes in the same element.
+    style_specs = element.getAttribute("style")
+    if style_specs:
+        for style_spec in style_specs.split(";"):
+            try:
+                key, value = style_spec.split(":")
+            except ValueError as e:
+                if not style_spec:
+                    # there was just a stray semicolon at the end, producing an emptystring
+                    pass
+                else:
+                    raise e
+            else:
+                style[key] = value
+
     return style
 
 
@@ -91,6 +120,26 @@ def parse_color_string(color_spec: str) -> str:
     return hex_color
 
 
+def fill_default_values(svg_style: Dict) -> None:
+    """
+    Fill in the default values for properties of SVG elements,
+    if they are not currently set in the style dictionary.
+
+    Parameters
+    ----------
+    svg_style : :class:`dict`
+        Style dictionary with SVG property names. Some may be missing.
+
+    Returns
+    -------
+    :class:`dict`
+        Style attributes; none are missing.
+    """
+    for key in SVG_DEFAULT_ATTRIBUTES:
+        if key not in svg_style:
+            svg_style[key] = SVG_DEFAULT_ATTRIBUTES[key]
+
+
 def parse_style(svg_style: Dict[str, str]) -> Dict:
     """Convert a dictionary of SVG attributes to Manim VMobject keyword arguments.
 
@@ -106,22 +155,7 @@ def parse_style(svg_style: Dict[str, str]) -> Dict:
     """
 
     manim_style = {}
-
-    # style attributes trump other element-level attributes,
-    # see https://www.w3.org/TR/SVG11/styling.html section 6.4, search "priority"
-    # so overwrite the other attribute dictionary values.
-    if "style" in svg_style:
-        for style_spec in svg_style["style"].split(";"):
-            try:
-                key, value = style_spec.split(":")
-            except ValueError as e:
-                if not style_spec:
-                    # there was just a stray semicolon at the end, producing an emptystring
-                    pass
-                else:
-                    raise e
-            else:
-                svg_style[key] = value
+    fill_default_values(svg_style)
 
     if "fill-opacity" in svg_style:
         manim_style["fill_opacity"] = float(svg_style["fill-opacity"])
@@ -138,7 +172,12 @@ def parse_style(svg_style: Dict[str, str]) -> Dict:
 
     if "stroke" in svg_style:
         if svg_style["stroke"] == "none":
-            manim_style["stroke_opacity"] = 0
+            # In order to not break animations.creation.Write,
+            # we interpret no stroke as stroke-width of zero and
+            # color the same as the fill color, if it exists.
+            manim_style["stroke_width"] = 0
+            if "fill_color" in manim_style:
+                manim_style["stroke_color"] = manim_style["fill_color"]
         else:
             manim_style["stroke_color"] = parse_color_string(svg_style["stroke"])
 

manim/mobject/svg/svg_mobject.py

@@ -77,7 +77,7 @@ def __init__(
         fill_opacity=1.0,
         **kwargs,
     ):
-        self.def_id_to_mobject = {}
+        self.def_map = {}
         self.file_name = file_name or self.file_name
         self.ensure_valid_file()
         self.should_center = should_center
@@ -125,17 +125,7 @@ def generate_points(self):
         """
         doc = minidom_parse(self.file_path)
         for svg in doc.getElementsByTagName("svg"):
-            mobjects = self.get_mobjects_from(
-                svg,
-                # these are the default styling specifications for SVG images,
-                # according to https://www.w3.org/TR/SVG/painting.html, ctrl-F for "initial"
-                {
-                    "fill": "black",
-                    "fill-opacity": "1",
-                    "stroke": "none",
-                    "stroke-opacity": "1",
-                },
-            )
+            mobjects = self.get_mobjects_from(svg, {})
             if self.unpack_groups:
                 self.add(*mobjects)
             else:
@@ -182,7 +172,9 @@ def get_mobjects_from(
         elif element.tagName in ["g", "svg", "symbol", "defs"]:
             result += it.chain(
                 *[
-                    self.get_mobjects_from(child, style, within_defs or is_defs)
+                    self.get_mobjects_from(
+                        child, style, within_defs=within_defs or is_defs
+                    )
                     for child in element.childNodes
                 ]
             )
@@ -191,8 +183,8 @@ def get_mobjects_from(
             if temp != "":
                 result.append(self.path_string_to_mobject(temp, style))
         elif element.tagName == "use":
-            # note, style is not passed down to "use" elements
-            result += self.use_to_mobjects(element)
+            # note, style is calcuated in a different way for `use` elements.
+            result += self.use_to_mobjects(element, style)
         elif element.tagName == "rect":
             result.append(self.rect_to_mobject(element, style))
         elif element.tagName == "circle":
@@ -210,7 +202,9 @@ def get_mobjects_from(
             result = [VGroup(*result)]
 
         if within_defs and element.hasAttribute("id"):
-            self.def_id_to_mobject[element.getAttribute("id")] = result
+            # it seems wasteful to throw away the actual element,
+            # but I'd like the parsing to be as similar as possible
+            self.def_map[element.getAttribute("id")] = (style, element)
         if is_defs:
             # defs shouldn't be part of the result tree, only the id dictionary.
             return []
@@ -235,7 +229,9 @@ def path_string_to_mobject(self, path_string: str, style: dict):
         """
         return SVGPathMobject(path_string, **parse_style(style))
 
-    def use_to_mobjects(self, use_element: MinidomElement) -> List[VMobject]:
+    def use_to_mobjects(
+        self, use_element: MinidomElement, local_style: Dict
+    ) -> List[VMobject]:
         """Converts a SVG <use> element to a collection of VMobjects.
 
         Parameters
@@ -244,22 +240,34 @@ def use_to_mobjects(self, use_element: MinidomElement) -> List[VMobject]:
             An SVG <use> element which represents nodes that should be
             duplicated elsewhere.
 
+        local_style : :class:`Dict`
+            The styling using SVG property names at the point the element is `<use>`d.
+            Not all values are applied; styles defined when the element is specified in
+            the `<def>` tag cannot be overriden here.
+
         Returns
         -------
         List[VMobject]
-            A collection of VMobjects that are copies of the defined objects
+            A collection of VMobjects that are a copy of the defined object
         """
 
         # Remove initial "#" character
         ref = use_element.getAttribute("xlink:href")[1:]
 
         try:
-            return [i.copy() for i in self.def_id_to_mobject[ref]]
+            def_style, def_element = self.def_map[ref]
         except KeyError:
             warning_text = f"{self.file_name} contains a reference to id #{ref}, which is not recognized"
             warnings.warn(warning_text)
             return []
 
+        # In short, the def-ed style overrides the new style,
+        # in cases when the def-ed styled is defined.
+        style = local_style.copy()
+        style.update(def_style)
+
+        return self.get_mobjects_from(def_element, style)
+
     def attribute_to_float(self, attr):
         """A helper method which converts the attribute to float.
 
@@ -385,20 +393,21 @@ def rect_to_mobject(self, rect_element: MinidomElement, style: dict):
 
         corner_radius = float(corner_radius)
 
+        parsed_style = parse_style(style)
+        parsed_style["stroke_width"] = stroke_width
+
         if corner_radius == 0:
             mob = Rectangle(
                 width=self.attribute_to_float(rect_element.getAttribute("width")),
                 height=self.attribute_to_float(rect_element.getAttribute("height")),
-                stroke_width=stroke_width,
-                **parse_style(style),
+                **parsed_style,
             )
         else:
             mob = RoundedRectangle(
                 width=self.attribute_to_float(rect_element.getAttribute("width")),
                 height=self.attribute_to_float(rect_element.getAttribute("height")),
-                stroke_width=stroke_width,
                 corner_radius=corner_radius,
-                **parse_style(style),
+                **parsed_style,
             )
 
         mob.shift(mob.get_center() - mob.get_corner(UP + LEFT))