sekoyo / react-image-crop

A responsive image cropping tool for React
ISC License
3.89k stars 344 forks source link

Working with Javascript-image-load #208

Closed MastroLindus closed 6 years ago

MastroLindus commented 6 years ago

Hi,

as many others using this library, I also have issues with cropping mobile images that have orientation metadata. I understand that this is not an issue specifically of this library, however it's kind of hard to work with the cropper in the current state, and I would like to ask if the library can help in my scenario.

Using the library javascript-load-image, I can successfully obtain a CanvasHTMLElement with the image rotated in the proper orientation. However I still have the issue, later on, to use the content of the canvas with react-image-crop in order to show the cropper to the user.

So far I have been calling canvas.toDataURL() and using the result as the src prop for the cropper.

However this is quite slow and inefficient, since we already have a canvas element loaded with the proper image, and we are reconverting it to a URL so that the cropper can reload it once again into another img.

Would it be possible for the cropper to receive as prop the canvas element in the first place, and visualize the cropper layer on top of it?

sekoyo commented 6 years ago

Hi,

I understand your pain but wish to keep this library small and focused. However it could be achieved with a HOC which could be written for that purpose, though at the moment I'm busy with other stuff so tbh it's not likely to be written by me any time soon.

One thing that you can drastically improve though is changing your use of toDataURL to https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/toBlob

toBlob is MUCH faster, try the example there and see if it fixes your issue - https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/toBlob#Getting_a_file_representing_the_canvas

sekoyo commented 6 years ago

If this is performant the library could potentially export a utility function which maps the image to a canvas, fixes the rotation, and returns a blob url, but as I said it's not likely to happen soon from me!

MastroLindus commented 6 years ago

Hi,

I understand that you are busy and that its out of scope of the library itself, so thank you for your answer anyway. Once you have the blob, would you pass it to react-image-prop by using URL.createObjectURL ?

sekoyo commented 6 years ago

Yes exactly, remember to call URL.revokeObjectURL(url) on the previous one when you need to recalc it or you will have a memory leak

sekoyo commented 6 years ago

I think you could use the onImageLoaded callback to revoke it. PS if/when you have a working solution please post it here so I can consider if it can somehow be integrated or exposed as a utility function @MastroLindus

MastroLindus commented 6 years ago

Hi Dominic,

I managed to make it work, unfortunately the results are not that great. Loading the image using javascript-image-load into canvas takes 70ms on my mobile, but then, to convert the canvas to a URL for react-image-crop: toDataURL takes more 3.9seconds toBlob uses 3.5seconds, that is better, but still just too slow, it is very very noticeable on the mobile, and it's a pity especially since the image is already loaded

moreover because my backend is expecting a dataUrl in base64, I still need to reconvert it to that after the cropping has been done, so it's even more painful (but that's out of scope)

I think I need to find a smarter way to do this, and in the worst case scenario I'll have to do the cropping myself on the canvas given by javascript-image-load, even if I really hope I won't have to go that far. Providing the canvas to react-image-crop so it can reuse it would be a much nicer solution, even if I understand your difficulties at the moment. I am also with my hands on many tasks at the moment (I was hoping that fixing the orientation of the images would have been a 10minutes thing and instead it took away most of my day and its not solved yet), but if I will find a better solution I'll definitely report it here.

Thank you for your support meanwhile

sekoyo commented 6 years ago

Ah that's a shame, it's not that I'm against the idea of passing a canvas or some other solution, the issue is that will take time to test and I'm not sure how it will behave in terms of re-sizing to the container as a canvas behaves differently to an image element etc, the onImageLoaded/Error would obviously not be there and probably other things I haven't considered so there is quite a bit of work/investigation involved

sekoyo commented 6 years ago

The advantage of toBlob over toDataURL is that it's async so you could potentially show a spinner rather than freezing the main thread of the window. Perhaps it's worth investigating uploading the blob to the server rather than converting it to a dataURL https://stackoverflow.com/questions/10313992/upload-html5-canvas-as-a-blob?

sekoyo commented 6 years ago

By the way I forgot to mention I hope you're using toDataURL('image/jpeg') and toBlob(callback, 'image/jpeg') otherwise the conversion to a png (default) will be significantly slower.

MastroLindus commented 6 years ago

With the jpeg type it is down at 2.5seconds. Not still fantastic, but I guess it's close to acceptable.

I guess that the proper solution would be indeed to allow react-image-crop to work on a passed in img/canvas as we discussed, but I understand its quite a big change and it won't happen anytime soon.

I just found other libraries that work that way and indeed it would be a better solution, but I am going to stick with react-image-crop for the time being since I really like the rest of it.

Thank you for all your suggestions so far.

sekoyo commented 6 years ago

Another thing that should bring it down to < 1 second is when you map the image to the canvas you can resize it to be say a max of 1000 pixels.

Can you link to the other project please? Want to have a look at how their canvas behaves when resizing.

On Wed, 10 Oct 2018 at 07:38, MastroLindus notifications@github.com wrote:

With the jpeg type it is down at 2.5seconds. Not still fantastic, but I guess it's close to acceptable.

I guess that the proper solution would be indeed to allow react-image-crop to work on a passed in img/canvas as we discussed, but I understand its quite a big change and it won't happen anytime soon.

I just found other libraries that work that way and indeed it would be a better solution, but I am going to stick with react-image-crop for the time being since I really like the rest of it.

Thank you for all your suggestions so far.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/DominicTobias/react-image-crop/issues/208#issuecomment-428455479, or mute the thread https://github.com/notifications/unsubscribe-auth/AAuZ-k9iEExrdyuakKnkWGtWpgOr613_ks5ujZX9gaJpZM4XTeRh .

MastroLindus commented 6 years ago

Sure,

I didn't try it, I only looked around yesterday night, but the one that looks like that allow you to pass a canvas to work on is https://github.com/fengyuanchen/cropperjs It's not react-based, nor declarative, but it is actually imperative/event-based and I prefer your API, but it looks like they support this use case

sekoyo commented 6 years ago

This should work on Android at least - https://developer.mozilla.org/en-US/docs/Web/CSS/image-orientation

img {
    image-orientation: from-image;
}

And I read rotation is correct in Safari anyway? Shame it doesn't have more support.

Have you considered detecting orientation and simply applying a css transform rotate to the image? See this answer - https://stackoverflow.com/a/47514730/414062

abel30567 commented 6 years ago

I tried to do this using this sample. https://github.com/codingforentrepreneurs/Try-Reactjs/blob/master/src/learn/ImgDropAndCrop.js

But instead of using FileReader in line 135 I used Javascript-Image-Load

handleFileSelect = event => {
    const files = event.target.files;
    if (files && files.length > 0) {
      const isVerified = this.verifyFile(files);
      if (isVerified) {
        const currentFile = files[0];
        loadImage(currentFile, (img) => {
          const base64data = img.toDataURL('image/jpeg');
          const img_src = base64data.replace(/^data\:image\/\w+\;base64\,/, '');
          this.setState({
            imgSrc: base64data,
            imgSrcExt: extractImageFileExtensionFromBase64(base64data),
          });
        }, { orientation: true });
      }
    }
  }

Hope that helps.

sekoyo commented 6 years ago

Nice @abel30567, since this library doesn't control how the image is loaded I'm going to close this.

I will add a section in the Readme and reference https://github.com/blueimp/JavaScript-Load-Image/