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

Impossible to make closed PathComponent #97

Open
typoland opened this issue Jul 14, 2021 · 19 comments
Open

Impossible to make closed PathComponent #97

typoland opened this issue Jul 14, 2021 · 19 comments

Comments

@typoland
Copy link

Hi, maybe this is not a issue, but I don't know make closed PathComponent. I have var curves : [CubicCurve] array where last.endingPoint == first.startPoint. I expect to get closed PathComponent, form PathComponent(curves: curves) but I receive "curves are not contiguous." error.

@typoland
Copy link
Author

typoland commented Jul 14, 2021

This is listing of .startPoint and .endPoint of curves send to PathComponent(curves: ...)

(548.9743825215003, 325.0) (341.15500052978894, 24.26896272382021)
(341.15500052978894, 24.268962723820202) (133.33561853807763, 325.0)
(133.33561853807763, 325.0) (341.15500052978894, 625.7310372761798)
(341.15500052978894, 625.7310372761798) (548.9743825215003, 325.0)`

Strange.

@hfutrell
Copy link
Owner

hfutrell commented Jul 14, 2021

@typoland If I'm understanding your data correctly it looks like your issue may be with minuscule differences in the y coordinate of the ending point of your first curve and starting point of your second. They need to be exactly equal, which I'd recommend forcing one way or the other or taking an average.

24.26896272382021

versus

24.268962723820202

@typoland
Copy link
Author

typoland commented Jul 14, 2021

Oh! In the world of CGFloats it could be difficult sometimes ;) . This time I assigned values from previous curve to the next and result is the same:

(548.9743825215003, 325.0) (341.15500052978894, 24.26896272382021)  
(341.15500052978894, 24.26896272382021) (133.33561853807763, 325.0)  
(133.33561853807763, 325.0) (341.15500052978894, 625.7310372761798)  
(341.15500052978894, 625.7310372761798) (548.9743825215003, 325.0)  

This listings made by:

componentCurves.curves.forEach({print (index, $0.startingPoint, $0.endingPoint)})

@typoland
Copy link
Author

typoland commented Jul 14, 2021

I suspect those lines in Init:

curves.forEach {
    assert($0.startingPoint == temp.last!, "curves are not contiguous.")
    temp += $0.points[1...]
}
...

@typoland
Copy link
Author

(Sorry for spamming here, I have to learn how to use this "issues". And markdown).

@hfutrell
Copy link
Owner

hfutrell commented Jul 14, 2021

it's definitely coming from this assert ... but strange you would hit this even when forcing the start and end points to be equal (I can't see how this would logically happen). Can you reproduce the issue with a code snippet I could run?

@typoland
Copy link
Author

I will. Give me some time. (I'm curious, maybe it's on my side somehow)

@typoland
Copy link
Author

OK. I rounded x and y of all points to third place after comma - no issue anymore.

BTW: Is it possible to automatically reverse inner PathComponent to maka a hole somehow?

@hfutrell
Copy link
Owner

hfutrell commented Jul 14, 2021

BTW: Is it possible to automatically reverse inner PathComponent to maka a hole somehow?

One way you can accomplish this is not by modifying the path, but when you go to render the path you can use the .evenOdd rule instead of .winding.

But if you must modify the path itself so that it works with the .winding fill rule: BezierKit has a .reversed() method that applies to BezierCurve, Path, and PathComponent which you may have seen. There is no way to apply this automatically, however one approach you could use is to find a point that you expect not to be filled, such as the center of your "hole" and test Path.contains(_ point: CGPoint, using rule: PathFillRule = .winding). If the result of the contains method is wrong then you know that your hole needs to be reversed.

@typoland
Copy link
Author

Super! I will try, thank you!

BTW: good job!

@typoland
Copy link
Author

typoland commented May 7, 2022

After almost a year I still don't know how to close a path. This is a small app showing a problem: https://github.com/typoland/HowToCloseCurveInBezierKit

@hfutrell
Copy link
Owner

hfutrell commented May 8, 2022

After almost a year I still don't know how to close a path. This is a small app showing a problem: https://github.com/typoland/HowToCloseCurveInBezierKit

It looks like you're creating 1 component for each curve. What this means is that when you stroke the path each curve will be stroked individually. This can make discontinuities because the stroking happens on a per-component basis and does not try to make different components continuous with each other. I think that might be the source of the things you've circled as "bad".

What you probably want is one component for the inner loop of the O and one component for the outer loop of the O. I haven't tested this, but something like this:

edit: this comment was wrong. I should have run the actual app

@typoland
Copy link
Author

typoland commented May 8, 2022

But curvesOfO: [[ BezierCurve ]] = [[...<Outer curve curves>...], [...<Inner curve curves>...]] and I add them in a loop

    for curve in curvesOfO {
        components.append(PathComponent(curves: curve))
    }

Should be OK, but it isn't :(

@hfutrell
Copy link
Owner

hfutrell commented May 8, 2022

this looks like it should work, what goes wrong in this case?

@typoland
Copy link
Author

typoland commented May 8, 2022

Paths are not closed. First point is not connected with a last one. It’s why I made this small app. I must close those shapes and I don’t know how.

@hfutrell
Copy link
Owner

hfutrell commented May 8, 2022

I see what's going on now. I was wrong in my initial assessment of what was going on in your app. Here is a workaround for you.

//
//  ContentView.swift
//  Shared
//
//  Created by Łukasz Dziedzic on 07/05/2022.
//

import SwiftUI
import BezierKit

extension BezierKit.Path {
    var closedCGPath: CGPath {
        let mutablePath = CGMutablePath()
        self.components.forEach {
            guard $0.isClosed else {
                mutablePath.addPath(BezierKit.Path(components: [$0]).cgPath)
                return
            }
            mutablePath.move(to: $0.startingPoint)
            for curve in $0.curves {
                let points = curve.points
                switch curve.order {
                case 1:
                    mutablePath.addLine(to: points[1])
                case 2:
                    mutablePath.addQuadCurve(to: curve.points[2],
                                             control: curve.points[1])
                case 3:
                    mutablePath.addCurve(to: curve.points[3],
                                         control1: curve.points[1],
                                         control2: curve.points[2])
                default:
                    break
                }
            }
            mutablePath.closeSubpath()
        }
        return mutablePath
    }
}

struct ContentView: View {
    var path: SwiftUI.Path {
        
        let curvesOfO: [[BezierCurve]] = [
           [ CubicCurve(p0: CGPoint(x: 118.0, y: 350.0),
                       p1: CGPoint(x: 187.0, y: 541.0),
                       p2: CGPoint(x: 233.0, y: 604.0),
                       p3: CGPoint(x: 300.0, y: 604.0)) as BezierCurve,
            CubicCurve(p0: CGPoint(x: 300.0, y: 604.0),
                       p1: CGPoint(x: 505.0, y: 604.0),
                       p2: CGPoint(x: 551.0, y: 541.0),
                       p3: CGPoint(x: 481.0, y: 350.0)) as BezierCurve,
            CubicCurve(p0: CGPoint(x: 481.0, y: 350.0),
                       p1: CGPoint(x: 481.0, y: 158.0),
                       p2: CGPoint(x: 436.0, y: 95.0),
                       p3: CGPoint(x: 300.0, y: 95.0)) as BezierCurve,
            CubicCurve(p0: CGPoint(x: 300.0, y: 95.0),
                       p1: CGPoint(x: 163.0, y: 95.0),
                       p2: CGPoint(x: 118.0, y: 158.0),
                       p3: CGPoint(x: 118.0, y: 350.0)) as BezierCurve],
            [CubicCurve(p0: CGPoint(x: 618.0, y: 350.0),
                       p1: CGPoint(x: 702.0, y: 580.0),
                       p2: CGPoint(x: 590.0, y: 705.0),
                        p3: CGPoint(x: 300.0, y: 705.0)) as BezierCurve,
            CubicCurve(p0: CGPoint(x: 300.0, y: 705.0),
                       p1: CGPoint(x: 177.0, y: 705.0),
                       p2: CGPoint(x: 65.0, y: 580.0),
                       p3: CGPoint(x: -18.0, y: 350.0)) as BezierCurve,
             CubicCurve(p0: CGPoint(x: -18.0, y: 350.0),
                        p1: CGPoint(x: -18.0, y: 119.0),
                        p2: CGPoint(x: 93.0, y: -5.0),
                        p3: CGPoint(x: 300.0, y: -5.0)) as BezierCurve,
             CubicCurve(p0: CGPoint(x: 300.0, y: -5.0),
                        p1: CGPoint(x: 506.0, y: -5.0),
                        p2: CGPoint(x: 618.0, y: 119.0),
                        p3: CGPoint(x: 618.0, y: 350.0)) as BezierCurve]]
        var components = [PathComponent]()
        
        for curve in curvesOfO {
            components.append(PathComponent(curves: curve))
        }
        
        return Path(BezierKit.Path(components: components).closedCGPath)
    }
    
    var body: some View {
        path.stroke(style: StrokeStyle(lineWidth: 75, lineCap: .butt, lineJoin: CGLineJoin.bevel, miterLimit: 6))
    }
}

struct ContentView_Previews: PreviewProvider {
    static var previews: some View {
        Group {
            ContentView()
        }
    }
}

What's going on is that CoreGraphics did not consider the paths closed for the purpose of stroking because they didn't have a .closeSubpath() command. I was unaware that CoreGraphics had this behavior and I try to match CoreGraphics behavior in BezierKit wherever possible. Thank you for filing the report. This is something I should fix.

@typoland
Copy link
Author

typoland commented May 8, 2022

Thank you!

Seems to work now. :)
(I can’t figure out what it does exactly, but maybe I don’t need it.

@hfutrell
Copy link
Owner

hfutrell commented May 8, 2022

Basically the workaround I gave you checks if BezierKit considers the path closed, and if so adds the missing .closeSubpath() command to the list of drawing commands to generate the CoreGraphics version of the path.

At some point I'll fix the issue so that the workaround won't be needed.

@typoland
Copy link
Author

typoland commented May 9, 2022

Great!

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

No branches or pull requests

2 participants