novus / nvd3

A reusable charting library written in d3.js
http://nvd3.org/
Other
7.22k stars 2.15k forks source link

Charts freeze and throw voronoi error if two series have the same point #873

Closed pcwiek closed 9 years ago

pcwiek commented 9 years ago

Hi there,

I have 3 charts with date series (x-axis) and numbers on y-axis, line chart, stacked area chart and bar chart. Each one of the series in any given chart has 240 points, 4 series on a chart.

In 1.1.15 everything is snappy and works really great on IE, Chrome and FF. My d3.js version is 3.5.5

The only error I'm getting in the console is this, when any and all of the charts are being rendered:

error in d3.js line 5232, col 25:
function d3_geom_voronoiHalfEdge(edge, lSite, rSite) {
    var va = edge.a, vb = edge.b;
    this.edge = edge;
    this.site = lSite;
    this.angle = rSite ? Math.atan2(rSite.y - lSite.y, rSite.x - lSite.x) : edge.l === lSite ? Math.atan2(vb.x - va.x, va.y - vb.y) : Math.atan2(va.x - vb.x, vb.y - va.y);
        vb is null ------^          
  }

But even then everything still works.

After the update nvd3 to 1.7.1, FF freezes until I kill d3.js, same with IE, Chrome just freezes and I can only kill the whole tab.

Firefox error:

A script on this page may be busy, or it may have stopped responding. You can stop the script now, open the script in the debugger, or let the script continue.

Script: http://localhost:55253/Scripts/d3/d3.js:5040 (or :5041)

Console says:

Error: Script terminated by timeout at:
d3_geom_voronoiCloseCells@http://localhost:55253/Scripts/d3/d3.js:5040:1
d3_geom_voronoi@http://localhost:55253/Scripts/d3/d3.js:5459:47
voronoi@http://localhost:55253/Scripts/d3/d3.js:5475:7
d3.geom.voronoi@http://localhost:55253/Scripts/d3/d3.js:5472:1
updateInteractiveLayer@http://localhost:55253/Scripts/nv.d3.js:9428:35

which points to:

        function d3_geom_voronoiCloseCells(extent) {
            var x0 = extent[0][0], x1 = extent[1][0], y0 = extent[0][1], y1 = extent[1][1], x2, y2, x3, y3, cells = d3_geom_voronoiCells, iCell = cells.length, cell, iHalfEdge, halfEdges, nHalfEdges, start, end;
            while (iCell--) {
              cell = cells[iCell];
              if (!cell || !cell.prepare()) continue;
              halfEdges = cell.edges;
              nHalfEdges = halfEdges.length;
              iHalfEdge = 0;
 line 5040 -> while (iHalfEdge < nHalfEdges) {
                end = halfEdges[iHalfEdge].end(), x3 = end.x, y3 = end.y;
                start = halfEdges[++iHalfEdge % nHalfEdges].start(), x2 = start.x, y2 = start.y;
                if (abs(x3 - x2) > ε || abs(y3 - y2) > ε) {
                  halfEdges.splice(iHalfEdge, 0, new d3_geom_voronoiHalfEdge(d3_geom_voronoiCreateBorderEdge(cell.site, end, abs(x3 - x0) < ε && y1 - y3 > ε ? {
                    x: x0,
                    y: abs(x2 - x0) < ε ? y2 : y1
                  } : abs(y3 - y1) < ε && x1 - x3 > ε ? {
                    x: abs(y2 - y1) < ε ? x2 : x1,
                    y: y1
                  } : abs(x3 - x1) < ε && y3 - y0 > ε ? {
                    x: x1,
                    y: abs(x2 - x1) < ε ? y2 : y0
                  } : abs(y3 - y0) < ε && x3 - x0 > ε ? {
                    x: abs(y2 - y0) < ε ? x2 : x0,
                    y: y0
                  } : null), cell.site, null));
                  ++nHalfEdges;
                }
             }
           }
         }

If I kill the script in FF/IE, I can actually see the charts. No luck with Chrome.

Another peculiar thing is memory used by the browser, only one tab (with the charts) open:

Browser 1.1.15 1.7.1
Firefox 285 MB 3.2 GB (!!)
Chrome 151 MB 900 MB +
IE 293 MB ~400 MB

For 1.1.15 memory pressure is stable and low.

Story is a bit different for 1.7.1: Chrome was still growing when I killed the whole tab. Firefox reaches 3.2 GB quickly and stays there. IE stays around ~400 MB (going up and down) until I kill the script.

Is there anything here that stands out for you guys? For now I'll just stay on the older version of nvd3 to keep the site usable :)

liquidpele commented 9 years ago

Hi there, I certainly haven't seen anything like this... Can you provide a jsfiddle that shows the issue using the latest nvd3 version?

pcwiek commented 9 years ago

I'll certainly try.

Maybe it's something 'local', but it's still very weird that previous version works without any issues and current one seems to be throwing a fit.

I'll work on a fiddle and see if I can reproduce it.

pcwiek commented 9 years ago

I narrowed it down to lineWithFocusChart. If I remove it from the page, everything works.

I kept digging through old issues and adding one line to lineWithFocusChart configuration also seems to make the issue go away:

chart.useVoronoi(false);

Then everything renders properly and there are no errors whatsoever in the console or otherwise.

Although, admittedly, overall performance (at least under FF) is better if I remove focusable line chart altogether from the page.

liquidpele commented 9 years ago

Now that is weird... there were issues with the old version and useVoronoi where chrome and IE would have performance issues, but I reworked that code based on http://bl.ocks.org/njvack/1405439 and I haven't seen any performance issues since then.

Is there any way you can provide the problem chart online where I an look at and debug it?

Are there errors in the js console?

Do the voronoi regions look okay if you do chart.showVoronoi(true) ?

pcwiek commented 9 years ago

I'll try to get a fiddle together, I can't promise it thought because of my time constraints and deadlines :)

Anyway, if I enable voronoi and kill d3.js when it hangs under FF, next render shows the chart, but the console says:

TypeError: vb is null

function d3_geom_voronoiHalfEdge(edge, lSite, rSite) {
    var va = edge.a, vb = edge.b;
    this.edge = edge;
    this.site = lSite;
    this.angle = rSite ? Math.atan2(rSite.y - lSite.y, rSite.x - lSite.x) : edge.l === lSite ? Math.atan2(vb.x - va.x, va.y - vb.y) : Math.atan2(va.x - vb.x, vb.y - va.y);
  }

Once I disable voronoi, there are no errors at all.

chart.showVoronoi doesn't seem to be there at all (showVoronoi is not a function).

liquidpele commented 9 years ago

Ah okay, that helps. This looks related: https://github.com/mbostock/d3/issues/1908

So this may be related to the known issue where we don't handle having two of the same point:

https://github.com/novus/nvd3/issues/368

liquidpele commented 9 years ago

Can you try removing any duplicate points (even if they're on different lines) to see if that makes the chart work properly? I know that's not a solution, but it would verify that the above is indeed the issue.

pcwiek commented 9 years ago

That's it! If I leave just one series on the chart (by cutting 3 other series out which share the same x-coordinates), everything works even with useVoronoi(true) without any errors.

Although I got that exact same error about vs being null even in 1.1.15, but all charts were working without any apparent issues then.

liquidpele commented 9 years ago

Okay, closing this then as a duplicate... I'll try to get this fixed soon, several people have brought it up.

liquidpele commented 9 years ago

Please check using this fix to see if it resolves your issue. This fixed the stackedareachart for me, which had the same bug.

https://github.com/novus/nvd3/pull/880

liquidpele commented 9 years ago

Actually I'll open this issue and close the other one since that one is reeeeealy old and I don't know if they'll ever reply and verify things are fixed :p

liquidpele commented 9 years ago

Can you confirm that the latest code in the development branch fixes your issue?

pcwiek commented 9 years ago

I hope I'll have a chance to test it on Monday

liquidpele commented 9 years ago

Hey there, any further problems? If not I'll close this issue.

pcwiek commented 9 years ago

Bumping the jitter also seems to work.

Anyway, I'll probably use useVoronoi(false) for now, just to be safe. Thanks for help!

liquidpele commented 9 years ago

Okay, closing this until a better solution is found (tracked by https://github.com/novus/nvd3/issues/330 and some potential things at https://github.com/novus/nvd3/pull/448)