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

[SR-2388] Forming a CFDictionary from a Swift Dictionary is now really difficult #44995

Closed
mattneub opened this issue Aug 18, 2016 · 21 comments
Closed
Assignees

Comments

@mattneub
Copy link

@mattneub mattneub commented Aug 18, 2016

Previous ID SR-2388
Radar rdar://problem/32003073
Original Reporter @mattneub
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Xcode Version 8.0 beta 6 (8S201h)

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee @belkadan
Priority Medium

md5: 0803c238279e54d409d7b137b9e8c7d0

Issue Description:

Before seed 6, I used to be able to say this:

        let d : [NSObject:AnyObject] = [
            kCGImageSourceShouldAllowFloat : true,
            kCGImageSourceCreateThumbnailWithTransform : true,
            kCGImageSourceCreateThumbnailFromImageAlways : true,
            kCGImageSourceThumbnailMaxPixelSize : w
        ]
        let imref = CGImageSourceCreateThumbnailAtIndex(src, 0, d)!

Now I have to take everything across the bridge by hand. All the CFString keys have to be cast as String or NSString. We don't get any automatic bridge-crossing, so we have to tell Swift explicitly what the value types are, and then we have to cast the resulting dictionary to CFDictionary as well. Even if we get past the compiler, it is still possible to do this wrong and thus end up with an invalid CFDictionary so that at runtime CGImageSourceCreateThumbnailAtIndex yields nil. The only way I've found to get things right is like this:

        let d = [
            kCGImageSourceShouldAllowFloat as String : true as NSNumber,
            kCGImageSourceCreateThumbnailWithTransform as String : true as NSNumber,
            kCGImageSourceCreateThumbnailFromImageAlways as String : true as NSNumber,
            kCGImageSourceThumbnailMaxPixelSize as String : w as NSNumber
        ]
        let imref = CGImageSourceCreateThumbnailAtIndex(src, 0, d as CFDictionary)!

This is absolutely horrible.

The situation is compounded by the fact that no one seems to have told NSDictionary that its Swift equivalent is now [AnyHashable : Any]. We see that in the APIs and it works, but when you try to cast a CFDictionary to it, the compiler complains:

        let result = CGImageSourceCopyPropertiesAtIndex(src, 0, nil)! as [AnyHashable:Any]
        // compile error

In my opinion, Swift now goes too far with its refusal to help things get across the bridges, the Swift to Objective-C bridge and the CFTypeRef toll-free bridge. (In fact, there now seems to be rather a heavy toll on that bridge. 🙂)

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

To work around the second problm, you can do nsDict as [NSObject: AnyObject] as [AnyHashable: Any], which is silly but should satisfy the compiler for now.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Aug 18, 2016

I think cfDict as NSDictionary as [AnyHashable: Any] also works.

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

Not yet, unfortunately, we still only allow explicit as conversions from NS classes to the "old" value type mappings [AnyObject]/[NSObject: AnyObject]/etc.

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

You can bridge `CFString` to `NSString` using `as NSString`. If you must make `CFString` `Hashable`, you can add this extension as a workaround:

extension CFString: Hashable {
  public static func ==(a: CFString, b: CFString) -> Bool {
    return CFEqual(a, b)
  }
}

@mattneub
Copy link
Author

@mattneub mattneub commented Aug 18, 2016

@jckarter That compiles (thank you!) but it doesn't actually work. Without the explicit as Number we fail to get a valid CFDictionary and we crash when we force-unwrap.

The reason is that merely saying true or w does not actually get us across the bridge to AnyObject — and typing the dictionary as [CFString:AnyObject] doesn't help, because we don't get automatic bridge-crossing that way any more either.

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

@mattneub What type is w? Bool, Int, and UInt should bridge to NSNumber when you convert to NSDictionary or CFDictionary, but if it's some other number type it may be getting boxed.

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

For example, this prints `true` for me:

let w = 1024

let d1 : [CFString: Any] = [
  kCGImageSourceShouldAllowFloat : true,
  kCGImageSourceCreateThumbnailWithTransform : true,
  kCGImageSourceCreateThumbnailFromImageAlways : true,
  kCGImageSourceThumbnailMaxPixelSize : w
]
let d2 : [CFString: AnyObject] = [
  kCGImageSourceShouldAllowFloat : true as NSNumber,
  kCGImageSourceCreateThumbnailWithTransform : true as NSNumber,
  kCGImageSourceCreateThumbnailFromImageAlways : true as NSNumber,
  kCGImageSourceThumbnailMaxPixelSize : w as NSNumber
]
print((d1 as NSDictionary).isEqual(to: d2 as NSDictionary))

@mattneub
Copy link
Author

@mattneub mattneub commented Aug 18, 2016

@jckarter It comes out as an NSDictionary but, as I said, it doesn't make it as a valid CFDictionary that ImageIO can understand. That is the bridge we are not crossing.

Here's the full code (along with your CFString extension):

        let url = Bundle.main.url(forResource:"colson", withExtension: "jpg")!
        let src = CGImageSourceCreateWithURL(url as CFURL, nil)!
        let scale = UIScreen.main.scale
        let w = self.iv.bounds.width * scale
        // have to cross over to Objective-C manually
        let d : [CFString:Any] = [
            kCGImageSourceShouldAllowFloat : true as NSNumber,
            kCGImageSourceCreateThumbnailWithTransform : true as NSNumber,
            kCGImageSourceCreateThumbnailFromImageAlways : true as NSNumber,
            kCGImageSourceThumbnailMaxPixelSize : w as NSNumber
        ]
        let imref = CGImageSourceCreateThumbnailAtIndex(src, 0, d as CFDictionary)!

I've attached my test project; launch, then tap the second button to view the image in the interface. I crash at that moment if all four as NSNumber casts have been removed. bk2ch23p829imageIO.zip

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

@belkadan, is there a chance the crash in CF is related to #4366 ? Does CFNumber not toll-free-bridge with NSNumber subclasses?

@belkadan
Copy link
Contributor

@belkadan belkadan commented Aug 18, 2016

It does, but CFBoolean is a distinct type from CFNumber at the CF layer, so it very well could be related.

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

Ah, that could definitely be it then.

@jckarter
Copy link
Member

@jckarter jckarter commented Aug 18, 2016

@mattneub If you do true as Bool as NSNumber, do you see the same crash as if you do no coercions at all? Do you crash if you pass w without as NSNumber?

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Aug 18, 2016

Comment by Wil Shipley (JIRA)

When I used this form (this is what the auto-updating stuff in Xcode ended up spitting out) it compiled but I do NOT get an actual image — the load fails.

            let thumbnailSideLength = 512
            let optionsDictionary = [
                kCGImageSourceCreateThumbnailFromImageIfAbsent as NSString: true,
                kCGImageSourceThumbnailMaxPixelSize as NSString: thumbnailSideLength,
            ] as CFDictionary
            guard let thumbnail = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, optionsDictionary) else {
                    print("\(#function) failed to load thumbnail for \(name)/\(maker)/\(productLine)")
                    throw PlatonicPieceOfFurnitureError.cannotCreateImageFromThumbnailImageSource
            }

When I change my dictionary to this it succeeds:

            let optionsDictionary = [
                kCGImageSourceCreateThumbnailFromImageIfAbsent as NSString: true as NSNumber,
                kCGImageSourceThumbnailMaxPixelSize as NSString: thumbnailSideLength as NSNumber,
            ] as CFDictionary

@belkadan
Copy link
Contributor

@belkadan belkadan commented Aug 19, 2016

Confirmed that with #4366 this succeeds without the as NSNumber. I'm using this bug to track making all CF types Hashable by default.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Aug 19, 2016

#4394

@belkadan
Copy link
Contributor

@belkadan belkadan commented Aug 19, 2016

CF types have been made Hashable in #4417 If there are any more improvements / fixes we can make around CF, let's put them in separate bug reports.

@gparker42
Copy link
Mannequin

@gparker42 gparker42 mannequin commented Aug 20, 2016

Reopening. #4394 and #4417 were reverted due to CI failures. New test Interpreter/SDK/cf.swift fails when run with build-script --test-optimized.

@belkadan
Copy link
Contributor

@belkadan belkadan commented May 5, 2017

@swift-ci create

@belkadan
Copy link
Contributor

@belkadan belkadan commented May 8, 2017

CF types have been remade Hashable in #4568 #finally

@belkadan
Copy link
Contributor

@belkadan belkadan commented May 12, 2017

And merged into Swift 4.0 in #9401

@mattneub
Copy link
Author

@mattneub mattneub commented Jul 24, 2017

@belkadan @gparker42 @jckarter I am now reaping the rewards of this change. I can say

let result = CGImageSourceCopyPropertiesAtIndex(src, 0, nil)! as! [AnyHashable:Any]

and this works because CFString is AnyHashable.

And I can say

let d : [AnyHashable:Any] = [
    kCGImageSourceShouldAllowFloat : true ,
    kCGImageSourceCreateThumbnailWithTransform : true ,
    kCGImageSourceCreateThumbnailFromImageAlways : true ,
    kCGImageSourceThumbnailMaxPixelSize : w
 ]
let imref = CGImageSourceCreateThumbnailAtIndex(src, 0, d as CFDictionary)!

That's convenient and intuitive. Thanks!

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants