| | [ Return to Bugs & Features | Roadmap 1.3 | Post Text | Post File | SVN ⇄ GIT | Prev | Next ]
STR #3214
| Application: | FLTK Library |
|---|
| Status: | 4 - Pending |
|---|
| Priority: | 2 - Low, e.g. a documentation error or undocumented side-effect |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | Core Library |
|---|
| Summary: | Fl_Tree::clear() does not delete widgets (only items) |
|---|
| Version: | 1.3-current |
|---|
| Created By: | Belgiansteve |
|---|
| Assigned To: | greg.ercolano |
|---|
| Fix Version: | Unassigned |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
[ Post File ]
Trouble Report Comments:
[ Post Text ]
| |
| #1 | Belgiansteve
09:43 Apr 10, 2015 |
| I have a Fl_Tree that contains many Fl_Widgets. I noticed when calling Fl_Tree::clear(), the child widgets are not deleted. (in contrast to Fl_Group::clear()). Calling (*Fl_Tree)->Fl_Group::clear() doesn't work since it deletes all the children of the Fl_Tree, including the hidden scrollbars. Deleting the Fl_Tree does delete the widgets, so I guess, in the end everything gets cleaned up properly, but I was wondering why Fl_Tree::clear() doesn't delete them. Is this a bug or by design? Thanks! | |
| |
| #2 | greg.ercolano
17:10 Apr 10, 2015 |
| Hmm, that might be an omission.
I hadn't conceptually thought of the FLTK widgets as part of the tree, so I didn't try to manage them. But I think you're right, clear() should probably include destroying any widgets associated with the tree.
Will investigate.. | |
| |
| #3 | greg.ercolano
17:12 Apr 10, 2015 |
| Assigning to me, adjusted the summary. | |
| |
| #4 | greg.ercolano
22:01 Apr 10, 2015 |
| Give this patch a try.
Adds Fl_Tree::clear_widgets() which you can call to clear
just the child widgets that you add to the items (or otherwise
parent to the tree).
You can also just clear the widgets, or just clear the items, or both.
So to clear the widgets and items:
tree->clear_items();
tree->clear();
Note: the patch includes mods to the test/tree program, which adds
a "Clear Widgets" button to test this new method. And you can hit 'Rebuild tree' to get them back. You should be able to use any combo
of "Clear Widgets", "Clear All", and "Rebuild Tree" without any bad
effects. | |
| |
| #5 | greg.ercolano
22:03 Apr 10, 2015 |
| CORRECTION:
So to clear the widgets and items:
tree->clear_widgets();
tree->clear(); | |
| |
| #6 | AlbrechtS
08:02 Apr 11, 2015 |
| I don't know the internals of Fl_Tree, so please correct me if anything I write or assume is wrong.
I believe that Fl_Tree::clear() should also delete the widgets, as the OP suggests. clear_widgets() is a useful method anyway, but it should be called directly from clear() - at least in the final version. But maybe you intended to do that anyway.
Fl_Tree is derived from Fl_Group, and as such clear() should .. clear the Fl_Tree widget and also delete the child widgets, unless I'm missing something important. In a future version (1.4) Fl_Group::clear() should also be virtual.
One note to the patch: using Fl::delete_widget(w) seems unnecessary. Fl_Group just delete's the children, and so should Fl_Tree. If you use delete, then Fl_Group::remove(*w); is not necessary because the widget will be removed anyway when it is deleted. Reasoning: Fl::delete_widget() might be slow if the tree is large (many child widgets) because it would add all children to the internal widget deletion list. Fl::delete_widget isn't really needed since FLTK 1.3 anyway, so we shouldn't use it internally. | |
| |
| #7 | greg.ercolano
08:57 Apr 11, 2015 |
| Lets move discussion to coredev; I'll open a thread. | |
| |
| #8 | Belgiansteve
12:48 Apr 11, 2015 |
| Thanks for the patch! It is working! For what it's worth, I do agree with Albrecht and would prefer if the clear() method cleans up both the items and the widgets. | |
| |
| #9 | greg.ercolano
14:29 Oct 20, 2017 |
| Updating patch specifically for 1.4.x (without the ABI macros)
for further investigation as clear_widgets_v2.patch.
I had a small block of time to revisit, but I might not be able to
dive in for another few days/weeks, so I wanted to save a snapshot of
where I am right now.
Patch leaves out the 1.3 ABI macros. Also some doc clarifications for
Fl_Tree_Item_Draw_Mode and Fl_Tree_Item::widget() and Fl_Tree::clear().
I'm leaving out the patch to test/tree.fl, as that probably needs to
be reworked for 1.4 anyway; didn't have time to do that today.
test/tree should be also be modified to test destruction of widgets
assigned to items with and without item->widget() during clear()
and individual item remove operations. (Example: make a MyWidget()
whose destructor prints messages using tty->printf() to show
destruction when clear() and remove() operations as used). Test should
also verify if clear_widgets() works as expected, removing the widgets
but leaving the tree.
Regarding Albrecht's Comment #6 and comments on Apr 11 2015 on fltk.coredev:
> I'd suggest the _item_ takes ownership of the widget, because it becomes
> a part of its "label".
Yes, it would and does when assigned to an item with item->widget().
And when it's not (e.g. caught up by begin()/end()), it's "owned"
by the group.
There is also the need to clear/replace the widgets but not the tree items;
when widgets are removed but the tree items remain, the tree items draw
as they normally would, using their own labels.
clear_widgets() would simply remove all the FLTK widgets (but not the scrollbars),
de-assigning any assigned to items, and remove()ing any /not/ assigned to items,
but are children of the group (such as invisible Fl_Box resizable placeholders).
It's important that Fl_Tree's clear() operations do NOT call Fl_Group::clear(),
as this would clear Fl_Tree's scrollbars, which needs to be avoided. Pretty much
this is the same design problem Fl_Scroll has; derives from Fl_Group, but carefully
keeps its scrollbars intact.
> One note to the patch: using Fl::delete_widget(w) seems unnecessary.
Hmm, I'm not sure; if Fl_Tree::clear()/clear_xxx() need to get rid of widgets,
whether assigned to items with item->widget() or not, they need to be destroyed.
We can't call Fl_Group::clear() (for scrollbar preservation), so we have to do
what Fl_Group::clear() would do, which does both a remove() and a "delete w".
Perhaps I'm missing something. | |
[ Return to Bugs & Features | Post Text | Post File ]
|
| |
