callstack / react-native-image-editor

A library providing an API for cropping images from the web and the local file system.
MIT License
379 stars 118 forks source link

iOS - determine the output image type from the input image type #44

Closed takameyer closed 4 years ago

takameyer commented 4 years ago

This pull request saves the cropped image as a JPEG in iOS, reflecting the behavior of the previous library. Android also preforms the same function.

takameyer commented 4 years ago

Fixes issue #43

Trancever commented 4 years ago

@takameyer We have recently added support on iOS for cropping transparent images (PNG) just like it is done on Android. We should change the extension of the saved file to "png" from "jpg" instead of using UIImageJPEGRepresentation back.

takameyer commented 4 years ago

If that is the case, then i require an extension so that it can be saved as jpeg. The old version worked this way and the app I am working on requires it.

I will reword and rework the pull request.

takameyer commented 4 years ago

I guess the big question is what the default should be and how the interface should behave. In our app, we are taking direct camera input and providing an interface to crop the picture. JPEG behaves well in this case since it will never have a transparency. This is also what iOS expects, which is the problem with issue #35 .

What I would like to see is:

const format = ImageCropFormat.JPEG
const croppedImagePath = await ImageEditor.cropImage(uri, cropData, format)

and ImageCropFormat would be defined as:

enum ImageCropFormat {
  JPEG = 'JPEG',
  PNG = 'PNG'
}

Since the old core library for ImageEditor used JPEG, I would set this as default. Let me know if this is suitable and I will update the pull request.

Trancever commented 4 years ago

@takameyer I am ok with having an optional format argument that defaults to "jpeg". We just need to make sure Android behaves in the same way. Feel free to submit a PR that implements it. Thanks!

takameyer commented 4 years ago

I reviewed the Android code and found out that it determines the mimetype of the input image and produces saves the cropped image as the determined mimetype, or JPEG if it cannot be determined. The changes in this pull request provide the same behavior in iOS. If a PNG is provided, than a PNG is the output. If anything else is provided, then JPEG is the output.

takameyer commented 4 years ago

@Trancever Are you okay with this pull request? It is a bit different than we discussed, but I feel it is the correct way to handle it.

Trancever commented 4 years ago

@takameyer Let me check it tomorrow :)

Trancever commented 4 years ago

@takameyer I've tested your changes and it doesn't work properly. contentTypeForImage function returns nil for both jpeg and png images. Looks like imageLoader from core of react-native somehow transforms the raw data or the code responsible for getting NSData from UIImage doesn't work correctly.

How did you test your changes? Did you load image to crop from local e.g. camera roll or from remote e.g. web url?

takameyer commented 4 years ago

I tested directly from camera and camera roll.

Trancever commented 4 years ago

@takameyer So it doesn't work for me when I test images taken from web. Can you check that?

takameyer commented 4 years ago

@Trancever So it seems that the method i was using to convert UIImage to NSData was not successful in any case. I have reverted to checking the input url for the file extension and creating a PNG if a PNG is provided. Otherwise JPEG is returned.
I have tested this both the camera roll and with a web image url.

Trancever commented 4 years ago

@takameyer Awesome, I will test it tomorrow and if everything works properly I will merge it.

azimgd commented 4 years ago

Hey @Trancever can we merge this now please ?