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

LineChartView.highlightValue causes CoreGraphics API errors #4043

Closed
diesal11 opened this issue Jun 27, 2019 · 4 comments
Closed

LineChartView.highlightValue causes CoreGraphics API errors #4043

diesal11 opened this issue Jun 27, 2019 · 4 comments
Assignees
Labels

Comments

@diesal11
Copy link

diesal11 commented Jun 27, 2019

What did you do?

Called LineChartView.highlightValue(x: Double, dataSetIndex: Int) with a valid x & dataSetIndex

What did you expect to happen?

I expected the Chart to Highlight the appropriate X/Y point for the given X value

What happened instead?

Nothing was drawn, console spews out the error: Error: this application, or a library it uses, has passed an invalid numeric value (NaN, or not-a-number) to CoreGraphics API and this value is being ignored. Please fix this problem.

Charts Environment

Charts version/Branch/Commit Number: 3.3.0
Xcode version: 10.2.1
Swift version: 5.0
Platform(s) running Charts: iOS 12.3.1
macOS version running Xcode: 10.14.4

Demo Project

        let chartView = LineChartView(frame: self.view.frame)

        let dataSet = LineChartDataSet(entries: [], label: "Test Dataset")
        let data = LineChartData(dataSets: [])
        chartView.data = data

        chartView.data!.addDataSet(dataSet)

        for i in 0..<10 {
            let entry = ChartDataEntry(x: Double(i), y: Double(i))
            chartView.data!.addEntry(entry, dataSetIndex: 0)
        }

        chartView.notifyDataSetChanged()

        chartView.highlightValue(x: 2.0, dataSetIndex: 0)

Note: From my own digging the issue here is in the LineChartRenderer.drawHighlighted func. Specifically the following lines:

            guard let e = set.entryForXValue(high.x, closestToY: high.y) else { continue }
            
            if !isInBoundsX(entry: e, dataSet: set)
            {
                continue
            }
            .......
            .......
            let x = high.x // get the x-position
            let y = high.y * Double(animator.phaseY)
            
            if x > chartXMax * animator.phaseX
            {
                continue
            }
            
            let trans = dataProvider.getTransformer(forAxis: set.axisDependency)
            
            let pt = trans.pixelForValues(x: x, y: y)
            
            high.setDraw(pt: pt)

            // draw the lines
            drawHighlightLines(context: context, point: pt, set: set)

Since high.y is NaN, the transformer attempts to transform a CGPoint where Y is NaN. This causes pt X & Y to both be NaN which is then passed to drawHighlightLines resulting in CoreGraphics API errors.

I believe a fix here would be to use the result from set.entryForXValue above? But i'm not sure what implications that would have on other highlighting funcs.

@diesal11
Copy link
Author

diesal11 commented Jun 29, 2019

I did some digging, and there's already a PR which addresses this #2568. But there looks to be some confusion about highlights being created with NaN Y values, however if you follow the code path from LineChartView.highlightValue this appears to be perfectly valid behaviour.

It's worth noting that using the result of set.entryForXValue matches the code & behaviour in MPAndroidChart as well:
https://github.com/PhilJay/MPAndroidChart/blob/726616d0794cabe878100ce8c7e49e9a7351f60c/MPChartLib/src/main/java/com/github/mikephil/charting/renderer/LineChartRenderer.java#L693-L717

liuxuan30 added a commit that referenced this issue Aug 7, 2019
1. change XBounds iterator to use self.min + self.range rather than self.x
2. align drawLinear,  drawCubicBezier to new XBounds iterator.
3. fix unexpected dash line during linear animation due to reading the next entry point
liuxuan30 added a commit that referenced this issue Aug 7, 2019
1. change XBounds iterator to use self.min + self.range rather than self.x
2. align drawLinear,  drawCubicBezier to new XBounds iterator.
3. fix unexpected dash line during linear animation due to reading the next entry point
@liuxuan30
Copy link
Member

liuxuan30 commented Aug 7, 2019

sorry to wrongly link this issue from 4093

I need to know why it's NaN then we can decide how to fix for this issue

@liuxuan30 liuxuan30 added the bug label Aug 7, 2019
@liuxuan30 liuxuan30 self-assigned this Aug 7, 2019
@liuxuan30
Copy link
Member

liuxuan30 commented Aug 7, 2019

OK so I took a look at your code and #2568.

LineChartView.highlightValue(x: Double, dataSetIndex: Int)

will eventually fall into

    @objc open func highlightValue(x: Double, dataSetIndex: Int, dataIndex: Int = -1, callDelegate: Bool)
    {
        highlightValue(x: x, y: .nan, dataSetIndex: dataSetIndex, dataIndex: dataIndex, callDelegate: callDelegate)
    }

and passing a nan for y value, because no one gives it. It should finally fall into

    open override func entryIndex(
        x xValue: Double,
        closestToY yValue: Double,
        rounding: ChartDataSetRounding) -> Int
    {

where it detects if yValue is NaN and do something:

            // Search by closest to y-value
            if !yValue.isNaN
            {
                while closest > 0 && self[closest - 1].x == closestXValue
                {
                    closest -= 1
                }
                
                var closestYValue = self[closest].y
                var closestYIndex = closest
                
                while true
                {
                    closest += 1
                    if closest >= endIndex { break }
                    
                    let value = self[closest]
                    
                    if value.x != closestXValue { break }
                    if abs(value.y - yValue) <= abs(closestYValue - yValue)
                    {
                        closestYValue = yValue
                        closestYIndex = closest
                    }
                }
                
                closest = closestYIndex
            }

but like you said, it later calls let y = high.y * Double(animator.phaseY), which seems buggy. at least it should try to fetch if detects NaN

@liuxuan30
Copy link
Member

it seems #2568 indeed is valid fix. Thanks for reporting!

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

No branches or pull requests

2 participants