tl;dr : sorry for the long text. Withdrawing my first idea, I
suggest we move child related data to the children and not
allocate it as part of the parent, and create a standardised
interface for that.
Ok, I see the points.
So changing Fl_Pack is not realistic.
OK, I'm glad you agree.
And since Fl_Flex is in fltk_rs, it's probably wise to keep
the method names as is.
I would still want to keep the option to change method names as long
as FLTK 1.4 is not yet released. fltk_rs could either keep the old
method names in their own implementation or follow the transition.
Relying on non-released software must be done on one's own risk. I
did already change some method names for FLTK compliance from the
original contribution (which had already been adopted before by
fltk-rs) and so we can do now too.
Fl_Groups seems to avoid successfully to have pointers
dangle, or we would know about it.
Maybe not. The only dangling pointer I can imagine is indeed the
resizable() pointer. We might not know about it because users won't
delete or remove the resizable() widget that often w/o resetting the
resizable() by themselves.
I'll check this because it would be a bug.
I am just very much in favour to point at something else only
once, because otherwise there are just too many locations to
keep track of. For example, if I delete and Widget, it may have
a parent that links back to it, but it also may be resizable, so
my parent may have a second pointer.
Yep, but that's not bad by itself. The parent could also store the
index of the resizable child but that's probably much more error
prone. But somehow the information must be stored.
Now with Fl_Flex, there may be just another pointer that
tells me that my widget has a fixed size.
This is IMHO unavoidable unless you keep a structure for each child
widget that contains a pointer and more attributes. However, that
structure would have to reserve special purpose attributes for every
container class that may own the child widget which is obviously not
easily doable. I believe that every container (group) widget must
store its own per-widget data in its own structure(s).
Hmm, well, after I wrote this and thought more about your group_data
approach (below), I think it is possible by subclassing the
group_data structure for every clase (as a subclass of its parent's
struct/class) but this begins to become more and more complicated.
This is by no means a drama, but if it is possible to avoid
it easily, I would. So here are my ideas:
I generally agree (it should be avoided), but emphasis is on both
"possible" and "easy".
- Storing relative width and height was a bad idea by me,
because different Groups may want to store very different
things per child.
I agree.
- Making the Group handle an array of widget data
(Fl_Group::init_size()) or widget pointers
(Fl_Group::resizable()) is inherently unsafe, even if we got
away with it for 30 years. But that's also part of what kept
us from implementing new Groups with better resizing options
like Flex and Grid.
I can't argue against "inherently unsafe". Yes, it needs some rules
but I don't see a better and "easy" solution yet.
I would however not agree that this "kept us from implementing new
Groups with better resizing options". Fl_Flex and Fl_Grid show that
it is possible but it's just a fact that nobody did it.
- Making Fl_Group::insert(), remove(), and clear() virtual
would make implementing Flex and Grid easier and safe.
Unfortunately it's too late to do this as I wrote before.
Inconsistent method naming in derived classes prevents this.
I would be glad to see that I'm wrong with this, but that's what I
found out so far. I could dig in my notes, maybe I can find some
info - although I dropped that idea.
So I withdraw my original idea and suggest that we add a
group_data pointer to every widget. This pointer is reserved
for use by the current parent exclusively, and the parent can
store whatever it wants.
Hmm, this doesn't seem very "easy". Imagine Fl_Flex, a group that
stores its own data in its own structure (lets say:
_Fl_Flex_Group_Data) similar to your example code below. That's
fine.
But what if you derive another class (FancyFlex) from Fl_Flex that
wants to store its own data in another structure that is linked by
the one and only per-widget group_data pointer? If this derived
class used its own structure to store the data it would destroy the
data stored by its base class Fl_Flex. OK, the derived class
FancyFlex could also subclass _Fl_Flex_Group_Data as
_FancyFlex_Group_Data and could use additional members in
_FancyFlex_Group_Data etc. etc. But that doesn't sound "easy", does
it?
It should delete that data when removing a widget, but if
it doesn't, the widget removes the data when being deleted (no
leak). If it is added to a new Group, the new Group will
simply replace the group_data with its own data.
Yes, it adds a pointer to Fl_Widget, but OTOH it removes
pointers and code in Fl_Group.
I'm not sure that the latter (removing pointers and code) would be
the case.
Let's talk about the resizable() widget pointer. It simplifies the
standard resizing algorithm by providing direct access to the single
resizable() widget that affects the resizing behavior of all its
children. If this information was stored in every child's group_data
structure, the resizing algorithm would have to search the child
array for the resizable child - or _all_ resizable children (?) -
before the calculation can begin. This costs CPU time and doesn't
help much. Yes, it removes one pointer from Fl_Group but it *adds*
code to the standard Fl_Group class and costs likely a performance
penalty.
To have a little code, I suggest:
class _Fl_Group_Data {
public:
virtual ~Fl_Group_Data();
};
class Fl_Widget {
~Fl_Widget() {
...
group_data(NULL);
}
void
group_data(_Fl_Group_Data*); // replace current group data
with new group data
_Fl_Group_Data *group_data();
};
class Fl_Fancy_Group {
class _Fl_Fancy_Group_Data :
public _Fl_Group_Data {
double width_factor;
int initial_width;
...
};
void insert(FL_Widget *w) {
w->group_data(new
_Fl_Fancy_Group_Data(50.0, w->w()));
...
}
void remove(Fl_Widget *w) {
...
w->group_data(NULL); // will
call 'delete' for us
}
};
This looks generally like a good idea.
However, the statement above:
`w->group_data(new
_Fl_Fancy_Group_Data(50.0, w->w()));`
is IMHO an oversimplification. It assumes that the group
(class) can allocate a new group data structure of its own and that
all parent classes can be initialized with default constructors (in
this example only _Fl_Group_Data). But how can this be achieved for
all nested classes of deeply subclassed group data structs?
[Potential answer: add the existing group_data pointer of the parent
class to the constructor?]
Please don't misunderstand me, these are only some thoughts I had
when reading your proposal. I believe it's worth thinking about it
and maybe implement it - if we can solve the open questions and
issues.
Thank you very much for this proposal, I believe it's worth
investigating, but as (you also) said before, we need to get 1.4.0
released soon and such big changes can't be done before that.
However, I believe that changes like these are only internal
implementation details and could be done in the next minor release
(maybe 1.5.0 or whatever) where we can break the ABI w/o issues and
lots of ABI guards in the code. Even if Fl_Flex and Fl_Grid do their
work differently in 1.4.x.
PS: Matthias, it's great to see you "back" and active again!