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.

Sign up for GitHub

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

Jump to bottom

Fix line chart x axis animation #4093, also close #3960 #4094

Merged
merged 1 commit into from
Sep 9, 2019
Merged

Fix line chart x axis animation #4093, also close #3960 #4094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Source/Charts/Renderers/BarLineScatterCandleBubbleRenderer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,14 @@ extension BarLineScatterCandleBubbleRenderer.XBounds: Sequence {
}

public func makeIterator() -> Iterator {
return Iterator(min: self.min, max: self.max)
return Iterator(min: self.min, max: self.min + self.range)
}
}

extension BarLineScatterCandleBubbleRenderer.XBounds: CustomDebugStringConvertible
{
public var debugDescription: String
{
return "min:\(self.min), max:\(self.max), range:\(self.range)"
}
}
11 changes: 6 additions & 5 deletions Source/Charts/Renderers/LineChartRenderer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,8 @@ open class LineChartRenderer: LineRadarRenderer
// Take an extra point from the left, and an extra from the right.
// That's because we need 4 points for a cubic bezier (cubic=4), otherwise we get lines moving and doing weird stuff on the edges of the chart.
// So in the starting `prev` and `cur`, go -2, -1
// And in the `lastIndex`, add +1

let firstIndex = _xBounds.min + 1
let lastIndex = _xBounds.min + _xBounds.range

var prevPrev: ChartDataEntry! = nil
var prev: ChartDataEntry! = dataSet.entryForIndex(max(firstIndex - 2, 0))
Expand All @@ -133,7 +131,7 @@ open class LineChartRenderer: LineRadarRenderer
// let the spline start
cubicPath.move(to: CGPoint(x: CGFloat(cur.x), y: CGFloat(cur.y * phaseY)), transform: valueToPixelMatrix)

for j in stride(from: firstIndex, through: lastIndex, by: 1)
for j in _xBounds.dropFirst() // same as firstIndex
{
prevPrev = prev
prev = cur
Expand Down Expand Up @@ -316,8 +314,8 @@ open class LineChartRenderer: LineRadarRenderer
// Allocate once in correct size
_lineSegments = [CGPoint](repeating: CGPoint(), count: pointsPerEntryPair)
}
for j in stride(from: _xBounds.min, through: _xBounds.range + _xBounds.min, by: 1)

for j in _xBounds.dropLast()
{
var e: ChartDataEntry! = dataSet.entryForIndex(j)

Expand All @@ -328,6 +326,9 @@ open class LineChartRenderer: LineRadarRenderer

if j < _xBounds.max
{
// TODO: remove the check.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat incorrect, and the iterator in XBounds is theoretically incorrect.
As the point is to iterate through min + range - where range could go over max due to a bouncing animation or something similar.

The thing is that this XBounds represents (in all current cases) a range of entries, and going over max could mean an IndexOutOfBounds.
That's why the check is there.

If the iterator only iterates until max - then it's a bug and it's not respecting the animation (range is basically (max - min) * animation.PhaseX).

Also I would change the iterator's fileprivate init(min: Int, max: Int) { to fileprivate init(min: Int, through: Int) {, as it's really confusing right now, both with the max property and with to vs through semantics.

Copy link
Member Author

@liuxuan30 liuxuan30 Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @danielgindi I came across here while I am working on the 4.0->master merge.

I'm a little confusing that you saying

the iterator in XBounds is theoretically incorrect.

Which iterator you say is wrong? my return Iterator(min: self.min, max: self.min + self.range) or return Iterator(min: self.min, max: self.max) ?

because from the old session I wrote here #4093
It seems the iterator of xbounds now is correct:

    public func makeIterator() -> Iterator {
        return Iterator(min: self.min, max: self.min + self.range)
    }

I will rename the iterator to through while merging 4.0.

// With the new XBounds iterator, j is always smaller than _xBounds.max
// Keeping this check for a while, if xBounds have no further breaking changes, it should be safe to remove the check
e = dataSet.entryForIndex(j + 1)

if e == nil { break }
Expand Down