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

Fix exif orientation issue when resizing images. #2001

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kaymes
Copy link

@kaymes kaymes commented Jun 18, 2021

This PR fixes the issue with exif orientation when resizing image described in #1967.

With this PR, Exif data is removed from the image before resizing it. This way the browser doesn't fix the orientation and everything is correct when the Exif information is added to the resized image afterwards.

Initially I tried to re-use some of the code of the existing ExifRestorer. However, it proved to be quite slow with to do the base64 encoding/decoding of that class. Thus, I wrote a much simpler and faster version of it which uses the builtin atob() and btoa() functions and subsequently uses strings instead of arrays.
Since this proved effective I also changed the Exif restore code to a simpler and faster function and removed the ExifRestorer.

Unfortunately I haven't worked out how to build the distribution files so I could only do limited testing.

kaymes and others added 2 commits June 18, 2021 11:25
Rewrite of the existing exif handling code to use built in base64 operations which are much faster.
@enyo
Copy link
Collaborator

enyo commented Oct 26, 2021

Woah what a PR! Thanks for that. I'd like to get this in, but how should I be testing this? Can you describe the use cases where it failed before and how you fixed it? I think using the atob and btoa functions is great call now that Dropzone is dropping IE support.

@kaymes
Copy link
Author

kaymes commented Oct 26, 2021

It's been a while since I wrote this. As far as I remember the failure case is the following:

  • user adds an image which has a non-standard exif orientation set. A portrait mode picture from a phone (or digital camera which automatically orients according to gravity) usually has non-standard exif orientation.
  • Dropzone resizes the image before uploading it to the server.

Before this PR: the version which arrives on the server is no longer oriented correctly when viewed with an exif-aware image viewer. (It is because the physical orientation has changed but the exif information wasn't updated which leads to the viewer to rotate the image again).
After this PR: the version which arrives on the server is oriented the same as the original. (because the physical orientation is unchanged so the exif information is still correct).

The test I would suggest is to take a picture with a mobile phone in portrait mode and configure Dropzone to resize images before the upload. If this all works as expected, then it should be ok.

@alex-levy
Copy link

@enyo any update on this? This is currently blocking our adoption of this library.

@Jaammeesss
Copy link

@enyo is there any updates on this as the issue for phone portrait images being uploaded in the incorrect orientation after upload is still an issue with resizing

@jordanade
Copy link

@enyo any update?

@TheDude70
Copy link

Will this get merged any time soon?

@DanielStout5
Copy link

DanielStout5 commented Jun 21, 2022

@enyo will this PR get merged? I'm seeing the issue it describes - upload a portrait oriented image with resizeWidth specified and it ends up being incorrect in the processed image. The thumbnail is correct.

Including the exif-js library actually made it worse - with that library available, both the thumbnail and the full image end up being incorrect.

I put up a npm package with @kaymes's fix merged into the 5.9.3 commit here: https://www.npmjs.com/package/dropzone-exif-fix and it seems to work great.

pryley added a commit to pryley/dropzone-v6 that referenced this pull request Aug 1, 2022
@dazbradbury
Copy link

Any chance the fix for this will be applied to master, or any of the new releases?

@Pes8
Copy link

Pes8 commented Dec 16, 2022

MERGE IT PLEASE 🐊

@MentalGear
Copy link

Merge for #2081

@hcw-rohan
Copy link

Currently using this lib in dev for my app but it won't make it to production unless this is fixed. Please merge 😄

NicolasCARPi added a commit to NicolasCARPi/dropzone that referenced this pull request Apr 13, 2024
@NicolasCARPi
Copy link

Hello everyone,

I have just merged this PR on the maintained fork (@deltablot/dropzone). Keep an eye out for the next release: https://github.com/NicolasCARPi/dropzone/releases

Best,
~Nicolas

@NicolasCARPi
Copy link

New release is live with this patch: https://github.com/NicolasCARPi/dropzone/releases/tag/7.1.1

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.

None yet