matt-way / gifuct-js

Fastest javascript .GIF decoder/parser
MIT License
447 stars 63 forks source link

Wrong interpretation of disposalType #35

Open raphaelrauwolf opened 3 years ago

raphaelrauwolf commented 3 years ago

In your demo https://matt-way.github.io/gifuct-js/ , GIFs don't get rendered correctly, when you try out more complex ones.

For example try out https://media0.giphy.com/media/KzW7esJ9VRB4SuEyIA/giphy.gif

Frame.disposalType addresses what happens to the frame data AFTER painting the frame.

Quote from your link

The disposal method specifies what happens to the current image data when you !!!move onto the next!!!. Our sample animated image has a value of 1 which tells the decoder to leave the image in place and draw the next image on top of it. A value of 2 would have meant that the canvas should be restored to the background color.

if (frame.disposalType ===  2) {
    gifCtx.clearRect(0, 0, c.width, c.height)
}

should be something like

const prevFrame = loadedFrames[frameIndex - 1];
if (prevFrame.disposalType ===  2) {
    const { width, height, left, top } = prevFrame.dims;
    gifCtx.clearRect(left, top, width, height);
}

Love the decoder though! <3

AdamJaggard commented 3 years ago

Hey @raphaelrauwolf, I think you have the clearRect params the wrong way round gifCtx.clearRect(width, height, left, top); should be gifCtx.clearRect(left, top, width, height);. Thanks for the tip though!

raphaelrauwolf commented 3 years ago

You are right, but not really the point 😋 it's not really a big issue, just wanted to let you know.

AdamJaggard commented 3 years ago

@raphaelrauwolf I'm not involved with the project at all but your comment came at a perfect time as I was in the middle of implementing the library and I wouldn't have noticed it myself.

matt-way commented 3 years ago

I am happy to put this change (and other changes like it) in, but just want to reemphasise that this lib is for parsing/decoding gifs, not necessarily rendering them. I made the demo quickly to showcase a rendering example.

Around half of the issues seem to be about rendering, so maybe there's a need for a proper rendering lib too. I'd happily point links to it if someone builds one.

raphaelrauwolf commented 3 years ago

I totally understand! And the lib, the parsing and decoding is clutch!

Just wanted to limit the number of devs that stumble over the same issue and make sure they don't waste their time!

sebavan commented 2 years ago

Hey @raphaelrauwolf you here :-) thanks a lot for the tip !!! was exactly running into this

johnhyde commented 7 months ago

Thank you so much for posting this, I was mimicking the code in the demo and the suggested reading didn't clarify that only for disposalType 2, only the area of the patch applied should be cleared before the next patch. I was clearing the whole canvas, which did not work as I had hoped!

It should be noted that despite the PR name "Correct use of disposalType in demo", the demo still incorrectly disposes of patches with type 2. It should only clear the portion of the canvas that was patched by the previous frame. (At least this has been my experience with this pikachu gif I've been testing with) pikachu