allow assign to array element

[jcupitt]

We might be able to allow assign to array element, meaning replace image band. So:

$image[1] = $image[0];

would mean:

$image = $image[0]->bandjoin([$image[0], $image[3]]);

[kleisauke]

@jcupitt I've made some untested code that might work:

public function offsetSet($offset, $value): Image
{
    if($value instanceof Image) {
        $bands = $this->bandsplit();

        $offset = !is_null($offset) && $this->offsetExists($offset) ? $offset : $this->bands - 1;
        unset($bands[$offset]);

        return $value->bandjoin($bands);
    } else {
        throw new \InvalidArgumentException("Image::offsetSet function only accepts instance of Image");
    }
}

public function offsetUnset($offset): Image
{
    $offset = !is_null($offset) && $this->offsetExists($offset) ? $offset : $this->bands - 1;

    $begin = $this->extract_band(0, ['n' => $offset]);
    $end = $this->extract_band($offset, ['n' => $this->bands - 1]);

    return $begin->bandjoin($end);
}

[kleisauke]

This enhancement is impossible to implement in our current setting, it requires that our Image class is a fluent interface. So for example:

$image = Image::newFromFile('example.png');

// Invert image
$image->invert();

// Rotate 90 degrees clockwise.
$image->rot90();

// Buffer contains a rotated-inverted image
$buffer = $image->writeToBuffer('.png')

Instead of:

$image = Image::newFromFile('example.png');

// Invert image
$image = $image->invert();

// Rotate 90 degrees clockwise.
$image = $image->rot90();

// Buffer contains a rotated-inverted image
$buffer = $image->writeToBuffer('.png')

[kleisauke]

If we make a internal function to get the the underlying vips image resource then it should be possible. See for example:

/**
 * Our ArrayAccess interface ... we allow [] to get band.
 *
 * @param mixed $offset The index to set.
 * @param Image $value  The band to insert
 */
public function offsetSet($offset, $value)
{
    if($value instanceof Image) {
        $bands = self::bandsplit();

        if(self::offsetExists($offset)) {
            unset($bands[$offset]);
        }

        $this->image = $value->bandjoin($bands)->getImageResource();
    } else {
        throw new \InvalidArgumentException("Image::offsetSet function only accepts instance of Image");
    }
}

/**
 * Our ArrayAccess interface ... we allow [] to get band.
 *
 * @param mixed $offset The index to remove.
 */
public function offsetUnset($offset)
{
    if(self::offsetExists($offset)) {
        $begin = self::extract_band(0, ['n' => $offset]);
        $end = self::extract_band($offset, ['n' => $this->bands - 1]);

        $this->image = $begin->bandjoin($end)->getImageResource();
    }
}

/**
 * Get the underlying vips resource.
 *
 * @return resource The underlying vips image resource.
 *
 * @internal
 */
public function getImageResource() {
    return $this->image;
}

Then we are able to do this (untested):

require __DIR__ . '/vendor/autoload.php';
use Jcupitt\Vips;

$image = Vips\Image::newFromFile($argv[1]);

$image[1] = $image[0];
unset($image[2]);

$image->writeToFile($argv[2]);

[jcupitt]

Here's a branch which implements all this:

https://github.com/jcupitt/php-vips/tree/add-array-access

Plus docs and some tests. Any thoughts?

[kleisauke]

Looks good to me! Added some inline comments.

[jcupitt]

OK, I think I've fixed the stuff you found.

I made it throw an exception if you try to delete all bands, does that sound right?

I made a small performance improvement: it only expands constants to images if they are the first band. This way it can use bandjoin_const for a small speed-up on band append.

[kleisauke]

Throwing an exception if you try to delete all bands, sounds right to me. We could also add this to the test case (so $this->expectException(BadMethodCallException::class); here).

Just 1 minor comment: my name is misspelled in that commit. For the rest, it all seems to be good. 😄

[jcupitt]

OK, done and merged to master. Thanks!

Let's leave it a day or two for any issues to shake out, then do 1.0.3.