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

AutoScaling yAxis during panning / zooming #95

Merged
merged 6 commits into from
May 26, 2015
Merged

AutoScaling yAxis during panning / zooming #95

merged 6 commits into from
May 26, 2015

Conversation

AlBirdie
Copy link
Contributor

This adds the auto scaling we've talked about in #89 . There is an issue that I haven't been able to solve yet, though. Sometimes during panning, the chart is kind of stuck in a re-drawing cycle which. I've got no idea why this is happening, but maybe you've got an idea, so please don't merge this as it is.

With autoscaling enabled, the drawRect method runs about 10% slower for the datasets of the ChartsDemo application. In our finance chart with 250 candles, this slow down isn't noticeable on an actual device.

@danielgindi
Copy link
Collaborator

Cool! I'll look into it
Just note that Xcode has a bug that causes the indentation of opening braces in properties, something really weird, I always un-indent it manually

@AlBirdie
Copy link
Contributor Author

Yes, I've noticed some inconsistencies during committing the files. I hope I kept all the formatting identical to the coding style of the library here on GitHub.

@dorsoft
Copy link

dorsoft commented May 26, 2015

Nice :)

I've managed to fix the re-drawing cycle issue on the candle chart be making the following changes:
ChartData.swift (lines 139,140) change:
_yMin = _dataSets[0].yMin;
_yMax = _dataSets[0].yMax;
to
_yMin = _dataSets[0].yMax;
_yMax = _dataSets[0].yMin;

CandleChartDataSet.swift (line 74) change
for (var i = 0; i <= endValue; i++)
to
for (var i = start; i <= endValue; i++)

BarLineChartViewBase.swift - call calcMinMax() only upon index change

  1. Declare vars:
    internal var _lastLowestVisibleXIndex: Int!
    internal var _lastHighestVisibleXIndex: Int!
  2. zero vars in initialize()
    _lastLowestVisibleXIndex = 0;
    _lastHighestVisibleXIndex = 0;
  3. check in drawRect() before calling calcMinMax()
    if (_autoScaleMinMaxEnabled)
    {
    if (_lastLowestVisibleXIndex != lowestVisibleXIndex || _lastHighestVisibleXIndex != highestVisibleXIndex) {
    calcMinMax();
    calculateOffsets();
    _lastLowestVisibleXIndex = lowestVisibleXIndex;
    _lastHighestVisibleXIndex = highestVisibleXIndex;
    }
    }
  4. There is an issue calculating _rightAxis.axisMinimum in calcMinMax() (line 309)

_rightAxis.axisMinimum = !isnan(_rightAxis.customAxisMin) ? _rightAxis.customAxisMin : (minRight - bottomSpaceRight);

when
A. _rightAxis.customAxisMin is undefined
B. minRight == 0.25f
C. bottomSpaceRight == 0.1f

_rightAxis.axisMinimum value jumps "sporadically" between 0.15f and -0.01f

Because I'm using only the right axis and my y values are always > 0 I've added the following to calcMinMax (line 311)

if (_rightAxis.axisMinimum < 0) {
_rightAxis.axisMinimum = 0;
}

And fixed the re-drawing cycle issue for my data set

@AlBirdie
Copy link
Contributor Author

Let me try that @dorsoft . If that does the the issue for the demo data set as well (I haven't had this issue on a CandleStickChart using our DataSets either), I'll merge your changes into my fork and create a new pull request.

@danielgindi
Copy link
Collaborator

I still haven't managed to debug and find the source for the render loop,
I'd like to try first before adding so many untested changes

On Tue, May 26, 2015 at 2:21 PM, AlBirdie notifications@github.com wrote:

Let me try that @dorsoft https://github.com/dorsoft . If that does the
the issue for the demo data set as well (I haven't had this issue on a
CandleStickChart using our DataSets either), I'll merge your changes into
my fork and create a new pull request.


Reply to this email directly or view it on GitHub
#95 (comment)
.

@dorsoft
Copy link

dorsoft commented May 26, 2015

I believe that the root cause for the render loop issue is an axisMinimum miscalculation in BarLineChartViewBase, calcMinMax(), line 309 (@AlBirdie pull request)

@AlBirdie
Copy link
Contributor Author

Yes, seems like that did the trick.

@danielgindi
Copy link
Collaborator

I'm testing it at this moment, and it doesn't compile because you forgot the bubble charts...
But do not touch I'm in a merge with the bugfixes right now

@AlBirdie
Copy link
Contributor Author

Whoops, sorry, totally forgot about those.

@danielgindi
Copy link
Collaborator

Well @dorsoft it doesn't seem to be the root cause, as @AlBirdie did not actually touch anything around line 309. Line 309 derives from minRight which then comes from data.getYMin(). So it's actually the reborn calcMinMax functions.

Something there seems weird to me - they are initialized with values that are not calculated yet (ymin/ymax of dataset[0], not calculated yet) and then iterating over all datasets. It should actually initialize with FLT_MIN/FLT_MAX.

I'm trying this, along with your fixes

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

4 participants