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

Bug with showing negative values and vertical zooming Horizontal Bar Chart #1702

Closed
valery-demin opened this issue Oct 23, 2016 · 6 comments
Closed

Comments

@valery-demin
Copy link

I am trying to use these Charts for showing negative values on Horizontal Bar Chart.
For horizontal zooming it works well but for vertical it has bug.
I have prepared GIF with this bug (from Demo project):
horizontal bar chart

I have added -i instead of i in this string (https://github.com/danielgindi/Charts/blob/master/ChartsDemo/Classes/Demos/HorizontalBarChartViewController.m line 122):
[yVals addObject:[[BarChartDataEntry alloc] initWithX:-i * spaceForBar y:val]];

How can I avoid this behavior?

@liuxuan30 liuxuan30 added bug and removed bug labels Oct 26, 2016
@liuxuan30
Copy link
Member

liuxuan30 commented Oct 26, 2016

At first I can't reproduce so I removed the bug tag, until I see it changes x values to negative;
Now I can reproduce

liuxuan30 added a commit to liuxuan30/Charts that referenced this issue Oct 26, 2016
…ing bottom fails

Similar to Bar chart renderer logic, continue if checking left fails, while break if checking right fails
@liuxuan30
Copy link
Member

liuxuan30 commented Oct 26, 2016

I did a quick look, it is because the check

        for j in stride(from: 0, to: buffer.rects.count, by: 1)
        {
            let barRect = buffer.rects[j]

            if (!viewPortHandler.isInBoundsTop(barRect.origin.y + barRect.size.height))
            {
                break  // <-- where negative values break at first, so no rendering
            }

            if (!viewPortHandler.isInBoundsBottom(barRect.origin.y))
            {
                continue
            }

that break. However while testing it is far more than I estimate the impact. It happens in both bar chart and horizontal bar chart while zooming and scrolling. So my commit does not fix it at all.

When trying to fix negative x values, it breaks positive x values; so I guess we can't simply fix it by changing around break/continue.

For horizontal bar chart, what I found is that for negative values, the buffer.barRects origin y values are ascending, while the positive values, they are descending. Forcing sorting the origin y values could be a solution, but not perfect. @danielgindi any insights rather than sort barRects?

Bar chart should be similar.

Also we need to fix drawValues() too.

@liuxuan30
Copy link
Member

liuxuan30 commented Oct 26, 2016

@valery-demin if you are hurry, one quick workaround is you could try to make sure x values are ascending (-5,-4,-3,-2,-1,0), when adding them.

@liuxuan30
Copy link
Member

liuxuan30 commented Oct 27, 2016

@danielgindi I have thought over it again, we had an implication that the x values should be ascending; people are not aware of this so weird thing happens.
Ideally we should handle this but it seems it's a big change to handle the x values' order.

@danielgindi
Copy link
Collaborator

@liuxuan30 It must be ordered by definition, as we use the order to make quick searches through the values, and also optimize rendering loops by stopping when passed the last value to render.

@valery-demin you should use addEntryOrdered if you don't want to order your data by yourself...

@liuxuan30
Copy link
Member

addEntryOrdered() never think about it.. so x values should be ordered as well now.

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

3 participants