Implemented interlace support for GD, Imagick, and Gmagick. #196

Merged
merged 4 commits into from Feb 5, 2013

Projects

None yet

3 participants

@anthonysterling
Contributor

I thought it best to raise my own PR for Interlace Support given PR #185 has been dormant for a month or so.

Apologies @vchrm.

This PR adds interlace support for the GD, Imagick, and Gmagick implementations. It still requires updated documentation and tests, as such, I would really appreciate some guidance on the best approach to add tests for this functionality.

I'm looking forward to having someone review this.

Thanks everyone.

Anthony.

@romainneutron romainneutron and 1 other commented on an outdated diff Feb 3, 2013
lib/Imagine/Gd/Image.php
@@ -479,6 +479,27 @@ public function layers()
return $this->layers;
}
+
+ /**
+ * {@inheritdoc}
+ **/
+ public function interlace($type)
+ {
+ $supportedInterlaceTypes = array(
@anthonysterling
anthonysterling Feb 3, 2013 Contributor

Sure, I'm happy to do that to help maintain consistency. :)

Thanks.

@anthonysterling anthonysterling commented on an outdated diff Feb 3, 2013
lib/Imagine/Image/ImageInterface.php
@@ -102,4 +107,13 @@ public function getColorAt(PointInterface $point);
* @return LayersInterface
*/
public function layers();
+
+ /**
+ * Enables or disables interlacing
+ *
+ * @throws InvalidArgumentException When an unsupported Interface type is supplied
+ *
+ * @return ImageInterface
+ */
+ public function interlace($type);
@anthonysterling
anthonysterling Feb 3, 2013 Contributor

I did notice that the Imagick and Gmagick objects call the interlace types scheme, I wonder if it's worth changing $type to scheme too.

ImageInterface::interlace($scheme);
@anthonysterling anthonysterling commented on an outdated diff Feb 3, 2013
lib/Imagine/Gd/Image.php
+ * {@inheritdoc}
+ **/
+ public function interlace($type)
+ {
+ static $supportedInterlaceTypes = array(
+ ImageInterface::INTERLACE_NONE => 0,
+ ImageInterface::INTERLACE_LINE => 1,
+ ImageInterface::INTERLACE_PLANE => 1,
+ ImageInterface::INTERLACE_PARTITION => 1,
+ );
+
+ if (!array_key_exists($supportedInterlaceTypes)) {
+ throw new InvalidArgumentException('Unsupported interlace type');
+ }
+
+ imageinterlace($this->resource, $supportedInterlaceTypes[$type]);
@anthonysterling
anthonysterling Feb 3, 2013 Contributor

I would still love to find out what interlacing scheme GD implements so this intended behaviour could be better handled and documented.

anthonysterling added some commits Feb 3, 2013
@anthonysterling anthonysterling Fixed missing param and renamed interface param name
- Added missing param for array_key_exists calls
- Renamed $type to $scheme within ImageInterface::interlace
bf01c45
@anthonysterling anthonysterling Fixed Typo f9df468
@anthonysterling
Contributor

Strange, if I use GD to create a set of interlaced images with:-

<?php
require_once __DIR__ . '/vendor/autoload.php';
$imagine = new Imagine\Gd\Imagine();
$image = $imagine->open('fixture.jpeg');
$image->interlace(Imagine\Image\ImageInterface::INTERLACE_LINE);
$image->save('interlaced.gif');
$image->save('interlaced.jpg');
$image->save('interlaced.png');

If I then use ImageMagick's identify utility to detect interlacing on the resulting images, I get a negative result for interlaced.gif.

➜  imagine-test  identify -verbose interlaced.gif | grep Interlace 
  Interlace: None
➜  imagine-test  identify -verbose interlaced.jpg | grep Interlace
  Interlace: JPEG
➜  imagine-test  identify -verbose interlaced.png | grep Interlace
  Interlace: PNG

I'll investigate further.

@avalanche123
Owner

Thanks, @AnthonySterling, I'm merging this in. Sorry for the lack of responses, I'm quite busy these couple of weeks

@avalanche123 avalanche123 merged commit 990b7ba into avalanche123:develop Feb 5, 2013

1 check passed

default The Travis build passed
Details
@anthonysterling anthonysterling deleted the unknown repository branch Feb 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment