JonasKruckenberg / imagetools

Load and transform images using a toolbox :toolbox: of custom import directives!
MIT License
894 stars 55 forks source link

Some images get rotated for some reason #700

Open Robin-Sch opened 4 months ago

Robin-Sch commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

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

Etheryte commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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

Etheryte commented 3 months ago

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

Etheryte commented 3 months ago

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

happycollision commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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.

https://github.com/sveltejs/kit/issues/12262

happycollision commented 1 month ago

I've also created a PR to address this issue: https://github.com/JonasKruckenberg/imagetools/pull/724

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

Robin-Sch commented 1 month ago

Nice! Let me know if you need help

happycollision commented 1 month ago

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 commented 3 weeks ago

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