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

Wrong interpretation of disposalType #35

Open
raphaelrauwolf opened this issue Apr 27, 2021 · 7 comments
Open

Wrong interpretation of disposalType #35

raphaelrauwolf opened this issue Apr 27, 2021 · 7 comments

Comments

@raphaelrauwolf
Copy link

raphaelrauwolf commented Apr 27, 2021

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
Copy link

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
Copy link
Author

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

@AdamJaggard
Copy link

@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
Copy link
Owner

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
Copy link
Author

raphaelrauwolf commented Apr 29, 2021

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
Copy link

sebavan commented Apr 25, 2022

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

benwest added a commit to benwest/gifuct-js that referenced this issue Apr 27, 2022
See matt-way#35: `frame.disposalType` is used incorrectly in the demo, which had me scratching my head for a while today.
@johnhyde
Copy link

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

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

5 participants