-
Notifications
You must be signed in to change notification settings - Fork 26
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
Circular arc approximation #72
base: master
Are you sure you want to change the base?
Conversation
import Foundation | ||
|
||
extension PathComponent { | ||
public convenience init( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make the public interface on Path
with an interface init(ellipseIn rect: CGRect)
. I'm trying to keep the API as similar to CGPath
and SwiftUI's Path
as much as possible which means it's desirable for the API to operate on Path
objects instead of PathComponent
objects where possible. For now I think that also means hiding the numberOfCurves
parameter as well (we can expose it later on potentially, it's just harder to take public API functionality away than to add it).
public convenience init( | ||
arcCenter center: CGPoint, | ||
radius: CGFloat, | ||
startAngle: CGFloat, | ||
endAngle: CGFloat, | ||
clockwise: Bool, | ||
numberOfCurves: Int = 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this private
for now. In the future I'm going to add an API for the creation of Path
objects that works similar to CGMutablePath
. When that's ready we can use your code to add the functionality of CGMutablePath.addArc(center: CGPoint, radius: CGFloat, startAngle: CGFloat, endAngle: CGFloat, clockwise: Bool, transform: CGAffineTransform = .identity)
CubicCurve(points: $0.points.map { | ||
$0.applying(finalTransform) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use copy(using: CGAffineTransform) -> Self
instead of transforming the array of points.
static let demo23 = Demo(title: "PathComponent(arcCenter: ...)", | ||
quadraticControlPoints: [], | ||
cubicControlPoints: [], | ||
drawFunction: {(context: CGContext, demoState: DemoState) in | ||
if demoState.quadratic { | ||
return | ||
} | ||
|
||
let pathComponent = PathComponent( | ||
arcCenter: CGPoint(x: 100, y: 105), | ||
radius: 100, | ||
startAngle: .pi / 2, | ||
endAngle: 2 * .pi, | ||
clockwise: true | ||
) | ||
Draw.drawPathComponent(context, pathComponent: pathComponent) | ||
for curve in pathComponent.curves { | ||
Draw.drawSkeleton(context, curve: curve) | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned previously, I think we'll have to make the arc functionality private for now.
But when it's ready to be public it'd be cool to make this demo more interactive. Maybe we could drag the endpoints to set the start and end angle, for example?
static let demo24 = Demo(title: "PathComponent(ovalIn rect: CGRect)", | ||
quadraticControlPoints: [], | ||
cubicControlPoints: [], | ||
drawFunction: {(context: CGContext, demoState: DemoState) in | ||
if demoState.quadratic { | ||
return | ||
} | ||
|
||
let pathComponent = PathComponent( | ||
ovalIn: CGRect( | ||
x: 0, y: 52.5, | ||
width: 200, height: 100 | ||
) | ||
) | ||
Draw.drawPathComponent(context, pathComponent: pathComponent) | ||
Draw.setColor(context, color: Draw.pinkish) | ||
Draw.drawBoundingBox(context, boundingBox: pathComponent.boundingBox) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's try to make this demo more interactive. We could let the user drag the origin and scale the oval perhaps
let transform = CGAffineTransform.identity | ||
.translatedBy(x: rect.origin.x, y: rect.origin.y) | ||
.scaledBy( | ||
x: rect.width / diameter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that if the rect is zero sized this will cause a divide by zero. It's an edge case, but actually quite likely to occur in practice. We should check how CGPath(ellipseIn:)
handles zero sized rects and use the same fallback behavior.
ovalIn rect: CGRect, | ||
numberOfCurves: Int = 4 | ||
) { | ||
let diameter = min(rect.width, rect.height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should look at how this behaves with negative dimension rectangles. Core Graphics actually allows this and we should be sure to behave in the same way. We should also consider what happens if the caller passes CGRect.null
for the rect
parameter.
This PR adds ability to create circles and ellipsis using pure BezierKit API and cubic curve arc approximation as described here.
P.S. I'm a total noob at writing tests thus sorry about this