On 11.07.2010, at 19:24, Greg Ercolano wrote:
> Albrecht Schlosser wrote:
>> Greg and others,
>>
>> I've seen some problems with Fl_Tree and included widgets with
>> the tree demo program. There may be an inconsistency or something
>> that needs clarification and documentation.
>>
>> Let me express it in a short question: Who "owns" the widgets
>> added to an Fl_Tree by calling Fl_Tree_Item::widget(Fl_Widget *)?
>
> The base class Fl_Group (that Fl_Tree derives from) manages
> all FLTK widgets added to the tree, meaning its the 'parent'.
Okay. We need clear "design decisions" what's intended to be able
to decide if it works correct.
> But it's true that once the association is made with item->widget(),
> the items should then 'own' it to the extent that the item's
> destructor should tell the Fl_Group to remove it. Should be relatively
> easy to add that.
I'd appreciate if you could do that, since you know the code best.
> I suppose as well, Fl_Tree should have some methods that trap any
> of Fl_Group's base class methods that manage child widgets
> (like Fl_Group::remove()) so that any caller trying to manipulate
> FLTK widgets that way get cleared from the tree to avoid any
> dangling pointers.
Yes, it's good that you say that. This is one more reason to make
Fl_Group::remove() virtual and then implement it in Fl_Tree to catch
widget removals. I thought that we need this for other reasons already,
but didn't bother to make it so ... yet.
Explanation: If a widget is deleted (in FLTK 1.3) and has a parent(),
it calls parent()->remove(this). If Fl_Group::remove() is virtual, then
it can be overloaded in Fl_Tree like this:
Fl_Tree::remove(Fl_Widget *widget) {
// pseudo code following ...
find the Fl_Tree_Item that owns the widget;
remove the widget from the Fl_Tree_Item;
Fl_Group::remove(widget); // call the base class's remove
}
Otherwise we would have dangling widget pointers in the Fl_Tree_Items.
> Glad to see the new widgets are getting the peer review they've needed.
Well, it was more by accident than by intention, but when I saw the
problem, I looked at the code - partially only, not systematically.
> I see myself as an applications programmer, not a core widget developer;
> I don't really know the FLTK API at the microscopic depth nor have the
> forward vision one should to write core widget APIs, especially with
> complex grouping widgets like Tables + Trees.
Nevertheless, I think your contributions are stable and valuable.
> Of course I get closer with every widget I make, but designing core
> widgets is not something I'd ever aspired to -- "necessity is the mother
> of all invention" and all that. I just wrote what I needed and
> offered it up, with the hopes I'd get some constructive feedback
> on how to make the widgets better.
I think that the best way would be to make clear documentation, also
about the design decisions (i.e. how to handle the widgets etc.). Doing
so will automatically show deficiencies and problems...
> Fl_Tree itself is relatively new, and I'm glad to see it's getting
> the peer review it has needed ;) The others could use this as well.
> (Fl_Table, FNFC..)
I don't use FNFC personally, but I always wanted to take a closer
look at Fl_Table and how to use it, because I believe that I could
use it in my app, but didn't find the time yet.
>> You can see the problem with the test/tree demo program. If you
>> clear a tree item while it (and thus its widget) is shown, then
>> the widget stays at its current position forever.
>> [[Example 1]]
>
> Right; I think some extra code added to the Fl_Tree_Item
> destructor can take care of that. It's basically an omission.
> Currently the destructor just NULLs out the widget() instead
> of telling the parent group to remove it.
Important question here: would it only remove it, or would it also
delete it? That must be clarified. Since deleting the Fl_Tree would
*delete* all child widgets, I think that we should decide to delete
the child widgets also when a tree item is deleted, but ... (any
other arguments here ?).
And something more would be needed: Fl_Tree_Item::widget() would
have to keep track of previous widgets and remove them, if a new
widget (or NULL) is assigned.
> I'll see about adding that.
Depending on how it is implemented, test/tree.cxx (i.e. tree.fl)
would probably need to be changed as well.
>> Example 2: start test/tree (or use "Rebuild Tree" after Example 1),
>> click on "Load Database..." [great new function, added by Matt -
>> thanks, BTW.),
>
> Hmm, I don't seem to see this 'Load Database' function.
> I'm looking at r7658?
It's been added in r 7672 by Matt. I think that his intention was
to look at the fltk.db file, and I used it this way. That's how
I found the problem with the widgets.
[...]
>> [1] this is not mentioned in the docs as well
>
> Yes, the Fl_Tree docs should probably have a section that describes
> how to manage FLTK widgets as children of the tree from a conceptual
> point of view.
That would be great.
> Having FLTK widgets be children of the widget was an afterthought,
> but seemed easy enough to add. It's just not fully fleshed out.
Okay then, could you take a look at it and ask here, if you need help?
I'd be willing to help, but you know the code and your initial design
and ideas better.
BTW.: I removed test/tree.cxx and tree.h from svn since they are not
needed anymore. The IDE and CMake files are now updated accordingly.
Albrecht
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev
