Keep Y-axis label precision when gridstep is sub-integer (#1326)

When SI scaling produces a fractional gridstep (e.g. 0.1 G for values
around 10 G+) but MaxY >= 10, the classic gridder formatted labels with
%4.0f and rounded neighbouring labels (10.0, 10.2, 10.4 G ...) to the
same integer, producing duplicate Y-axis labels on tall graphs.

Extend the existing 'MaxY < 10' precision check to also trigger when
scaledstep * labfact < 1 (sub-integer label step) at all three label
formatting sites: no-SI, with-SI, and the second axis.

Also bump im->unitslength inside calc_horizontal_grid for the classic
branch, mirroring the existing ALTYGRID code path, so the label column
is sized appropriately when the call order is eventually reworked.

Adds tests/graph4 as a regression test that verifies Y-axis labels are
unique via pdftotext when pdftotext is available.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

diff --git a/CHANGES b/CHANGES
index 7c64407421e7e1abfa0a2a177a1cd553071fcfed..c4c2e06075fc0ea704bebd2e96054a9f3951663f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -3,6 +3,10 @@ RRDtool - master ...
 ====================
 Bugfixes
 --------
+* Y-axis labels keep enough fractional precision when the gridstep is
+  fractional after SI scaling (e.g. values around 10G+), avoiding
+  duplicate labels on tall graphs. Issue #1326 reported by @muusik,
+  fixed by @oetiker
 
 Features
 --------

diff --git a/src/rrd_graph.c b/src/rrd_graph.c
index 951e0a589358bd2422ce8280738403979efc904c..a3bdad58f0dc9cb698e5864932e97c954c3088ea 100644 (file)
--- a/src/rrd_graph.c
+++ b/src/rrd_graph.c
@@ -2412,6 +2412,12 @@ int calc_horizontal_grid(
                          (im->symbol != ' ' ? " %c" : ""));
             }
         } else {        /* classic rrd grid */
+            double    grid_scaled;
+            double    label_step;
+            double    maxabs;
+            int       int_digits;
+            int       needed;
+
             for (i = 0; ylab[i].grid > 0; i++) {
                 pixel = im->ysize / (scaledrange / ylab[i].grid);
                 gridind = i;
@@ -2428,6 +2434,35 @@ int calc_horizontal_grid(
             }
 
             im->ygrid_scale.gridstep = ylab[gridind].grid * im->magfact;
+
+            /* If the label step is sub-integer, draw_horizontal_grid will
+             * format labels with one fractional digit (%4.1f) to keep
+             * neighbouring labels distinct (issue #1326). Make sure the
+             * vertical label column is wide enough to hold them. */
+            grid_scaled = ylab[gridind].grid * im->viewfactor;
+            label_step = grid_scaled * (double) im->ygrid_scale.labfact;
+            maxabs = max(fabs(im->maxval), fabs(im->minval))
+                     * im->viewfactor / im->magfact;
+            if (maxabs < 10) {
+                int_digits = 1;
+            } else {
+                int_digits = (int) ceil(log10(maxabs + 0.5));
+            }
+            if (label_step < 1 || maxabs < 10) {
+                /* digits + '.' + 1 fractional digit */
+                needed = int_digits + 2;
+            } else {
+                needed = int_digits;
+            }
+            if (im->minval < 0) {
+                needed += 1;       /* sign */
+            }
+            if (im->symbol != ' ') {
+                needed += 2;       /* " <SI>" suffix */
+            }
+            if (im->unitslength < needed) {
+                im->unitslength = needed;
+            }
         }
     } else {
         im->ygrid_scale.gridstep = im->ygridstep;
@@ -2480,7 +2515,9 @@ int draw_horizontal_grid(
                                          im->ygrid_scale.labfmt,
                                          scaledstep * (double) i);
                             } else {
-                                if (MaxY < 10) {
+                                if (MaxY < 10
+                                    || scaledstep *
+                                       (double) im->ygrid_scale.labfact < 1) {
                                     snprintf(graph_label, sizeof graph_label,
                                              "%4.1f",
                                              scaledstep * (double) i);
@@ -2505,7 +2542,9 @@ int draw_horizontal_grid(
                                          im->ygrid_scale.labfmt,
                                          scaledstep * (double) i, sisym);
                             } else {
-                                if (MaxY < 10) {
+                                if (MaxY < 10
+                                    || scaledstep *
+                                       (double) im->ygrid_scale.labfact < 1) {
                                     snprintf(graph_label, sizeof graph_label,
                                              "%4.1f %c",
                                              scaledstep * (double) i, sisym);
@@ -2586,14 +2625,24 @@ int draw_horizontal_grid(
                             }
                             sval /= second_axis_magfact;
 
-                            if (MaxY < 10) {
-                                snprintf(graph_label_right,
-                                         sizeof graph_label_right, "%5.1f %s",
-                                         sval, second_axis_symb);
-                            } else {
-                                snprintf(graph_label_right,
-                                         sizeof graph_label_right, "%5.0f %s",
-                                         sval, second_axis_symb);
+                            {
+                                double    right_step =
+                                    im->ygrid_scale.gridstep *
+                                    im->second_axis_scale /
+                                    second_axis_magfact *
+                                    (double) im->ygrid_scale.labfact;
+
+                                if (MaxY < 10 || fabs(right_step) < 1) {
+                                    snprintf(graph_label_right,
+                                             sizeof graph_label_right,
+                                             "%5.1f %s", sval,
+                                             second_axis_symb);
+                                } else {
+                                    snprintf(graph_label_right,
+                                             sizeof graph_label_right,
+                                             "%5.0f %s", sval,
+                                             second_axis_symb);
+                                }
                             }
                         } else {
                             snprintf(graph_label_right,

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 55a6bb267c9a1aca912b517cf8ae1764db1a710e..796c1e5fc18c2e96b8ec33e5d9eeb6b273fa940f 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -6,7 +6,7 @@ TESTS = modify1 modify2 modify3 modify4 modify5 \
        create-with-source-1 create-with-source-2 create-with-source-3 \
        create-with-source-4 create-with-source-and-mapping-1 \
        create-from-template-1 dcounter1 vformatter1 xport1 xport2 xport3 list1 \
-       pdp-calc1 graph3
+       pdp-calc1 graph3 graph4
 
 EXTRA_DIST = Makefile.am \
        functions $(TESTS) \
@@ -19,7 +19,7 @@ EXTRA_DIST = Makefile.am \
        tune1-testa-mod1.dump tune1-testa-mod2.dump tune1-testorg.dump \
        tune2-testa-mod1.dump tune2-testorg.dump \
        valgrind-supressions dcounter1 dcounter1.output graph1.output graph2.output vformatter1 rpn1.output rpn2.output \
-       xport1.json.output xport1.xml.output xport2 xport3 graph3 \
+       xport1.json.output xport1.xml.output xport2 xport3 graph3 graph4 \
        pdp-calc1 pdp-calc1-1-avg-60.output pdp-calc1-1-avg-300.output pdp-calc1-1-max-300.output
 
 # NB: AM_TESTS_ENVIRONMENT not available until automake 1.12
@@ -30,6 +30,7 @@ AM_TESTS_ENVIRONMENT = \
 
 CLEANFILES = *.rrd \
        ct.out dur.out graph1.output.out graph2.output.out \
+       graph4.pdf \
        modify5-testa1-mod.dump modify5-testa2-mod.dump \
        modify5-testa1-mod.dump.tmp modify5-testa2-mod.dump.tmp \
        rpn1.out rpn1.output.out

diff --git a/tests/graph4 b/tests/graph4
new file mode 100755 (executable)
index 0000000..b9d4c8c
--- /dev/null
+++ b/tests/graph4
@@ -0,0 +1,51 @@
+#!/bin/bash
+
+. $(dirname $0)/functions
+
+if [ -n "$MSYSTEM" ]; then
+    BUILD=graph4
+else
+    BUILD=$BUILDDIR/graph4
+fi
+
+# Issue #1326: With Y-axis values in the 10G+ range and a tall graph,
+# the gridstep becomes fractional (e.g. 0.1 G) after SI scaling, but the
+# legacy formatter chose %4.0f whenever MaxY >= 10, rounding 10.2, 10.4,
+# 10.6 all to "10" and producing duplicate Y-axis labels.
+
+$RRDTOOL create ${BUILD}.rrd \
+    --start 1748087640 --step 60 \
+    DS:value:GAUGE:120:U:U \
+    RRA:AVERAGE:0.5:1:60
+report "create"
+
+is_cached && exit 0
+
+# Populate with 10.1G .. 11.5G in 0.1G increments
+for i in $(seq 1 15); do
+    t=$((1748087640 + i*60))
+    v=$(awk -v i="$i" 'BEGIN { printf "%.0f", 10e9 + i*1e8 }')
+    $RRDTOOL update ${BUILD}.rrd ${t}:${v}
+done
+report "update"
+
+# Generate a tall graph so the autoscaler picks a small gridstep
+$RRDTOOL graph ${BUILD}.pdf --imgformat PDF \
+    --start 1748087700 --end 1748088600 --height 1000 \
+    DEF:v=${BUILD}.rrd:value:AVERAGE \
+    LINE1:v#000000 > /dev/null
+report "graph"
+
+# If pdftotext is available, verify Y-axis labels are unique
+if command -v pdftotext > /dev/null; then
+    LABELS=$(pdftotext ${BUILD}.pdf - | grep -E '^[0-9]+\.[0-9]+ G$')
+    UNIQUE=$(echo "$LABELS" | sort -u | wc -l)
+    TOTAL=$(echo "$LABELS" | wc -l)
+    if [ "$UNIQUE" = "$TOTAL" ] && [ "$TOTAL" -ge 10 ]; then
+        ok "Y-axis labels are unique ($TOTAL distinct labels)"
+    else
+        fail 1 "Y-axis labels are duplicated (got $UNIQUE distinct out of $TOTAL)"
+    fi
+else
+    ok "pdftotext not available, skipping label uniqueness check"
+fi