Skip to content

Fix bug in XBounds calculation of minimum/maximum visible entry index.#4839

Open
fumoboy007 wants to merge 1 commit intoChartsOrg:masterfrom
fumoboy007:master
Open

Fix bug in XBounds calculation of minimum/maximum visible entry index.#4839
fumoboy007 wants to merge 1 commit intoChartsOrg:masterfrom
fumoboy007:master

Conversation

@fumoboy007
Copy link

XBounds looks for data entries on the boundary of or outside of the visible range. However, there may not be any such entries for a particular data set.

Imagine a chart with two data sets, one of which has a bigger range of x-values than the other. The chart’s zoomed-out visible range would be the range of the wider data set. When the renderer tries to render the narrower data set, it would fail to correctly calculate the starting and ending entry indices because the starting/ending entries are strictly inside of the visible range.

The fix is to add fallback logic to look for data entries inside of the visible range if the initial algorithm does not find anything.

The XBounds bug fix also requires a bug fix in ChartDataSet’s binary search algorithm.

`XBounds` looks for data entries on the boundary of or outside of the visible range. However, there may not be any such entries for a particular data set.

Imagine a chart with two data sets, one of which has a bigger range of x-values than the other. The chart’s zoomed-out visible range would be the range of the wider data set. When the renderer tries to render the narrower data set, it would fail to correctly calculate the starting and ending entry indices because the starting/ending entries are strictly inside of the visible range.

The fix is to add fallback logic to look for data entries inside of the visible range if the initial algorithm does not find anything.

The `XBounds` bug fix also requires a bug fix in `ChartDataSet`’s binary search algorithm.
Copy link

@k1ran-ak k1ran-ak left a comment

Choose a reason for hiding this comment

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

Reviewed. No crashes. I didn't see any Lower bound <= upper bound errors anymore

@FelixHerrmann
Copy link
Contributor

This conflicts with #4829 where I fixed the partitioningIndex algorithm change

@FelixHerrmann
Copy link
Contributor

My PR might fix your whole problem as you can see in the screenshot below! Could you test my fork and confirm?

image

@fumoboy007
Copy link
Author

@FelixHerrmann Cool! Confirmed that #4829 also partially fixes the XBounds calculation. However, I commented on part of the change, which I believe is incorrect.

@tri75
Copy link

tri75 commented Jul 12, 2022

Please merge this commit soon. thanks.

Joe-BOD pushed a commit to beachbodydigital/Charts that referenced this pull request Aug 2, 2022
* Update for the two fixes in PR now for Charts
* Swift 5.7 - ChartsOrg#4874
* XAxis beyond Chart Dataset - ChartsOrg#4839
@lundjrl
Copy link

lundjrl commented Dec 20, 2022

Hey all, any updates on merging this in? Love the library, awesome work on it!

@FelixHerrmann
Copy link
Contributor

@lundjrl should be fixed with #4829

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.

5 participants