Skip to content

EXIF Orientation Support - #1379

Closed
fstrube wants to merge 8 commits into
foliojs:masterfrom
More-Vang:exif-orientation-support
Closed

EXIF Orientation Support#1379
fstrube wants to merge 8 commits into
foliojs:masterfrom
More-Vang:exif-orientation-support

Conversation

@fstrube

@fstrube fstrube commented Jul 19, 2022

Copy link
Copy Markdown

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

fstrube commented Jul 24, 2022

Copy link
Copy Markdown
Author

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

@fstrube

fstrube commented Aug 1, 2022

Copy link
Copy Markdown
Author

@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

blikblum commented Aug 1, 2022

Copy link
Copy Markdown
Member

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

fstrube commented Aug 25, 2022

Copy link
Copy Markdown
Author

@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
Copy Markdown

@fstrube Can we merge this change looks good to me

@fstrube

fstrube commented Sep 13, 2022

Copy link
Copy Markdown
Author

@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

halstjas commented Sep 14, 2022

Copy link
Copy Markdown

@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

fstrube commented Mar 20, 2023

Copy link
Copy Markdown
Author

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
Copy Markdown
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
Copy Markdown
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

blikblum commented Dec 3, 2023

Copy link
Copy Markdown
Member

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