invalid charts rendering when value is constant and yRangePad = 1 #781

Open
ktsaou opened this Issue Sep 29, 2016 · 12 comments

Projects

None yet

3 participants

@ktsaou
ktsaou commented Sep 29, 2016 edited

Hi,

Using the latest git version, when there is a chart with a constant non-zero value, the chart is not rendered properly.

The following images come from the same browser, same application, but different dygraph versions.

On the left the git version f6ec7be and on the right the latest release:

Note that the git version does not show the y-axis labels at all. It is like the chart is rendered below the visible area.

image

image

image

@danvk
Owner
danvk commented Sep 29, 2016

I merged in a major change yesterday which requires you to source a CSS file to get a proper rendering. Maybe you're running into that?
#678

@ktsaou
ktsaou commented Sep 29, 2016 edited

I am reading it now (I see I use the killed options). I'll use the new ones and come back.
Keep in mind I made the same test 2 weeks ago, and had the same result.

@danvk
Owner
danvk commented Sep 29, 2016

OK. If you do still see the issue, please provide a self-contained repro. There are many charts in tests/ which do not appear broken.

@ktsaou
ktsaou commented Sep 29, 2016

Yeap, I included dygraph.css, the problem remains. I will try to make a test for it.

@ktsaou
ktsaou commented Sep 29, 2016

Found it.

The problem is yRangePad: 1.
If I remove this, it works.

Here is a test:

<!DOCTYPE html>
<html>
  <head>
    <link rel="stylesheet" href="../css/dygraph.css">
    <title>dygraph</title>
    For production (minified) code, use:
    <script type="text/javascript" src="../dist/dygraph-combined.js"></script>
    <script type="text/javascript" src="../src/extras/smooth-plotter.js"></script>
    <!--
    <script type="text/javascript" src="../dist/dygraph.js"></script>
    -->

  </head>
  <body>
    <p>Minimal example of a dygraph chart:</p>
    <div id="graphdiv"></div>
    <script type="text/javascript">
      g = new Dygraph(document.getElementById("graphdiv"),
                      "Date,Temperature\n" +
                      "2008-05-07,10\n" +
                      "2008-05-08,10\n" +
                      "2008-05-09,10\n");
    </script>

    <p>Same data, specified in a parsed format:</p>
    <div id="graphdiv2"></div>
    <script type="text/javascript">
      g2 = new Dygraph(document.getElementById("graphdiv2"),
                       [ [ new Date("2008/05/07"), 10],
                         [ new Date("2008/05/08"), 10],
                         [ new Date("2008/05/08"), 10],
                         [ new Date("2008/05/10"), 10],
                         [ new Date("2008/05/11"), 10],
                         [ new Date("2008/05/12"), 10],
                         [ new Date("2008/05/13"), 10],
                         [ new Date("2008/05/14"), 10],
                         [ new Date("2008/05/15"), 10],
                         [ new Date("2008/05/16"), 10],
                         [ new Date("2008/05/17"), 10],
                         [ new Date("2008/05/18"), 10],
                         [ new Date("2008/05/19"), 10],
                         [ new Date("2008/05/20"), 10],
                         [ new Date("2008/05/21"), 10]
                       ],
                       {
                          rightGap: 5,
                          showRangeSelector: false,
                          showRoller: false,
                          title: 'hello world',
                          titleHeight: 19,
                          legend: 'always',
                          labelsSeparateLines: true,
                          labelsShowZeroValues: true,
                          labelsKMB: false,
                          labelsKMG2: false,
                          showLabelsOnHighlight: true,
                          hideOverlayOnMouseOut: true,
                          includeZero: false,
                          xRangePad: 0,
                          yRangePad: 1,
                          valueRange: null,
                          ylabel: 'percent',
                          labels: [ "Date", "Temperature" ]
                       });
    </script>
  </body>
</html>
@ktsaou
ktsaou commented Sep 29, 2016

image:
image

@ktsaou
ktsaou commented Sep 29, 2016

I used that option because on a few charts the minimum value was clipped by 1 pixel (I also use smooth plotter if that matters). With this option I prevented that tiny clipping.

@danvk
Owner
danvk commented Sep 29, 2016

Could you try to figure out which commit broke this? There have been very few in the last two weeks.

@ktsaou ktsaou changed the title from invalid charts rendering when value is constant to invalid charts rendering when value is constant and yRangePad = 1 Sep 29, 2016
@ktsaou
ktsaou commented Sep 29, 2016

I am not sure this happened in the last 2 weeks. The version I used that does not have this problem is the released one (v1.1.1). It might be something committed a year ago...

@danvk danvk added the bug label Sep 29, 2016
@danvk danvk added this to the 2.0.0 milestone Sep 29, 2016
@danvk
Owner
danvk commented Sep 29, 2016

I can confirm that this regressed from 1.1.1 to HEAD.

@danvk
Owner
danvk commented Sep 30, 2016

I ran git bisect with your example and found the culprit: #732

@klausw hope you're doing well! Any ideas why this broke? I'd like to fix this before cutting the 2.0.0 release candidate.

@klausw
Contributor
klausw commented Sep 30, 2016 edited

I think I can see the issue. The if (!axis.valueWindow && !ypadCompat) path doesn't currently have the special case for span == 0 that's used in non-padding mode, so the Y axis ends up with equal top and bottom values which doesn't render right.

I don't currently have time to do a proper patch, but roughly something like this should work:

 if (!axis.valueWindow && !ypadCompat) {
   // When using yRangePad, adjust the upper/lower bounds to add
   // padding unless the user has zoomed/panned the Y axis range.
   if (logscale) {
     y0 = axis.computedValueRange[0];
     y1 = axis.computedValueRange[1];
     var y0pct = ypad / (2 * ypad - 1);
     var y1pct = (ypad - 1) / (2 * ypad - 1);
     axis.computedValueRange[0] = utils.logRangeFraction(y0, y1, y0pct);
     axis.computedValueRange[1] = utils.logRangeFraction(y0, y1, y1pct);
   } else {
     y0 = axis.computedValueRange[0];
     y1 = axis.computedValueRange[1];
     span = y1 - y0;

.... add this
  // special case: if we have no sense of scale, center on the sole value.
   if (span === 0) {
     if (y1 !== 0) {
       span = Math.abs(y1);
     } else {
       // ... and if the sole value is zero, use range 0-1.
       y1 = 1;
       span = 1;
      }
    }
... to here

     axis.computedValueRange[0] = y0 - span * ypad;
     axis.computedValueRange[1] = y1 + span * ypad;
   }
 }

The logscale branch may need something similar, not sure what expected behavior is there.

@ktsaou ktsaou referenced this issue in firehol/netdata Oct 13, 2016
Open

Build packaging for debian #42

@danvk danvk modified the milestone: 2.0.0, 2.1.0 Dec 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment