chartjs / Chart.js

Simple HTML5 Charts using the <canvas> tag
https://www.chartjs.org/
MIT License
64.69k stars 11.92k forks source link

Line goes below zero #424

Closed knyga closed 10 years ago

knyga commented 10 years ago

See screenshot: http://prntscr.com/3zmwat

To solve it I did change in my app to control tension, but is not good solution. I think something is wrong with spline realization.

point.controlPoints = helpers.splineCurve(dataset.points[index-1],point,dataset.points[index+1], //if coordinates are the same - tension is zero dataset.points[index-1].value == dataset.points[index].value ? 0 : //and reduce tension dataset.points[index-1].value == 0 ? this.options.bezierCurveTension : this.options.bezierCurveTension * dataset.points[index+1].value / dataset.points[index-1].value);

mikeziri commented 10 years ago

noticed this also.

using bezier curves with points that go from anything > 0 to 0 then up again, with a bezierCurveTension > 0 (i think i'm on default 0.4) the line goes under the 0 on the X axis

rzds commented 10 years ago

What about when you have two consecutive low values ? Any fixes for that ? two-consecutive-zero

nnnick commented 10 years ago

That's a bit of a tricky one. I've been working a bit on bezier curving this weekend for another issue regarding sparse datasets, mentioned above (#435).

To provide a smooth bezier curve between a set of pre-described points, by its very nature the line may have to go above / below some of the points, especially when you have large then small values next to each other.

Try playing around with the demo here and you'll see what I mean: http://www.particleincell.com/blog/2012/bezier-splines/

All ears if someone can come up with some logic / maths around how Chart.js should provide bezier curving between points.

Just capping the curve tension at zero isn't really a solution because as the demo above shows the bottom of the chart may not be zero. We might also be dealing with numbers at the top of the chart, so there's also that to consider.

Comparing with previous values doesn't work either because in cases like this: http://jsbin.com/licunala/3/edit we'd have no bezier curve at all - just a straight line.

I'm not sure if there is actually a solution to this other than reducing the bezier curve tension.

rept commented 10 years ago

Hi nnnick,

First of all: thanks for the library, great work!

I have the same problem. I believe a lot of graphs don't need to go below zero and then it would be perfectly fine to just flatten the line at zero instead of going under zero.

For example the following chart now looks weird because nobody can work negative hours. I think having an option in the chart (the global options) to flatten out zero's or to have a value under which no line should be drawn would fix this for a lot of users.

worked

wunderdojo commented 10 years ago

I've been banging around on this all day and I think I've got a reasonable work around for now. The issue is that for certain values the two control points that are plotted for drawing the Bezier curve fall below the x axis. The solution is to simply catch those points and manually give them a y value equal to the y value of the x axis.

Here's what it looks like when you've got control points below the x axis: control-points-out-of-bounds

By adding a check to the loop that plots the data points we can force those low hanging control points up to the x axis. In Chart.js (the non-minified full version) at approx line 2655 add the two lines after //wunderdojo edit show below:

// Control points need to be calculated in a seperate loop, because we need to know the current x/y of the point
// This would cause issues when there is no animation, because the y of the next point would be 0, so beziers would be skewed
if (this.options.bezierCurve){
    helpers.each(dataset.points,function(point,index){
    //If we're at the start or end, we don't have a previous/next point
    //By setting the tension to 0 here, the curve will transition to straight at the end
    if (index === 0){
        point.controlPoints = helpers.splineCurve(point,point,dataset.points[index+1],0);
    }
    else if (index >= dataset.points.length-1){
        point.controlPoints = helpers.splineCurve(dataset.points[index-1],point,point,0);
    }
    else{
        point.controlPoints = helpers.splineCurve(dataset.points[index-1],point,dataset.points[index+1],this.options.bezierCurveTension);
    }

// wunderdojo edit
    if(point.controlPoints.inner.y > this.scale.endPoint) {point.controlPoints.inner.y = this.scale.endPoint;}
    if(point.controlPoints.outer.y > this.scale.endPoint) {point.controlPoints.outer.y = this.scale.endPoint;}
    },this);
}

What you now get is this: control-points-in-bounds

nnnick commented 10 years ago

Interesting solution, it might be worthwhile setting the x to the same as the point itself as well, not sure. I'd be really interested to see how other charting libraries deal with this particular issue. Anyone got any insight on that?

We'd certainly need to repeat that same capping logic for the top of the scale, as the same effect could occur.

wunderdojo commented 10 years ago

I've been using custom Y scales for my particular project so I just have it automatically building in a buffer, ie: If the highest plotted value is 100 and the steps are 25 then the top value shown on the Y axis is 125. It would definitely work to bound the top just like the bottom, just not something that's an issue for what I'm doing right now.

I actually spent a couple of days looking into Bezier curves and various formulas for them and approaches to drawing them before I tackled this issue. This tutorial: http://pomax.github.io/bezierinfo/ -- specifically section 13 on Bounding Boxes -- is what helped me narrow in on a solution. My algebra skills weren't necessarily up to re-coding the control points formula to keep the bounding boxes within the proper bounds, but it did prompt me to come up with my hacky little workaround.

wunderdojo commented 10 years ago

Oh, and btw, thanks for all of your work on this. Fighting my way through your code is definitely teaching me a few things about javascript!

rept commented 10 years ago

Hi Nick,

From the screenshot it looks like the fix of wunderdojo is good. Will this be integrated in the regular version?

Kind regards.

cuu508 commented 10 years ago

As for other libraries, morris.js seems to deal OK with this issue. From the looks of it, it does same thing as suggested by wunderdojo: cap the control points to some "bottom" value:

https://github.com/morrisjs/morris.js/blob/master/lib/morris.line.coffee#L272

jivinivan commented 10 years ago

:+1: I'm also seeing this.

nnnick commented 10 years ago

This was resolved in https://github.com/nnnick/Chart.js/commit/179d80a93b64cf107ee219886c43eb6e212594f5, forgot to close this.

@jivinivan which version are you using?

jivinivan commented 10 years ago

@nnnick thanks for the response!

Was relying on https://github.com/lgsilver/angles for the chart.js version. I've added to our own list of dependencies with the latest version and also opened an issue for them to update. https://github.com/lgsilver/angles/issues/57

brianpkelley commented 9 years ago

I'm leaving this bit here as this is where I landed when searching how to limit the curve from going above the value when sequential values are the same.

if (this.options.bezierCurve){
    helpers.each(pointsWithValues, function(point, index){
        var tension = (index > 0 && index < pointsWithValues.length - 1) ? this.options.bezierCurveTension : 0;

// My Changes
        var previousPointObj = previousPoint(point, pointsWithValues, index);
        var nextPointObj = nextPoint(point, pointsWithValues, index)
        point.controlPoints = helpers.splineCurve(
            previousPointObj,
            point,
            nextPointObj,
            tension
        );

        // Prevent curve from going over the value and "flatten" the bezier handles 
        if ( point.value == nextPointObj.value ) {
            point.controlPoints.outer.y = nextPointObj.y;
            point.controlPoints.inner.y = nextPointObj.y;
        }

        if ( point.value == previousPointObj.value ) {
            point.controlPoints.outer.y = previousPointObj.y;
            point.controlPoints.inner.y = previousPointObj.y;
        }
// end My Changes
        // Prevent the bezier going outside of the bounds of the graph
        // Cap outer bezier handles to the upper/lower scale bounds
        if (point.controlPoints.outer.y > this.scale.endPoint){
            point.controlPoints.outer.y = this.scale.endPoint;
        }
        else if (point.controlPoints.outer.y < this.scale.startPoint){
            point.controlPoints.outer.y = this.scale.startPoint;
        }

        // Cap inner bezier handles to the upper/lower scale bounds
        if (point.controlPoints.inner.y > this.scale.endPoint){
            point.controlPoints.inner.y = this.scale.endPoint;
        }
        else if (point.controlPoints.inner.y < this.scale.startPoint){
            point.controlPoints.inner.y = this.scale.startPoint;
        }
    },this);
}
subhendupsingh commented 8 years ago

Still have the same issue on the latest version, how to fix it?

PropelNZ commented 3 years ago

Setting tension: 0 took much of the aesthetic out so I edited the non-minified version of Chart.js v2.9.4

Find at at line 2498: ctx.bezierCurveTo( flip ? previous.controlPointPreviousX : previous.controlPointNextX, flip ? previous.controlPointPreviousY : previous.controlPointNextY, flip ? target.controlPointNextX : target.controlPointPreviousX, flip ? target.controlPointNextY : target.controlPointPreviousY, target.x, target.y); Edit to: if (previous.y == target.y) { ctx.lineTo(target.x, target.y); } else { ctx.bezierCurveTo( flip ? previous.controlPointPreviousX : previous.controlPointNextX, flip ? previous.controlPointPreviousY : previous.controlPointNextY, flip ? target.controlPointNextX : target.controlPointPreviousX, flip ? target.controlPointNextY : target.controlPointPreviousY, target.x, target.y); }

Basically checks start and end points. If they are on the same y height, just draw a straight line.

Fixed my issue from this: bezier_default

To this: bezier_flat

kurkle commented 3 years ago

You should use monotone interpolation: https://www.chartjs.org/docs/latest/charts/line.html#cubicinterpolationmode https://www.chartjs.org/samples/latest/charts/line/interpolation-modes.html

PropelNZ commented 3 years ago

You should use monotone interpolation: https://www.chartjs.org/docs/latest/charts/line.html#cubicinterpolationmode https://www.chartjs.org/samples/latest/charts/line/interpolation-modes.html

Thanks so much for that. I didnt see that anywhere on my hunt for a solution.

For anyone else, this was the setting that achieved the same result as my hack: elements: { line: { cubicInterpolationMode: 'monotone', } },

Dance225 commented 7 months ago

who will be use library react-native-gifted-charts and have same problem use in your LineChart like this: export enum CurveType { CUBIC, QUADRATIC, } <LineChart ... curveType={CurveType.QUADRATIC} ... its solved to me