photopea / UPNG.js

Fast and advanced PNG (APNG) decoder and encoder (lossy / lossless)
MIT License
2.1k stars 259 forks source link

Typo #47

Closed blitzbohne closed 4 years ago

blitzbohne commented 4 years ago

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

Seems it is the same issue as stated here:

https://github.com/photopea/UPNG.js/issues/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 commented 4 years ago

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

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

@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 commented 4 years ago

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.