ladybird icon indicating copy to clipboard operation
ladybird copied to clipboard

Tracking issue for EXIF image rotation

Open shlyakpavel opened this issue 1 year ago • 7 comments

Tracking Issue: Rotation Support for Image Formats

We need to add rotation support for the following image formats in Ladybird:

  • [x] PNG
  • [ ] JPEG
  • [ ] JPEG XL
  • [x] TIFF
  • [ ] WebP
  • [ ] AVIF

Current Progress

  • PNG
    • PR: #1821 (Merged)
  • WebP
    • PR: #2122 (Rejected)

Reference

  • EXIF Orientation Tests: https://zpl.fi/exif-orientation-in-different-formats/

I will update this issue as new PRs are opened and formats are supported.

shlyakpavel avatar Oct 30 '24 19:10 shlyakpavel

TIFF is also already implemented.

justus2510 avatar Nov 02 '24 06:11 justus2510

https://libjxl.readthedocs.io/en/latest/api_decoder.html#_CPPv416JxlDecoderStatus

”Exif”: a box with EXIF metadata. Starts with a 4-byte tiff header offset (big-endian uint32) that indicates the start of the actual EXIF data (which starts with a tiff header). Usually the offset will be zero and the EXIF data starts immediately after the offset field. The Exif orientation should be ignored by applications; the JPEG XL codestream orientation takes precedence and libjxl will by default apply the correct orientation automatically (see JxlDecoderSetKeepOrientation).

However, as I have no test for that atm, I'm not marking that as done in the issue

shlyakpavel avatar Nov 02 '24 11:11 shlyakpavel

The PRs for this seem to not honor https://drafts.csswg.org/css-images/#the-image-orientation . Is that intentional?

nico avatar Nov 02 '24 16:11 nico

@nico In my PRs, that was unintentional. However, setting 'none' as the default value doesn’t align with the behavior of other browsers. Setting it to 'from-image' essentially matches what we currently do but with the added flexibility to change it through CSS. I believe rotating all images according to EXIF data works for now, and we can implement that CSS rule later if someone is interested in doing so. I’m open to further discussion on this.

shlyakpavel avatar Nov 02 '24 17:11 shlyakpavel

Up to maintainers (which I'm not).

The current approach of doing this in the decoders is kinda incompatible with implementing the spec later -- for that you'll have to let each decoder return orientation from exif and then rotate at some later time (consider two img elements showing an image from the same URL with different image-orientation tags). So IMHO doing more of the current approach causes more, not less, future work.

(But again, I'm not a maintainer.)

nico avatar Nov 02 '24 17:11 nico

A useful link mentioned in Discord: https://image-orientation-test.vercel.app/

shlyakpavel avatar Nov 05 '24 21:11 shlyakpavel

I'm working on a PR for this. I have moved exif orientation code from happening when decoding to rendering. I have also made the exif orientation part of the bitmap class so it can be read out by the renderer. This same PR of mine implements the image-orientation CSS property. Not doing this when decoding is necessary for the image-orientation property, since that prop will determine whether or not the exif orientation is used or disregarded. Adding support for the other image formats is then up to the individual formats' decoders, they are to pass the orientation data as an argument to the Bitmap constructor.

JohnyTheCarrot avatar Mar 02 '25 08:03 JohnyTheCarrot