ChartsOrg / Charts

Beautiful charts for iOS/tvOS/OSX! The Apple side of the crossplatform MPAndroidChart.
Apache License 2.0
27.54k stars 5.99k forks source link

Crash building iterator. Range requires lowerBound <= upperBound #4798

Closed sebastienhannay closed 2 years ago

sebastienhannay commented 2 years ago

What did you do?

Upgrade from 3.6.0 to 4.0.2

What happened ?

Most drawing of charts leads to a crash when building iterator in extension BarLineScatterCandleBubbleRenderer.XBounds: Sequence because Range requires lowerBound <= upperBound

let entryTo = dataSet.entryForXValue(high, closestToY: .nan, rounding: .up) is nil. It looks like the closest found is < endIndex in the dataset.

I use custom minimum and maximum axis values and visible range.

The same code was working perfectly in v3.6. I also tried to rollback to 3.6 as a temporary workaround but the project doesn't compile anymore with XCode 13.3.

Charts Environment

Charts version/Branch/Commit Number: 4.0.2 Xcode version: 13.3 (13E113) Swift version: 5 Platform(s) running Charts: iOS macOS version running Xcode: 12.0.1 (21A559)

pavlo-kravchenko commented 2 years ago

Some problem

image
k1ran-ak commented 2 years ago

This can happen when max values are 0 or negative values. make sure the x values are not 0 or negative in the data entry x value array you add to the Chart Data entry. you can add a guard let to check if the max > min then proceed with make iterator as a temporary solution

sebastienhannay commented 2 years ago

hello @k1ran-ak, I just checked and I don't have any negative value in my data :

Charts.ChartDataSet Charts.ChartDataSet 
Charts.ChartBaseDataSet Charts.ChartBaseDataSet 
entries [Charts.ChartDataEntry] 60 values   
_yMax   Double  80.135000000000005
_yMin   Double  49.600000000000001
_xMax   Double  17737.416666666668
_xMin   Double  16777.458333333332

Tried to swap min and max before building iterator if min > max and I got the chart display without a crash but when I scroll to the very end of it, the line disappear.

sebastienhannay commented 2 years ago

Found a quick fix (available in above pull request) that is not perfect and doesn't solve the source of the issue but at least it doesn't crash anymore

k1ran-ak commented 2 years ago

Hi @sebastienhannay. can you provide your 60 ChartDataEntry values? so I get more information about the crash.

k1ran-ak commented 2 years ago

Make sure you use x values are index. You can later change X values with value formatter.

Example: for index in 0..<high.count { let dataEntry = CandleChartDataEntry(x:Double(index) , shadowH: high, shadowL: low, open: open, close: close) candledataEntries.append(dataEntry) }

sebastienhannay commented 2 years ago

We are building a timeline so x values are a function on timeIntervalSince1970 (and so distance between 2 points may vary a lot depending on data that we have). We already use value formatter (calculation on timeIntervalSince1970 was to avoid huge values but it will be difficult to reduce more than that as the timeline may be quite long)

Here are my 60 values that cause the crash :

▿ 0 : ChartDataEntry, x: 16777.458333333332, y 78.546
▿ 1 : ChartDataEntry, x: 16782.458333333332, y 79.335
▿ 2 : ChartDataEntry, x: 16796.458333333332, y 78.836
▿ 3 : ChartDataEntry, x: 16804.458333333332, y 78.652
▿ 4 : ChartDataEntry, x: 16812.458333333332, y 78.523
▿ 5 : ChartDataEntry, x: 16813.458333333332, y 78.591
▿ 6 : ChartDataEntry, x: 16818.458333333332, y 78.976
▿ 7 : ChartDataEntry, x: 16821.458333333332, y 78.228
▿ 8 : ChartDataEntry, x: 16830.458333333332, y 79.165
▿ 9 : ChartDataEntry, x: 16838.458333333332, y 79.55
▿ 10 : ChartDataEntry, x: 16840.458333333332, y 78.473
▿ 11 : ChartDataEntry, x: 16841.458333333332, y 79.062
▿ 12 : ChartDataEntry, x: 16842.458333333332, y 78.668
▿ 13 : ChartDataEntry, x: 16855.458333333332, y 80.135
▿ 14 : ChartDataEntry, x: 16889.416666666668, y 78.914
▿ 15 : ChartDataEntry, x: 16895.416666666668, y 78.829
▿ 16 : ChartDataEntry, x: 16903.416666666668, y 79.331
▿ 17 : ChartDataEntry, x: 16909.416666666668, y 79.175
▿ 18 : ChartDataEntry, x: 16915.416666666668, y 78.453
▿ 19 : ChartDataEntry, x: 16921.416666666668, y 79.129
▿ 20 : ChartDataEntry, x: 16923.416666666668, y 78.438
▿ 21 : ChartDataEntry, x: 16926.416666666668, y 78.309
▿ 22 : ChartDataEntry, x: 16931.416666666668, y 78.263
▿ 23 : ChartDataEntry, x: 16932.416666666668, y 78.858
▿ 24 : ChartDataEntry, x: 16935.416666666668, y 79.253
▿ 25 : ChartDataEntry, x: 16936.416666666668, y 78.571
▿ 26 : ChartDataEntry, x: 16939.416666666668, y 79.557
▿ 27 : ChartDataEntry, x: 16943.416666666668, y 78.748
▿ 28 : ChartDataEntry, x: 16950.416666666668, y 78.992
▿ 29 : ChartDataEntry, x: 16951.416666666668, y 78.624
▿ 30 : ChartDataEntry, x: 16953.416666666668, y 79.237
▿ 31 : ChartDataEntry, x: 16958.416666666668, y 78.7615
▿ 32 : ChartDataEntry, x: 16962.416666666668, y 78.832
▿ 33 : ChartDataEntry, x: 16968.416666666668, y 78.335
▿ 34 : ChartDataEntry, x: 16971.416666666668, y 78.099
▿ 35 : ChartDataEntry, x: 16982.416666666668, y 78.939
▿ 36 : ChartDataEntry, x: 16989.416666666668, y 78.634
▿ 37 : ChartDataEntry, x: 16990.416666666668, y 78.604
▿ 38 : ChartDataEntry, x: 16994.416666666668, y 78.796
▿ 39 : ChartDataEntry, x: 16997.416666666668, y 79.1465
▿ 40 : ChartDataEntry, x: 16998.416666666668, y 78.414
▿ 41 : ChartDataEntry, x: 17014.416666666668, y 79.58
▿ 42 : ChartDataEntry, x: 17018.416666666668, y 78.153
▿ 43 : ChartDataEntry, x: 17021.416666666668, y 78.876
▿ 44 : ChartDataEntry, x: 17025.416666666668, y 77.746
▿ 45 : ChartDataEntry, x: 17029.416666666668, y 78.315
▿ 46 : ChartDataEntry, x: 17031.416666666668, y 78.468
▿ 47 : ChartDataEntry, x: 17033.416666666668, y 62.72666666666667
▿ 48 : ChartDataEntry, x: 17203.458333333332, y 75.0
▿ 49 : ChartDataEntry, x: 17543.458333333332, y 79.0
▿ 50 : ChartDataEntry, x: 17546.458333333332, y 80.0
▿ 51 : ChartDataEntry, x: 17549.458333333332, y 78.0
▿ 52 : ChartDataEntry, x: 17670.416666666668, y 49.8
▿ 53 : ChartDataEntry, x: 17672.416666666668, y 49.6
▿ 54 : ChartDataEntry, x: 17675.416666666668, y 50.5
▿ 55 : ChartDataEntry, x: 17677.416666666668, y 51.0
▿ 56 : ChartDataEntry, x: 17734.416666666668, y 53.0
▿ 57 : ChartDataEntry, x: 17735.416666666668, y 51.0
▿ 58 : ChartDataEntry, x: 17736.416666666668, y 50.0
▿ 59 : ChartDataEntry, x: 17737.416666666668, y 51.0
k1ran-ak commented 2 years ago

okay values seems to be fine. Do you have any X offset or Y offset assigned to your chart view ?

k1ran-ak commented 2 years ago

if you have any offset values try this

fileprivate init(min: Int, max: Int) { self.iterator = max >= min ? (min...max).makeIterator() : (min...min+50).makeIterator() }

50 - is your range given in setVisibleXRange(minXRange: 1, maxXRange: 50)

sebastienhannay commented 2 years ago

I have offset on X and also custom min and max values on both X and Y (add some padding to the charts so it matches our requirements). I can scroll in the chart horizontally but cannot zoom in or zoom out.

I just noticed that the crash happens when the scroll reach the very last point of the dataset (didn't notice earlier as I was automatically scrolling to the end on display) so maybe it is related to the dataset or custom viewing window.

Tried your solution, it prevent the crash but as soon as the last point is displayed, the line disappear but i still have the datapoints

k1ran-ak commented 2 years ago

I can scroll in the chart horizontally but cannot zoom in or zoom out -> for this issue check if the scaleY is enabled and pinchZoom enabled

Can you share all of your offset codes along with the offset values

sebastienhannay commented 2 years ago

I can scroll in the chart horizontally but cannot zoom in or zoom out This is not an issue, this is how I set it in my chart, mentioned it just in case it could help.

About the offset, I tried to isolate the issue as much as possible and it looks like setting any xAxis.axisMaximum bigger than data.xMax will make it crash as soon as we reach the end of the chart. Looks like setting it to something lower will not make it crash as we can't reach the end of the chart but it mess up the display.

For the data that I shared, I set xAxis.axisMaximum = 17742.41665509259 but I tried 17738 (round up of data.xMax) and 17737 (round down of data.xMax) and both had bad behavior

k1ran-ak commented 2 years ago

For the data that I shared, I set xAxis.axisMaximum = 17742.41665509259 but I tried 17738 (round up of data.xMax) and 17737 (round down of data.xMax) and both had bad behavior

setting xAxis.axisMaximum will force the chart to stop at that particular value.Check if you have same functionality without setting axisMaximum.

sebastienhannay commented 2 years ago

Yes it works correctly when I don't set axisMaximum so it can be a quick fix but it will have some differences in the display. In my case setting an axisMaximum adds some padding in the chart. The charts frame shows data by week and the axis maximum is set to display the full week even on the very last week. So if my last data value is on Tuesday, it extends the chart to Sunday. It gives a better UI so it would be a shame to lose it... And It still works perfectly on axisMinimum

sebastienhannay commented 2 years ago

Seems like the issue is when you try to find the closest point to the max.

The comparison seems to be : var closest = partitioningIndex { $0.x >= xValue } which is not really the closest but the closest after specific xValue. In my case closest x is lower than xValue as I don't have any point >= axisMaximum

k1ran-ak commented 2 years ago

Seems like the issue is when you try to find the closest point to the max.

The comparison seems to be : var closest = partitioningIndex { $0.x >= xValue } which is not really the closest but the closest after specific xValue. In my case closest x is lower than xValue as I don't have any point >= axisMaximum

Yeah I think you are using x values in Data Entry as timestamps instead you can try to use the index of the loop in which you append the values as x values. then use value formatter to IndexAxisFormatter and give your timestamps

And then you can set Axis Maximum. Lets say I have total of 60 values my Axis Maximum would be 70 and I should manually add the timestamps for the 10 extra points that's needed to be shown in the graph(I'm not sure about this part)

In case if you are not using timestamps as x values To achieve a padding you need to assign x offset(while assigning don't give it more than the x axis range you have assigned).

sebastienhannay commented 2 years ago

They are not directly timestamps but derive of timestamps. The problem is that I don't have values for every day and can't add extra points to my chart without losing its accuracy. I need a scale that represent a real time scale and all points that are on my chart should be associated with a value added by the user. So your solution doesn't seems to fit my needs...

Correct me if I'm wrong but if I use xOffset instead of xAxis.axisMaximum I will get the same padding at the beginning and end of the chart which, again, doesn't really fit my needs.

k1ran-ak commented 2 years ago

Correct me if I'm wrong but if I use xOffset instead of xAxis.axisMaximum I will get the same padding at the beginning and end of the chart which, again, doesn't really fit my needs.

Yes you will get at both ends but you can move your chart view to last xValue with moveViewToX which displays your chart view's last x point with padding on right side alone , unless user decides to scroll to very first value.

sebastienhannay commented 2 years ago

I see, could also be a quick fix but it's it would be better if could work the same way as in v3.6. Looks like this version bring more limitations than the previous one... I saw a branch that restore the old way of calculating xbounds (https://github.com/danielgindi/Charts/tree/bugfix/xbounds-iterator) that may fix my issue but I don't know if it limits some newer features. Would be awesome if I could have the same display as before without adding some tricks in the code to compensate what seems to be a regression.

k1ran-ak commented 2 years ago

I see, could also be a quick fix but it's it would be better if could work the same way as in v3.6. Looks like this version bring more limitations than the previous one... I saw a branch that restore the old way of calculating xbounds (https://github.com/danielgindi/Charts/tree/bugfix/xbounds-iterator) that may fix my issue but I don't know if it limits some newer features. Would be awesome if I could have the same display as before without adding some tricks in the code to compensate what seems to be a regression.

The version of Charts you linked is of Version 4.0.0 I think you won't lose much on new features probably 4.0.2 have some bug fixes.

sebastienhannay commented 2 years ago

I mean as it roll back the new calculation of xbounds introduce in 4.0.0 to the one in 3.6. I guess there must be a reason why it changed that much between 3.6 and 4.0.x. Again this could probably solve my issue but it would probably be better to fix the new calculation method than rollback to the old one

sjmerel commented 2 years ago

I am also seeing this in version 4.0.2. When I set a maximum value on the x axis, I get a sporadic crash, and the line disappears when I zoom out all the way.

k1ran-ak commented 2 years ago

I am also seeing this in version 4.0.2. When I set a maximum value on the x axis, I get a sporadic crash, and the line disappears when I zoom out all the way.

Did you set visibleXRange ?

sjmerel commented 2 years ago

No, will that help? It seems to automatically limit the zoom so I can't zoom out past the extent of the data. (Just to clarify my last post, the line disappears when I zoom out so all the data points are visible; I don't need to zoom out a crazy amount.)

k1ran-ak commented 2 years ago

No, will that help? It seems to automatically limit the zoom so I can't zoom out past the extent of the data. (Just to clarify my last post, the line disappears when I zoom out so all the data points are visible; I don't need to zoom out a crazy amount.)

In case if you don't want to scale a crazy amount you can set maximum scale limit with

if barChartView.scaleX > 1.3
{
barChartView.notifyDataSetChanged()
barChartView.scaleXEnabled = false
}

Change 1.3 to match whatever zoom level you wanted to limit it.

PWrzesinski commented 2 years ago

I'm also seeing this issue. I'm using time interval since 1970 as x values and drawing a CombinedChartView with candlesticks, line and bar data. Not sure if this helps, but it seems like the crash occurs when there is a big gap between 2 entries and the user tries to pan the view to show this part.

k1ran-ak commented 2 years ago

I'm also seeing this issue. I'm using time interval since 1970 as x values and drawing a CombinedChartView with candlesticks, line and bar data. Not sure if this helps, but it seems like the crash occurs when there is a big gap between 2 entries and the user tries to pan the view to show this part.

@PWrzesinski Yes charts will not function properly when time stamps are used. You can make use of indexes instead of timestamps and change the x axis values later with Axis Formatter Class. Like this https://github.com/danielgindi/Charts/issues/4798#issuecomment-1090169068

PWrzesinski commented 2 years ago

I'm also seeing this issue. I'm using time interval since 1970 as x values and drawing a CombinedChartView with candlesticks, line and bar data. Not sure if this helps, but it seems like the crash occurs when there is a big gap between 2 entries and the user tries to pan the view to show this part.

@PWrzesinski Yes charts will not function properly when time stamps are used. You can make use of indexes instead of timestamps and change the x axis values later with Axis Formatter Class. Like this #4798 (comment)

Thanks @k1ran-ak, I've reduced the numbers so instead of starting 1.01.1970 they will start with the lowest date from all values, so if my earliest data point is 1.01.2021 that will be value 0. This fixed the crash for me.

Still I must say, Charts is pretty close to working with dates since 1970, apart from this crash I've only encountered the issue with candlesticks being very thin which feels like it should be fixable. There is also one unrelated issue, but otherwise I'm super happy with the framework!

k1ran-ak commented 2 years ago

@PWrzesinski If you are using timestamps as X-axis values, you will be getting a thin candle stick chart as the difference between two consequetive timestamps is large. Thats why I earlier mentioned to use indices of the timestamps array ie. 0,1,2 .... and later change the x axis labels with Axis formatter class. If you want me to explain in detail I can do it so let me know if there is anything I could do to help you.

PWrzesinski commented 2 years ago

@PWrzesinski If you are using timestamps as X-axis values, you will be getting a thin candle stick chart as the difference between two consequetive timestamps is large. Thats why I earlier mentioned to use indices of the timestamps array ie. 0,1,2 .... and later change the x axis labels with Axis formatter class. If you want me to explain in detail I can do it so let me know if there is anything I could do to help you.

Thank you, I appreciate the help! There are a few reasons I'd like to avoid using the index approach you're suggesting, but your comment clarified for me why the candlesticks are so thin. In the end I simply divided the timestamps by 10 after offsetting them to the start date and now they look good. Thanks! 😅

k1ran-ak commented 2 years ago

@PWrzesinski Glad I could be of any help 😊.

sjmerel commented 2 years ago

This does appear to be a bug. It's happening to me on a LineChartView with the following data points: (-3, 0) (-2, 0) (0, 0)

It only happens if I set the xMin and xMax values of the x axis (I'm setting them to -4 and 7). Some of the points are not rendered, and then if I zoom in a little and pan to the left, I hit this crash.

I'm not too familiar with the code, but it looks like the problem is in BarLineScatterCandleBubbleRenderer.XBounds.set(). I think it's trying to figure out the range of indices of points to render, but if you've set the xMin and xMax to be even a little larger than the full range of the data (which seems to a reasonable thing to do) then the range is not calculated correctly.

(looking back in this thread, this is the same place in the code that @sebastienhannay identified also.)

Would love to get this resolved; I'm currently stuck on version 3.6 due to this bug! 😀

msaps commented 2 years ago

Also experiencing this bug - seems like #4839 is the cure 😄

FelixHerrmann commented 2 years ago

Can someone try with the fix from #4829? (it is already in master) It fixes a bug related to the swift-algorithms change (released in 4.0.0) and might cause this issue here as well!

sjmerel commented 2 years ago

@FelixHerrmann I verified that the latest build from the master branch does fix the problem!

FelixHerrmann commented 2 years ago

Awesome, thanks for checking!! If @sebastienhannay can confirm too, this issue can be closed I would say...

sebastienhannay commented 2 years ago

Sorry for the late answer, the latest version seems to work well on my side too.