Skip to content

add new initializer for transform rotation to CGAffineTransform extension #395

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.

Sign up for GitHub

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

Merged
michaelknoch merged 4 commits into from
Aug 7, 2024
Merged

add new initializer for transform rotation to CGAffineTransform extension #395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5377644
add new API
michaelknoch Jul 31, 2024
eba0cd4
cleanup
michaelknoch Jul 31, 2024
2140bec
update xcode
michaelknoch Aug 7, 2024
789b1eb
Merge branch 'master' of github.com:flowkey/UIKit-SDL into add-api-UI…
michaelknoch Aug 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/AffineTransform.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,4 +359,4 @@ extension AffineTransform: CustomStringConvertible {
public var debugDescription: String {
return description
}
}
}
5 changes: 4 additions & 1 deletion Sources/CGAffineTransform.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ public extension CGAffineTransform {
self.init(scaleByX: scaleX, byY: y)
}

init(rotationAngle: CGFloat) {
self.init(rotationByRadians: rotationAngle)
}
Comment on lines +16 to +18

@ephemer ephemer Aug 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit confused by this and just looked into it. Apparently AffineTransform is a Foundation (only?) type that has this rotationByRadians initializer. CGAffineTransform, which AFAIK is what is actually used by UIView in UIKit, appears to have different initializers.

Our CGAffineTransform type is copied from Foundation's AffineTransform type, so it's a bit of a weird mix of both. I'm not sure what implications this has for code re-use. It's probably "close-enough" API-wise that we can use it as is, especially given this new extension. But we should be aware of the differences

@michaelknoch @reckjn

@michaelknoch michaelknoch Aug 7, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the info. I was confused as well tbh. Do you think our CGAffineTransform needs to be refactored because of this?

@ephemer ephemer Aug 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we actually intend to create a real CGAffineTransform type here. So I would suggest to actually rename the rotationByRadians initializer to be called rotationAngle, instead of the change in this PR.

It would be a bit weird to see both in code completion, and then not be actually source compatible if you choose the wrong one.


var isIdentity: Bool {
return self == .identity
}

}