cgifsave: number of colours in palette is one less than it could/should be #2852
Labels
Comments
|
Ooop, good catch, I'll fix it in my branch. |
|
Should be fixed with #2853 |
|
I noticed that this change produces artefacts with $ vips copy skate.gif[n=-1] x.gifReverting it back to @@ -731,7 +731,7 @@ vips_foreign_save_cgif_build( VipsObject *object )
/* Set up libimagequant.
*/
cgif->attr = vips__quantise_attr_create();
- vips__quantise_set_max_colors( cgif->attr, 1 << cgif->bitdepth );
+ vips__quantise_set_max_colors( cgif->attr, (1 << cgif->bitdepth) - 1 );
vips__quantise_set_quality( cgif->attr, 0, 100 );
vips__quantise_set_speed( cgif->attr, 11 - cgif->effort ); |
|
This looks like it might be something to do with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The number of colours in the palette of GIF output via cgifsave is currently set to one less than it could be due to this line:
libvips/libvips/foreign/cgifsave.c
Line 685 in bae0342
This can be highlighted by setting
bitdepthto 1, resulting in a call toliq_set_max_colorswith the value 1, which is out of the 2-256 range it supports. We currently ignore the error return code so the output GIF has the (default) bitdepth of 8, which is incorrect.I'm probably to blame here as I originally wrote this code, with the logic of decrementing by 1 to allow space for an extra "colour" for the transparency, if any. However I think with all the recent updates to this code that this logic is no longer required, so the following change should fix this.
I'm aware there's more refactoring going on in and around this code so am creating an issue to track it rather than open a PR that may conflict with any work-in-progress.
The text was updated successfully, but these errors were encountered: