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

Picture misplaced #217

Open
wuweikd opened this issue Sep 1, 2021 · 19 comments
Open

Picture misplaced #217

wuweikd opened this issue Sep 1, 2021 · 19 comments

Comments

@wuweikd
Copy link

wuweikd commented Sep 1, 2021

Your library is great, but I have some problems

I don't know what I did wrong, and occasionally some pictures will be misplaced. I framed them with a red frame.

The probability of error is about 5%, and our testers and developers cannot reproduce it. But users can always have this situation. It should be related to the type of phone, but I don't have definite evidence. I want to know why.

反馈2

反馈3

The following code is my compression method. Accept a File type and return the compressed File.

const pica = require('pica')();

const maxSize = 2097152 // 2M
// const maxSize = 1000000
const isLarge = (size) => {
    return size > 2097152 * 3 // 6M
}
/*
* @params myFile: type is File
* */
export default (myFile) => {
    try {
        return new Promise((res, rej) => {
            let reader = new FileReader();
            reader.readAsDataURL(myFile);
            reader.onload = function (theFile) {
                let image = new Image();
                image.src = theFile.target.result;
                image.onload = function () {
                    const offScreenCanvas = document.createElement('canvas')
                    const big = this.width > 2000 || this.height > 2000
                    const veryBig = this.width > 4000 || this.height > 4000
                    offScreenCanvas.width = veryBig ? this.width * 0.3 : (big ? this.width * 0.5 : this.width)
                    offScreenCanvas.height = veryBig ? this.height * 0.3 : (big ? this.height * 0.5 : this.height)
                    pica.resize(image, offScreenCanvas, {
                        quality: 1,
                    }).then(result => pica.toBlob(result, 'image/jpeg', isLarge(myFile.size) ? 0.4 : 0.6))
                        .then(blob => {
                            let files = new window.File([blob], myFile.name, {type: 'image/jpeg'})
                            if (files.size > maxSize) {
                                rej('图片太大:' + files.size)
                            }
                            res(files) // return new File
                        });
                };
            };
        })
    } catch (e) {
        if (myFile.size > maxSize) {
            return false
        }
        return myFile
    }
}
@cukecooker
Copy link

Same issue here. try disable wasm and ww features for you Pica instance;like new Pica({features:['js']})

@puzrin
Copy link
Member

puzrin commented Dec 10, 2021

@wuweikd

  • Try v9
  • Try reduce .tile to 512
  • Try disable webworkers

Let me know if problem is still actual.

@chebum
Copy link

chebum commented Dec 10, 2021

  • Try reduce .tile to 512

@puzrin why reducing tile size may be helpful?

@puzrin
Copy link
Member

puzrin commented Dec 10, 2021

@chebum, in theory, that reduces memory consumption 4x (in all concurrent workers), if device has low limits and crashes or abnormal behaviour happens. But i have no conditions to investigate behaviour of all possible hardware.

1024*1024 => 4mb per tile + ~4mb for intermetiate buffer (in v9). With 4 workers => 32mb. Not too much.

The similar effect may be reached with restriction of peak workers or disabling those at all.

@chebum
Copy link

chebum commented Dec 14, 2021

@puzrin Disabling web workers also solves accidental STATUS_ACCESS_VIOLATION in Edge 96.0.1054.53. I'm able to reproduce the problem in 100% cases using specific set of images.
image

Chrome 96.0.4664.93 is working fine, but Chrome 96.0.4664.110 crashes. Disabling WebWorkers does NOT solve the crash in Chrome 96.0.4664.110.

@puzrin
Copy link
Member

puzrin commented Dec 14, 2021

@chebum i guess, that's another bug, from wasm. Try do disable it. It that helps - try to review source, may be i missed some zero division or byte/word overflow.

Anyway, please report that separate, with example how to reproduce.

@chebum
Copy link

chebum commented Dec 14, 2021

Anyway, please report that separate, with example how to reproduce.

That's the hardest part. I was able to reproduce it in the bigger application, but wasn't yet able to reproduce the problem separately :(

I found a fix for Chrome as well - I have to clone source canvas before passing it to pica.resize. This way it worked even with web workers. Cloning source canvas solved the problem both in Edge and Chrome.

I tested with 7.0.0 as well. It behaves the same way as 9.0.0, so I doubt it's the latest change.

Will report in a separate issue if I find more information about it.

@puzrin
Copy link
Member

puzrin commented Dec 14, 2021

I tested with 7.0.0 as well. It behaves the same way as 9.0.0, so I doubt it's the latest change.

Thank you for info, that's important to know for me.

In our app, uploader has 2 stage resizer - client-side & server-side. If client-side code fails, image is sent "as is".

@asia215
Copy link

asia215 commented Feb 27, 2022

Ihre Bibliothek ist großartig, aber ich habe einige Probleme

Ich weiß nicht, was ich falsch gemacht habe, und gelegentlich werden einige Bilder verlegt. Ich habe sie mit einem roten Rahmen eingerahmt.

Die Fehlerwahrscheinlichkeit liegt bei ca. 5 % und kann von unseren Testern und Entwicklern nicht reproduziert werden. Aber Benutzer können diese Situation immer haben. Es sollte mit der Art des Telefons zusammenhängen, aber ich habe keine eindeutigen Beweise. Ich will wissen warum.

反馈2 反馈3

Der folgende Code ist meine Komprimierungsmethode. Akzeptieren Sie einen Dateityp und geben Sie die komprimierte Datei zurück.

const pica = require('pica')();

const maxSize = 2097152 // 2M
// const maxSize = 1000000
const isLarge = (size) => {
    return size > 2097152 * 3 // 6M
}
/*
* @params myFile: type is File
* */
export default (myFile) => {
    try {
        return new Promise((res, rej) => {
            let reader = new FileReader();
            reader.readAsDataURL(myFile);
            reader.onload = function (theFile) {
                let image = new Image();
                image.src = theFile.target.result;
                image.onload = function () {
                    const offScreenCanvas = document.createElement('canvas')
                    const big = this.width > 2000 || this.height > 2000
                    const veryBig = this.width > 4000 || this.height > 4000
                    offScreenCanvas.width = veryBig ? this.width * 0.3 : (big ? this.width * 0.5 : this.width)
                    offScreenCanvas.height = veryBig ? this.height * 0.3 : (big ? this.height * 0.5 : this.height)
                    pica.resize(image, offScreenCanvas, {
                        quality: 1,
                    }).then(result => pica.toBlob(result, 'image/jpeg', isLarge(myFile.size) ? 0.4 : 0.6))
                        .then(blob => {
                            let files = new window.File([blob], myFile.name, {type: 'image/jpeg'})
                            if (files.size > maxSize) {
                                rej('图片太大:' + files.size)
                            }
                            res(files) // return new File
                        });
                };
            };
        })
    } catch (e) {
        if (myFile.size > maxSize) {
            return false
        }
        return myFile
    }
}

Gu

@capc0
Copy link

capc0 commented Nov 7, 2024

I can reproduce this on android using the Android System Webview Dev-Channel. This only happens when Webworkers are used. Removing ww from the features (https://github.com/nodeca/pica?tab=readme-ov-file#new-picaconfig) prevents this issue.

Maybe it is not handled correctly that the response order of webworkers is effectivly random?

image

@Airdevelopments
Copy link

It seems this issue was introduced due to some changes coming with Chromium 131. Since this version is being shipped at this very moment, I would expect the bug to appear more frequently.

Some notes that might help tracking down the cause:

  • as mentioned by others before: not using WebWorkers but simply using plain 'js' does not seem to cause the issue
  • I tried various files, some worked flawlessly, some had issues. While my sample size of files was not very large, here are some things I observed
    • having an image taken with an android camera in HDR mode (still outputting a JPEG though) seemed to make this issue more likely
    • from those HDR images, only the ones that had EXIF Orientation set to for example "90 degrees CW" (portrait mode pictures) seemed to be having the issue when scaling
      • clearing the EXIF Orientation field of those images and then trying the resize again did not result in the mentioned artifacts

I attached an image (hopefully GitHub will not alter it) that I could use to reproduce the issue 100% of the time on desktop Chrome version 131. (Make sure to use WebWorker feature). If you require any further info / a small reproduction repository, I am happy to help.

Thank you for this great library!
IMG_20240708_154607312_HDR

@gkostov
Copy link

gkostov commented Nov 18, 2024

@Airdevelopments , thanks so much for the sample image - finding a way to reproduce a problem is often a major problem on its own. It failed straight away on a desktop Chrome 131 when I used it and the output image was having its blocks misplaced. I'll see what pica setting may help avoid it.

PS.

Tried and confirmed that the problem in desktop Chrome 131 disappears with

    new Pica({features:['js']})

Cheers

@Airdevelopments
Copy link

I believe I was able do track down the source of the issue.

As mentioned before, I suspected EXIF Orientation to play a major role here. When I took a closer look at how the images were mangled, I realized the symmetry around the two main axis:

image

While debugging through the pica code in Chrome and Firefox at the same time I was able to find the difference in behavior. The culprit seems to be createImageBitmap (https://developer.mozilla.org/en-US/docs/Web/API/Window/createImageBitmap).

On Chrome 131 the region parameters of the function (sx: number, sy: number, sw: number, sh: number) seem to no longer respect the EXIF Orientation. In Firefox they still do.

I created a simple repository to show the discrepancy. Open it in Chrome 131 and the latest Firefox version (I used 132) and you should see the difference (mirrored coordinate spaces).

I am not sure which behavior the ECMAScript Spec defines, but it does feel like this might be a regression in chromium.

This Chromium issue had some fairly recent activity which could have introduced the regression. Although I obviously can not be sure at this point.

@samueldominguez
Copy link

I also managed to solve it with new Pica({features:['js']}) which is OK in my case since I do not use WW. Tested on Chrome 131.0.6778.86 (arm64)

@didotsonev
Copy link

Thank you for your work 🙏

I've faced the same issue here, but only with one specific image size of 4,2MB 4000x3000 made on samsung s24 ultra (as well for Galaxy S22 and Galaxy TAB S9+).
Locally I've reproduced it on chrome (131.0.6778.86 (arm64) and 131.0.6778.109 (arm64)), while on Firefox 130 and 133.0 (aarch64) it's working as expected.

Removing the web worker seems to fix the issue:
From Pica({ features: ['js', 'wasm', 'ww', 'cib'] }); to Pica({ features: ['js', 'wasm', 'cib'] });

Original image:
12-part-original-image-samsung-s24-ultra

Resized:
test

@Airdevelopments
Copy link

@didotsonev I would be careful with the 'cib' feature (though it technically is the fastest scaling operation afaik), as it is the root cause for the issue on Chrome >= 131 when tiling is used (not adjusted coordinate space). In practice the preconditions to use 'cib' to scale the image directly are rarely fulfilled. That means even though you have the feature in your "features" array, it will likely not be used. I have not done any testing in regard to issues with EXIF-Orientation and direct usage of 'cib'-scaling on Chrome >= 131. It may work perfectly fine since no tiling operation is used and therefore the described issue itself should not occur.

Lets hope this gets fixed soon by the Chromium Team.

@puzrin
Copy link
Member

puzrin commented Dec 6, 2024

@Airdevelopments thank you for sharing details. If you have time, i'd suggest experiments with disabling offscreen canvas use in ww.

https://github.com/nodeca/pica/blob/master/lib/utils.js#L102

Patch this to always reject the promise. The data will be extracted in the main thread and sent to ww as a typed array. This can increase main thread blocking a bit, but maybe it will help. It's difficult to say clearly which step orientation data is missing.

@omprakashyadav
Copy link

I’m experiencing an issue where, even with non-rotated images, the problem is not consistently reproducible. However, when uploading an image with rotation and EXIF orientation metadata, the output appears scrambled and tiled. Disabling Web Workers resolves the issue.

Does anyone know why this is happening? Would this issue occur even if we bypass Pica and use a plain HTML Canvas for resizing? Any insights would be appreciated!

@sabercoy
Copy link

sabercoy commented Dec 22, 2024

async function createImageBitmap_WITH_RESPECT(img, x, y, width, height) {
  return new Promise((res, rej) => {
    const offscreenCanvas = new OffscreenCanvas(width, height);
    const contextOffscreen = offscreenCanvas.getContext('2d');

    contextOffscreen.drawImage(img, x, y, width, height, 0, 0, width, height);
    
    res(offscreenCanvas.transferToImageBitmap());
  });
}

async function resize() {
    const file = upload.files.item(0);
    if (!file) {
        return;
    }

    const base64ImageData = await getImageDataAsBase64(file);
    const img = await createHTMLImage(base64ImageData);

    const tileX = tileXInput.valueAsNumber;
    const tileY = tileYInput.valueAsNumber;
    const tileWidth = 3072 / 4;
    const tileHeight = 4080 / 4;
    // const bitmap = await createImageBitmap(img, tileWidth * tileX, tileHeight * tileY, tileWidth, tileHeight);
    const bitmap = await createImageBitmap_WITH_RESPECT(img, tileWidth * tileX, tileHeight * tileY, tileWidth, tileHeight);

    const context = outputCanvas.getContext('2d');
    context.drawImage(bitmap, 0, 0, tileWidth, tileHeight);
}

@Airdevelopments If we pass it through the veil of an OffscreenCanvas, which apparently respects orientation unlike createImageBitmap(), maybe we can make it work until its resolved lol

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