sebsauvage / MinigalNano

GNU Affero General Public License v3.0
162 stars 50 forks source link

Bug: MediaBox lightbox overlay image distorded/squared due to commit 3fc42c0 #98

Closed Lucas-C closed 9 years ago

Lucas-C commented 9 years ago

This problem appear on photos whose EXIF metadata specify they have been rotated, when clicking on the miniature in the gallery to display the MediaBox.

Let start from the beginning of my journey:

exiftool my_photo_ko.JPG | grep -E 'Orient|Image Size'
# Orientation : Rotate 270 CW
# Image Size : 1469x1102
convert my_photo_ko.JPG -auto-orient my_photo_ok.JPG
exiftool my_photo_ok.JPG | grep -E 'Orient|Image Size'
# Orientation : Horizontal (normal)
# Image Size : 1102x1469

But now the MediaBox image was squared: https://chezsoi.org/lucas/minigalnano_bug_reproduction/squared/

Hence the bug report: shouldn't this commit be reverted, as it seems to introduce new issues while fixing others ? Another solution would be to properly fix this patch code to cope with my original distortion issue. But this would probably require detecting the EXIF orientation in such problematic photo:

var img = new Image()
img.src = "https://chezsoi.org/lucas/minigalnano_bug_reproduction/distorted/photos/30percent_resized_nonautoriented_P1020697.JPG"
console.log(img.width, img.height) // "Wrong", unoriented dimensions: 1469 1102
jniggemann commented 9 years ago

initially, I was making a photo gallery, and found out that some pictures where distorted: https://chezsoi.org/lucas/minigalnano_bug_reproduction/distorded

==> 404...

to fix this, I used ImageMagick to get rid of the EXIF information and resave the photo with the proper orientation

Removing metadata or changing the orientation is unwanted by most, so our approach is to overcome the need for any alteration of the original images. MinigalNano shall display images, nothing more.

But now the MediaBox image was squared

Intended behaviour, at least somewhat. Let's recall the path that lead there: In 2014 I found an issue in thumbnail creation; images were not properly rotated (see #2 and #41). I set up a test suite of images and contributed some code that did proper rotation. This code was later factored out with commit https://github.com/sebsauvage/MinigalNano/commit/8ab86967f7ca402f30af4f4f2ee7a59c3308c387, leading to a regression in thumbnail generation.

92 fixed that regression.

While putting together #92, I realized that the full images in the lightbox still weren't properly rotated. I found that FF > 26 used image-orientation and implemented 3fc42c0 to fix this. Because this only works in FF >26, I later wrote a patch (see the directory "patches") that FF-users can apply, obsoleting 3fc42c0.

Conclusion: 3fc42c0 should be reverted. Users using FF>26 exclusively can apply the supplied patch.

Sidenote: The issue with FF stretching or shrinking images may be due to the screen size / resolution. Someday, when the kids have grown up and I'm old and have plenty of time, I might look into that. Until then, everyone else is welcome to do so :-)

Lucas-C commented 9 years ago

Thanks for your detailed answer.

The 404 was due to a spelling mistake: distorded -> distorted. I fixed the URL in my initial post.

jniggemann commented 9 years ago

@Lucas-C Can you please pull from my repo and

Please check for 2 things: 1) are all thumbnails properly rotated? 2) are the images distorted

Images to test with can be found in https://github.com/jniggemann/exif-orientation-examples

Lucas-C commented 9 years ago

Sorry for the 6 weeks answer delay, I had completely forgotten about this :( I just checked and it all works fine,for the thumbnails and the overlay view. Thank you very much !