Fl_Group/Fl_Tree/etc: document how to properly delete widgets/items from container widgets

13 views
Skip to first unread message

Greg Ercolano

unread,
Jul 13, 2021, 5:36:49 PMJul 13
to fltkc...@googlegroups.com
Our docs for Fl_Group should probably both describe and show recommended examples
for how to delete widgets from the group (i.e. remove() and destroy), showing proper
techniques especially in contexts where one has to be careful (or used to have to be),
such as when deleting from callbacks vs. event handlers vs. other situations.

Similarly, docs for Fl_Browser::remove() and Fl_Tree::remove() should elaborate on how they also
destroy the items, so as not to be confused with Fl_Group::remove() (which does not), and in the
case of Fl_Tree, what happens with Fl_Widget's that have been associated with tree items, and how
to properly reassign or destroy those.

Gmail Ercolano

unread,
Jul 13, 2021, 8:59:45 PMJul 13
to fltkc...@googlegroups.com

On 7/13/21 2:36 PM, Greg Ercolano wrote:

Our docs for Fl_Group should probably both describe and show recommended examples
for how to delete widgets from the group (i.e. remove() and destroy), showing proper
techniques especially in contexts where one has to be careful (or used to have to be),
such as when deleting from callbacks vs. event handlers vs. other situations.

Since the best way to get traction on this is to suggest a change and seek input, as it's easier to make corrections to someone else's suggestion than to suggest something from scratch. ; )

For Fl_Group, we could perhaps add a section to the class description, something along the lines of (I'm not actually the one to write this, as I'm not sure all the ins and outs of when we supported what.. I think we used to have an article about this but I couldn't find it):

Removing vs. Deleting Children

Child widgets can be removed from the group without destruction using remove(int) or remove(Fl_Widget*), allowing the widget to remain allocated so it can later be reparented elsewhere.

To delete as well as destroy a child widget, you can use any one of:
  • Fl::delete_widget(w) which will schedule destruction on the next iteration of the app loop

  • In fltk 1.3.x (and up, 'x' unknown) you can simply use the c++ 'delete' operator to destroy the widget, and that automatically schedules it for removal just like Fl::delete_widget()

  • In fltk 1.1.x (and back) order of execution was important; you had to use Fl_Group::remove() first, then c++ delete.

  • ..etc..
..and include some example code showing the proper use in a callback() vs handle() vs other contexts. Again, I'm probably not the one to write this, as I'm not familiar with all the internals and possible gotchyas.


Similarly, docs for Fl_Browser::remove() and Fl_Tree::remove() should elaborate on how they also
destroy the items, so as not to be confused with Fl_Group::remove() (which does not), and in the
case of Fl_Tree, what happens with Fl_Widget's that have been associated with tree items, and how
to properly reassign or destroy those.

This should be easy, just a small elaboration to the existing docs (in green):

Fl_Browser::remove(int)

    Remove entry for given line number, making the browser one line shorter.
    This frees the memory associated with the browser item immediately.
    You must call redraw() to make any changes visible. 


Fl_Tree::remove(Fl_Tree_Item* item)

    Remove the specified 'item' from the tree.

    'item' may not be NULL. If the item has children, all those are removed too.
    If item being removed has focus, no item will have focus.

    remove() frees the memory associated with the specified Fl_Tree_Item immediately.
    remove() does not affect any FLTK widget assigned to the item with Fl_Tree_Item::widget(),

    To destroy an FLTK widget assigned to the item, you can do that cleanly this way:

        // Delete the Fl_Tree_Item 'item' and any associated FLTK widget
        Fl_Widget *w = item->widget();   // save any widget for the item
        tree->remove(item);              // remove the item, destroying it
        if ( w ) Fl::delete_widget(w);   // delete the widget last, if there was one

    For more info, see Fl_Group section on deleting FLTK child widgets.

    Note that after a remove(), any widget assigned to the item will remain a child
    of Fl_Tree's Fl_Group, which will continue to 'own' the widget. This allows the
    widget to be reassigned to another item without having to recreate it. Destroying
    the tree will automatically destroy any child fltk widgets.


I think specific details regarding proper deleting of FLTK widgets should appear
only in Fl_Group, and other docs simply refer to that documenation as needed,
so there aren't multiple descriptions of the process.

There may be other widgets besides Fl_Browser and Fl_Tree that may need elaboration on whether memory is freed or not with methods like remove().



  

Gmail Ercolano

unread,
Jul 13, 2021, 10:51:28 PMJul 13
to fltkc...@googlegroups.com


On 7/13/21 5:59 PM, Gmail Ercolano wrote:
Fl_Tree::remove(Fl_Tree_Item* item)

    Remove the specified 'item' from the tree.

    'item' may not be NULL. If the item has children, all those are removed too.
    If item being removed has focus, no item will have focus.

    remove() frees the memory associated with the specified Fl_Tree_Item immediately.
    remove() does not affect any FLTK widget 
assigned to the item with Fl_Tree_Item::widget(),

    To destroy an FLTK widget assigned to the item, you can do that cleanly this way:
    [..]

    Hmm, it might be better to move the description of how to remove a widget assigned to an item
    with Fl_Tree_Item::widget() to the docs for that method, and simply refer to that here
    in remove()'s docs, e.g.

remove() does not destroy any FLTK widget assigned to the item with widget(Fl_Widget*).
For more on how to manage that, see Fl_Tree_Item::widget().

    This way anyone not even using widget() assignment doesn't have to see a lot of detail
    in the remove() docs that doesn't concern them.

Albrecht Schlosser

unread,
Jul 15, 2021, 9:00:37 AMJul 15
to fltkc...@googlegroups.com
On 7/14/21 2:59 AM Gmail Ercolano wrote:

On 7/13/21 2:36 PM, Greg Ercolano wrote:

Our docs for Fl_Group should probably both describe and show recommended examples
for how to delete widgets from the group (i.e. remove() and destroy), showing proper
techniques especially in contexts where one has to be careful (or used to have to be),
such as when deleting from callbacks vs. event handlers vs. other situations.

I agree. Documentation could be improved in some cases.


Since the best way to get traction on this is to suggest a change and seek input, as it's easier to make corrections to someone else's suggestion than to suggest something from scratch. ; )

For Fl_Group, we could perhaps add a section to the class description, something along the lines of (I'm not actually the one to write this, as I'm not sure all the ins and outs of when we supported what.. I think we used to have an article about this but I couldn't find it):

Removing vs. Deleting Children

Child widgets can be removed from the group without destruction using remove(int) or remove(Fl_Widget*), allowing the widget to remain allocated so it can later be reparented elsewhere.

To delete as well as destroy a child widget, you can use any one of:


I would elaborate that this could sometimes be "too late". We've seen an example where "clicking quickly" on a button could trigger the callback twice if the widget is not deleted (or at least deactivated) immediately. Note that this can only happen if FLTK is busy servicing events and the 2nd click gets added to the event queue before Fl::wait() is called internally.

I believe that Fl::delete_widget(w) is no longer necessary at all, but I do also think that we can't be absolutely sure about that. It's difficult.


  • In fltk 1.3.x (and up, 'x' unknown) you can simply use the c++ 'delete' operator to destroy the widget, and that automatically schedules it for removal just like Fl::delete_widget()

should read: "... and that automatically removes the widget from its parent Fl_Group (if any)"

  • In fltk 1.1.x (and back) order of execution was important; you had to use Fl_Group::remove() first, then c++ delete.

Yep.


  • ..etc..
..and include some example code showing the proper use in a callback() vs handle() vs other contexts. Again, I'm probably not the one to write this, as I'm not familiar with all the internals and possible gotchyas.

I could try to do this. Generally there shouldn't be any difference between handle() and in callbacks. But handle() methods need to take care never to access a widget after the callback *if* the callback deleted the widget. This is what Fl_Widget_Tracker is for.



Similarly, docs for Fl_Browser::remove() and Fl_Tree::remove() should elaborate on how they also
destroy the items, so as not to be confused with Fl_Group::remove() (which does not), and in the
case of Fl_Tree, what happens with Fl_Widget's that have been associated with tree items, and how
to properly reassign or destroy those.

This should be easy, just a small elaboration to the existing docs (in green):

Fl_Browser::remove(int)

    Remove entry for given line number, making the browser one line shorter.
    This frees the memory associated with the browser item immediately.
    You must call redraw() to make any changes visible. 


Fl_Tree::remove(Fl_Tree_Item* item)

    Remove the specified 'item' from the tree.

    'item' may not be NULL. If the item has children, all those are removed too.
    If item being removed has focus, no item will have focus.

    remove() frees the memory associated with the specified Fl_Tree_Item immediately.
    remove() does not affect any FLTK widget assigned to the item with Fl_Tree_Item::widget(),

    To destroy an FLTK widget assigned to the item, you can do that cleanly this way:

        // Delete the Fl_Tree_Item 'item' and any associated FLTK widget
        Fl_Widget *w = item->widget();   // save any widget for the item
        tree->remove(item);              // remove the item, destroying it
        if ( w ) Fl::delete_widget(w);   // delete the widget last, if there was one

I would prefer:

        // Delete the Fl_Tree_Item 'item' and any associated FLTK widget
        Fl_Widget *w = item->widget();   // save any widget associated with the item
        item->widget(0);                 // disconnect the widget
        tree->remove(item);              // remove the item, destroying it
        delete w;                        // delete the widget, if there was one


Note that operator delete allows NULL as argument, but it's also fine to use if(w) and also Fl::delete_widget(w) (if you're paranoid). I would explain the last part in the text (too lazy now).

I would suggest to call item->widget(0) just in case a future method would delete the associated widget. Or you could use Fl_Widget_Tracker, but I'd say this is overkill.

I *believe* it would be best if 'tree->remove(item)' would also delete the associated widget. I have no idea how to achieve this with backwards compatibility in mind though. :-(


    For more info, see Fl_Group section on deleting FLTK child widgets.

    Note that after a remove(), any widget assigned to the item will remain a child
    of Fl_Tree's Fl_Group, which will continue to 'own' the widget. This allows the
    widget to be reassigned to another item without having to recreate it. Destroying
    the tree will automatically destroy any child fltk widgets.

I think specific details regarding proper deleting of FLTK widgets should appear
only in Fl_Group, and other docs simply refer to that documenation as needed,
so there aren't multiple descriptions of the process.


I agree.


There may be other widgets besides Fl_Browser and Fl_Tree that may need elaboration on whether memory is freed or not with methods like remove().

Yep.


Bill Spitzak

unread,
Jul 15, 2021, 2:34:59 PMJul 15
to fltkc...@googlegroups.com
It is pretty easy to write callbacks that will crash if some widget has been deleted, which is the reason the deferred deletion is provided.
However if fltk or any of it's built-in widgets crashes because a widget was deleted by a callback then this is a bug that should be fixed.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/2b28917e-ea29-c2a9-dfc9-cde40ce05fc3%40online.de.

Albrecht Schlosser

unread,
Jul 15, 2021, 3:44:33 PMJul 15
to fltkc...@googlegroups.com
On 7/15/21 8:34 PM Bill Spitzak wrote:
> It is pretty easy to write callbacks that will crash if some widget
> has been deleted, which is the reason the deferred deletion is provided.

I agree. This has probably been done because I experienced crashes
around FLTK 1.1.5rcN when someone (IIRC Mike) changed code to reset the
changed() flag after the callback. At that time I was only a "user" of FLTK.

> However if fltk or any of it's built-in widgets crashes because a
> widget was deleted by a callback then this is a bug that should be fixed.

I agree again. This is the reason why the class Fl_Wiget_Tracker was
invented (by me) - after some work of Matt who made this class possible.

The conclusion is:

(1) if user code accesses deleted widgets in a callback it's their fault
- and we should warn not to do this.

(2) No FLTK widget must access a widget after the callback w/o checking
that it hasn't been deleted - which is done by using Fl_Widget_Tracker
and testing whether the widget still exists after calling the callback.

The only reason why Fl::delete_widget() would be needed today (in FLTK
1.4) is if something mentioned above is not taken care of. I hope I
found all cases and fixed the code in 1.1.x or at least in 1.3.x (I
don't remember exactly). Hence Fl::delete_widget() should never be
necessary WRT FLTK widgets, but users might still want it if their own
code is "broken" (or hard to fix).

Note that there's one fatal drawback of using Fl::delete_widget(): if
you ever call one of the standard FLTK dialogs or own code that uses
Fl::wait() inside a callback this could lead to unexpected (pemature)
widget deletion because Fl::wait() would delete all widgets scheduled
for deletion by Fl::delete_widget().

Reply all
Reply to author
Forward
0 new messages