Updated bindings to support GLFW 3.1 functionality.

[filonik]

I have extended the bindings to support GLFW 3.1. Feel free to merge if you like.

[adamlwgriffiths]

Looks good, I'll check it out locally soon and merge if there aren't any problems. Thanks for your work =)

[adamlwgriffiths]

I can't actually confirm the code at the moment as homebrew on OS-X hasn't updated the build for glfw to 3.1. But the code itself seems fine, so I'm sure there won't be any problems. The only change I made was to modify the version to 3.1.0.1 to allow for GLFW's major.minor.micro + a local version number.

I created a glfw_3.0 branch and backported some of the fixes. I pushed the new 3.0 to pypi, but not the 3.1 until I can actually build it myself.

Cheers =)

[adamlwgriffiths]

The README has the following for windows builds:

set INCLUDE=%INCLUDE%;<path to headers>
set LIB=%LIB%;<path to lib>
python setup.py build_ext -i

Can you verify if this is correct? The setup scripts added GLFW_ROOT env var, I assume that setting INCLUDE and LIB weren't enough to get it building?

So it seems the correct build commands are:

set GLFW_ROOT=<path to glfw>
python setup.py build_ext -i

?

[filonik]

Ah yes, I introduced the GLFW_ROOT environment variable to make the builds more convenient on Windows. It is possible to build without it following the instructions in the README , however the following part in 'setup.py' is required:

if 'win32' in platform:
    glfw_lib = 'glfw3dll'

A benefit of having GLFW_ROOT is that it can be used to find the headers in 'generate_cython.py'. However, I suppose it would be best to make it optional.

[filonik]

Also, I haven't exposed the following member from GLFWimage struct through the python interface:

unsigned char* pixels

Not sure what the preferred way of exposing such an array to python is. However, this is the only notable omission.

[adamlwgriffiths]

This is the only cython code I've written so I'm not sure how. Technically you could use a ctypes.void_p type, but I find those quite annoying to use. Returning a list or numpy.ndarray would be best, and accepting the same in any char* functions.

I think GLFW_ROOT is optional as of this commit: https://github.com/adamlwgriffiths/cyglfw3/commit/18dbfab73fdc6abdc67f4b3b93909c41070626c6 I'm not in a position to test it however. If theres something wrong feel free to fix and push.

I've given you push access to this repo now, so feel free to fix at will =)

[adamlwgriffiths]

Looking at the char* handling in generate_cython.py (it's been a while), it seems cython handles char* conversion to / from python strings for you. So essentially you could pass in a string, but thats a bit crap.

[filonik]

Yes, that's what I thought. Especially because cython's char* conversion only works for null-terminated strings as far as I understand. I guess a memory buffer or a numpy array would be more appropriate, but I wasn't sure and didn't want to drag in an extra dependency.

PS: The optional GLFW_ROOT works perfect, thanks!

[adamlwgriffiths]

Yeah, it would be nice to not drag in another dependency, numpy is quite a large one for such a simple function. But if it makes it easier, so be it. Numpy should convert to list pretty easily though, so if it can accept a list then that should work for both cases.

[filonik]

I guess following what is currently being used for GLFWgammaramp, it would look something like that (untested):

property pixels:
    def __get__(self):
        cdef int size = self._this_ptr.width * self._this_ptr.height
        pixels = [self._this_ptr.pixels[i] for i in range(size)]
        return pixels

However, in this case you probably want to be able to manipulate the pixels, so from what I can gather this calls for either numpy or typed memoryviews.

[adamlwgriffiths]

Hmm I don't have much experience with python memory buffers / views. I don't mind having to convert to numpy manually, end user can always wrap that in a function and example code can be provided.

Perhaps just expose it as a ctype.void_p. The following code goes from an integer (pointer) -> ctypes buffer -> np array (used for opengl mapped memory buffers):

import ctypes
import numpy as np
from numpy.core.multiarray import int_asbuffer

<snip>
    def _ptr_to_np(self, ptr, access):
        func = ctypes.pythonapi.PyBuffer_FromMemory
        func.restype = ctypes.py_object

        buf = int_asbuffer(ptr, self._nbytes)
        buf = np.frombuffer(buf, self._dtype)
        buf.shape = self._shape
        return buf

[filonik]

I have toyed around a bit with numpy, I have it at the stage where you can do things like this:

width, height = (32, 32)

image = glfw.Image()
image.pixels = [[[0, 255*(x/width), 255*(y/height), 255] for x in range(width)] for y in range(height)]
image.pixels[::4,:] = [0,0,0,255] # Manipulate pixels using fancy numpy indexing.

cursor = glfw.CreateCursor(image, width/2, height/2)

glfw.SetCursor(window, cursor)

(Loosely adapted from GLFW.)

However, I am hesitant to recommend going down this path. For one, my current implementation is rather brittle, since the memory is not owned by the image. You can get invalid memory accesses and crashes. It is really close to C, but in python this behavior would be unexpected.

Edit: I have updated the implementation to store a reference to the numpy array that owns the pixel data, keeping it alive. If we go with numpy, I guess this would be a decent way to do it. I have pushed the changes to my fork.

[adamlwgriffiths]

Ok, so the GLFWImage struct supports width, height and unsigned char*. GLFWImage is passed to a GLFWCursor object and then to GLFW via the new API functions. The GLFWImage is free'ed by GLFW with a call to glfwDestroyCursor.

So the issue you're describing is when Python free's the memory itself, or GLFW frees a numpy array.

Perhaps using ctypes instead would resolve this?

import ctypes
arr = (ctypes.c_int * len(pyarr))(*pyarr)

That should allocate a new array and copy the contents of the list. The memory is free'ed when the ctypes object is destroyed. Might be able to set _b_needsfree_ to False.

If its a multidimensional list the conversion probably wouldn't work. I'm curious to see how PyOpenGL does this with it's ctypes mappings as it handles all of these situations.

On Tue, Feb 10, 2015 at 7:05 PM, filonik notifications@github.com wrote:

I have toyed around a bit with numpy, I have it at the stage where you can do things like this:

width, height = (32, 32) pixels = np.asarray([[[0, 255(x/width), 255(y/height), 255] for x in range(width)] for y in range(height)], dtype=np.ubyte)

image = glfw.Image() image.pixels = pixels image.pixels[::4,:] = [0,0,0,255] # Set every 4th line black (i.e. fancy numpy indexing)

cursor = glfw.CreateCursor(image, width/2, height/2)

glfw.SetCursor(window, cursor)

(Loosely adapted from GLFW https://github.com/glfw/glfw/blob/master/tests/cursor.c)

However, I am hesitant to recommend going down this path. For one, my current implementation is rather brittle, since the memory is not owned by the image. You can get invalid memory accesses and crashes. It is really close to C, but in python this behavior would be unexpected.

— Reply to this email directly or view it on GitHub https://github.com/adamlwgriffiths/cyglfw3/pull/12#issuecomment-73659250 .

[filonik]

The issue that I was describing refers to when Python frees the memory itself. GLFW does not allocate the pixel data (or the GLFWimage for that matter), so it does not attempt to free it. The struct is just for passing data to glfwCreateCursor as far as I can tell.

The current numpy solution in my repository works well, but ctypes may also be worthwhile exploring to avoid the extra dependency.

[filonik]

One neat thing about numpy though, using an image as your cursor is pretty much a one liner:

image.pixels = PIL.Image.open('data/image/logo.png').convert("RGBA")

[adamlwgriffiths]

You could potentially do the opposite of this

https://stackoverflow.com/questions/23872946/force-numpy-ndarray-to-take-ownership-of-its-memory-in-cython

Remove the OWNDATA flag from the np array once it's passed in. I'm still leaning towards a ctypes method with numpy functions if numpy can be imported, but I can't test the code easily without glfw 3.1 being installed =/.

[filonik]

That could work, but I do not think it would be an improvement over the current solution. Suppose removing the OWNDATA flag would in fact give you your own buffer with full ownership (which I am not sure it would guarantee, since numpy arrays can reference each others data, so if you get one that only holds a reference, I am not sure what happens). Nevertheless, even if that worked, you would then have to make sure to manually free the memory.

In the current solution, the user can pass in anything that can be converted to a contiguous numpy array of the right shape, which is held internally by the Image object (and therefore guarantees that the data stays valid throughout the lifetime of the object). The only drawback I see is the dependence on numpy, but I don't have the time to look into ctypes at the moment.

[filonik]

Here is an implementation of the Image extension type, based on cython's typed memory views. Conceptually, it works the same as my numpy implementation, storing an internal reference to the buffer to make sure it is kept alive.

from libc.stdlib cimport malloc, free
from cython cimport view

cdef getitem(obj, key, default=None):
    try:
        return obj[key]
    except (TypeError, KeyError, IndexError):
        return default

cdef getlen(obj, default=None):
    try:
        return len(obj)
    except TypeError:
        return default

cdef getshape(obj):
    lengths = []
    while True:
        length, obj = getlen(obj), getitem(obj, 0)
        if length is None: break
        lengths.append(length)
    return tuple(lengths)

cdef class Image:
    cdef cglfw3.GLFWimage * _this
    cdef unsigned char[:,:,::1] _data

    property width:
        def __get__(self):
            return self._this.width

    property height:
        def __get__(self):
            return self._this.height

    property size:
        def __get__(self):
            return (self.width, self.height)

    property pixels:
        def __get__(self):
            return self._data

        def __set__(self, value):
            cdef unsigned char[:,:,::1] data
            if isinstance(value, (tuple, list)):
                shape = getshape(value)
                data = view.array(shape=shape, itemsize=sizeof(unsigned char), format="c")
                for i in range(shape[0]):
                    for j in range(shape[1]):
                        for k in range(shape[2]):
                            data[i,j,k] = value[i][j][k]
            else:
                # must be a memory view or a buffer type
                data = value

            self._this.width = data.shape[0]
            self._this.height = data.shape[1]
            self._this.pixels = &data[0][0][0]

            self._data = data

    def __cinit__(self):
        self._this = NULL

    def __init__(self):
        self._this = <cglfw3.GLFWimage *>malloc(sizeof(cglfw3.GLFWimage))

    def __dealloc__(self):
        free(self._this)
        self._this = NULL

    def __richcmp__(Image self, Image other, int op):
        if op == 0:
            # <
            return self._this < other._this
        elif op == 1:
            # <=
            return self._this <= other._this
        elif op == 2:
            # ==
            return self._this == other._this
        elif op == 3:
            # !=
            return self._this != other._this
        elif op == 4:
            # >
            return self._this > other._this
        elif op == 5:
            # >=
            return self._this >= other._this

    def __nonzero__(self):
        return self._this != NULL

    def __hash__(self):
        return <size_t>self._this

One limitation that I have come across is that memory views cannot hold read-only buffers. As a result, it is necessary to make a copy of pixel data from PIL, for example. Anyway, I am happy to push this if desired.

[adamlwgriffiths]

Thanks for furthering this =)

Is it possible to handle the PIL case transparently? Even if its not performant, I'd like to get a nice, consistent interface / usage pattern down, and then the internals can change as much as they want.

If you're happy with it then feel free to push.

[filonik]

Well, currently the following works, but it isn't quite as nice as the old numpy version:

def load_cursor_image(path):
    import PIL.Image
    result = glfw3.Image()
    result.pixels = np.array(PIL.Image.open(path).convert("RGBA"), copy=True)
    return result

Perhaps it would be possible to test whether the buffer is read-only and make a copy internally so that it is transparent to the client code. I'd have to look into that.

[adamlwgriffiths]

Well rgba convert is pretty standard. So is the np conversion. I don't see a problem with that code. Just put it on the read me (since there's no docs) and that's fine. Last suggestion. Is it possible to do the pixel assignment equivalent in the constructor too? Or does it not make sense? Other than that. Looks good. Push away =)

Sent from my iPhone

On 01/03/2015, at 1:07 AM, filonik notifications@github.com wrote:

Well, currently the following works, but it isn't quite as nice as the old numpy version:

def load_cursor_image(path): import PIL.Image result = glfw3.Image() result.pixels = np.array(PIL.Image.open(path).convert("RGBA"), copy=True) return result Perhaps it would be possible to test whether the buffer is read-only and make a copy internally so that it is transparent to the client code. I'd have to look into that.

— Reply to this email directly or view it on GitHub.

[filonik]

Alright, I have pushed the latest version, which includes an optional pixels argument in the constructor, and created an example to document the usage.

[adamlwgriffiths]

Awesome! GLFW3.1 got updated on OS-X so I'll try it out, tag it and release it soon.

On Sun, Mar 1, 2015 at 1:06 PM, filonik notifications@github.com wrote:

Alright, I have pushed the latest version, which includes an optional pixels argument in the constructor, and created an example to document the usage.

— Reply to this email directly or view it on GitHub https://github.com/adamlwgriffiths/cyglfw3/pull/12#issuecomment-76563756 .

[filonik]

Sweet, let me know if you run into any issues. I have only tested on Windows so far.

[adamlwgriffiths]

Tested, tagged, and pushed to PyPi! Thanks again =)