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

Builds and runs on Swift 3 #24

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Builds and runs on Swift 3 #24

wants to merge 7 commits into from

Conversation

algal
Copy link

@algal algal commented Feb 7, 2017

Hello. Love your book. (We met briefly at a conference a while ago, btw.)

Anyway, I've updated the project so it builds on Swift 3 with zero errors or warnings.

I have not rigorously tested this. I just wanted to get it running so I could play with it. But it seems to work.

Mostly this was automatic updates except for the following manual changes:

  • AnyObject -> Any
  • breaking out some tricky double-dictionary unwrapping with optionals into a function
  • Adding an optionals within a nested UnsafePointer structure to use it with vImage.
  • updates to remove deprecated GCD methods

@algal algal changed the title Builds and runs on Swift 3.1 Builds and runs on Swift 3 Feb 7, 2017
@FlexMonkey
Copy link
Owner

FlexMonkey commented Feb 8, 2017

Hi Alex,

Thanks a million for this. Because of the terms of employment with my new employer, I need to get approval to merge your changes. I've noticed a few issues - default NSNumber* values seem a bit funky on some of the custom filters and the Metal based filters crash (I'd like to move those to the new CI image processor).

I'll try to find time to look at those issues and get your merge signed off 🙂

Thanks again!

Simon

*Yep - looks like having parameters as CGFloat no longer works, they need to be NSNumber with some other tweaks throughout the code.

@algal
Copy link
Author

algal commented Feb 9, 2017

Hi. This fixes one bug in fixFilterParameterValues() by adding a cast from Float to NSNumber. This improves behavior where, for instance, one is using a slider to set RGBA values individually to specify a color.

I don't know the expected behavior of the app everywhere, so I'm not sure how to look for all other cases of this sort of problem systematically.

I'm not 100% sure what is going on. But the key thing, I think, is that whenever one is setting a value of a dictionary that will be consumed by CoreImage, to assign an NSNumber and not a Float or CGFloat. But if you're reading a value from the dictionary, then you can do an optional binding to Float successfully, no matter if the original value was an NSNumber or Float or nil.

And weirdly, trying d["key"] is Float and d["key"] is NSNumber will both return true, so you cannot use that to distinguish the type of instance that was saved into the dictionary.

@algal
Copy link
Author

algal commented Feb 9, 2017

If you can give me a pointer about what to look for to find other places that need some of these casting fixes, I'm happy to chase them down.

On second thought, I'm not sure why the as NSNumber should be necessary at all, because I believe it should automatically convert from Float to NSNumber. I am confused.

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

Successfully merging this pull request may close these issues.

2 participants