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

PHPicker NSUrl is only a temporary file #54

Closed
iruizmar opened this issue Feb 4, 2024 · 13 comments · Fixed by #85 or #86
Closed

PHPicker NSUrl is only a temporary file #54

iruizmar opened this issue Feb 4, 2024 · 13 comments · Fixed by #85 or #86

Comments

@iruizmar
Copy link
Contributor

iruizmar commented Feb 4, 2024

According to PHPicker's documentation, the NSUrl returned on the loadFileRepresetion handler is only temporary and will get deleted when the callback finishes:

https://developer.apple.com/documentation/foundation/nsitemprovider/2888338-loadfilerepresentation/#discussion

This method writes a copy of the file’s data to a temporary file, which the system deletes when the completion handler returns.

This means that if you get the byteArray right away from the KmpFile on the callback in compose everything will work as expected, but if you store the KmpFile for a future usage, for instance uploading to the server, this won't work.

I'm not sure what's the best solution. WDYT?

@amrfarid140
Copy link

I am thinking the image picker can make its own temporary file copy, update KmpFile to be disposable and expose .dispose() method to delete the file when it's not needed anymore.

We can also add safeguards to call dispose when the file picker is removed from composition. (i.e. use a DisposableEffect).

How does that sound?

@iruizmar
Copy link
Contributor Author

That sounds like the correct way, yes. The problem is how to "make its own temporary file copy". Do you know how to achieve this?

@bmc08gt
Copy link
Contributor

bmc08gt commented Feb 20, 2024

@amrfarid140
Copy link

I ended up forking and going with my own logic. Basically replace usage of KmpFile type with kotlinx.io.Path and here's my open and copy to temp file logic. Not the prettiest but works™️

val pickerDelegate = remember(type, selectionMode, onResult) {
        object : NSObject(), PHPickerViewControllerDelegateProtocol {
            override fun picker(picker: PHPickerViewController, didFinishPicking: List<*>) {
                picker.dismissViewControllerAnimated(true, null)
                println("didFinishPicking: $didFinishPicking")
                didFinishPicking.forEach {
                    val result = it as? PHPickerResult ?: return@forEach

                    // this gives us NSData instead of the file 
                    result.itemProvider.loadDataRepresentationForTypeIdentifier(
                        typeIdentifier = UTTypeImage.identifier
                    ) { data, error ->
                        if (error != null) {
                            println("Error: $error")
                            return@loadDataRepresentationForTypeIdentifier
                        }
                        coroutineScope.launch(Dispatchers.IO) {
                            // if we have data then read it as a UIImage. I ended up adding some logic to handle image rotation properly
                            // We might not need the image rotation logic here.                        
                            data?.let {
                                UIImage.imageWithData(it)
                                    ?.let { image ->
                                        if (image.imageOrientation == UIImageOrientation.UIImageOrientationUp) {
                                            image
                                        } else {
                                            UIGraphicsBeginImageContextWithOptions(
                                                image.size,
                                                false,
                                                image.scale
                                            )
                                            val rect = CGRectMake(
                                                x = 0.0,
                                                y = 0.0,
                                                width = image.size.useContents { this.width },
                                                height = image.size.useContents { this.height }
                                            )
                                            image.drawInRect(rect)
                                            val normalisedImage = UIGraphicsGetImageFromCurrentImageContext()
                                            UIGraphicsEndImageContext()
                                            normalisedImage
                                        }
                                    }
                                    ?.let { image ->
                                        // Use NSFileManager to write the image data into a file in the cache directory. We can as well use the 
                                        // temp directory here.
                                        NSFileManager.defaultManager.URLForDirectory(
                                            NSCachesDirectory,
                                            NSUserDomainMask,
                                            null,
                                            true,
                                            null
                                        )?.let {
                                            val fileName = result.itemProvider.suggestedName ?: "temp_image"
                                            val url = NSURL.fileURLWithPath("$fileName.jpg", it)
                                            val jpeg = UIImageJPEGRepresentation(image, 0.5)
                                            val hasWritten = jpeg?.writeToURL(url, true)
                                            if (hasWritten == true) {
                                                onResult(listOfNotNull(url.toPath()))
                                            }
                                        }
                                    }
                            }
                        }
                    }
                }
            }
        }
    }

@bmc08gt
Copy link
Contributor

bmc08gt commented Feb 20, 2024

I ended up forking and going with my own logic. Basically replace usage of KmpFile type with kotlinx.io.Path and here's my open and copy to temp file logic. Not the prettiest but works™️

val pickerDelegate = remember(type, selectionMode, onResult) {
        object : NSObject(), PHPickerViewControllerDelegateProtocol {
            override fun picker(picker: PHPickerViewController, didFinishPicking: List<*>) {
                picker.dismissViewControllerAnimated(true, null)
                println("didFinishPicking: $didFinishPicking")
                didFinishPicking.forEach {
                    val result = it as? PHPickerResult ?: return@forEach

                    // this gives us NSData instead of the file 
                    result.itemProvider.loadDataRepresentationForTypeIdentifier(
                        typeIdentifier = UTTypeImage.identifier
                    ) { data, error ->
                        if (error != null) {
                            println("Error: $error")
                            return@loadDataRepresentationForTypeIdentifier
                        }
                        coroutineScope.launch(Dispatchers.IO) {
                            // if we have data then read it as a UIImage. I ended up adding some logic to handle image rotation properly
                            // We might not need the image rotation logic here.                        
                            data?.let {
                                UIImage.imageWithData(it)
                                    ?.let { image ->
                                        if (image.imageOrientation == UIImageOrientation.UIImageOrientationUp) {
                                            image
                                        } else {
                                            UIGraphicsBeginImageContextWithOptions(
                                                image.size,
                                                false,
                                                image.scale
                                            )
                                            val rect = CGRectMake(
                                                x = 0.0,
                                                y = 0.0,
                                                width = image.size.useContents { this.width },
                                                height = image.size.useContents { this.height }
                                            )
                                            image.drawInRect(rect)
                                            val normalisedImage = UIGraphicsGetImageFromCurrentImageContext()
                                            UIGraphicsEndImageContext()
                                            normalisedImage
                                        }
                                    }
                                    ?.let { image ->
                                        // Use NSFileManager to write the image data into a file in the cache directory. We can as well use the 
                                        // temp directory here.
                                        NSFileManager.defaultManager.URLForDirectory(
                                            NSCachesDirectory,
                                            NSUserDomainMask,
                                            null,
                                            true,
                                            null
                                        )?.let {
                                            val fileName = result.itemProvider.suggestedName ?: "temp_image"
                                            val url = NSURL.fileURLWithPath("$fileName.jpg", it)
                                            val jpeg = UIImageJPEGRepresentation(image, 0.5)
                                            val hasWritten = jpeg?.writeToURL(url, true)
                                            if (hasWritten == true) {
                                                onResult(listOfNotNull(url.toPath()))
                                            }
                                        }
                                    }
                            }
                        }
                    }
                }
            }
        }
    }

Nice. Your fork published by chance? Considering just rolling the file picker source into an SDK im working on in the interim.

@amrfarid140
Copy link

That fork is part of a private project, happy to share snippets but the git repo isn't public.

@bmc08gt
Copy link
Contributor

bmc08gt commented Feb 21, 2024

I was able to make this work without a complete fork actually :)

@bmc08gt
Copy link
Contributor

bmc08gt commented Feb 22, 2024

Have another solution that I'll be putting up as a PR soonish

@iruizmar
Copy link
Contributor Author

 @bmc08gt Can you share your solution? :) Maybe we can help you with the PR.

@MohamedRejeb
Copy link
Owner

With KmpFile you can access NSUrl in iosMain and then you can use the `NSFileManager to save that picked file permanently.
I don't think that it's the best option to handle this logic in the library unless there is an official API for that.

@iruizmar
Copy link
Contributor Author

The problem is that if this is not handled by the library, then the behaviour is different on Android than on iOS.
You wouldn't want to make the copy on Android since the obtained URL is permanent.

Would it make sense to add a configuration parameter for this?

@bmc08gt
Copy link
Contributor

bmc08gt commented Apr 12, 2024

@bmc08gt Can you share your solution? :) Maybe we can help you with the PR.

Sorry for the delay. Had some personal stuff come up that prevented me from getting this extracted into a PR from our private repo. Good news is that its now OSS.

https://github.com/BotStacks/mobile-sdk/blob/main/chat-sdk/src/iosMain/kotlin/com/mohamedrejeb/calf/io/TemporaryFileURL.kt

https://github.com/BotStacks/mobile-sdk/blob/main/chat-sdk/src/iosMain/kotlin/com/mohamedrejeb/calf/picker/FilePickerLauncher.ios.kt#L80

@MohamedRejeb
Copy link
Owner

MohamedRejeb commented May 29, 2024

Hopefully, I will release a new version today. Now you won't need to create a temp file manually, it will be the default behavior and you can store KmpFile and use it later without issues.

Edit: v0.4.1 contains the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants