Fixed sequential access

[deluxetom]

• resolves Use sequential mode #56

• Added a check to make sure the image is in memory when needed

[olivervogel]

Thanks. I still don't understand exactly why Core::ensureInMemory() is only called in four classes. Can you say something more about this and why exactly these classes? My first thought was that they are classes that have nothing to do with resizing but that doesn't seem to be the case.

[deluxetom]

@olivervogel from what I understand, it depends on the operations that iterate over the image pixels.
When getPoint is called, it iterates and then the pointer to the image is null, so you can't do anything anymore. Ensuring it's in memory before we use it allows for more operations afterward.

I updated PlaceMofidier again, it wasn't needed it there after all.

[jcupitt]

I think my 2p would be that you shouldn't try to automate copyMemory(). Exactly where it gets placed in the pipeline is crucial for performance, and an automatic system is likely to have many mysterious corner cases where users see unexpectedly terrible performance and memory use with no easy way to fix it.

I think I would leave the default case as it is (ie. random access, so all operations work), but add some way for users to add a "please open in sequential mode" hint if they know in advance that they will only need to iterate once. How is stringOptions set? It could maybe be done there.

You could perhaps add some API to let users add copyMemory() nodes to the pipeline, if it's possible to make this into a no-op with other backends.

[deluxetom]

I'd like to keep the sequential access option as the default if it's the fastest option. So far, it's only automating copyMemory() for commands that will prevent any other command from running after it.

@olivervogel I think adding a driver option for it could be useful but I'm not sure how to extend the current Config

[olivervogel]

I cannot judge whether it is faster or better to use sequential access. I think if it's not too much of a difference, we should listen to @jcupitt and stick to the default "random access" in both classes and not set automatic calls of copyMemory(). I also think we should not include manual control over sequential access for now.

The main goal of Intervention Image is to provide an easy-to-use image manipulation layer for various image manipulation extensions in PHP. Since different backends are covered, the current state is that I have not gone into too much detail or configuration of individual drivers. My guideline was to make it as simple as possible for the user of the library and not necessarily cover a full feature set.

I think if a more advanced implementation is needed (where it is important to the developer where copyMemory() is placed or if sequential access is needed or not), Intervention Image is not the right library anyway and it is better to use libvips directly.

[jcupitt]

@olivervogel that sounds reasonable.

One wrinkle here is that thumbnail() runs in sequential mode, so if you add a special case for this, you'll be a little limited in the operations you can run on that.

A simple workaround would be to add a copyMemory() call to the output of thumbnail().