Skip to content

Implement NSAffineTransform #93

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

Jump to bottom
Merged
merged 10 commits into from
Dec 9, 2015
Merged

Implement NSAffineTransform #93

merged 10 commits into from
Dec 9, 2015
+371 −45

Conversation

nubbel
Copy link

@nubbel nubbel commented Dec 9, 2015

Hi,

The changes in this PR improve the implementation of the transformation methods. The new implementation makes use of the fact that a 2D affine transformation matrix is not an arbitrary 3x3 matrix (see code comments), and is both simpler and more efficient by skipping unnecessary computation steps.
Also included is:

  • Implementation of NSAffineTransformStruct.init() which initializes all entries to zeros.
  • Implementation of NSAffineTransform.init(transform:)
  • Implementation of NSCopying methods for NSAffineTransform

Dominique d'Argent added 3 commits December 9, 2015 03:12
Implement NSAffineTransformStruct.init()
e7e1774 
Initializes all entries to zeros.
Implement NSAffineTransform.init(transform:).
a61d89f 
Initializes the receiver's matrix using another transform object and returns the receiver.
Implement NSCopying methods for NSAffineTransform
7aede49 
The `copy()` override is necessary because `NSObject.copy()` always returns `self` so far.
@nubbel nubbel force-pushed the pr/NSAffineTransform branch from 5b08c4a to 7926b21 Compare December 9, 2015 02:12
Simplify point/size transformations
47602c4 
…while making them more efficient.
@nubbel nubbel force-pushed the pr/NSAffineTransform branch from 7926b21 to 47602c4 Compare December 9, 2015 02:16
jessesquires
jessesquires reviewed Dec 9, 2015
View reviewed changes
Foundation/NSAffineTransform.swift

public init() {
(self.m11, self.m12, self.m21, self.m22) = (CGFloat(), CGFloat(), CGFloat(), CGFloat())
(self.tX, self.tY) = (CGFloat(), CGFloat())
Copy link
Contributor

Choose a reason for hiding this comment

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

this init could (should?) just call the full memberwise init below 😄

Copy link
Author

Choose a reason for hiding this comment

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

Ah, you're right. Thanks!

Dominique d'Argent added 2 commits December 9, 2015 11:52
NSAffineTransformStruct.init() calls memberwise initializer
b2dd23e
Improve readability
34d8865 
…by introduction helper varaibles and better spacing
@nubbel
Copy link
Author

nubbel commented Dec 9, 2015

Thank you, @jessesquires!
I've incorporated your feedback and updated the PR.

@nubbel nubbel changed the title Improve implementation of NSAffineTransform.transform{Point,Size} Implement NSAffineTransform methods Dec 9, 2015
@nubbel
Copy link
Author

nubbel commented Dec 9, 2015

I've updated the PR to include the following changes:

  • Implementation of translation, scaling and rotation methods with tests
  • Implementation for prependTransform and appendTransform methods

Tests for composed transformations are missing - working on it!
Implementation for invert is still in progress, I'm not happy with it yet.

@nubbel nubbel changed the title Implement NSAffineTransform methods Implement (most of) NSAffineTransform Dec 9, 2015
Implement NSAffineTransformStruct.invert() and tests
cb38823 
It also adds missing arithmetic operators to `CGFloat` which are needed for the matrix inversion.
@nubbel
Copy link
Author

nubbel commented Dec 9, 2015

I've updated the PR to include the following changes:

  • Implementation for invert
  • Added arithmetic operators for CGFloat which were needed for the matrix inversion.

Notes:

  • The original implementation throws an exception. My implementation uses preconditionFailure in this case.
  • NSAffineTransform implementation is almost complete now. Only the NSCoding methods are missing, but NSCoder itself isn't implemented yet.

@nubbel nubbel changed the title Implement (most of) NSAffineTransform Implement NSAffineTransform Dec 9, 2015
This was referenced Dec 9, 2015
Merged
Merged
@phausler phausler merged commit 1d6803c into swiftlang:master Dec 9, 2015
@parkera
Copy link
Contributor

parkera commented Dec 9, 2015

Be sure to update the Status doc when we finish a feature!

https://github.com/apple/swift-corelibs-foundation/blob/master/Docs/Status.md

parkera
parkera reviewed Dec 9, 2015
View reviewed changes
Foundation/NSAffineTransform.swift
[ 0 0 1 ]
*/
static func rotation(radians angle: CGFloat) -> NSAffineTransformStruct {
let α = Double(angle)
Copy link
Contributor

Choose a reason for hiding this comment

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

Love the use of unicode variable name here!

Copy link
Author

Choose a reason for hiding this comment

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

Cool, glad you like it. I wasn't sure if that is considered good style, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to use it in limited places like this, where it actually conveys some meaning.

@nubbel nubbel mentioned this pull request Dec 10, 2015
Merged
@nubbel
Copy link
Author

nubbel commented Dec 10, 2015

@parkera Sorry, I didn't know about that doc. I updated it in #117.

@nubbel nubbel mentioned this pull request Dec 16, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nubbel @parkera @jessesquires @phausler