Conversation
|
(it's pretty much just the player extracted from https://movableink.github.io/gif-inspector/) |
|
Sorry for the delay, did you close it because it sat for too long or another reason? Thanks! |
deanm
left a comment
There was a problem hiding this comment.
Thanks for doing this! Looks pretty good, a few things I probably need to check (there have been some recent github issues about how looping/disposal/etc works, and it has been years so I don't actually remember anymore)
| } else if (previousFrameInfo && previousFrameInfo.disposal === 2) { | ||
| // disposal was "restore to background" which is essentially | ||
| // "restore to transparent" | ||
| context.clearRect(previousFrameInfo.x, |
There was a problem hiding this comment.
It's been a long time, I would have to refresh myself on all of the disposal stuff again, but I am pretty sure to do it properly you need to be able to fill with a background colour and stuff like that. I know that my plask example didn't do this either, maybe this is a chance for me to refresh myself on the details again...
There was a problem hiding this comment.
Yes, there were a number of disposal methods in my original code that weren't quite right but should be fixed now. For background disposal, in practice all browsers dispose to transparent so I did that here as well.
|
|
||
| // get ready to draw next frame | ||
| previousFrameInfo = frameInfo; | ||
| frameNumber = (frameNumber + 1) % reader.numFrames(); |
There was a problem hiding this comment.
Why do this modulus and not just frameNumber === reader.numFrames() below?
There was a problem hiding this comment.
I've rearranged it to use frameNumber === reader.numFrames()
| frameNumber = (frameNumber + 1) % reader.numFrames(); | ||
|
|
||
| if (frameNumber === reader.numFrames() - 1) { | ||
| if (--loops === 0) { |
There was a problem hiding this comment.
This could just be if (frameNumber === reader.numFrames()) { frameNumber = 0; ... }
| } | ||
| } | ||
|
|
||
| timeout = setTimeout(draw, frameInfo.delay * 10); |
There was a problem hiding this comment.
Technically (although I am not exactly sure how the timers work), you would probably actually want to set the timer before doing the decode and blit. The reason is otherwise the time spent in decode/blit is skewing your timer back every frame.
There was a problem hiding this comment.
Thanks, I've done so and I believe it makes the timer more precise.
|
@deanm thanks for the review, there were definitely some edge cases that this wasn't catching. I think I've addressed them all now. |
|
@deanm ping |
import { GifReader } from 'omggif';
export const loadGifFrameList = async (
gifUrl: string,
): Promise<ImageData[]> => {
const response = await fetch(gifUrl);
const blob = await response.blob();
const arrayBuffer = await blob.arrayBuffer();
const intArray = new Uint8Array(arrayBuffer);
const reader = new GifReader(intArray as Buffer);
const info = reader.frameInfo(0);
return new Array(reader.numFrames()).fill(0).map((_, k) => {
const image = new ImageData(info.width, info.height);
reader.decodeAndBlitFrameRGBA(k, image.data as any);
return image;
});
}; |
| // save this frame in case we need to revert to it later | ||
| previousImageData = context.getImageData(0, 0, reader.width, reader.height); | ||
| previousImageData.frame = frameNumber; | ||
| } |
There was a problem hiding this comment.
should there be an else that makes previousImageData = null ?
There was a problem hiding this comment.
I don't think so, see https://github.com/deanm/omggif/pull/31/files/6b64106d7f5f3abd71b9166d166aa08e395b487e#diff-3ade314beb37a2490a4953585acfed048cef729832de94350bce3085be9f3e0aR73-R74 -- even if this frame doesn't get saved for later reversion, a future frame may use "restore to previous" and want to revert to the most recent saved frame.
|
Highly appreciated |
|
Ok |
This library works great but there were a few gotchas in using it to render gifs on the web with canvas and support most gif files. I thought this example might be useful for people trying to use
omggifin a web context.