Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for crash when interval is NaN #2804

Closed
wants to merge 1 commit into from
Closed

fix for crash when interval is NaN #2804

wants to merge 1 commit into from

Conversation

pingd
Copy link

@pingd pingd commented Sep 20, 2017

After clearing a working chart and then adding new data (dataset), the chart crashes in AxisRendererBase.swift line 125 with "Double value cannot be converted to Int because it is either infinite or NaN". The reason for that is "min" and "max" are NaN (because the Transformer._matrixValueToPx is somewhat invalid(?), why does it happen?).

The issue is referenced here: https://stackoverflow.com/questions/45496912/how-to-clear-a-chart-and-then-add-data-to-it-in-swift-3-using-chartview-clear and here: #2168 (josman185hotm's comment from 17 Jun).

@codecov-io
Copy link

codecov-io commented Sep 20, 2017

Codecov Report

Merging #2804 into master will decrease coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2804     +/-   ##
=========================================
- Coverage   19.64%   19.54%   -0.1%     
=========================================
  Files         113      113             
  Lines       13791    13878     +87     
=========================================
+ Hits         2709     2713      +4     
- Misses      11082    11165     +83
Impacted Files Coverage Δ
Source/Charts/Renderers/AxisRendererBase.swift 59.02% <100%> (ø) ⬆️
...s/Renderers/Scatter/ChevronDownShapeRenderer.swift 0% <0%> (ø) ⬆️
...rts/Renderers/Scatter/ChevronUpShapeRenderer.swift 0% <0%> (ø) ⬆️
Source/Charts/Highlight/Highlight.swift 0% <0%> (ø) ⬆️
...Charts/Renderers/Scatter/CircleShapeRenderer.swift 0% <0%> (ø) ⬆️
...arts/Renderers/Scatter/TriangleShapeRenderer.swift 0% <0%> (ø) ⬆️
.../Charts/Renderers/Scatter/CrossShapeRenderer.swift 0% <0%> (ø) ⬆️
Source/Charts/Filters/DataApproximator.swift 0% <0%> (ø) ⬆️
...urce/Charts/Renderers/Scatter/XShapeRenderer.swift 0% <0%> (ø) ⬆️
...Charts/Renderers/Scatter/SquareShapeRenderer.swift 0% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c083f3...58c33f3. Read the comment docs.

@liuxuan30
Copy link
Member

liuxuan30 commented Sep 22, 2017

#2377 is my thought to fix. Have you checked it's the reason why you saw NaN on your side?

@liuxuan30
Copy link
Member

liuxuan30 commented Sep 22, 2017

I looked at your change again, it seems not the same thing. range should not be NaN at all.
Do you have the exact values that causes NaN? We'd like to investigate.

In the code, min/max are coming from:

    @objc open func computeAxis(min: Double, max: Double, inverted: Bool)
    {
        var min = min, max = max
        
        if let transformer = self.transformer
        {
            // calculate the starting and entry point of the y-labels (depending on zoom / contentrect bounds)
            if let viewPortHandler = viewPortHandler
            {
                if viewPortHandler.contentWidth > 10.0 && !viewPortHandler.isFullyZoomedOutY
                {
                    let p1 = transformer.valueForTouchPoint(CGPoint(x: viewPortHandler.contentLeft, y: viewPortHandler.contentTop))
                    let p2 = transformer.valueForTouchPoint(CGPoint(x: viewPortHandler.contentLeft, y: viewPortHandler.contentBottom))
                    
                    if !inverted
                    {
                        min = Double(p2.y)
                        max = Double(p1.y)
                    }
                    else
                    {
                        min = Double(p1.y)
                        max = Double(p2.y)
                    }
                }
            }
        }
        
        computeAxisValues(min: min, max: max)
    }

which basically take the contentRect coordinates to calculate. If this goes wrong, the chart setup is a mess in the first place then, which should not happen at all.

Adding a NaN check is the last resort to prevent crash, but we don't want to silent the crash first, it will be more difficult to notice.

@liuxuan30
Copy link
Member

please open an issue to track NaN crash for your case, with details or values that causes NaN, thanks.

@pingd
Copy link
Author

pingd commented Sep 22, 2017

I am not able to open an issue with actual code for about two weeks, as I am away. However, it happens roughly when you do something like in the SO issue.

In my case, the steps go like this:

  1. Set up a CandleStickChartView.
  2. Create a new CandleChartData with no underlaying dataset. Set chart.data to it.
  3. Create an empty CandleChartDataSet. Add it to chart data.
  4. Add values to chart. (At this point everything works correctly, the chart is displayed, the values can be update real time and so on).
  5. When I need to switch to another ViewController, I stop my data updates and clear chart.
  6. When I go back to a ViewController with chart, I repeat the process from step 2. After adding values and calling notifyDataSetChanged(), I get "Double value cannot be converted to Int because it is either infinite or NaN" on line 125 in AxisRendererBase.

I will open an actual issue with code once I am back.

@maciejtrybilo
Copy link
Contributor

+1 Having the same issue.

@liuxuan30 liuxuan30 closed this Oct 13, 2017
@liuxuan30
Copy link
Member

liuxuan30 commented Oct 13, 2017

I logged the reason in #2845 why closing this.

@liuxuan30 liuxuan30 reopened this Oct 13, 2017
@liuxuan30 liuxuan30 closed this Oct 13, 2017
@SeRG1k17
Copy link

SeRG1k17 commented Feb 7, 2018

@liuxuan30
I think this will be useful to many.
And, it is required to use only a ChartData class, because any subclass also calls crash( < ios11).

override var data: ChartData? {
        get { return super.data }
        set {
            guard let newData = newValue, newData.entryCount > 0 else {
                super.data = ChartData(dataSet: ChartDataSet(values: [ChartDataEntry()]))
                return
            }
            
            super.data = newData
            notifyDataSetChanged()
        }
    }

@atalayasa
Copy link

Hello @SeRG1k17 I had same issue too but no other solutions worked for me where should I put your code ? Thanks

@atalayasa
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants