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

Enable loop #19

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

andrey-moura
Copy link

Small changes that allow programmer to set the loop bytes

Small changes that allow programmer to set the loop bytes
gif.h Outdated
@@ -735,7 +735,7 @@ typedef struct
// Creates a gif file.
// The input GIFWriter is assumed to be uninitialized.
// The delay value is the time between frames in hundredths of a second - note that not all viewers pay much attention to this value.
bool GifBegin( GifWriter* writer, const char* filename, uint32_t width, uint32_t height, uint32_t delay, int32_t bitDepth = 8, bool dither = false )
bool GifBegin( GifWriter* writer, const char* filename, uint32_t width, uint32_t height, uint32_t delay, uint16_t loop = 0, int32_t bitDepth = 8, bool dither = false )

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment about the loop variable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment added

@charlietangora
Copy link
Owner

How well-supported is the loop setting these days? Back 10 years or so ago, I found that lots of browsers just totally ignored it and looped forever anyway. So I worry it might cause confusion to expose the loop bytes - "I tried to turn off looping, why is my gif still looping when I view it in Chrome?"

I'd also prefer to preserve API compatibility by adding new parameters at the end of the parameter list - with this change as is, if some client is setting bitdepth they will see a behavior change if they update the .h.

@andrey-moura
Copy link
Author

How well-supported is the loop setting these days? Back 10 years or so ago, I found that lots of browsers just totally ignored it and looped forever anyway.

I made a little test here, drag and drop the gif to play on browser and also putting an img in a html file to see those that respect the loop, these are the results with loop = 1:

Browser Drag and drop HTML image
Chrome ✔️ ✔️
FireFox ✔️ ✔️
IE ✔️
Android HTML Reader - ✔️
Firefox Android - ✔️
Chrome Android - ✔️

IE don't play the gif image, so no way to know if it works.

Also, I tested in some others applications:

App Works
Gmail Web ✔️
Gmail Android ✔️
DIscord Web ✔️
Android Galery

So I worry it might cause confusion to expose the loop bytes - "I tried to turn off looping, why is my gif still looping when I view it in Chrome?"

How about a comment just like the comment in the delay parameter?

The delay value is the time between frames in hundredths of a second - note that not all viewers pay much attention to this value.

I'd also prefer to preserve API compatibility by adding new parameters at the end of the parameter list - with this change as is, if some client is setting bitdepth they will see a behavior change if they update the .h.

I can change the parameter order if you want.

@andrey-moura
Copy link
Author

Sorry for the delay.

Now I reduced the comment size, changed loop to repeat (which makes more sense), and the repeat variable is on the end of the argument list

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

Successfully merging this pull request may close these issues.

3 participants