Save and call previous callback installed by user

[akramparvez]

User installed call backs are lost if installed before initializing GlfwRenderer, the change save previous callback and chains them. Based on change in imgui example.

[coveralls]

Coverage decreased (-0.3%) to 53.875% when pulling 57288e4 on akramparvez:improvements/glfw-integrations into c84315e on swistakm:master.

[swistakm]

Yeah, replacing of callbacks is the exact reason why this mechanism can be disabled with: attach_callbacks=False argument. It such situation (having pre-existing callbacks) users were supposed to link these manually after instantiating GlfwRendered. I understand this is inconvenient and we can do better.

Your solution is quite good but has following issues:

• there is small risk that some users actually had some of their callbacks replaced knowingly, this would bring them up and cause unexpected behaviour
• would be great to be able to control the point where new callbacks are attached (first or last)

I'm OK with changing the default behaviour as long as there would be a way to have old behaviour back on-demand.

Following is something that I quickly drafted that would add new argument to the renderer to control the way callbacks are attached:

from enum import Enum


class CallbackMode(Enum):
    REPLACE = auto()
    FIRST = auto()
    LAST = auto()


def _chain_glfw_callback(window, mode, setter, callback):
    if mode not in CallbackMode:
        raise ValueError("Unknown callback mode: {}".format(mode))

    chain = [callback]

    def chained_call(*args, **kwargs):
        for call in chain:
            call(*args, **kwargs)

    previous = setter(window, chained_call)

    if previous and mode != CallbackMode.REPLACE:
        chain.append(previous)

    if mode == CallbackMode.LAST:
        chain.reverse()


class GlfwRenderer(ProgrammablePipelineRenderer):
    def __init__(self, window, attach_callbacks=True, callback_mode=CallbackMode.REPLACE):
        super(GlfwRenderer, self).__init__()
        self.window = window

        if attach_callbacks:
            _chain_glfw_callback(window, callback_mode, glfw.set_key_callback, self.keyboard_callback)
            _chain_glfw_callback(window, callback_mode, glfw.set_cursor_pos_callback, self.mouse_callback)
            _chain_glfw_callback(window, callback_mode, glfw.set_window_size_callback, self.resize_callback)
            _chain_glfw_callback(window, callback_mode, glfw.set_char_callback, self.char_callback)
            _chain_glfw_callback(window, callback_mode, glfw.set_scroll_callback, self.scroll_callback)
        ...

Then you could get the desired behaviour using:

impl = GlfwRenderer(window, callback_mode=CallbackMode.LAST)

This retains full backwards compatibility (although we can make more sensible default here) and allows for a bit more control in future. Unfortunately it depends on enum that is not available in Python<3.4. As we will probably get rid of old Python versions support at some point of time we can use a conditional backport in setup.py:

setup(
    #  ...,
    install_requires=[
        "enum34>=1.1.10; python_version < '3.4'",
    ],
    # ...
)

Would you be willing to integrate this idea into your PR?

[swistakm]

Just a small reminder: I have just merged #158 so the pyglet integration classes have changed slightly.