Fix image memory issues

[corranwebster]

Fixes #1108 (hopefully).

This stashes a back-reference to a QImage when it is used to create a QPixmap to ensure that the QImage isn't gc-d until we're done with it.

The example from TraitsUI looks like this after the fix:

Unfortunately I don't see a way to create a regression test.

For a reviewer - to replicate set up a Windows/Python3.6/Pyside2 environment with the main branch of TraitsUI and the main branch of Pyface - you should see corruption or get a segfault. Switching to this branch should fix the problem.

[jwiggins]

How is the QImage being created? Is it using the same strategy to keep a reference to its backing buffer? See https://github.com/python-pillow/Pillow/blob/7.2.x/src/PIL/ImageQt.py#L166-L170

[corranwebster]

How is the QImage being created? Is it using the same strategy to keep a reference to its backing buffer?

For arrays we stash a reference to (a shuffled copy) of the array here:

pyface/pyface/ui/qt4/util/image_helpers.py

Line 169 in 8adf2a1

image._numpy_data = data

For a PIL ImageQt the original Image instance is stored in a trait on the PILImage class:

pyface/pyface/i_pil_image.py

Line 34 in 8adf2a1

image = Instance("PIL.Image.Image")

Edit: plus it looks like PIL is also stashing a reference.

[mdickinson]

Suggest adding a comment explaining why this is necessary.

[jwiggins]

If this code is only consuming QImage instances that are also created by pyface and have a ref to their underlying buffer, perhaps having the QPixmap reference the buffer would avoid keeping a QImage around unnecessarily?

I'm speculating a little, so don't take this too seriously. I'm 👍 on the proposed solution but only wonder if perhaps the QImage object is made redundant here.

[mdickinson]

LGTM, but a comment explaining why this code is doing what it's doing would be useful.

[corranwebster]

If this code is only consuming QImage instances that are also created by pyface and have a ref to their underlying buffer, perhaps having the QPixmap reference the buffer would avoid keeping a QImage around unnecessarily?

There's no way I know of to create a QPixmap directly from a numpy array - it can be created from various file formats directly, but not an array of RGB(A) as far as I can see - this likely because the QPixmap data is device-dependent.

There is a question of whether the ImageEditor should be storing a QPixmap or a QImage - I suspect QPixmap was chosen because that was what was easily available (but maybe for speed/efficiency too).

[corranwebster]

LGTM, but a comment explaining why this code is doing what it's doing would be useful.

Done.