-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Proposal: opt-in mode to pre-orient images #4144
Comments
Thanks for the detailed proposal. My understanding of this is that there are 2 main topics being discussed here, both of which are a good idea.
(Providing a runtime environment variable to control this feels like pain for me rather than benefit for thee, so no interest in that, sorry.) |
Sounds like a winning compromise! I completely understand that taking on new features is very tricky since now you have to support them. I think adding the syntactic sugar for And I could be wrong (because I got in and out of several patches of weeds) but I think that |
I'm having exactly this problem. I need to pipe rotations to 1st auto-orient and that add display rotation. |
Hey @labsforge, I’ve got a PR open in this repo with a branch that might do what you want. It is definitely not official and I will almost certainly rebase it as @lovell requests changes and considers how best to incorporate it into such a long-standing project. It seems like he is really busy at the moment, so I doubt that work will be accomplished soon. In the meantime, it’s probably safe enough to fork my branch to have your own copy that won’t change out from under you, and follow the instructions on how to build it locally. If all of that is a bit much, the workaround you’ve been using ought to work and—I am only guessing—probably doesn’t degrade your images in a noticeable fashion. |
Thanks both, the |
Hi @happycollision I've seen your PR and can't wait to see it on master 🙌 for now I'll be using the workaround and watch how this discussion develops. I believe |
Proposal: add an opt-in mode to conceptualize all commands starting from display orientation instead of actual orientation
Summary
Sharp manipulates images based on actual orientation instead of display orientation. This creates surprising results that many developers can fix with
.rotate()
. But sometimes.rotate()
is not viable or unavailable. (Read more below for why that might be.)I'd like to propose an opt-in mode that allows users to issue commands to Sharp based on starting from the display orientation of an image instead of the actual orientation. In other words, Sharp should effectively load an image, auto-orient it and then let users continue to issue
.rotate
and.flip
/.flop
commands.See the "This solution" section below for the finer details.
Terminology
If there is agreed-upon language for these concepts, I am unaware, so I will use the following:
8
no value
Problems inherent with EXIF-oriented image rotation in Sharp
Anyone who's spent enough time manipulating images with Sharp or a library which uses it under the hood knows that some images are oriented via EXIF data and some are oriented by virtue of how the pixels are arranged in the file. When you strip the metadata from images that use it for orientation, your images might display differently than they did before. A solution to this problem is to use the
.rotate
method without any arguments. This flips/flops and rotates an image to orient it based on the EXIF orientation, then strips the EXIF orientation data. The result is an image whose (removed) EXIF orientation is now baked in to the image. The display orientation is now the actual orientation, and it matches the previous EXIF orientation's instructions.There are some problems that arise even when developers are aware of Sharp's
.rotate(undefined)
functionality.Certain orientations prevent flip/flop after
.rotate()
If you use
.rotate()
on an image with certain EXIF orientations, you will not be able to flip or flop after that, because those operations were already performed.You cannot rotate again from the display orientation
What if you want to apply the EXIF orientation and then rotate 90 degrees? Sharp does not allow multiple calls to
.rotate
, sosharp(img).rotate().rotate(90)
will not work.Workarounds
Sharp gives you all the tools you need to work around these situations yourself. So if you are the one using Sharp directly you have two options:
Workaround 1: Buffer out first, then read and manipulate again
One solution is to
sharp(sharp(image).rotate().toBuffer()).rotate(90)
.Workaround 2: Do the orientation yourself, correcting for desired rotation/flip/flop
Another solution is to do the entire re-orientation manually.
You basically pretend Sharp doesn't have an auto-rotate feature at all.
How these workarounds fall short
Either of these work, but there are two issues I see with them: Users of Sharp get more complexity than they bargained for, and users of downstream tools might be completely unable to achieve what they desire.
Users of Sharp
Users of Sharp directly need to dig into EXIF orientation and understand how it works, which might be more than they bargained for when they wanted to rotate an image. Many of them will probably be just fine with
.rotate()
and call it a day. But anyone who wants to also apply their own rotations or flip/flop will need to strap in and read up on EXIF orientations.Users of downstream tools
Users of downstream tools are stuck if the tool creator didn't provide a way to deal with EXIF-oriented images. (That is what led me here.)
An example is the
imagetools
library which gives users a way to declare image transformations at import. That tool is used by an even further downstream tool for SvelteKit called@sveltejs/enhanced-img
, which modifies your HTML to create responsive images viasrcset
. It is incredibly convenient, but as of now, there is no great way to deal with images that are oriented via their EXIF data.There are obviously tons of other tools on NPM that use Sharp. How many of them handle EXIF orientation in ways that work for all their users?
You could argue that the creators of all these tools should consider this problem themselves and do the requisite corrections directly in their own code or provide an API that actually allows users to declare what they want. You could also argue that the developers who use these tools should pre-transform their images as well. (They could use Sharp to do it!)
Why should Sharp address this issue?
You can already manipulate images in Sharp to get your desired rotation. All it takes is for developers to write more/different code when they use Sharp. Here's what everyone can do with the tools that Sharp already provides:
But that is a lot of slow churn with a low chance of success.
Sharp already has all the information needed to satisfy all these users' desire: "I want to manipulate my images starting from how I see them displayed. I don't care about EXIF orientation."
If Sharp implements this proposal, none of the churn mentioned above needs to happen, and surely some code can be removed from projects already working around this issue.
This solution
It seems to me there are two ways to think about manipulating images with Sharp (or any tool).
Sharp uses option 1.
I'd like to propose an opt-in mode to Sharp that behaves as if all images start from their display orientation (whether actual or EXIF), and manipulates images from there.
You could opt in to this mode on a per instance basis via options.
Now, if you want to rotate an image slightly from how it displays, you can just do this:
Alternatively—for people like me using libraries built on Sharp that do not expose it—you could use an environment variable to default Sharp into this mode for all instances.
I hope this proposal is worth some consideration. I do have a POC branch going as I mentioned before. I have added several tests to ensure that the behavior I am describing works for all EXIF orientations. I am now in the process of fixing the approximately 15 existing tests that fail when you use the ENV var.
The text was updated successfully, but these errors were encountered: