-
Notifications
You must be signed in to change notification settings - Fork 379
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
Enable configuration of post processors using parameters (closes #720) #759
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,17 +10,37 @@ | |
|
||
class OptiPngPostProcessor implements PostProcessorInterface | ||
{ | ||
/** @var string Path to optipng binary */ | ||
protected $optipng; | ||
/** | ||
* @var string Path to optipng binary | ||
*/ | ||
protected $optipngBin; | ||
|
||
/** | ||
* If set --oN will be passed to optipng. | ||
* | ||
* @var int | ||
*/ | ||
protected $level; | ||
|
||
/** | ||
* If set --strip=all will be passed to optipng. | ||
* | ||
* @var bool | ||
*/ | ||
protected $stripAll; | ||
|
||
/** | ||
* Constructor. | ||
* | ||
* @param string $optipngBin Path to the optipng binary | ||
* @param int $level Optimization level | ||
* @param bool $stripAll Strip metadata objects | ||
*/ | ||
public function __construct($optipngBin = '/usr/bin/optipng') | ||
public function __construct($optipngBin = '/usr/bin/optipng', $level = 7, $stripAll = true) | ||
{ | ||
$this->optipngBin = $optipngBin; | ||
$this->level = $level; | ||
$this->stripAll = $stripAll; | ||
} | ||
|
||
/** | ||
|
@@ -44,7 +64,16 @@ public function process(BinaryInterface $binary) | |
} | ||
|
||
$pb = new ProcessBuilder(array($this->optipngBin)); | ||
$pb->add('--o7'); | ||
|
||
if ($this->level !== null) { | ||
$pb->add(sprintf('--o%d', $this->level)); | ||
} | ||
|
||
if ($this->stripAll) { | ||
$pb->add('--strip=all'); | ||
} | ||
|
||
$pb->add($input = tempnam(sys_get_temp_dir(), 'imagine_optipng')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for my late comment, but I have noticed that there is merge conflict. $input gets added twice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, nice catch! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you do a follow up PR? |
||
$pb->add($input); | ||
|
||
if ($binary instanceof FileBinaryInterface) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just looking at this myself during the rebase - it's originally from here 81ee48a#diff-9cd4d6a634315c7cf9b3483cb8b37db8R45 so I chose to keep it.
Setting --max disables lossless mode and will reduce the quality factor of jpegs (if greater than its current value) to the current setting - My assumption then is that it doesn't matter, as 100 is the max quality factor for the jpeg format anyway, so removing the flag shouldn't matter much.
I can run a few test cases using jpegoptim by hand to see if there's any difference, if that'd be preferred?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I guess defaulting to
null
is better then