NF: Return data from _createTexture

[EitanHemed]

This addition aims to make the array of image values being available by default, as an attribute of ImageStim.

[peircej]

@EitanHemed the tests are failing:

___________________________ TestImage.test_img_data ____________________________

self = <psychopy.tests.test_visual.test_image.TestImage object at 0x13db60520>

    def test_img_data(self):
        """Test the accessibility of image array values"""
>       img_ar = self.getImageData()
E       AttributeError: 'TestImage' object has no attribute 'getImageData'

By the way, could you use the prefixes, like NF, BF, FF for the commit title so that when we view the commit history we can filter by different types. The pull request title and description is just within GitHub not in the git history itself. That's better to be written to be informative to the user

[EitanHemed]

Thanks Jon. Sure, somehow i missed that it applies for the commits.

[peircej]

@EitanHemed just so you're aware, I like this idea but it feels like a big change to make this close to a release date. I'd prefer we wait a few weeks and then pull it in so that it will go live in the winter release instead, giving us longer to see if there are any problems with the concept

[EitanHemed]

I see no problem with this (especially as i haven't fixed the remaining issues on the added tests yet).

[mdcutone]

I think to save on memory we should try using pixel buffer objects (PBO). All ImageStim textures now have a PBO associated with them. You can map the buffer to client memory using the glMapBuffer command. We can put that into a method or property to get the underlying texture in ndarray format.

Example from ImageStim:

 # bind pixel unpack buffer
GL.glBindBuffer(GL.GL_PIXEL_UNPACK_BUFFER, self._pixbuffID)

# Free last storage buffer before mapping and writing new frame
# data. This allows the GPU to process the extant buffer in VRAM
# uploaded last cycle without being stalled by the CPU accessing it.
GL.glBufferData(
    GL.GL_PIXEL_UNPACK_BUFFER,
    nBufferBytes * ctypes.sizeof(GL.GLubyte),
    None,
    GL.GL_STREAM_DRAW)

# Map the buffer to client memory
bufferPtr = GL.glMapBuffer(
    GL.GL_PIXEL_UNPACK_BUFFER,
    GL.GL_READ_WRITE) 

# allow the mapped buffer to be read as a numpy array
bufferArray = numpy.ctypeslib.as_array(    
    ctypes.cast(bufferPtr, ctypes.POINTER(GL.GLubyte)),
    shape=(nBufferBytes,))

# create a copy of the array if we want
copyArray = bufferArray.copy()

# Very important that we unmap the buffer data after copying, but
# keep the buffer bound for setting the texture.
GL.glUnmapBuffer(GL.GL_PIXEL_UNPACK_BUFFER)

[peircej]

So, if the above example code became a new stim.getData() method, then existing usage would be unchanged in terms of performance. Presumably the slight performance difference between this and @EitanHemed's solution is that in his version there is never a need for any copying of the data - the data would be passed back as a pointer to an array that had already been created in memory ready for re-use.
So, as I see it the trade-off is:

• store data when creating the texture, even if it isn't going to be used by most users (slight memory hit)
• fetch a copy of the data as needed (slight speed hit if the data are needed)

[peircej]

@EitanHemed @mdcutone we need to get a decision on this and pull in one or other option to be included in the 2024.1.0 release