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

Typo #47

Closed
blitzbohne opened this issue Apr 13, 2020 · 5 comments
Closed

Typo #47

blitzbohne opened this issue Apr 13, 2020 · 5 comments

Comments

@blitzbohne
Copy link

Hi,

There's a typo in here: https://github.com/photopea/UPNG.js/blob/master/UPNG.js#L672

It should be

p32 = new Uint32Array(pimg.buffer)

Otherwise the diff for the changed region doesn't work at all and always returns 0,0,maxWidth,maxHeight

I fixed it locally for me and I can now generate GIFs correctly with changed regions, however I can now see artificacts any idea why? When I dismiss the region using your UPNG.compress function and always set 0,0,maxWidth,maxHeight in my gif it works perfectly?

@blitzbohne
Copy link
Author

Seems it is the same issue as stated here:

#40

When I call compress via

const pres = UPNG.encode.compress(
frames.map(frame => frame.data),
frames[0].width,
frames[0].height,
256,
[true, false, false, 0, false]
)

instead of

const pres = UPNG.encode.compress(
frames.map(frame => frame.data),
frames[0].width,
frames[0].height,
256,
[false, false, false, 0, false]
)

So I enforce blend its gone. However this can not be the idea right? Any idea what's wrong? @photopea

@blitzbohne
Copy link
Author

blitzbohne commented Apr 13, 2020

test2

^^ This is the original GIF I used

express-new-test (22)

^^ This is generated gif with artifacts (slow-mo so u can see) AFTER my proposed typo fix so regions are set correctly but WITHOUT setting alwaysBlend to true

express-new-test (25)

^^ This is the generated gif with alwaysBlend set to true and the region fix proposed. This has no more artifacts but I surely do not want to enforce blending if not possible to the second case seem to be broken?

Btw great library!

@photopea
Copy link
Owner

Hi, are you talking about this?

var pimg = new Uint8Array(bufs[j-1-it]), p32 = new Uint32Array(bufs[j-1-it]);

If the array "bufs" is the array of ArrayBuffers, the "bufs[j-1-it]" should be equal to pimg.buffer . Are you sure you are not sending Uint8Arrays in "bufs"?

@blitzbohne
Copy link
Author

@photopea I am actually sending the image data directly which is an UInt8 array that might have been the mistake indeed though changing the code line accordingly fixes it too and one can directly send canvas image data ;)

@photopea
Copy link
Owner

When you have a Uint8Array, use the .buffer property to access its ArrayBuffer. I am not a fan of allowing multiple types as a parameter of a function. Just pre-process the input data before givint it to UPNG.

@photopea photopea closed this as completed May 7, 2020
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

2 participants