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

Handling critical Y values with autoScaleMinMaxEnabled #2053

Closed
Fahrenheit-sp opened this issue Jan 10, 2017 · 22 comments
Closed

Handling critical Y values with autoScaleMinMaxEnabled #2053

Fahrenheit-sp opened this issue Jan 10, 2017 · 22 comments

Comments

@Fahrenheit-sp
Copy link

Fahrenheit-sp commented Jan 10, 2017

Hi. I have a LineChartView with autoScaleMinMaxEnabled option enabled. I retrieve some data from internet and dynamically add entries to my chartView, based on the data retrieved.
Here is some sample code:

let receivedEntry = ChartDataEntry(x: json["time"].doubleValue, y:json["value"].doubleValue)
set.addEntry(receivedEntry)
chartView.notifyDataSetChanged()

everything works just well, but if receivedEntry.y is higher than my current yAxisMaximum, or lower than minimum, the new value gets drawn beyond the bounds (it means that some part of chart goes beyond the top of chartView, or beyond the bottom, depending on receivedEntry.y value), and gets autoscaled only if I translate or scale it manually.

I tried to set axisMaximum/axisMinimum manually, and then reset it by calling resetAxisMinMax, when I start scaling/translating chartView

func handleCriticalValues() {
        if receivedEntry.y >= chartView.rightAxis.axisMaximum - (chartView.rightAxis.axisRange * Double(chartView.rightAxis.spaceTop)) {
            chartView.rightAxis.axisMaximum = receivedEntry.y + (chartView.rightAxis.axisRange * Double(chartView.rightAxis.spaceTop))
            chartView.notifyDataSetChanged()
        }
        
        if receivedEntry.y <= chartView.rightAxis.axisMinimum + (chartView.rightAxis.axisRange * Double(chartView.rightAxis.spaceTop)) {
            chartView.rightAxis.axisMinimum = receivedEntry.y - (chartView.rightAxis.axisRange * Double(chartView.rightAxis.spaceTop))
            chartView.notifyDataSetChanged()
        }
    }

(this gets called right before chartView.notifyDataSetChanged() from previous code sample). This solution works almost perfect, but when I translate chart to the current position, and critical value arrives, chart goes beyond the bounds anyway.

If any additional information or logs needed, please let me know. Thanks for any help and thanks again for such awesome library.

@liuxuan30
Copy link
Member

liuxuan30 commented Jan 10, 2017

Your code sample seems fine, the chart will recalculate and trigger a new draw(), but I am not sure what you mean

"but when I translate chart to the current position, and critical value arrives, chart goes beyond the bounds anyway."

You have to debug a little to see when you translate and add new entries, what happened to your yAxisMinimum/maximum

In draw(), when you turn on autoScaleMinMaxEnabled, it will trigger another recalculation:

        if _autoScaleMinMaxEnabled
        {
            autoScale()
        }

in autoScale():

    internal func autoScale()
    {
        guard let data = _data
            else { return }
        
        data.calcMinMaxY(fromX: self.lowestVisibleX, toX: self.highestVisibleX)
        
        _xAxis.calculate(min: data.xMin, max: data.xMax)
        
        // calculate axis range (min / max) according to provided data
        _leftAxis.calculate(min: data.getYMin(axis: .left), max: data.getYMax(axis: .left))
        _rightAxis.calculate(min: data.getYMin(axis: .right), max: data.getYMax(axis: .right))
        
        calculateOffsets()
    }

I don't see obvious reason why it draw beyond, though it's another calculation, but the values should be the same when you trigger notifyDataSetChanged(), can you help to debug? e.g. debug autoScale() to see if any weird value appears.

@Fahrenheit-sp
Copy link
Author

@liuxuan30, thanks for your reply. I'll try to find anything weird, and give you an answer as soon as possible.

@cartmanguo
Copy link

This also happens in my app ,do you have a solution?@Fahrenheit-sp

@Fahrenheit-sp
Copy link
Author

@cartmanguo actually no, I still have this issue

@JokerMZD
Copy link

JokerMZD commented Feb 20, 2017

+1 - have this issue in my app
http://take.ms/AJlo6

And as we can see with autoscaleminmax top space is not calculated properly - http://take.ms/OAw5P

@liuxuan30
Copy link
Member

@Fahrenheit-sp Can you check if #2177 fix your issue?

@Fahrenheit-sp
Copy link
Author

@liuxuan30, have not much time right now. Will check in a couple of days and reply. Thank you for your participation.

@JokerMZD
Copy link

@liuxuan30 , I've checked the latest Master code and issue still exist for iOS version.
Here is different behavior of iOS and Android versions of Charts. Dataset is the same.
iOS - http://take.ms/CCxxU
Android - http://take.ms/3aCHh

@liuxuan30
Copy link
Member

@JokerMZD so issue is different

@liuxuan30 liuxuan30 added the bug label Feb 27, 2017
@aelam
Copy link
Contributor

aelam commented Feb 27, 2017

Is it needed to call notifyDataSetChanged
After set.addEntry(receivedEntry)?

@Fahrenheit-sp
Copy link
Author

Fahrenheit-sp commented Mar 6, 2017

@liuxuan30 sorry for the late reply. Actually, no. It doesn't fix the problem. seems like handling min/max manually is the only option atm.

@liuxuan30
Copy link
Member

@aelam it depends when you add it. If you chart is displayed, and you manually do that, you should

@aelam
Copy link
Contributor

aelam commented Mar 10, 2017

@lixuan maybe try my PR to see if can solve this ? I believe some import entries which are the maxY or minY are ignored

@aelam
Copy link
Contributor

aelam commented Mar 10, 2017

Debug the method calculateMinMaxY(fromX, toX)
If the entry with index indexTo is visible and this entry is overlapped, then that is the problem

@aelam
Copy link
Contributor

aelam commented Mar 10, 2017

#2229

@JokerMZD
Copy link

@aelam I've checked your pull request - it fixes my issue. Thanks.

@Fahrenheit-sp
Copy link
Author

@aelam yes, that fixes it. Thank you a lot!

@liuxuan30
Copy link
Member

@danielgindi I am not sure if #2229 will potentially impact other components though it fix some..

@aelam
Copy link
Contributor

aelam commented Mar 14, 2017 via email

@liuxuan30
Copy link
Member

yeah, what I am concerned is this could be a big change if any other code relies on it

@aelam
Copy link
Contributor

aelam commented Mar 15, 2017

Just need to confirm if the indexTo needs to be included. Then it's clear

@aelam
Copy link
Contributor

aelam commented Jan 29, 2018

I think this got fixed, So close it ?

@jjatie jjatie closed this as completed Jan 30, 2018
@ChartsOrg ChartsOrg locked as resolved and limited conversation to collaborators Jan 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants