hidefromkgb / gif_load

A slim, fast and header-only GIF loader written in C
80 stars 6 forks source link

Usage from c++ has a minor flaw #7

Closed wcout closed 5 years ago

wcout commented 5 years ago

Thanks for this awesome work!

I use GIFLIB currently for decoding, but am tempted to switch to this header only masterpiece.

There is only one minor flaw using it in class contexts:

I am including gif_load.h in the source file of my class. Declaring the private callback method static MyClass::gifLoadCallback(void *, GIF_WHDR *) in the corresponding header is unfortunately not possible, as one cannot forward declare a typedef struct.

[Declaring the callback locally as a non member function in the source file is not an option, because called methods by MyClass would need to be declared public and with a void *argument for GIF_WHDR - both is not very nice].

As a workaround I have changed gif_load.h by declaring the struct without typedef as _GIF_WHDR and follow it with a typedef struct _GIF_WHDR GIF_WHDR and use _GIF_WHDR for the forward declaration.

Here is the diff:

diff --git a/gif_load.h b/gif_load.h
index e2cace7..28e3200 100644
--- a/gif_load.h
+++ b/gif_load.h
@@ -46,7 +46,7 @@ extern "C" {
 #define _GIF_SWAP(h) ((GIF_BIGE)? ((uint16_t)(h << 8) | (h >> 8)) : h)

 #pragma pack(push, 1)
-typedef struct {                 /** ======== frame writer info: ======== **/
+struct _GIF_WHDR {               /** ======== frame writer info: ======== **/
     long xdim, ydim, clrs,       /** global dimensions, palette size      **/
          bkgd, tran,             /** background index, transparent index  **/
          intr, mode,             /** interlace flag, frame blending mode  **/
@@ -56,8 +56,9 @@ typedef struct {                 /** ======== frame writer info: ======== **/
     struct {                     /** [==== GIF RGB palette element: ====] **/
         uint8_t R, G, B;         /** [color values - red, green, blue   ] **/
     } *cpal;                     /** current palette                      **/
-} GIF_WHDR;
+};
 #pragma pack(pop)
+typedef struct _GIF_WHDR GIF_WHDR;

 enum {GIF_NONE = 0, GIF_CURR = 1, GIF_BKGD = 2, GIF_PREV = 3};

Do you think this is a good solution or do you have a better idea?

hidefromkgb commented 5 years ago

Seems like I`ve got a better option. Let`s just rename GIF_WHDR to struct GIF_WHDR, getting rid of the typedef once and for all.

hidefromkgb commented 5 years ago

Resolved in c283802. Please confirm.

wcout commented 5 years ago

Works like a charm (and in C++ one does not even need to use struct in the parameter declarations). Certainly the best solution. Thank you for your work!

hidefromkgb commented 5 years ago

Great! Please don`t forget to report GIFs it decodes wrong in case you stumble upon any =) Closing as fixed.