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

Change compression to 0-100, update docs. #12

Closed
wants to merge 2 commits into from

Conversation

wshaver
Copy link

@wshaver wshaver commented Dec 4, 2013

There is a difference in file size between compression of 99 and 100. The current code sets an int of '9' to a compression of 99, and other ints to always end in a 9.

For example I have a generated file with these sizes:
compression level : file size
50: 133kb
89: 156kb
90: 118kb
99: 155kb
100: 118kb
119: 155kb

Frankly I have no idea why the compressor has much bigger files when you give it an option that ends in in '9'. But the current way the compression options are passed to the compression forces them to end in a 9.

@wshaver
Copy link
Author

wshaver commented Dec 5, 2013

Note: this is with the 'gm' compositor.

@selaux
Copy link
Owner

selaux commented Dec 5, 2013

Yes there is a compression difference. The thing is that the quality parameter for graphicsmagick consists of two parts when creating a png file.

The first is the png-compression-level which is calculated as min(quality/10,9)
The second is the filter method which is taken from quality % 10

With my configuration the file size gets smaller when specifying a filter of 9 (which is translated to PNG_ALL_FILTERS in graphicsmagick), but it seems different in you case. Nevertheless, i'd rather fix this by specifying a separate filter parameter to keep the api the same between all compositors. I already prepared a pull request exactly for that case here, but I still need to wait for my pull request to node-canvas to be merged, because the filter parameter in node-canvas is broken otherwise.

Sorry, no merge, but thanks anyways for the pull request 🍺

@selaux selaux closed this Dec 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants