libglui / glui

GLUI is a GLUT-based C++ user interface library which provides controls such as buttons, checkboxes, radio buttons, and spinners to OpenGL applications. It is window-system independent, using GLUT or FreeGLUT.
Other
200 stars 81 forks source link

Extensible callback (std::function?) #99

Open m-7761 opened 5 years ago

m-7761 commented 5 years ago

I'm changing GLUI_CB right now, and so hunted down https://github.com/libglui/glui/pull/82 that is not an Issue (took me a while to find it.)

It recommends deleting the ID and using std::function which seems downright radical compared to anything I've ever put forward here.

I'm not personally interested in GLUI because GLUT is so small and restrictive... though in my search I found conversation about moving GLUI away from GLUT. I'm more interested in the design aspect, since I think despite GLUI being an absolute mess, it has some obvious virtues.

I'm not straight up endorsing std::function but I am making the callback system extensible. I don't understand how std::function is implemented. How large is it in memory, etc?

The ID system is a fine system, and back-compat might as well be thrown out the window, along with deprecated functions if ID were removed from the constructor signatures.

Instead of storing two function pointers, I'm storing a type of function, or 0 for none, and a pointer, that is called differently depending on the type. The pointer could also be union with a member-function pointer size. Or it could perhaps store a std::function object if they are something that's like unique_ptr.

I think it's a good idea to add a function that takes an event ID so that a callback can handle more than one kind of notification. I don't have a use for that, but I don't know what the callbacks correspond to for the most part.

I've added a "fallback_callback" member to the GLUI object that is called for every control that doesn't have a callback. And I've made a note to add another fallback to the GLUI_Master object that is called if the GLUI object doesn't have a callback, and can even be used with controls that don't belong to a GLUI. Which might sound crazy, but it's actually possible to use GLUI without GLUT to render some things. Anyway, having one callback for all GLUI is also a perfectly valid approach.

And I intend to add a font to GLUI_Master also, and if present, to make the default font NULL for the GLUI objects.

If an object is passed as a callback, it can be wrapped in a virtual method interface, and copied into a new allocated object, and deleted in the destructor. You can do a lot with templates. I can write constructors (which I've standardized) so that the ID field can be a callback or an int ID.

If a function object is passed, it can be determined if it takes a control pointer or not, and if it wants an event code.

I've already added a template so that the type of control the function accepts can be specialized. It's not type safe, although it could be made so if the GLUI_CB parameters were aware of type the constructor is constructing.

I'm saying here is an opportunity to hash this out. It's possible to be backward compatible, etc. Whatever system I devise I will carry over to my forthcoming Widgets 95 effort (https://sourceforge.net/projects/widgets-95/) so it's no loss for me. The more similar it is to GLUI the easier it is to cross pollinate between them.

I'm staking out GLUI_VERSION 3.00 for this code. I haven't seen https://github.com/josch around since I started on this work. It's probably a hundred hours of work on GLUI, which it obviously needs. I'm surprised anyone uses it, but I'm surprised people write code like it to begin with. I don't mean to insult its contributors, but I'm a very fastidious programmer, and I just can't imagine the process that develops such undisciplined (and harrowing) code as GLUI's.

EDITED: Here is example code:


/*** Requires definition of Control ***/
inline void UI::CB::operator()(Control *ctrl, int event=0)const
{
    switch(_sig)
    {
    case 1: ((Update_CB)(void*)_cb)(ctrl->user_id); break;
    case 2:
    case 3: ((Control_CB)(void*)_cb)(ctrl); break;
    case 4:
    case 5: ((Control2_CB)(void*)_cb)(ctrl,event); break;
    }
}

I don't know if the void* cast makes a difference, but I think compilers have been iffy about function pointer casts. 3 & 5 just communicate the type is specialized, which is something that doesn't make sense for shared callbacks.

The idea is just to add more to this list of callback types, as many as desired.

nigels-com commented 5 years ago

PR82 Was a proof-of-concept taking GLUI in a direction of being more aligned with modern C++11 (and beyond) such as lambda functions. But since I'm not actively doing much with GLUI nowadays (not new code, especially) I'm a little reluctant to impose that decision (a breaking change) on the user base without some feedback one way or the other. I'm not committed personally to refactoring GLUI for modern C++, but I do feel this little experiment was successful.

nigels-com commented 5 years ago

As for raw function pointers, that's still very much the way of things for C APIs. Probably not what I'd consider for new C++ code, but probably the option you have when you don't have C++11 and/or boost in the mix.

m-7761 commented 5 years ago

Modern C++ is all about templates. It doesn't matter what the type is. I'm sure std::function supports raw functions too.

The only thing I can add, is nothing here is a breaking change, and probably instead of an int for an Event identifier, it should be a data structure so that it can carry more information about a given event.

If I make a day of this, it would support your code too. I think your code with lambdas is a useful option to have, but I would hate to be stuck with only using it. Of course, having an option for a void callback is fine, but it's obviously impossible to capture the control itself in your demo. And I think it doesn't make for the most readable code, but it's certainly a fine option where it makes sense.

I don't know if GLUI is unique in this design, but personally I like it, and for lack of imagination I think it makes a lot of sense. I think the Win32 model is very good too, and other models often are inadequate. The ID passing model is close to Win32 style, except it would use one callback for the entire node-tree; which is why I feel that needs to be an option too. (EDITED: That's actually the pattern the demos are using... they are just stupidly passing the same callback to ever constructor, which is obviously an inelagent solution.)

m-7761 commented 5 years ago

What is your https://github.com/libglui/glui/pull/82 suggestion to use std::bind? It looks possibly interesting for reducing all callback inputs into a single manageable object. But I'm unclear. It's pretty exotic looking. It's not a class but it seems to be used like one, via auto.

I worry the template architecture might end up heavy handed, but the basic idea is to replace the ID and CB parameters with a generic class that can be constructed from different inputs, including an integer. I'm thinking the ID should also be a union that can include a user_id or user_ptr, and the pointer will have to used to get a pointer-to-member callback to fly. Perhaps if the type matches the control's it could use the control instead.

From inside the callback generating constructor, it would need to wrap it in an interface, that knows what arguments to forward, and use new to copy it, and use a virtual method then to delete it.

std::bind depends on C++11. It's probably possible to implement everything without a C++11 dependency. That is, the client can use lambdas, etc, but older clients don't get build errors. I am familiar enough with bind. I've seen it around. It might be easy to get working before rolling something more friendly from scratch. (EDITED: Maybe I meant to say unfamiliar. I don't know. I'm definitely not versed in applications of bind.)

nigels-com commented 5 years ago

Yes, the implication of std::function is to require C++11. std::bind can certainly be used with that, but is not specifically required.

m-7761 commented 5 years ago

That's what I meant... what do you mean by "used with that"? I guess my idea is to not have to use std::function. It has to take a signature from what I can see, you use a void signature. So it might be inadequate.

nigels-com commented 5 years ago

Quoting from std::function

Class template std::function is a general-purpose polymorphic function wrapper. Instances of std::function can store, copy, and invoke any Callable target -- functions, lambda expressions, bind expressions, or other function objects, as well as pointers to member functions and pointers to data members.

m-7761 commented 5 years ago

Quoting from std::function

I can read.

After some sleep I have a better idea of what to do here.

Firstly, passing an event ID I think is not good. I said yesterday it needed to communicate more than an ID. So what makes more sense is to just have an event descriptor in the GLUI_Master object that is meaningful inside the callback. Single-thread, obviously.

So that's that.

Next, what I said about a user_ptr I want to scratch. That might be a useful option. But it's not a basis for this call support. A member-function callback will just be an object like a lambda that will require wrapping in an interface and new allocating a copy. A member-function pointer may be a wide-pointer anyway, including its this pointer, and it probably could not be cast into anything readily callable that is portable.

The last option available is to pass a pointer to the interface object. In that case it can be user-managed, and so not require making a new copy. It could be a global object.

This system is straightforward. There is one argument, that is either void, int, or a control pointer. I think I have enough of an idea to implement something from this.

m-7761 commented 5 years ago

Note on function-pointer casting: https://github.com/libglui/glui/pull/71

Background: https://stackoverflow.com/questions/36645660/why-cant-i-cast-a-function-pointer-to-void