Andy Ross wrote:

Indeed this patch (below) doesn't work the way you want it to.  It is
not impossible to have a dialog element with a user defined width or
height.  The layout management code will clobber that value -- you
should be able to see this by deleting and recreating the dialog
repeatedly.  Different components will grow or shrink each iteration.

(There was actually a bug reported, and worked around, long ago like
this.  We now know that it was because removeChild() was failing to
actually remove the children.)

I *strongly* recommend reverting this change and fixing removeChild()

The way the code works is like this (I show it only for "width", but it's almost the same for "x", "y", and "height":

1. See if there is a property called "../x"
    --> bool userw = props->hasValue("width");

2. If not, automatically assign it:
    --> if(!userw || !userh)
    -->    wid.calcPrefSize(&pw, &ph);

3. Get the specified width if it exists, otherwise return the
   automatically calculated values:
    --> pw = props->getIntValue("width", pw);

4. Make a new object using these values:
    --> wid.layout(px, py, pw, ph);

This is useful when the file gets read every time the dialog opens (the way it used to be), but doing this every time while the previous value is still resident in the property tree (the current behavior) is useless.

Now, does this this patch change this behavior; not in any way.
The user still can specify it's own set of x, y, width and height properties and the code accepts them happily. If the user omits them the code still automatically assigns the best values the first time (and uses these settings every time afterwards).

Unless somebody proves me I'm wrong I won't revert this patch.

This doesn't mean the removeChild() code couldn't be wrong, I haven't tested that yet. But using the dialog code to prove that the removeChild() function doesn't work is *clearly* wrong.


Flightgear-devel mailing list

Reply via email to