php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71685 getiteratorindex returns wrong values
Submitted: 2016-02-28 20:59 UTC Modified: 2016-03-01 17:43 UTC
From: lee dot traynor at skeptic dot de Assigned: danack (profile)
Status: Assigned Package: imagick (PECL)
PHP Version: 5.6.18 OS: Windows 10
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2016-02-28 20:59 UTC] lee dot traynor at skeptic dot de
Description:
------------
---
From manual page: http://www.php.net/imagick.getiteratorindex
---
When iterating over a multi image object, getiteratorindex returns 2 "0", and all indices up to (n - 2), but not (n - 1). GetNumberImages() returns n.

Also $im->current () does not return the current images, but all images.

Test script:
---------------
$files = array_map ("realpath", "*.JPG");
$files = array_map ("utf8_encode", $files);
$im = new Imagick ($files);
$im->resetIterator ();
while ($im->hasNextImage ())
{
	echo " " . $im->getIteratorIndex ();
	$im->nextImage ();
	$im_temp = $im->current ();
	echo " " . $im_temp->getNumberImages ();
}
var_dump ($im->getNumberImages ());

Expected result:
----------------
Consecutive numbers from 0 to n - 1, interspersed with "1":
0 1 1 1 2 1 3 1 ... (n - 1) 1 (int) n

Actual result:
--------------
0 n 0 n 1 n 2 n .... (n - 2) n (int) n

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-02-28 21:01 UTC] lee dot traynor at skeptic dot de
Minor correction:

Also $im->current () does not return the current image, but all images.
 [2016-02-29 12:43 UTC] lee dot traynor at skeptic dot de
And a further correction: The first line of the code should read:

$files = array_map ("realpath", glob ("*.JPG"));
 [2016-02-29 21:50 UTC] danack@php.net
-Assigned To: +Assigned To: danack
 [2016-02-29 21:50 UTC] danack@php.net
Yeah.........long story short, I recommend not using the iterator built instead just use:

for ($x=0; $x<$imagick->getNumberImages(); $x++) {
    $imagick->setImageIndex();
    //processing for particular index here.
}

Or if you want to extract a single image:

$files = array_map ("realpath", glob ("*.jpg"));
$im = new Imagick($files);
$im->setIteratorIndex(1);
$im_temp = $im->getImage();
$canvas = new Imagick();
$canvas->addImage($im_temp);


> Also $im->current () does not return the current image, but all images.

That is exactly the problem. The iterator stuff that is included in each Imagick object makes it very confusing as to whether the images are separate things that can be iterated over, or whether there is just an internal counter 'pointing' to the 'current' image. The idea of a current image is an aspect of the ImageMagick library that is best accessed via $imagick->setImageIndex() rather than an inaccurate abstraction.

I'm tempted to just remove the iterator stuff.....although in theory it's a nice idea, because it doesn't actually represent what's going on internally, it's a bad abstraction, and so just leads to confusion.

In practice, that would probably annoy a lot of people....so I'm holding off on doing that for now.
 [2016-02-29 21:59 UTC] danack@php.net
And that should be - $imagick->setImageIndex($x);
 [2016-03-01 17:43 UTC] lee dot traynor at skeptic dot de
There appears to be a subtle difference between resetIterator () and setFirstIterator (), both of which I have used interchangeably up to now. E.g. before appendImages (), combineImages (), or after morphImages ().

The following modified test script works as intended with setFirstIterator (), but not with resetIterator ():

$files = array_map ("realpath", glob ("*.jpg"));
$files = array_map ("utf8_encode", $files);
$im = new Imagick ($files);
$im->setFirstIterator ();
$x = 0;
do
{
        $x++;
	echo " " . $im->getIteratorIndex ();
	$im_temp[$x] = $im->getImage ();
	echo " " . $im_temp[$x]->getNumberImages ();
} while ($im->nextImage ());
var_dump (count ($im_temp), $im->getNumberImages ());

So the question resolves to what to do with resetIterator () - I would recommend deprecating it and suggesting the use of setFirstIterator () instead.

In the end for me it was much easier to load the images into an array and work with arrays.

Ta.
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Nov 19 01:31:42 2017 UTC