php-imagine / Imagine

PHP Object Oriented image manipulation library
https://imagine.readthedocs.io
Other
4.42k stars 530 forks source link

png_compression_filter behaves different between types / saving files #523

Open marcvanduivenvoorde opened 7 years ago

marcvanduivenvoorde commented 7 years ago

Both the Gd/Image and Imagick/Image use some validation on the 'png_compression_filter'. This validation differs between the types which prevents seamless switching of the backends.

The GD version supports the Php GD constants, while the Imagick supports an integer between 0 and 9.

Imagick/Image.php : 690 and down

// second digit: compression filter (default: 5)
            if (isset($options['png_compression_filter'])) {
                if ($options['png_compression_filter'] < 0 || $options['png_compression_filter'] > 9) {
                    throw new InvalidArgumentException('png_compression_filter option should be an integer from 0 to 9');
                }
                $compression += $options['png_compression_filter'];
            } else {
                $compression += 5;
            }

Gd/Image.php : 566 and down

if (isset($options['png_compression_filter'])) {
                if (~PNG_ALL_FILTERS & $options['png_compression_filter']) {
                    throw new InvalidArgumentException('png_compression_filter option should be a combination of the PNG_FILTER_XXX constants');
                }
                $args[] = $options['png_compression_filter'];
            }

I think it would be a better idea to create your own constants and convert them to the specific output section when needed. This would centralize the validation code and remove duplicates.

I think it would also be nice to have some sort of writer factory to separate the specificities based on filetype, instead of having all jpeg and png checks in one method.

avalanche123 commented 7 years ago

these sound like great ideas. would you like to become a maintainer?

marcvanduivenvoorde commented 7 years ago

First off, thank you :). I don't have really much time in my personal life to become a full maintainer, might do some small things. But I'll check with my employer if it's possible to throw in some company time ;).