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

Progressive quality digression to meet max_size requirements. #8

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

s2young
Copy link
Collaborator

@s2young s2young commented Sep 3, 2014

Ok, this one may be off the rails. But I'm encountering size issues with some of my splash screens, so I thought it would be nice to have some flexibility in setting the 'quality' property during the render process.

This change adds the following properties:

max_size - this value, in bytes, tells web2splash the max file size goal. It does NOT prevent the output of a splash image if the size cannot be attained, but a warning message is attached to the image output in the onRenderImage function.

min_quality - this allows you to specify that you don't want your PNG to drop below a certain quality, even if the max_size cannot be attained.

quality_increment - this integer is used in decrementing the quality setting on a retry. Default is 5.

In short, web2splash would first attempt a render at 100% quality. If the file size is below max_size, great. If not, the quality would be decremented by the quality_increment amount (default 5) and the render would retry. Once the max_size value is reached, it moves on. If it can't be reached, then a 'warn' value is added to the image hash the is sent to the onRenderImage function. Something like this:

{
 name: 'ios-ipad-2x-portrait.png',
  width: 1536,
  height: 2008,
  pixelRatio: 2,
  size: 9646890,
  quality: 50,
  warn: 'Could not render image under the max_size setting (2048000) without going under the min_quality setting (50).' 
}

Stuart Young added 5 commits September 3, 2014 11:35
…ages at lower quality if the output PNG size exceeds a specified limit. Added: max_size - an optional value (in bytes) that will cause a re-try of the render at progressively lower quality until the max_size limit is satisfied. Added: min_quality - an integer value that will provide a lower bound to the attempts at shrinking the PNG size. Added: quality_increment - an integer value representing the quality increment to descend as the program attempts to satisfy the max_size limit. Default is 5. Added: render_timeout - allows user to specify the milliseconds to wait before rendering the page. Default is 200.
…r. Maybe I have phantomjs installed wrong?
…r. Maybe I have phantomjs installed wrong?
…r. Maybe I have phantomjs installed wrong?
@s2young
Copy link
Collaborator Author

s2young commented Sep 19, 2014

Anyone have comments on this?

/* Attempt first render at 100% quality */
(function attemptRender(quality){
/* wait a bit then render the image */
setTimeout(function () {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be function() {

@mwbrooks
Copy link
Owner

Hi @s2young,

The feature is a little off the rails, but if you have a use for it then I imagine others do as well!

Here are a few points for feedback:

Naming conventions:

Let's keep the camelCase convention going. So we should change the various setting variables from max_size to maxSize.

Exposing the options:

Let's encapsulate the options under an options object. This will keep the programmatic API more aligned with other node projects that use a similar API design.

web2splash.options = {
    maxSize: 9646890,
    minQuality: 50
}
web2splash.render(input, output, function(e) {
    // ...
}

quality-exports

Maybe a dumb question, but I'm confused where the variable quality-exports comes from.

CLI

If we offer this sort of feature, it needs to be exposed from the CLI as well.

Documentation

We need to update the following documentation:

  • README.md for the CLI
  • README.md for the Node Interface
  • CLI help output should explain the options available

Formatting

I've left a few note within the commit diff on matching the code formatting.

Cheers!
Michael

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.

2 participants