Skip to content
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

Some images get rotated for some reason #700

Open
Robin-Sch opened this issue Mar 1, 2024 · 18 comments
Open

Some images get rotated for some reason #700

Robin-Sch opened this issue Mar 1, 2024 · 18 comments

Comments

@Robin-Sch
Copy link

I'm using @sveltejs/enhanced-img, and for some reason, my optimized images get "flipped" while the original files are the correct side up.

+page.server.js:

/** @type {import('./$types').PageServerLoad} */
export function load() {
	return {
		images: Object.values(import.meta.glob('$lib/assets/gallerij/*.{jpeg,png}', { query: { enhanced: true }, import: 'default', eager: true })),
		images2: Object.values(import.meta.glob('$lib/assets/gallerij/*.{jpeg,png}', { import: 'default', eager: true })),
	};
}

+page.svelte

<script>
    /** @type {import('./$types').PageData} */
    export let data;
</script>
{#each data.images as image, i}
    <div class="col-12 col-md-4 col-lg-3 h-auto p-2" style="order: {2 * i}">
        <enhanced:img src={image} class="w-100 h-auto" />
    </div>
{/each}
{#each data.images2 as image, i}
    <div class="col-12 col-md-4 col-lg-3 h-auto p-2" style="order: {2 * i + 1}">
        <img src={image} class="w-100 h-auto" />
    </div>
{/each}

This does however not happen with all images for some reason???
Example image which gets rotated:
062nbppmfl20hed1

See below: left image is optimized, right image is original
image

@JonasKruckenberg
Copy link
Owner

Can you confirm this also happens with imagetools directly? The enhanced query parameter is not defined by us but by the svelte package you are using, so unless this is reproducible with imagetools directly I would assume it's a fault on their side not ours.

@Robin-Sch
Copy link
Author

Sorry for my late reponse, will take a look into this in a few weeks. Life's busy atm

@Etheryte
Copy link

Etheryte commented Mar 30, 2024

I'm running into the same issue and the problem is reproducible with the vite-simple demo project in this repo. The picture ends up 90 degrees rotated, even though it looks alright in the OS image viewer and likewise when you simply open it in your browser.

I've attached an archive of the jpg that causes the issue so that the file doesn't get touched by any minifiers on Github's side: example.jpg.zip

The file is a jpg right out of a camera, and while I'm not too familiar with image formats, it seems to me the image data is kept unrotated and then metadata is used to rotate it as it should be? One online exif tool tells me the Orientation field is left-bottom, another says Rotate 270 CW. If all metadata is stripped away, including orientation information, that would explain the issue.

@benmccann
Copy link
Collaborator

@Etheryte are you saying that the issue reproduces using one of the existing images in vite-simple? If so, which one and what OS are you on? Or are you saying that if I extract the example.jpg.zip that you've provided and put it in the vite-simple project that it will reproduce the issue?

@Etheryte
Copy link

@benmccann The latter, that the example file I provided reproduces the issue when you put it in the vite-simple project.

@Etheryte
Copy link

I've made a minimal code demo that replicates the issue for me: https://github.com/Etheryte/imagetools-demo.

@Etheryte
Copy link

This seems to be a bug in Sharp itself, at least for my image, so I've gone ahead and filed an issue there: lovell/sharp#4047

@happycollision
Copy link

@Etheryte's issue on Sharp reveals the way to work around the problem, but it seems like we cannot take advantage of it via the exposed API in imagetools.

Summary (hopefully correct): Some images are rotated via metadata, and some are rotated via rearranging the pixels. Sharp drops metadata during resize and so any images rotated via metadata will appear in a surprising orientation after said resize.

There is a workaround for this problem, but I think that when we apply rotation via ?enhanced&rotate=0, either nothing happens or it is applied after the metadata is already stripped.

@Etheryte
Copy link

Yeah that's a good summary, and as far as Sharp's author is concerned it's working as intended. To fix the problem for Imagetools, the mentioned workaround would have to be integrated here, using the workaround from the currently exposed API does not work as far as I tried it.

@happycollision
Copy link

happycollision commented May 26, 2024

Ah, okay. There is a way for imagetools users to work around this by changing the config to the following:

imagetools({
  removeMetadata: false,
})

Seems like there is also potentially a way to use a custom directive to do thing the way you want, but I am not sure if it gives you control over the order of operations, which I think is the sticking point with the rotate() workaround.

It'd be nice if there was a way to whitelist certain metadata and let all the other metadata be stripped. Then we might do

imagetools({
  removeMetadata: true,
  allowMetadataFields: ["orientation"],
})

Or something like that.


Either way, this does not help in the case of enhanced:img because we have no way to pass the config for imagetools into enhanced:img. Unless there is some way of configuring Vite that I am unaware of. So now I am wondering if the workaround does indeed need to bubble up to enhanced:img after all. @benmccann, what do you think?

Update: Rather than wait for @benmccann to answer and then wait again for me to open an issue... I went ahead and opened the issue already. Feel free to close it if enhanced:img is not the place to solve this problem.

sveltejs/kit#12262

@happycollision
Copy link

happycollision commented May 29, 2024

I've also created a PR to address this issue: #724

My initial take on it was naive, and I have force-pushed a new approach as of this morning.

Update: Upstreamed into Sharp

@Robin-Sch
Copy link
Author

Nice! Let me know if you need help

@happycollision
Copy link

I am actually playing with proposing this even further upstream to Sharp. But still playing with an implementation there, as I’ve never worked with C++ before.

The PR I have open works, but uses a buffer back into Sharp to set initial orientation in cases where Sharp won’t allow the transformation to work logically (that I could find). If you’d like to use my fork for now, feel free! If the maintainer of Sharp doesn’t like my proposal, I’ll keep tinkering on this to see if I can get it to work without the buffer step in the middle.

@happycollision
Copy link

I've finally gotten the code to work, and have opened an issue in Sharp proposing the new feature: lovell/sharp#4144

@Robin-Sch
Copy link
Author

Btw, do you have any idea when this will be merged? Iirc, I can not (easily) use the work around in @sveltejs/enhanced-img right? Or would this be very easy to do so

@happycollision
Copy link

happycollision commented Sep 18, 2024

Not soon, but it is still on the map. In the meantime you could potentially employ the same trick I am using in my project to get it to work: apply small patches to both this library and to Sharp.

Have a look at my patches directory here: https://github.com/postplayhouse/postplayhouse.com/tree/f524f9549ee7085e5cc6bd0b5a81e2a62347b8b9/patches

You’ll want to either use the NPM patch package library, or the built in patching tools provided by pnpm or yarn2.

@Robin-Sch
Copy link
Author

Robin-Sch commented Sep 24, 2024

This gives an error for me tho:

[imagetools] Could not load .../blabla.jpeg?enhanced=true (imported by blabla): Input file has corrupt header: webp: unable to parse image
     at Sharp.metadata (node_modules/sharp/lib/input.js:487:17)
     at Object.load (file://node_modules/vite-imagetools/dist/index.js:113:74)
     at async PluginDriver.hookFirstAndGetPlugin (file://node_modules/rollup/dist/es/shared/node-entry.js:19836:28)
     at async file://node_modules/rollup/dist/es/shared/node-entry.js:19017:33
     at async Queue.work (file://node_modules/rollup/dist/es/shared/node-entry.js:20046:32)

@happycollision
Copy link

happycollision commented Jan 18, 2025

Great news! The next version of Sharp will contain the fulfillment of my proposal to add features to make this whole problem much easier for downstream libraries to solve.

This means that imagetools can take advantage of a group of new features, all known as "autoOrient".

New Sharp Features (coming in 0.34.0)

Instance method

sharp.autoOrient()

Once you have a sharp instance, you can explicitly call instance.autoOrient() on it, which will read the EXIF orientation data, apply the appropriate transformations, then drop the orientation data, thus "baking in" the correct orientation of the image. This transformation is logically applied before all other rotations, flips, or flops. If there is no EXIF orientation data, then it does nothing.

Essentially, it makes Sharp behave the way people think it would if they don't realize their images are oriented with metadata instead of with the actual arrangement of pixels in the file.

Option

sharp(img, {autoOrient: true})

Same as the above, but you can call it at instantiation instead of after. Both do the same thing under the hood.

Composite Option

sharp(background).composite([{ input: image, autoOrient: true }])

The same thing, available as a compositing option.

Resulting height and width in metadata

New properties are available on image metadata that give you the resulting height and width assuming that you wish to invoke autoOrient somewhere. Since Sharp's metadata information is gathered before transformations are applied, the developer has to track whether or not they want the original or the altered height and width.

let { 
  height: originalHeight, 
  width: originalWidth,
  autoOrient: {
    height: afterOrientHeight,
    width: afterOrientWidth,
  }
} = await sharp(image).metadata()

Recommendations

I've been digging into this issue rather deeply, starting back in Svelte's enhanced-img where I first realized it was not possible for me to use it (easily and reliably) on my website. (Yes, there are workarounds, but now we have better tools!)

I would suggest adding some kind of config option and an explicit url param for consumers.

import { defineConfig } from 'vite'
import { imagetools } from 'vite-imagetools'
    
export default defineConfig({
  plugins: [imagetools({

    // Runs `.autoOrient()` on all images, unless opted out in the url
    defaultAutoOrient: true

    // Only runs `.autoOrient()` on images when opted in via the url
    defaultAutoOrient: false

  })]
})

At the call site:

import img1 from "./img1.jpg?autoOrient=true"
import img2 from "./img2.jpg?autoOrient=false"

Furthermore, I'd suggest the default behavior for imagetools should be

// Next minor release
{ defaultAutoOrient: false }

// Next major release
{ defaultAutoOrient: true }

I am guessing it is far less surprising to do the autoOrient on all images by default than the current way things work. But @JonasKruckenberg knows his users better than I!

Future work

I'd be happy to do the development work on this once the next version of Sharp is released. Honestly, the entire reason I worked on these new features for Sharp were to get them into imagetools and then into enhanced-img, so doing the work to get it shipped here would be kinda fun.

Assuming this feature is eventually added, I'd suggest that enhanced-img pass basically the same features down to its users as well, @benmccann. I would also be happy to do that, seeing these features through to the last mile.

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

No branches or pull requests

5 participants