From: Magnus Jacobsson <Magnus.Jacobsson@berotec.se>
Date: Wed, 10 Aug 2022 14:40:44 +0000 (+0200)
Subject: tests: SVGAnalyzer: add handling of the 'fill-opacity' attribute
X-Git-Tag: 6.0.1~10^2~3
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fefd86375cc60d7a78011bdb053ea81599ab7660;p=graphviz

tests: SVGAnalyzer: add handling of the 'fill-opacity' attribute
---

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 9e2915113..9d1b82692 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -94,4 +94,5 @@ CREATE_TEST(subgraph_layout)
 CREATE_TEST(subgraphs)
 CREATE_TEST(svg_analyzer)
 CREATE_TEST(svg_analyzer_color)
+CREATE_TEST(svg_analyzer_fillcolor)
 CREATE_TEST(svg_analyzer_penwidth)
diff --git a/tests/svg_analyzer.cpp b/tests/svg_analyzer.cpp
index f46d96be2..f6f2df93f 100644
--- a/tests/svg_analyzer.cpp
+++ b/tests/svg_analyzer.cpp
@@ -219,6 +219,10 @@ void SVGAnalyzer::set_fill(std::string_view fill) {
   current_element().attributes.fill = fill;
 }
 
+void SVGAnalyzer::set_fill_opacity(double fill_opacity) {
+  current_element().attributes.fill_opacity = fill_opacity;
+}
+
 void SVGAnalyzer::set_height(double height) {
   current_element().attributes.height = height;
 }
diff --git a/tests/svg_analyzer.h b/tests/svg_analyzer.h
index db0c0fb9c..e2cd2e3f1 100644
--- a/tests/svg_analyzer.h
+++ b/tests/svg_analyzer.h
@@ -40,6 +40,7 @@ public:
   void set_font_family(std::string_view font_family) override;
   void set_font_size(double font_size) override;
   void set_fill(std::string_view fill) override;
+  void set_fill_opacity(double fill_opacity) override;
   void set_height(double height) override;
   void set_id(std::string_view id) override;
   void set_rx(double rx) override;
diff --git a/tests/svg_analyzer_interface.h b/tests/svg_analyzer_interface.h
index f2b44e3d4..2633a367c 100644
--- a/tests/svg_analyzer_interface.h
+++ b/tests/svg_analyzer_interface.h
@@ -36,6 +36,7 @@ public:
   virtual void set_font_family(std::string_view font_family) = 0;
   virtual void set_font_size(double font_size) = 0;
   virtual void set_fill(std::string_view fill) = 0;
+  virtual void set_fill_opacity(double fill_opacity) = 0;
   virtual void set_height(double height) = 0;
   virtual void set_id(std::string_view id) = 0;
   virtual void set_rx(double rx) = 0;
diff --git a/tests/svg_element.cpp b/tests/svg_element.cpp
index 76a5b9ce3..b00bcf016 100644
--- a/tests/svg_element.cpp
+++ b/tests/svg_element.cpp
@@ -221,6 +221,21 @@ std::string SVG::SVGElement::id_attribute_to_string() const {
   return fmt::format(R"(id="{}")", attributes.id);
 }
 
+std::string SVG::SVGElement::fill_opacity_attribute_to_string() const {
+  if (attributes.fill_opacity == 1) {
+    // Graphviz doesn't set `fill-opacity` to 1 since that's the default
+    return "";
+  }
+
+  if (attributes.fill_opacity == 0) {
+    // Graphviz doesn't set `fill-opacity` to 0 since in that case it sets
+    // `fill` to "none" instead
+    return "";
+  }
+
+  return fmt::format(R"(fill-opacity="{}")", attributes.fill_opacity);
+}
+
 std::string SVG::SVGElement::points_attribute_to_string() const {
   std::string points_attribute_str = R"|(points=")|";
   const char *separator = "";
@@ -305,6 +320,7 @@ void SVG::SVGElement::to_string_impl(std::string &output,
   switch (type) {
   case SVG::SVGElementType::Ellipse:
     append_attribute(attributes_str, fill_attribute_to_string());
+    append_attribute(attributes_str, fill_opacity_attribute_to_string());
     append_attribute(attributes_str, stroke_attribute_to_string());
     append_attribute(attributes_str, stroke_width_attribute_to_string());
     append_attribute(attributes_str, stroke_opacity_attribute_to_string());
@@ -323,6 +339,7 @@ void SVG::SVGElement::to_string_impl(std::string &output,
     break;
   case SVG::SVGElementType::Path: {
     append_attribute(attributes_str, fill_attribute_to_string());
+    append_attribute(attributes_str, fill_opacity_attribute_to_string());
     append_attribute(attributes_str, stroke_attribute_to_string());
     append_attribute(attributes_str, stroke_width_attribute_to_string());
     append_attribute(attributes_str, stroke_opacity_attribute_to_string());
@@ -348,6 +365,7 @@ void SVG::SVGElement::to_string_impl(std::string &output,
   }
   case SVG::SVGElementType::Polygon:
     append_attribute(attributes_str, fill_attribute_to_string());
+    append_attribute(attributes_str, fill_opacity_attribute_to_string());
     append_attribute(attributes_str, stroke_attribute_to_string());
     append_attribute(attributes_str, stroke_width_attribute_to_string());
     append_attribute(attributes_str, stroke_opacity_attribute_to_string());
diff --git a/tests/svg_element.h b/tests/svg_element.h
index 5dd4918cf..da557aa13 100644
--- a/tests/svg_element.h
+++ b/tests/svg_element.h
@@ -60,6 +60,7 @@ struct SVGAttributes {
   double cx;
   double cy;
   std::string fill;
+  double fill_opacity = 1;
   std::string font_family;
   double font_size;
   double height;
@@ -162,6 +163,7 @@ private:
                         const std::string &attribute) const;
   std::string id_attribute_to_string() const;
   std::string fill_attribute_to_string() const;
+  std::string fill_opacity_attribute_to_string() const;
   std::string points_attribute_to_string() const;
   std::string stroke_attribute_to_string() const;
   std::string stroke_opacity_attribute_to_string() const;
diff --git a/tests/svgpp_context.cpp b/tests/svgpp_context.cpp
index de7b397e5..003f5916b 100644
--- a/tests/svgpp_context.cpp
+++ b/tests/svgpp_context.cpp
@@ -126,6 +126,10 @@ void SvgppContext::set(svgpp::tag::attribute::fill, color_t color,
   m_svg_analyzer->set_fill(to_color_string(color));
 }
 
+void SvgppContext::set(svgpp::tag::attribute::fill_opacity, const double v) {
+  m_svg_analyzer->set_fill_opacity(v);
+}
+
 void SvgppContext::set(svgpp::tag::attribute::stroke, svgpp::tag::value::none) {
   m_svg_analyzer->set_stroke("none");
 }
diff --git a/tests/svgpp_context.h b/tests/svgpp_context.h
index ab346caa3..250cc43d0 100644
--- a/tests/svgpp_context.h
+++ b/tests/svgpp_context.h
@@ -99,6 +99,8 @@ public:
     throw std::runtime_error{
         "this flavor of the 'fill' attribute is not yet implemented"};
   };
+
+  void set(svgpp::tag::attribute::fill_opacity, double v);
   void set(svgpp::tag::attribute::stroke, svgpp::tag::value::none);
   void set(svgpp::tag::attribute::stroke, svgpp::tag::value::currentColor);
   void set(svgpp::tag::attribute::stroke, color_t color,
diff --git a/tests/svgpp_document_traverser.cpp b/tests/svgpp_document_traverser.cpp
index 1d386638c..9d64213ac 100644
--- a/tests/svgpp_document_traverser.cpp
+++ b/tests/svgpp_document_traverser.cpp
@@ -35,6 +35,7 @@ void traverseDocumentWithSvgpp(SvgppContext &context, char *text) {
                         svgpp::tag::attribute::cy,             //
                         svgpp::tag::attribute::d,              //
                         svgpp::tag::attribute::fill,           //
+                        svgpp::tag::attribute::fill_opacity,   //
                         svgpp::tag::attribute::font_family,    //
                         svgpp::tag::attribute::font_size,      //
                         svgpp::tag::attribute::height,         //
diff --git a/tests/test_svg_analyzer_color.cpp b/tests/test_svg_analyzer_color.cpp
index b4bce838f..685e600ab 100644
--- a/tests/test_svg_analyzer_color.cpp
+++ b/tests/test_svg_analyzer_color.cpp
@@ -19,26 +19,8 @@ TEST_CASE("SvgAnalyzer color",
   INFO(fmt::format("Color: {}", color));
   const auto color_attr = color.empty() ? "" : fmt::format(" color={}", color);
 
-  // FIXME: Edge arrowheads use `color` also for fill when `fillcolor` is not
-  // set. This can result in `fill-opacity' being set which we do not yet
-  // support. We therefore temporarily avoid specifying an edge color when
-  // opacity is not 0 or 100 %.
-  const auto edge_color_attr =
-      !color.ends_with("00\"") && !color.ends_with("ff\"")
-          ? ""
-          : fmt::format(" color={}", color);
-
   auto dot = fmt::format("digraph g1 {{node [shape={}{}]; edge [{}]; a -> b}}",
-                         shape, color_attr, edge_color_attr);
+                         shape, color_attr, color_attr);
 
-  if (shape == "point" && !color.ends_with("00\"") &&
-      !color.ends_with("ff\"")) {
-    // FIXME: The `point` shape implicitly uses style="filled" and this in turn
-    // causes `color` to be used for fill when `fillcolor` is not set.
-    // This can result in `fill-opacity' being set which we do not yet
-    // support. We therefore avoid checking the SVG for the `point` shape when
-    // the opacity is not 0 or 100 %.
-  } else {
-    SVGAnalyzer::make_from_dot(dot).re_create_and_verify_svg();
-  }
+  SVGAnalyzer::make_from_dot(dot).re_create_and_verify_svg();
 }
diff --git a/tests/test_svg_analyzer_fillcolor.cpp b/tests/test_svg_analyzer_fillcolor.cpp
new file mode 100644
index 000000000..3f31f3bc5
--- /dev/null
+++ b/tests/test_svg_analyzer_fillcolor.cpp
@@ -0,0 +1,31 @@
+#include <string_view>
+
+#include <catch2/catch.hpp>
+#include <fmt/format.h>
+
+#include "svg_analyzer.h"
+#include "test_utilities.h"
+
+TEST_CASE("SvgAnalyzer fillcolor",
+          "Test that the SvgAnalyzer can recreate the original SVG with the "
+          "correct `fill` and `fill-opacity` attributes when the Graphviz "
+          "`fillcolor` attribute is used for nodes and edges ") {
+
+  const auto shape = GENERATE(from_range(all_node_shapes));
+  INFO(fmt::format("Shape: {}", shape));
+
+  const std::string_view fillcolor =
+      GENERATE("", "\"#10204000\"", "\"#10204080\"", "\"#102040ff\"");
+  INFO(fmt::format("Fillcolor: {}", fillcolor));
+  const auto fillcolor_attr =
+      fillcolor.empty() ? "" : fmt::format(" fillcolor={}", fillcolor);
+  const std::string_view node_style = fillcolor.empty() ? "" : "filled";
+  const auto node_style_attr =
+      node_style.empty() ? "" : fmt::format(" style={}", node_style);
+
+  auto dot =
+      fmt::format("digraph g1 {{node [shape={}{}{}]; edge [{}]; a -> b}}",
+                  shape, node_style_attr, fillcolor_attr, fillcolor_attr);
+
+  SVGAnalyzer::make_from_dot(dot).re_create_and_verify_svg();
+}