Add support for rounded corners for BarChart#3754
Add support for rounded corners for BarChart#3754skarol wants to merge 1 commit intoChartsOrg:roundedBarsfrom
Conversation
|
except for CI errors, is problem solved for what I asked #1066 (comment) ? |
|
|
@skarol I mean if the bars are thin enough, it becomes an oval more like a bar with rounded corners. That's what I said a flaw in #1066 (comment), and hold similar PRs till now. Have you solved this? (sorry I don't have time to download your PR and test it on my side so far) If so, I could take a deep look and maybe we could fix the CI errors and get this merged. |
|
@liuxuan30 yes, I've solved it. Basically, corner radius is half of bar's width for |
|
That's fantastic. we will take time for this. But maybe longer :( |
|
@jjatie which branch do you think we should use to merge this PR? I'm asking because this is a nice feature that waited for too long (if it solves my questions), so I'm hoping to add them soon in master. But I'm seeing conflicts with 4.0 refactor. |
| if dataProvider.isDrawBarShadowEnabled | ||
| { | ||
| for j in stride(from: 0, to: buffer.rects.count, by: 1) | ||
| for firstIndexInBar in stride(from: 0, to: buffer.rects.count, by: 1) |
There was a problem hiding this comment.
why change name here? seems nothing about first index
There was a problem hiding this comment.
I have the same question. If anything the change should be:
for barRect in buffer.rects
where viewPortHandler.isInBoundsLeft(barRect.maxX)
{
guard viewPortHandler.isInBoundsRight(barRect.origin.x) else
{
break
}
context.setFillColor(dataSet.barShadowColor.cgColor)
context.fill(barRect)
}
| return element | ||
| } | ||
|
|
||
| private func findTopRectInBar(barRects: [CGRect], firstIndexInBar: Int, lastIndexInBar: Int) -> CGRect { |
There was a problem hiding this comment.
let's put all { to another line. This is the code style for this library
| high.setDraw(x: barRect.midY, y: barRect.origin.x + barRect.size.width) | ||
| } | ||
|
|
||
| override internal func createBarPath(for rect: CGRect, roundedCorners: UIRectCorner) -> UIBezierPath { |
There was a problem hiding this comment.
could bar renderer and horizontal bar renderer share the same createBarPath and findMostLeftRectInBar somehow?
| FBSnapshotVerifyView(chart, identifier: Snapshot.identifier(UIScreen.main.bounds.size), tolerance: Snapshot.tolerance) | ||
| } | ||
|
|
||
| func testRoundedCorners() { |
There was a problem hiding this comment.
not seeing updated images for testing rounded corners.
| context.setFillColor(dataSet.color(atIndex: 0).cgColor) | ||
| } | ||
|
|
||
| context.setStrokeColor(borderColor.cgColor) |
There was a problem hiding this comment.
it seems you are using drawBorder properties for rounded corners. Why force setting setLineCap(.square)?
But what if people disable drawBorder?
| let stackSize = isStacked ? dataSet.stackSize : 1 | ||
|
|
||
| for j in stride(from: 0, to: buffer.rects.count, by: 1) | ||
| for firstIndexInBar in stride(from: 0, to: buffer.rects.count, by: stackSize) |
There was a problem hiding this comment.
Seems like it was done for the stacked bar char case, if round each bar when we will have this situation (from my rounding implementation).
Author's example:
There was a problem hiding this comment.
Seems like it was done for the stacked bar char case, if round each bar when we will have this situation (from my rounding implementation).
Author's example:
That's exactly what I wanted to do. Round corner each bar with space in between. Do you have a demo project?
There was a problem hiding this comment.
Hello @panncherry
You can check my fork/branch. Do not remember if bothered with an example adjustment as not going to upload a pull request, but the changes should be pretty straightforward to update existing StackedBarChar from the ChartsDemo-iOS
|
@skarol it seems CI failed because multiple bar chart tests failures. If rounded corner is turned on by default, then that's the reason. we need to have a switch to turn it off and add a new rounded bar test. |
| if dataProvider.isDrawBarShadowEnabled | ||
| { | ||
| for j in stride(from: 0, to: buffer.rects.count, by: 1) | ||
| for firstIndexInBar in stride(from: 0, to: buffer.rects.count, by: 1) |
There was a problem hiding this comment.
I have the same question. If anything the change should be:
for barRect in buffer.rects
where viewPortHandler.isInBoundsLeft(barRect.maxX)
{
guard viewPortHandler.isInBoundsRight(barRect.origin.x) else
{
break
}
context.setFillColor(dataSet.barShadowColor.cgColor)
context.fill(barRect)
}
| { | ||
| // Set the color for the currently drawn value. If the index is out of bounds, reuse colors. | ||
| context.setFillColor(dataSet.color(atIndex: j).cgColor) | ||
| for index in firstIndexInBar...lastIndexInBar { |
There was a problem hiding this comment.
let barRects = buffer.rects[firstIndexInBar...lastIndexInBar]
for (index, rect) in zip(barRects, barRects.indices)
where viewPortHandler.isInBoundsLeft(barRect.maxX)
{
...
}
| } | ||
|
|
||
| var height: CGFloat = 0 | ||
| for index in firstIndexInBar...lastIndexInBar { |
There was a problem hiding this comment.
topRectInBar.size.height = barRects[firstIndexInBar...lastIndexInBar]
.reduce(0) { $0 + $1.height }
| } | ||
|
|
||
| /// Creates path for bar in rect with rounded corners | ||
| internal func createBarPath(for rect: CGRect, roundedCorners: UIRectCorner) -> UIBezierPath { |
There was a problem hiding this comment.
internal is default and is not needed.
|
@liuxuan30 I think we should start only targeting 4.0.0. Will encourage us to get it out faster and with more features in it, encourage people to update and help us keep it bug free. |
|
Any update on this @skarol? @liuxuan30? :) |
|
not really.. have to postpone, working on 3.3 release and PRs |
|
Sorry, have been busy lately :/ I'll try to address this soon |
|
One more question about this PR for @skarol and @liuxuan30 - the rounded corners seem to be perfectly semicircular - what about exposing the option to round corners with a given corner radius? This would be much more flexible and design agnostic. I'm guessing it is not really in the spirit of such a library to dictate design decisions like this? :) |
|
@samjarman I decided to not expose corner radius and calculate it as a semicircular on a purpose. When creating a chart you have no idea about bars width and height so it is hard to pass good looking corner radius and semicircular always looks good :) |
|
Hmm, I thought it'd be possible for a developer to calculate their own corner radius (such as the one on these input fields and comments on github) based on the width of the bar? If you can't get the width of the bar, maybe set a corner radius and then a minimum width for the corner radius to be used? |
|
I think "semicircular always looks good" is our designers decisions not really a framework's decision, y'know? :) |
|
Some news about this? |
|
Really hoping to see this feature implemented |
|
If the main problem is that it does not look "right" in some situations, why not add an additional parameter? Something like enum Radius {
case none
case widthPercentage(CGFLoat)
case absolute(CGFLoat)
}With |
|
Some news about this? |
|
Looking forward to use this feature, any news? Thanks for the great work on this lib :D |
|
BarChartView support rounded corners now?I have took a deep look code,didn't look corner... |
|
Any updates on getting this merged @skarol @liuxuan30? A highly requested feature for sure! 👍 |
|
@narendra-ct unfortunately this is what I'm concerned about. You have to try to fix it yourself, or use the old implementation of rounded bars, if your bars are not too many. |
|
Can someone help me? I got This one works: This one doesn't: Anybody faced with this problem? pod 'Charts', :git => 'https://github.com/danielgindi/Charts.git', :branch => 'master' |
|
Maybe it helps someone. To get this feature works, add these to your pod file: If you have the same crash as me, just reinstall Charts in pod file. |
|
|
||
| let cornerRadius = rect.width / 2.0 | ||
|
|
||
| let path = UIBezierPath(roundedRect: rect, |
There was a problem hiding this comment.
@skarol I see a problem here when the rect is smaller than the cornerRadius, in this case you'll have a smashed rounded rectangle that looks bad(I haven't tested this code, but I worked in this feature in my fork, and at the start I had used the UIBezierPath like you with the result that I have commented, some screenshots at the end, apologies for the quality).
My proposal to solve this is to split it into two cases(Take into consideration that I haven't used the open var roundedCorners: UIRectCorner = []. I will always round all corners)
rect.height >= rect.widthin this case we can perform the UIBezierPath as you dorect.height < rect.widthin this case we can draw a portion of a circumference, I leave the algorithm
let height = rect.size.height
let hypotenuse = width / 2.0
let radius = hypotenuse
let startAngle: CGFloat
let endAngle: CGFloat
if height < radius {
let oLeg = radius - height
startAngle = asin(oLeg/hypotenuse)
endAngle = CGFloat(Double.pi) - startAngle
} else {
let oLeg = height - radius
startAngle = -asin(oLeg/hypotenuse)
endAngle = CGFloat(Double.pi) - startAngle
}
let y = rect.origin.y - (width - rect.size.height)
let center = CGPoint(x: rect.origin.x + radius, y: y + radius)
let bezierPath = UIBezierPath(arcCenter: center, radius: radius, startAngle: startAngle, endAngle: endAngle, clockwise: true)
Before
After
|
why this was closed? @danielgindi |
|
Rounded Rect seems an important requirement @danielgindi . Please reopen it and merge changes. |
Friends I can’t remember every PR but please read the history here. There are issues that weren’t addressed and there is another branch that does this. |
|
@danielgindi it's weird our branch |
|
I will reopen this one as this seems the best rounded bar by far. The poblem is we need help from @skarol, or we take it from here. |
|
Hi! Could you be nice to process this pull request? Cheers |
|
Hi everyone! There is something I could do to add the feature ASAP? Thanks, |
|
I put up a PR that adds this functionality as well: #4375 No clue if it will get in (likely not, for similar reasons as this one) - but its a more up to date approach you can probably pull in / play with. |
|
Hello.
Edge cases:
Corners |
|
just FYI that we are actively working on the long waited 4.0 merge right now, so if anyone want to contribute on this one, wait until we merge 4.0. |
Can I use this function for rounded corner? |
|
@liuxuan30 Could you say when this feature will be in the master branch? |
|
nope, not any estimatable time. I mean it's a big change and the author seems not responding. Besides, we are having 4.0 branch merging ahead, and 4.0 branch is already big enough and will cause more code break for this PR. Without the author's help, I don't think we can merge this fairly. |
|
Hello @liuxuan30 |
|
I want to give a corner radius to shadow in a bar chart view graph. How can I do this? @liuxuan30 |
|
why this was closed? @jjatie |
20 participants
Issue Link 🔗
Rounding the corners of bars #1066
Goals ⚽
BarChartViewandHorizontalBarChartView.dataSet.roundedCorners = [.allCorners].Implementation Details 🚧
roundedCornersproperty forBarChartDataSet, where you can specify which corners should be rounded.drawDataSetfunction inBarChartRendererandHorizontalBarChartRenderer, whereUIBezierPathis created to clip bar filling.drawHighlightedfunction inBarChartRenderer.Testing Details 🔍
Add
testRoundedCornerstest toBarChartTests.