GtkAda signal handlers interface proposal

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

GtkAda signal handlers interface proposal

Dmitry A. Kazakov

The use case is a widget (or GObject) handling signals from other widgets, usually its children. Presently this requires instantiation of a Gtk.Handlers.* package because it requires passing the widget as a signal parameter. This quite difficult to get right, especially the Connect call.

The proposal is to have a method similar to already provided parameterless handlers. As an example let's take Gkt.Button. It has On_Clicked declared as a primitive operation.

   procedure On_Clicked
      (Self  : not null access Gtk_Button_Record;
       Call  : Cb_Gtk_Button_Void;
       After : Boolean := False);

Here the handler procedure is Cb_Gtk_Button_Void:

   type Cb_Gtk_Button_Void is not null access
      procedure (Self : access Gtk_Button_Record'Class);

Now the package Gtk.Button will also have an interface declaration:

   type Clicked_Handler is interface;
   procedure On_Clicked
             (  Emitter : not null access Gtk_Button_Record'Class;
                       -- Additional signal-specific parameters go here
                Handler : not null access Clicked_Handler
             )  is abstract;

The Gtk_Button_Record type will get yet another primitive operation

   procedure Handle_Clicked
             (  Self    : not null access Gtk_Button_Record;
                Handler : not null access Clicked_Handler'Class;
                After   : Boolean := False
             );

Since GtkAda is generated, it should be relatively easy to do. The implementation of Handle_Clicked is straightforward. Gtk.Button will have an instance Gtk.Handlers.User_Callback with

   type Clicked_Handler_Ptr is access all Clicked_Handler'Class;

From the procedure called on signal it will dispatch to On_Clicked.

The client code will look like this:

   type My_Widget_Record is new Gtk.Grid.Gtk_Grid_Record
                            and Clicked_Handler with
   record
      Button1 : Gtk_Button;
      Button2 : Gtk_Button;
      ...
   end record;
   overriding
      procedure On_Clicked
                (  Emitter : not null access Gtk_Button_Record'Class;
                   Widget  : not null access My_Widget_Record
                );

Connecting signals in Initialize:

   procedure Initialize
             (  Widget : not null access My_Widget_Record'Class
             )  is
   begin
      ...
      Widget.Button1.Handle_Clicked (Widget);
      Widget.Button2.Handle_Clicked (Widget);
   end Initialize;

The handler:

   procedure On_Clicked
             (  Emitter : not null access Gtk_Button_Record'Class;
                Widget  : not null access My_Widget_Record
             )  is
   begin
      if Emitter = Widget.Button1 then
         ... -- This is from Button1
      elsif Emitter = Widget.Button2 then
         ... -- This is from Button2
      end if;
   end On_Clicked;

-- 
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de

_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Emmanuel Briot
> The use case is a widget (or GObject) handling signals from other widgets, usually its children. Presently this requires instantiation of a Gtk.Handlers.* package because it requires passing the widget as a signal parameter. This quite difficult to get right, especially the Connect call.

From our conversation on comp.lang.ada, I am wondering whether you missed the Slot parameter
to the On_* signal procedures, or the Object_Connect procedures in Gtk.Handlers ?
These seem to be exactly what is needed here, and for sure this is what we use when we develop
GPS.
The only drawback is that the Slot object is passed as a GObject, so in the callback you need to convert
back to your widget type.


> Now the package Gtk.Button will also have an interface declaration:
>
>    type Clicked_Handler is interface;
>    procedure On_Clicked
>              (  Emitter : not null access Gtk_Button_Record'Class;
>                        -- Additional signal-specific parameters go here
>                 Handler : not null access Clicked_Handler
>              )  is abstract;
>
> The Gtk_Button_Record type will get yet another primitive operation
>
>    procedure Handle_Clicked
>              (  Self    : not null access Gtk_Button_Record;
>                 Handler : not null access Clicked_Handler'Class;
>                 After   : Boolean := False
>              )


I was thinking along the same line yesterday: callbacks could be objects, rather than access to
subprograms. We also use something like that in GPS, for all actions exported to the user, or
for internal "hooks".
This is indeed very flexible, since the actual handler can then embed its own data (and thus we
no longer need, ever, to instantiate the complex handlers packages from gtk.handlers with
User_Data.
The minor drawback is that it is slightly heavier to write (need to create a new type), and slightly
harder to navigate the code (in the case of GPS, we have lots of "Execute" subprograms which
we then need to look at). In your proposal, we would have On_Clicked, On_Draw,... so this
problem is slightly less visible, and the proposal is nice because one object can then handle
multiple events.
One other thing we need to pay attention to is to allow something like:

    Button.On_Click           --  I prefer to keep the current names rather than Handle_Clicked
          (new Click_Handler);    --  unless we do something, this raises a compiler error because of
                                                --  accessibility checks

To handle this, we need unfortunately a 'Unrestricted_Access somewhere internally.

And the other difficulty is for the lifecycle, since we need to know when the handler can be freed
(which also either requires the handler to be refcounted, or the user has to guarantee that he doesn't
pass the same handler object to multiple widgets).  That part is difficult to get right (you might have
missed that part since in your later example the handler is the widget itself, whose lifecycle is
already handled -- but nothing prevents the handler from being a different object).


>    procedure Initialize
>              (  Widget : not null access My_Widget_Record'Class
>              )  is
>    begin
>       ...
>       Widget.Button1.Handle_Clicked (Widget);
>       Widget.Button2.Handle_Clicked (Widget);
>    end Initialize;

Right now, you can already use

     Widget.Button1.On_Click (On_Clicked'Access, Slot => Widget);

to achieve a very similar effect. The full power of having objects for the handlers only comes when
you need to pass additional user data to the callback, which is not so frequent but still worth handling
properly.


>    procedure On_Clicked
>              (  Emitter : not null access Gtk_Button_Record'Class;
>                 Widget  : not null access My_Widget_Record
>              )  is
>    begin
>       if Emitter = Widget.Button1 then
>          ... -- This is from Button1
>       elsif Emitter = Widget.Button2 then
>          ... -- This is from Button2
>       end if;
>    end On_Clicked;

I am not sure I would use this style of a long list of "if .. elsif", but that of course depends on the code
you need to write.

As you set up your example (a primitive On_Clicked operation on the widget itself, used by all nested
widgets), this is fairly restrictive. You need this long list, and end up with a design similar to what the
first versions of Java were doing (was is AWT at the time, I don't remember the name) ? They were
forcing this approach, and this ended up being so restrictive that later versions chose a different and
more flexible approach. Your proposal is flexible, but the example you give is not really a good one I
think.

I would have:

     type My_Widget_Record is new Gtk_Button_Record with ...

     type My_Click_Handler is new Clicked_Handler with null record;
     overriding procedure On_Click (...)

     Widget.Button1.On_Click (new My_Click_Handler)


Emmanuel



_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Dmitry A. Kazakov

On 28/04/2016 10:04, Emmanuel Briot wrote:
The use case is a widget (or GObject) handling signals from other widgets, usually its children. Presently this requires instantiation of a Gtk.Handlers.* package because it requires passing the widget as a signal parameter. This quite difficult to get right, especially the Connect call.
From our conversation on comp.lang.ada, I am wondering whether you missed the Slot parameter
to the On_* signal procedures, or the Object_Connect procedures in Gtk.Handlers ?
These seem to be exactly what is needed here, and for sure this is what we use when we develop
GPS.
The only drawback is that the Slot object is passed as a GObject, so in the callback you need to convert
back to your widget type.
That reason and because it requires instantiation from Gtk.Handlers anyway. Automatic handler disconnection is rarely an issue.

      
Now the package Gtk.Button will also have an interface declaration:

   type Clicked_Handler is interface;
   procedure On_Clicked
             (  Emitter : not null access Gtk_Button_Record'Class;
                       -- Additional signal-specific parameters go here
                Handler : not null access Clicked_Handler 
             )  is abstract;

The Gtk_Button_Record type will get yet another primitive operation

   procedure Handle_Clicked
             (  Self    : not null access Gtk_Button_Record;
                Handler : not null access Clicked_Handler'Class;
                After   : Boolean := False
             )
I was thinking along the same line yesterday: callbacks could be objects, rather than access to
subprograms. We also use something like that in GPS, for all actions exported to the user, or
for internal "hooks". 
This is indeed very flexible, since the actual handler can then embed its own data (and thus we
no longer need, ever, to instantiate the complex handlers packages from gtk.handlers with
User_Data.
The minor drawback is that it is slightly heavier to write (need to create a new type), and slightly
harder to navigate the code (in the case of GPS, we have lots of "Execute" subprograms which
we then need to look at). In your proposal, we would have On_Clicked, On_Draw,... so this 
problem is slightly less visible, and the proposal is nice because one object can then handle
multiple events.
The abstract procedure/function could be named "Handle" or "Execute", but Something_<signal name> has that big advantage that the handler object may handle many different signals by implementing multiple interfaces:

   type My_Widget_Record is ...
       and Clicked_Handler
       and Button_Press_Handler
       and Button_Release_Handler
      ...

So you would not want conflicting signal handler names.
One other thing we need to pay attention to is to allow something like:

    Button.On_Click           --  I prefer to keep the current names rather than Handle_Clicked
          (new Click_Handler);    --  unless we do something, this raises a compiler error because of
                                                --  accessibility checks

To handle this, we need unfortunately a 'Unrestricted_Access somewhere internally.
Actually I considered means to prevent this sort of misuse. E.g. by having the handler object as a plain parameter rather than access-to-object. This construct does not make sense in my view, because the purpose is to have some existing (visible elsewhere) object passed to the handler callback. If the handler object can be that sort anonymous, the existing callback procedures do the job nicely.
And the other difficulty is for the lifecycle, since we need to know when the handler can be freed
(which also either requires the handler to be refcounted, or the user has to guarantee that he doesn't
pass the same handler object to multiple widgets).  That part is difficult to get right (you might have
missed that part since in your later example the handler is the widget itself, whose lifecycle is
already handled -- but nothing prevents the handler from being a different object).
I thought about that, but unfortunately Ada does not have proper multiple-inheritance to deal with that. The interface would be a GObject_Record descendant if Ada didn't have that flaw.

I think it could still be possible to fool the compiler into it. We could change Glib.Object this way:

   type GObject_Interface is interface;
   -- Primitive operations of GObject_Record are declared on GObject_Interface:
   procedure Ref (Object : access GObject_Interface) is abstract;

Now the GObject_Record:

   type GObject_Record is new GObject_Interface with private;
   -- Primitive operations of GObject_Record are overriding of GObject_Interface:
   overriding procedure Ref (Object : access GObject_Record) is abstract;

The handler interfaces will then be

   type Clicked_Handler is interface and GObject_Interface;

This will allow calling Ref/Unref or even conversion to GObject_Record (risking Constraint_Error at run-time). The user will not be able to use an arbitrary type as a handler's base. Only descendants of GObject_Record will be eligible or else he will be forced to implement reference counting operations.

   procedure Initialize
             (  Widget : not null access My_Widget_Record'Class
             )  is
   begin
      ...
      Widget.Button1.Handle_Clicked (Widget);
      Widget.Button2.Handle_Clicked (Widget);
   end Initialize;
Right now, you can already use

     Widget.Button1.On_Click (On_Clicked'Access, Slot => Widget);

to achieve a very similar effect. The full power of having objects for the handlers only comes when
you need to pass additional user data to the callback, which is not so frequent but still worth handling
properly.
The purpose is to have My_Widget_Record as a parameter. Handling automatic disconnection is not so interesting, however, having *_Handler a descendant of GObject_Interface will allow transparent disconnection handling if the handler gets prematurely destroyed.


   procedure On_Clicked
             (  Emitter : not null access Gtk_Button_Record'Class;
                Widget  : not null access My_Widget_Record
             )  is
   begin
      if Emitter = Widget.Button1 then
         ... -- This is from Button1
      elsif Emitter = Widget.Button2 then
         ... -- This is from Button2
      end if;
   end On_Clicked;
I am not sure I would use this style of a long list of "if .. elsif", but that of course depends on the code
you need to write.
The purpose was to illustrate that handling a signal from multiple sources were possible. It is worth to consider an additional parameter for the procedure On_Click (in your nomenclature) connecting the handler object, Connection_ID, a plain Positive passed along to the handler procedure On_Clicked. It will make implementation slightly more difficult but more comfortable for the user since he will be able to use a case-statement instead.

As you set up your example (a primitive On_Clicked operation on the widget itself, used by all nested
widgets), this is fairly restrictive. You need this long list, and end up with a design similar to what the
first versions of Java were doing (was is AWT at the time, I don't remember the name) ? They were
forcing this approach, and this ended up being so restrictive that later versions chose a different and
more flexible approach. Your proposal is flexible, but the example you give is not really a good one I
think.

I would have:

     type My_Widget_Record is new Gtk_Button_Record with ...

     type My_Click_Handler is new Clicked_Handler with null record;
     overriding procedure On_Click (...)

     Widget.Button1.On_Click (new My_Click_Handler)
This is a very specific case, which better be:
   type My_Widget_Record is new Gtk_Button_Record with private;
       
   type My_Click_Handler is new Clicked_Handler with null record;
   overriding procedure On_Click (...)

   Handler : My_Click_Handler; -- Or placed in My_Widget_Record

   Widget.Button1.On_Click (My_Click_Handler);
Existing callback procedures do this better.
-- 
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de

_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Emmanuel Briot
> That reason and because it requires instantiation from Gtk.Handlers anyway. Automatic handler disconnection is rarely an issue.

For the instantiation => you can either use the already existing instances from gtkada-handlers.ads,
or use the generated On_* procedure. Then there is no need for any extra instance, which indeed makes
the code easier to read.

> The abstract procedure/function could be named "Handle" or "Execute", but Something_<signal name> has that big advantage that the handler object may handle many different signals by implementing multiple interfaces:

Agreed

> Actually I considered means to prevent this sort of misuse. E.g. by having the handler object as a plain parameter rather than access-to-object. This construct does not make sense in my view, because the purpose is to have some existing (visible elsewhere) object passed to the handler callback. If the handler object can be that sort anonymous, the existing callback procedures do the job nicely

Whether to pass an existing object or a new object is left to the user (the proposal does not force
either), so we need to handle that properly.

> The handler interfaces will then be
>
>    type Clicked_Handler is interface and GObject_Interface;
>
> This will allow calling Ref/Unref or even conversion to GObject_Record (risking Constraint_Error at run-time). The user will not be able to use an arbitrary type as a handler's base. Only descendants of GObject_Record will be eligible or else he will be forced to implement reference counting operations.

Right, that's forcing all handlers to be GObject compatible, which is not so attractive (there are
other pure-Ada ways to have refcounted types, for instance using controlled types).

Personally not a big fan of having all handlers be descendants of GObject (although admittedly this
is fully compatible with having the widget themselves be the handlers. But then when a dialog contains
multiple checkboxes, for instance, the handler indeed would have to be a big list of "if .. elsif" statements,
which I don't like because that's not quite "object-oriented" enough.

> The purpose is to have My_Widget_Record as a parameter. Handling automatic disconnection is not so interesting, however, having *_Handler a descendant of GObject_Interface will allow transparent disconnection handling if the handler gets prematurely destroyed.

Can you explain "handling automatic disconnection" is not so interesting ?
To me, it is important that handlers be disconnected when the object is destroyed, and then memory freed
appropriately.

>> I would have:
>>
>>      type My_Widget_Record is new Gtk_Button_Record with ...
>>
>>      type My_Click_Handler is new Clicked_Handler with null record;
>>      overriding procedure On_Click (...)
>>
>>      Widget.Button1.On_Click (new My_Click_Handler)
>>
> This is a very specific case, which better be:
>    type My_Widget_Record is new Gtk_Button_Record with private;
>        
>    type My_Click_Handler is new Clicked_Handler with null record;
>    overriding procedure On_Click (...)
>
>    Handler : My_Click_Handler; -- Or placed in My_Widget_Record
>
>    Widget.Button1.On_Click (My_Click_Handler);
>
> Existing callback procedures do this better.

To me, existing callbacks do all the examples that have been discussed so far better. If all we want
is to pass a parent widget, this is already fully handled by the Slot argument, with no additional
instances needed, and are not worth the extra development or the increase in size of libgtkada.

The new proposal is only interesting to replace the User_Data packages from Gtk.Handlers, which in
my own experience are almost never needed.

Emmanuel




_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Dmitry A. Kazakov

On 28/04/2016 11:37, Emmanuel Briot wrote:
Actually I considered means to prevent this sort of misuse. E.g. by having the handler object as a plain parameter rather than access-to-object. This construct does not make sense in my view, because the purpose is to have some existing (visible elsewhere) object passed to the handler callback. If the handler object can be that sort anonymous, the existing callback procedures do the job nicely
Whether to pass an existing object or a new object is left to the user (the proposal does not force
either), so we need to handle that properly.
We should not encourage memory leaks.
The handler interfaces will then be

   type Clicked_Handler is interface and GObject_Interface;

This will allow calling Ref/Unref or even conversion to GObject_Record (risking Constraint_Error at run-time). The user will not be able to use an arbitrary type as a handler's base. Only descendants of GObject_Record will be eligible or else he will be forced to implement reference counting operations.
Right, that's forcing all handlers to be GObject compatible, which is not so attractive (there are
other pure-Ada ways to have refcounted types, for instance using controlled types).
But this is compatible with GtkAda design. If you wanted controlled objects, then the first candidates would be GObject, Gtk_Widget and all other GtkAda's access types. They must have been controlled doing Ref/Unref in the first place. I understand that GtkAda started as rather thin bindings which made Gtk_Widget etc plain access type. It is probably much too late to fix. But then there is no need to worry about new types to be controlled either.
Personally not a big fan of having all handlers be descendants of GObject (although admittedly this
is fully compatible with having the widget themselves be the handlers. But then when a dialog contains
multiple checkboxes, for instance, the handler indeed would have to be a big list of "if .. elsif" statements,
which I don't like because that's not quite "object-oriented" enough.
The purpose is to have My_Widget_Record as a parameter. Handling automatic disconnection is not so interesting, however, having *_Handler a descendant of GObject_Interface will allow transparent disconnection handling if the handler gets prematurely destroyed.
Can you explain "handling automatic disconnection" is not so interesting ?
Because widgets practically never handle signals outside the containment hierarchy and otherwise than child to parent. I had only 2-3 cases overall where widgets were unrelated. And this is easily handled using either explicit Ref/Unref or else a controlled reference-holder object, or in even more rare cases by a weak reference controlled object.
To me, it is important that handlers be disconnected when the object is destroyed, and then memory freed
appropriately.
And this is never an issue in my case. If there is a remote possibility that a signal will be delivered to a prematurely destroyed object, I see no way how something may get leaked. All objects are put into some containers which deal with their referencing/unreferencing. Anything else is bad design, in my view, it should be prevented from happening rather than dealt with at run time.
I would have:

     type My_Widget_Record is new Gtk_Button_Record with ...

     type My_Click_Handler is new Clicked_Handler with null record;
     overriding procedure On_Click (...)

     Widget.Button1.On_Click (new My_Click_Handler)

This is a very specific case, which better be:
   type My_Widget_Record is new Gtk_Button_Record with private;
       
   type My_Click_Handler is new Clicked_Handler with null record;
   overriding procedure On_Click (...)

   Handler : My_Click_Handler; -- Or placed in My_Widget_Record

   Widget.Button1.On_Click (My_Click_Handler);

Existing callback procedures do this better.
To me, existing callbacks do all the examples that have been discussed so far better. If all we want
is to pass a parent widget, this is already fully handled by the Slot argument, with no additional
instances needed, and are not worth the extra development or the increase in size of libgtkada.
Not really, we don't want to be forced to type conversions when using public interfaces. Especially the conversions that may fail at run-time and crash whole application.
-- 
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de

_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Emmanuel Briot
>>
> And this is never an issue in my case. If there is a remote possibility that a signal will be delivered to a prematurely destroyed object, I see no way how something may get leaked. All objects are put into some containers which deal with their referencing/unreferencing. Anything else is bad design, in my view, it should be prevented from happening rather than dealt with at run time.

Right, and that's why I am saying that the current proposal, as it stands, is bad design. It is easy to get leaks, or
even invalid memory access.
Knowing when to call Unref is not so easy (the answer is that you need a Weak_Ref on the emitted widget, and I
would guess that most people don't know what a weak ref is).

> Not really, we don't want to be forced to type conversions when using public interfaces. Especially the conversions that may fail at run-time and crash whole application

Sorry, can't crash your application.
You will get a Constraint_Error (assuming you are passing the wrong type), and such exceptions are handled by GtkAda,
always, before it gives the control back to the C layer. So no crash.
_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Dmitry A. Kazakov



On 28/04/2016 14:28, Emmanuel Briot wrote:
And this is never an issue in my case. If there is a remote possibility that a signal will be delivered to a prematurely destroyed object, I see no way how something may get leaked. All objects are put into some containers which deal with their referencing/unreferencing. Anything else is bad design, in my view, it should be prevented from happening rather than dealt with at run time.
Right, and that's why I am saying that the current proposal, as it stands, is bad design. It is easy to get leaks, or
even invalid memory access.
Not if the parameter where plain object.

But even if it is an access type, it is no different from any other place in GtkAda where you can call new Gtk_Button and pass an uninitialized object almost everywhere. If you want prevent this, all types must be limited unconstrained to prevent illegitimate object declarations and allocators.
Knowing when to call Unref is not so easy (the answer is that you need a Weak_Ref on the emitted widget, and I
would guess that most people don't know what a weak ref is).

Not really, we don't want to be forced to type conversions when using public interfaces. Especially the conversions that may fail at run-time and crash whole application
Sorry, can't crash your application.
You will get a Constraint_Error (assuming you are passing the wrong type), and such exceptions are handled by GtkAda,
always, before it gives the control back to the C layer. So no crash.
The scenario is when the user forgets to specify the Slot parameter, and when he will try to convert the parameter Constraint_Error will propagate and consequently crash the application. Using the Slot parameter is untyped and inherently unsafe.
-- 
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de

_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada
Reply | Threaded
Open this post in threaded view
|

Re: GtkAda signal handlers interface proposal

Emmanuel Briot
>>
> The scenario is when the user forgets to specify the Slot parameter, and when he will try to convert the parameter Constraint_Error will propagate and consequently crash the application. Using the Slot parameter is untyped and inherently unsafe.

No, as I explained it won't crash the application.
It isn't untyped at all. It cannot be statically checked by the compiler, granted, but it is of course
fully type safe.

Since this seems to be the issue you are trying to solve with your proposal, that sounds way too
much work (not to mention again that the design is dangerous as it stands) to be worth adding to
GtkAda. This is already handled using either the Slot object or generic instances with User_Data
(I should hope in that case that you are using a Watch on the object, so that the handler is removed
when the user data is destroyed !)

_______________________________________________
gtkada mailing list
[hidden email]
http://lists.adacore.com/mailman/listinfo/gtkada