-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Respect Exif orientation in LaTeX, Docx, ODT output #10386
Open
silby
wants to merge
5
commits into
jgm:main
Choose a base branch
from
silby:exif
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+156
−18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
silby
force-pushed
the
exif
branch
4 times, most recently
from
November 20, 2024 01:21
c00b4d1
to
72f7da0
Compare
silby
changed the title
wip: respect Exif orientation
Respect Exif orientation in LaTeX, Docx, ODT output
Nov 20, 2024
libreoffice bug for the image rotation thing |
Transforms images based on exif metadata
The desired size of the final image in the docx output wants to be based on the rotated dimensions, not the original dimensions, but then we have to set the extents of the image object based on the original aspect ratio, so we have to swap those dimensions back. An earlier iteration of this diff added the swapping to imageSize, but I thought better of it. rotatedDesiredSizeInPoints is a new function because T.P.ImageSize is in the public API and I didn't want to create an API break.
Extensive experimentation with LibreOffice 24.8.3.2 on Mac didn't come up with a way to create an ODT with an image that is displayed uncropped, at the correct aspect ratio, with the correct size of bounding box, after rotating 90° or 270°, while anchored "as character". As of this commit, if the EXIF orientation says an image needs to be quarter-rotated, the solution chosen has a bounding box the original, unrotated size of the image, which displays the image correctly rotated and with the correct aspect ratio, but cropped. It is unknown to this author if this looks correct in other software than LibreOffice. Pandoc doesn't currently pull in any dependencies capable of rotating the actual pixels in image data. Document authors needing to mitigate this issue will have to edit their images themselves.
Ordinarily the LaTeX writer doesn't really need to get the bytes of the images it's including, but we have to check them to get an EXIF orientation. If the media fetch fails we're just punting and assuming a default orientation. It would be slightly dubious to bother doing this at all but LaTeX is the default format for PDF output so it's worth making the effort.
jgm
reviewed
Dec 20, 2024
@@ -451,3 +495,32 @@ webpSize opts img = | |||
case AW.parseOnly pWebpSize img of | |||
Left _ -> Nothing | |||
Right sz -> Just sz { dpiX = fromIntegral $ writerDpi opts, dpiY = fromIntegral $ writerDpi opts} | |||
|
|||
imageTransform :: ByteString -> ImageTransform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imageTransform
seems too general a name for a function that just operates on exif.
Also, all exported functions and types should have Haddock comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch uses EXIF orientation metadata in image files to affect the output of images in Docx, ODT, and LaTeX writers. Addresses #10311 for these three writers only.
Having worked on this for a few days I am a bit dubious that this in scope for Pandoc. On the one hand, it's probably somewhat common for users to encounter images with a flipped/rotated orientation, given the behavior smartphone cameras and so on, and common to not know what's going on or how to fix it when you generate a PDF with Pandoc. On the other hand, the problem has to be addressed in a specific way for each output format and the results can questionable.
The LaTeX case is particularly bad here, I think. For one thing, it introduces a need to read the image bytes in the LaTeX writer where it was previously unnecessary, and so I'm just ignoring all
fetchItem
exceptions and defaulting to no rotation. There's also no necessary relationship between the environment wherepandoc
is being run and the environment where LaTeX is being run: if someone has a rotatedfoo.jpg
in their working directory when they run pandoc, then they execute LaTeX in a directory with an unrotatedfoo.jpg
, have we done the right thing or not?In the ODT case, there's no good-looking solution available without actually rotating the pixels of the image data. See the commit message for cc73c01.
I think the approach here is more or less reasonable but I don't wholeheartedly endorse it.
Incidentally, typst natively respects EXIF orientation, seemingly by actually rotating and flipping the pixels, see here