TokamakUI / Tokamak

SwiftUI-compatible framework for building browser apps with WebAssembly and native apps for other platforms
Apache License 2.0
2.62k stars 111 forks source link

Implement Path.addArc(tangent1End:...). #526

Closed filip-sakel closed 1 year ago

filip-sakel commented 2 years ago

The addArc(tangent1End:tangent2End:radius:transform:) method was left unimplemented and added nothing to the path components. This implementation uses some vector arithmetic on CGPoint to create an arc. There are probably better approaches out there, but I think this a good starting point.

filip-sakel commented 2 years ago

Swiftlint is complaining about using a = a + b, but there’s no reason to implement += just for these operations. I don't know how I can disable this check for the places where this is used.

filip-sakel commented 2 years ago

I saw how other snapshot tests are written and think I understand how to write one, but I'm not sure how I can generate a snapshot to compare against.

I'll add the snapshot tests later in the week.

carson-katri commented 2 years ago

I think you could just run the test and copy the image it writes in, then compare against that going forward. Layout tests compare directly against a rendered SwiftUI view.

filip-sakel commented 2 years ago

I realized that my code doesn't really choose the start-end/clockwise combinations correctly. This means that the start of the arc may be on the second tangent and the arc's end will be on the first tangent.

So, I'll keep working on this to get everything exactly like the native implementations.

filip-sakel commented 1 year ago

I fixed the issue and added some more tests. However, I was unable to get the project to build so I could run the snapshot tests. I run the following according to the contribution guide to no avail:

swift build --product TokamakPackageTests && `xcrun --find xctest` .build/debug/TokamakPackageTests.xctest

I’m not sure why, but there seems to be a problem with core-graphics types, such as CGLineJoin and CGPoint. @carson-katri could you add the snapshot tests?