FLIF-hub / FLIF

Free Lossless Image Format
Other
3.72k stars 229 forks source link

Try to mention flif_encoder_set_alpha_zero_lossless somewhere #470

Open lixo6500 opened 7 years ago

lixo6500 commented 7 years ago

The API is simple and pretty much self-explanatory, which is great.

But I feel like you should mention somewhere the importance of flif_encoder_set_alpha_zero_lossless in case someone cares about the original color channel values when the alpha channel is 0.

I was testing the library and my tests were failing and I spent quite a while trying to figure out what I was doing wrong. Then I remembered a similar situation in WEBP, where they set the pixel to 0 when the alpha is 0, in order to improve compression. Only then I start to hunt for another function that could be controlling that, because either there is no mention about it or, if there is, then it is certainly well hidden :)

jonsneyers commented 6 years ago

I added a comment in flif_enc.h (https://github.com/FLIF-hub/FLIF/blob/master/src/library/flif_enc.h#L59), I hope that is enough.

The default is indeed to not encode RGB values of fully transparent pixels. Whether or not that is the right thing to do, can of course be debated. It will depend on the use case. If alpha is being used as some kind of "selection mask", then surely it can be useful to keep the RGB values of the "non-selected" pixels, so you can create a different "selection mask" if you want. To me that behavior is mostly useful for image editing formats like XCF (with no or simple compression), not for image compression formats. In a "final" image with transparency, the RGB values of invisible pixels are meaningless (in a sense they are the results of divisions by zero), and it can be a significant cost to encode them (and you might be unintentionally revealing parts of an image that were intended to be "cut out", like a cropped or otherwise censored JPEG image that still has a preview of the full original photo in its Exif metadata). This is why I think the default makes sense. But I agree that it should be made clear that that's the default and you can change it using that function.