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

png_compression_filter behaves different between types / saving files #523

Open
marcvanduivenvoorde opened this issue Jan 5, 2017 · 2 comments

Comments

@marcvanduivenvoorde
Copy link

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
Copy link
Collaborator

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

@marcvanduivenvoorde
Copy link
Author

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 ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants