Skip to content

png_compression_filter behaves different between types / saving files #523

Open

Description

@marcvanduivenvoorde

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.

Activity

  1. avalanche123 commented on Jan 5, 2017

    @avalanche123
    Collaborator

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

  2. marcvanduivenvoorde commented on Jan 9, 2017

    @marcvanduivenvoorde
    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions