arrows: arrow_length_normal: take edge penwidth into account when calculating length

This causes the edge stem to be clipped at the visual outline of the
edge arrow instead of at its "ideal" bounding box which fixes the too
large overlap between them that was introduced by eariler commits in
this series.

The test_max_edge_stem_arrow_overlap_simple.cpp,
test_edge_node_overlap_normal_and_inv_edge_arrows
test_edge_node_overlap_simple and test_edge_node_overlap_node_shapes
test cases now succeed, so the expectancy that they should fail is
removed.

Towards https://gitlab.com/graphviz/graphviz/-/issues/372.

diff --git a/lib/common/arrows.c b/lib/common/arrows.c
index f033255762f4ab18f39f5d467898db67f9bfa1d9..979d0ad76c8182c48dea915f62bae57b01a42801 100644 (file)
--- a/lib/common/arrows.c
+++ b/lib/common/arrows.c
@@ -9,6 +9,7 @@
  *************************************************************************/
 
 
+#include <assert.h>
 #include <common/render.h>
 #include <math.h>
 #include <stdbool.h>
@@ -122,9 +123,10 @@ static pointf arrow_type_curve(GVJ_t * job, pointf p, pointf u, double arrowsize
 static pointf arrow_type_gap(GVJ_t * job, pointf p, pointf u, double arrowsize, double penwidth, int flag);
 
 static double arrow_length_generic(double lenfact, double arrowsize, double penwidth, int flag);
+static double arrow_length_normal(double lenfact, double arrowsize, double penwidth, int flag);
 
 static const arrowtype_t Arrowtypes[] = {
-    {ARR_TYPE_NORM, 1.0, arrow_type_normal, arrow_length_generic},
+    {ARR_TYPE_NORM, 1.0, arrow_type_normal, arrow_length_normal},
     {ARR_TYPE_CROW, 1.0, arrow_type_crow, arrow_length_generic},
     {ARR_TYPE_TEE, 0.5, arrow_type_tee, arrow_length_generic},
     {ARR_TYPE_BOX, 1.0, arrow_type_box, arrow_length_generic},
@@ -903,3 +905,43 @@ static double arrow_length_generic(double lenfact, double arrowsize,
 
   return lenfact * arrowsize * ARROW_LENGTH;
 }
+
+static double arrow_length_normal(double lenfact, double arrowsize,
+                                  double penwidth, int flag) {
+  pointf a[5];
+  // set arrow end point at origin
+  const pointf p = {0, 0};
+  // generate an arrowhead vector along x-axis
+  const pointf u = {lenfact * arrowsize * ARROW_LENGTH, 0};
+
+  // arrow start point
+  pointf q = arrow_type_normal0(p, u, penwidth, flag, a);
+
+  const pointf base1 = a[1];
+  const pointf base2 = a[3];
+  const pointf tip = a[2];
+  const double full_length = q.x;
+  assert(full_length > 0 && "non-positive full length");
+  const double nominal_length = fabs(base1.x - tip.x);
+  const double nominal_base_width = base2.y - base1.y;
+  assert(nominal_base_width > 0 && "non-positive nominal base width");
+  // the full base width is proportionally scaled with the length
+  const double full_base_width =
+      nominal_base_width * full_length / nominal_length;
+  assert(full_base_width > 0 && "non-positive full base width");
+
+  // we want a small overlap between the edge path (stem) and the arrow to avoid
+  // gaps beetween them in case the arrow has a sharp corner towards the edge
+  // path
+  const double overlap_at_base = penwidth / 2;
+  // overlap the tip to a point where its width is equal to the penwidth.
+  const double length_where_width_is_penwidth =
+      full_length * penwidth / full_base_width;
+  const double overlap_at_tip = length_where_width_is_penwidth;
+
+  const double overlap = flag & ARR_MOD_INV ? overlap_at_tip : overlap_at_base;
+
+  // arrow length is the x value of the start point since the arrow points along
+  // the positive x axis and ends at origin
+  return full_length - overlap;
+}

diff --git a/tests/test_edge_node_overlap_normal_and_inv_edge_arrows.cpp b/tests/test_edge_node_overlap_normal_and_inv_edge_arrows.cpp
index e4e91099753b8fbd40f763c5fd105555e7926ef4..6a72b52814c41c16bd6936cf55b815a51c5a2357 100644 (file)
--- a/tests/test_edge_node_overlap_normal_and_inv_edge_arrows.cpp
+++ b/tests/test_edge_node_overlap_normal_and_inv_edge_arrows.cpp
@@ -5,10 +5,10 @@
 #include "test_edge_node_overlap_utilities.h"
 #include "test_utilities.h"
 
-TEST_CASE("Edge node overlap for normal and inv arrow",
-          "[!shouldfail] An edge connected to a node shall touch that node and "
-          "not overlap it too much, regardless of if the arrow shape is "
-          "'normal' or 'inv'") {
+TEST_CASE(
+    "Edge node overlap for normal and inv arrow",
+    "An edge connected to a node shall touch that node and not overlap it too "
+    "much, regardless of if the arrow shape is 'normal' or 'inv'") {
 
   std::string filename_base = AUTO_NAME();
 

diff --git a/tests/test_edge_node_overlap_polygon_node_shapes.cpp b/tests/test_edge_node_overlap_polygon_node_shapes.cpp
index bfdcae46e229a81e8e5f74158568b988d4133a30..0fabd5dce1a0a078aebad72c6207aa1bf8e207d0 100644 (file)
--- a/tests/test_edge_node_overlap_polygon_node_shapes.cpp
+++ b/tests/test_edge_node_overlap_polygon_node_shapes.cpp
@@ -6,9 +6,8 @@
 
 TEST_CASE(
     "Overlap polygon node shapes",
-    "[!shouldfail] Test that an edge connected to a polygon based node touches "
-    "that node and does not overlap it too much, regardless of the node "
-    "shape") {
+    "Test that an edge connected to a polygon based node touches that node and "
+    "does not overlap it too much, regardless of the node shape") {
 
   const auto shape = GENERATE(from_range(node_shapes_consisting_of_polygon));
   INFO("Node shape: " << shape);

diff --git a/tests/test_edge_node_overlap_simple.cpp b/tests/test_edge_node_overlap_simple.cpp
index 1989d98ed6e2661dc5f446dd11e7acfadacf30b4..ac655d7fda8eba8c8a5e55b6ed52c2b073098488 100644 (file)
--- a/tests/test_edge_node_overlap_simple.cpp
+++ b/tests/test_edge_node_overlap_simple.cpp
@@ -5,9 +5,8 @@
 #include "test_edge_node_overlap_utilities.h"
 #include "test_utilities.h"
 
-TEST_CASE("Edge node overlap",
-          "[!shouldfail] An edge connected to a node shall touch that node and "
-          "not overlap it too much") {
+TEST_CASE("Edge node overlap", "An edge connected to a node shall touch that "
+                               "node and not overlap it too much") {
 
   const graph_options graph_options = {
       .node_shape = "polygon",

diff --git a/tests/test_max_edge_stem_arrow_overlap_simple.cpp b/tests/test_max_edge_stem_arrow_overlap_simple.cpp
index f6c0da9d9a26c2461ebb3221c61f8e09b4d38283..d080e63836d26c8195032e1a1f309b2385cc50eb 100644 (file)
--- a/tests/test_max_edge_stem_arrow_overlap_simple.cpp
+++ b/tests/test_max_edge_stem_arrow_overlap_simple.cpp
@@ -6,8 +6,7 @@
 #include "test_utilities.h"
 
 TEST_CASE("Maximum edge stem and arrow overlap",
-          "[!shouldfail] Test that an edge stem doesn't overlap its arrow "
-          "heads too much") {
+          "Test that an edge stem doesn't overlap its arrow heads too much") {
 
   const graph_options graph_options = {
       .node_shape = "polygon",