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

text value on the last entry is missing from line chart #1323

Closed
kennycoding opened this issue Aug 16, 2016 · 10 comments
Closed

text value on the last entry is missing from line chart #1323

kennycoding opened this issue Aug 16, 2016 · 10 comments

Comments

@kennycoding
Copy link

kennycoding commented Aug 16, 2016

repro: run the ios demo, show the first line chart, notice that the value (text) on the last entry is missing, although the value point is drawn.

@kennycoding
Copy link
Author

my current work around is to add a dummy entry with same value as the last

@danielgindi
Copy link
Collaborator

On what version of Charts?

@kennycoding
Copy link
Author

kennycoding commented Aug 17, 2016

version 2.2.5, build 21, downloaded zip yesterday

@danielgindi
Copy link
Collaborator

Did you try on 3.0?

@spinosa
Copy link

spinosa commented Sep 28, 2016

@danielgindi this is still happening on 3.0 as of 5540503 (latest code as of right now; September 28th).

Run the demo. See the last data point drawn without a label:

screen shot 2016-09-28 at 12 41 07 pm

It seems drawValues(context:) may not be calculating stride correctly (LineChartRenderer.swift:552).

@liuxuan30 liuxuan30 added the bug label Sep 29, 2016
@liuxuan30
Copy link
Member

liuxuan30 commented Sep 29, 2016

I tried to fix it by changing to to through in

for j in stride(from: _xBounds.min, to: Int(ceil(Double(_xBounds.max - _xBounds.min) * phaseX + Double(_xBounds.min))), by: 1)

This did fix the missing label, however, I found a new issue that when zoomed in, the animation on x axis is not in sync: the value text will jump out first and then come with the line. Changing back to to the animation will be good. awkward...
@danielgindi any insights?

@ryanschneider
Copy link

Sorry, didn't see this issue before opening #1585 and the #1586 PR. I'll keep my comments here, but will leave it up to you guys on if you want to close those.

Anyways, what about adjusting the stride based on phaseX?

In pseudo-code:

open override func drawValues(context: CGContext) {
    ///snip
    let isAnimatingX = phaseX != 1.0
    let through = isAnimatingX ?
        Int(ceil(Double(_xBounds.max - _xBounds.min) * phaseX + Double(_xBounds.min))) - 1 :
        Int(ceil(Double(_xBounds.max - _xBounds.min) * phaseX + Double(_xBounds.min)))

    for j in stride(from: _xBounds.min, through: through, by: 1) {

    }
    ///snip
}

I think that resolves the issue, but some comments or rationale on the equation used to calculate the to/through value would be helpful. That equation is pretty dense so personally I'd prefer to see it broken down into separate lines or a helper function on XBounds (maxVisibleValue(forPhaseX:) or something like that).

@ryanschneider
Copy link

Hmm, actually, after playing with the above code in the ChartsDemo, I think the issue is actually related to XBounds.

In both master and the code above, there's a completely separate issue in the animation where the last value pops in much later than the other ones.

This is very easy to see if you use the "Animate XY" option in the demo app Line Chart with the above code change.

I probably won't get much more time to look at it today, but my guess is there's something subtly wrong in XBounds.set(...), and that if that was fixed it would work for both drawValues and the other drawing functions for circles and lines.

@liuxuan30
Copy link
Member

I agree, it's different issue. However, I am not able to fine where the lag happens..

@ryanschneider
Copy link

FWIW I've been using the proposed fix:

for j in stride(from: _xBounds.min, to: Int(ceil(Double(_xBounds.max - _xBounds.min) * phaseX + Double(_xBounds.min))), by: 1)

In our app. However, there seems to be an edge case that's not handled now, as we are getting some crashes on the next line:

guard let e = dataSet.entryForIndex(j) else { break }

Symbolication seem to be completely broken in iOS XCode 8, so I'm having to do this manually, but the crash looks to be because j is out of bounds for _value in entryForIndex. If I manually run symbols /path/to/My.app/Frameworks/Charts.framework/Charts then work back through the addresses in the crash log, I get:

- 0x000000000005c7f8 (    0x30) specialized Array._checkSubscript(Int, wasNativeTypeChecked : Bool) -> _DependenceToken [FUNC, PEXT, NameNList, MangledNameNList, Merge
d, NList, FunctionStarts]
- 0x000000000005ac64 (   0x104) ChartDataSet.entryForXValue(Double, rounding : ChartDataSetRounding) -> ChartDataEntry? [FUNC, EXT, NameNList, MangledNameNList, Merged
, NList, FunctionStarts]
- 0x00000000000ac0ac (   0xd70) specialized LineChartRenderer.drawValues(context : CGContext) -> () [FUNC, EXT, LENGTH, NameNList, MangledNameNList, Merged, NList, Dwa
rf, FunctionStarts]

I'm still not sure if the root cause is in XBounds or the "ceil * phase" mapping to a value in the through: argument, just wanted to leave this here so people know this fix has potential issues.

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

No branches or pull requests

5 participants