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

Highlighter fix #4721

Merged
merged 5 commits into from
May 25, 2022
Merged

Highlighter fix #4721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
41ae51d
Fixed the use of partitioningIndex(where:) in ChartDataSet.entriesFor…
mbharry Sep 23, 2021
799ec91
Merge branch 'master' into 4719-highlighter-fix
kcome Mar 20, 2022
85cc87d
Fix rounding method to "closest" if there is a result.
kcome Apr 1, 2022
e2d2038
Add back `setNeedsDisplay` when deselect currently highlighted bar.
kcome Apr 3, 2022
30538b3
Update closestXValue when rounding method is closest.
kcome Apr 12, 2022
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
1 change: 1 addition & 0 deletions Source/Charts/Charts/ChartViewBase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ open class ChartViewBase: NSUIView, ChartDataProvider, AnimatorDelegate
{
delegate?.chartValueNothingSelected?(self)
}
setNeedsDisplay()
return
}

Expand Down
25 changes: 18 additions & 7 deletions Source/Charts/Data/Implementations/Standard/ChartDataSet.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ open class ChartDataSet: ChartBaseDataSet

guard !isEmpty else { return }

let indexFrom = entryIndex(x: fromX, closestToY: .nan, rounding: .down)
let indexTo = entryIndex(x: toX, closestToY: .nan, rounding: .up)
let indexFrom = entryIndex(x: fromX, closestToY: .nan, rounding: .closest)
var indexTo = entryIndex(x: toX, closestToY: .nan, rounding: .up)
if indexTo == -1 { indexTo = entryIndex(x: toX, closestToY: .nan, rounding: .closest) }

guard indexTo >= indexFrom else { return }
// only recalculate y
Expand Down Expand Up @@ -197,9 +198,11 @@ open class ChartDataSet: ChartBaseDataSet
open override func entriesForXValue(_ xValue: Double) -> [ChartDataEntry]
{
let match: (ChartDataEntry) -> Bool = { $0.x == xValue }
Copy link

@PWrzesinski PWrzesinski May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I believe this line should be:

let match: (ChartDataEntry) -> Bool = { $0.x >= xValue }

Note the >= instead of ==.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain why?

Copy link

@PWrzesinski PWrzesinski May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Actually nevermind, sorry for bothering you! Please disregard this and other comments - I'll delete them.

My thinking was that partitioningIndex function requires that all values after the desired index match the expression, but after looking more carefully I see the added partition function before running partitioningIndex - I'm guessing the intention is to make it possible to search in non-ordered data. I'm a little worried about the performance here, but otherwise I think the implementation is correct, except one detail: comparing Doubles directly. This is probably bigger discussion about the library design, but basically due to Double representation in memory, we should never compare Doubles using the equality operator. This will break for example in a simple case: 0.3 != 0.1 + 0.2. But as I said, that's probably a separate discussion.

let i = partitioningIndex(where: match)
var partitioned = self.entries
_ = partitioned.partition(by: match)
let i = partitioned.partitioningIndex(where: match)
guard i < endIndex else { return [] }
return self[i...].prefix(while: match)
return partitioned[i...].prefix(while: match)
}

/// - Parameters:
Expand All @@ -214,9 +217,9 @@ open class ChartDataSet: ChartBaseDataSet
rounding: ChartDataSetRounding) -> Int
{
var closest = partitioningIndex { $0.x >= xValue }
guard closest < endIndex else { return -1 }
guard closest < endIndex else { return rounding == .closest ? (endIndex-1) : -1 }

let closestXValue = self[closest].x
var closestXValue = self[closest].x

switch rounding {
case .up:
Expand All @@ -234,7 +237,15 @@ open class ChartDataSet: ChartBaseDataSet
}

case .closest:
break
// The closest value in the beginning of this function
// `var closest = partitioningIndex { $0.x >= xValue }`
// doesn't guarantee closest rounding method
if closest > 0 {
let distanceAfter = abs(self[closest].x - xValue)
let distanceBefore = abs(self[closest-1].x - xValue)
distanceBefore < distanceAfter ? closest -= 1 : ()
closestXValue = self[closest].x
}
}

// Search by closest to y-value
Expand Down