Skip to content

EXIF Orientation Support #1379

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

Closed
wants to merge 8 commits into from

Conversation

fstrube
Copy link

@fstrube fstrube commented Jul 19, 2022

What kind of change does this PR introduce?
This change adds support for obeying the EXIF orientation on JPEG images. To avoid introducing a breaking change, this PR introduces an obeyOrientation option on both the PDFDocument constructor and the PDFDocument#image() function, which defaults to false.

The following dependencies have been added in this branch:

  • jpeg-exif: for parsing EXIF orientation on JPEG buffer

Checklist:

  • Unit Tests
  • Documentation
  • Update CHANGELOG.md
  • Ready to be merged

Related Issues:

@fstrube fstrube changed the title Exif Orientation Support EXIF Orientation Support Jul 20, 2022
@fstrube
Copy link
Author

fstrube commented Jul 24, 2022

@blikblum @liborm85 @devongovett any eyes on this would be greatly appreciated. Thanks!

@fstrube
Copy link
Author

fstrube commented Aug 1, 2022

@blikblum @liborm85 @devongovett
Has anyone had a chance to look at this yet? Let me know if there is anything else that I can do.

@blikblum
Copy link
Member

blikblum commented Aug 1, 2022

IMO obeyOrientation should be true by default.

This seems to be the default behavior in other programs . I will test in some apps to see if they obey orientation by default

@fstrube
Copy link
Author

fstrube commented Aug 25, 2022

@blikblum @liborm85 @devongovett Where do we land on the default behavior? Should we have it obey orientation by default? Or not?

Let me know and I'm happy to refactor this.

@halstjas
Copy link

@fstrube Can we merge this change looks good to me

@fstrube
Copy link
Author

fstrube commented Sep 13, 2022

@fstrube Can we merge this change looks good to me

I don't have maintainer privileges so I can't make the merge myself. I was waiting on feedback whether or not I should refactor this to obey orientation by default instead of relying on options.

@halstjas
Copy link

halstjas commented Sep 14, 2022

@fstrube @liborm85 @devongovett , my take on the default behaviour would to be that in order to ensure that existing consumers/users are not effected by this change the default behaviour should match the existing behaviour which is to obey orientation in EXIF meta and execute the translation.
Hence default behaviour should be as follows:

  • obeyOrientation = true
  • translation performed as prescribed by EXIF meta

  • obeyOrientation = false
  • No translation performed (EXIF meta ignored)

@fstrube
Copy link
Author

fstrube commented Mar 20, 2023

Hi @blikblum, I'd like to resume work on this PR and get it ready to merge. Do you think it is good as-is? Or do you think I should change the behavior to be obeyOrientation = true by default?

@blikblum
Copy link
Member

Hi @blikblum, I'd like to resume work on this PR and get it ready to merge. Do you think it is good as-is? Or do you think I should change the behavior to be obeyOrientation = true by default?

Yes. I want only see how other apps or libraries behave when you add a rotated image. I believe it does but want be sure.

@blikblum
Copy link
Member

Hi, i finally got time to look at some pdfkit issues.

I investigated how other tools / programs handle exif images by default:

PDF Editors

  • 👍 Adobe Acrobat - respect exif orientation
  • 👎 Foxit Editor - do NOT respect exif orientation
  • 👎 PDF Candy - do NOT respect exif orientation
  • 👎 htmlPDF (itext) - do NOT respect exif orientation

Text processors

  • 👍 Word - respect exif orientation
  • 👎 Libre Office - do NOT respect exif orientation

Web Engines

  • 👍 Chrome, Firefox - respect exif orientation

So, while behaviors differ, i think the most reasonable one is to obey the orientation by default because: 1. the main tools do 2. is more ergonomic for the developer (do not need to check if a image is rotated or not)

Given this, i will adapt this PR to obey orientation by default. Will add a option ignoreOrientation to opt out

@blikblum blikblum self-assigned this Dec 3, 2023
@blikblum blikblum mentioned this pull request Dec 3, 2023
4 tasks
@blikblum
Copy link
Member

blikblum commented Dec 3, 2023

Commited in #1482

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.

3 participants