[SOLVED] Cleanup problem of GroupBox

[BrightBit]

I created a custom GroupBox that has some extra AutoWindows (in addition to "__auto_contentpane__"). Everything works fine. However, during its cleanup these additional windows aren't removed.

This is the corresponding entry in my looknfeel file:

Code: Select all

<WidgetLook name="BNBWidgets/Belt">

   <!--PropertyDefinitions-->

   <Child type="BNBWidgets/HealthBar" nameSuffix="Health">
      <!--Area Definition, etc.-->
   </Child>
   <Child type="BNBWidgets/StaminaBar" nameSuffix="Stamina">
      <!--Area Definition, etc.-->
   </Child>
   <Child type="DefaultWindow" nameSuffix="__auto_contentpane__">
      <!--Area Definition, etc.-->      
   </Child>

   <ImagerySection name="Label">
      <!--TextComponent, Area Definition, etc.-->
   </ImagerySection>

   <ImagerySection name="Normal">
      <!--FrameComponents, Area Defintions, etc.-->
   </ImagerySection>

   <StateImagery name="Enabled">
      <Layer>
         <Section section="Normal" />
         <Section section="Label" controlProperty="LabelEnabled"><ColourProperty name="NormalTextColour" /></Section>
      </Layer>
   </StateImagery>
   <StateImagery name="Disabled">
      <Layer>
         <Section section="Normal" />
         <Section section="Label" controlProperty="LabelEnabled" ><ColourProperty name="DisabledTextColour" /></Section>
      </Layer>
   </StateImagery>

</WidgetLook>

These are the corresponding entries in my scheme file:

Code: Select all

<FalagardMapping WindowType="BNBWidgets/HealthBar" TargetType="CEGUI/ProgressBar" Renderer="Falagard/ProgressBar" LookNFeel="BNBWidgets/HealthBar" />
<FalagardMapping WindowType="BNBWidgets/StaminaBar" TargetType="CEGUI/ProgressBar" Renderer="Falagard/ProgressBar" LookNFeel="BNBWidgets/StaminaBar" />
<FalagardMapping WindowType="BNBWidgets/Belt" TargetType="CEGUI/GroupBox" Renderer="Falagard/Default" LookNFeel="BNBWidgets/Belt" />


CEGUI doesn't simply crash during the cleanup. It gets stuck in an endless loop:

Code: Select all

void Window::cleanupChildren(void)
{
    while(getChildCount() != 0)
    {
        Window* wnd = d_children[0];

        // always remove child
        removeChildWindow(wnd);

        // destroy child if that is required
        if (wnd->isDestroyedByParent())
            WindowManager::getSingleton().destroyWindow(wnd);
    }
}

The root of this problem seems to be this method:

Code: Select all

void GroupBox::removeChild_impl(Window* wnd)
{
   if (wnd)
   {   // Auto pane itself?
        if (wnd->getName().find(ContentPaneNameSuffix) != String::npos)
        {   // Yes
            Window::removeChild_impl(wnd);
            WindowManager::getSingleton().destroyWindow(wnd);
        }
        else
        {   // Remove child from out auto pane
            Window* wndPane = getContentPane();
            if (wndPane)
            {
                wndPane->removeChildWindow(wnd);
              if (wnd->isDestroyedByParent())
              {
                 WindowManager::getSingleton().destroyWindow(wnd);
              }
            }
        }
   }
}

These additional AutoWindows are neither the auto pane itself nor children of the content pane. Please correct me, if I'm wrong.

Here's my CEGUI.log in case it is needed.

EDIT: It's a bit off topic but: Is there a way to calculate Custom Properties (PropertyDefinitions) in a looknfeel file? Or can I define a named area and refer to it in ImageSections? I just want to reduce code duplication.

[CrazyEddie]

I created a custom GroupBox that has some extra AutoWindows (in addition to "__auto_contentpane__"). Everything works fine. However, during its cleanup these additional windows aren't removed.

Agh! Thanks for this. I've not looked at it yet (aside from your post), but can pretty much confirm this as a bug in GroupBox and likely other widgets that use a similar approach. I will fix this ASAP

It's a bit off topic but: Is there a way to calculate Custom Properties (PropertyDefinitions) in a looknfeel file? Or can I define a named area and refer to it in ImageSections? I just want to reduce code duplication.

There's no way to do calculations in a property definition. In the development code (1.0) we have enhanced the system to reduce repetition and be even more flexible - there you can use a NamedArea or a Property that describes an area as the area for any component (Image, Text, Frame and Child), in addition it's also possible to have a WidgetLook inherit from another WidgetLook. Unfortunately these new abilities are not available in the v0-7 branch code.

CE

[BrightBit]

You're welcome as always. Here's another thing I noticed. I'm not sure if this is related to the "Additional AutoWindow" problem I mentioned above, so I didn't open a new post:

When I add the attribute "layoutOnWrite='true'" to a PropertyDefinition AND have an additional AutoWindow in a corresponding FrameWindow-looknfeel-description, CEGUI throws several exceptions:

Code: Select all

UnknownObjectException in file c:\...\ceguipropertyset.cpp(109) : There is no Property named 'BarWidth' available in the set.
UnknownObjectException in file c:\...\ceguipropertyset.cpp(109) : There is no Property named 'BarWidth' available in the set.
UnknownObjectException in file c:\...\ceguiwindowmanager.cpp(256) : WindowManager::getWindow - A Window object with the name 'Belt__auto_contentpane__' does not exist within the system
UnknownObjectException in file c:\...\ceguiwindowmanager.cpp(256) : WindowManager::getWindow - A Window object with the name 'BeltHealth' does not exist within the system
UnknownObjectException in file c:\...\ceguiwindowmanager.cpp(256) : WindowManager::getWindow - A Window object with the name 'BeltStamina' does not exist within the system
UnknownObjectException in file c:\...\ceguiwindowmanager.cpp(256) : WindowManager::getWindow - A Window object with the name 'Belt__auto_contentpane__' does not exist within the system

I also was able to reproduce this with the default TaharezLook.looknfeel:

Code: Select all

<WidgetLook name="TaharezLook/FrameWindow">
   <PropertyDefinition name="ClientAreaColour" initialValue="FF141B38" redrawOnWrite="true" layoutOnWrite="true" />
   <!-- PropertyLinks and Properties, etc. -->

   <Child type="TaharezLook/Editbox" nameSuffix="Temp">
      <Area>
         <Dim type="LeftEdge"><AbsoluteDim value="0" type="TopEdge" /></Dim>
         <Dim type="Width"><UnifiedDim scale="1" type="Width"/></Dim>
         <Dim type="TopEdge"><AbsoluteDim value="0" type="TopEdge" /></Dim>
         <Dim type="BottomEdge"><UnifiedDim scale="1" type="BottomEdge" /></Dim>
      </Area>
   </Child>

<!-- all the rest goes here -->


[CrazyEddie]

When I add the attribute "layoutOnWrite='true'" to a PropertyDefinition AND have an additional AutoWindow in a corresponding FrameWindow-looknfeel-description, CEGUI throws several exceptions

I'm pretty sure this is the same as an issue I fixed in default branch. I will confirm this and back-port the fix to v0-7.

Cheers,

CE.

[CrazyEddie]

Hi,

Both of these issues should now be fixed in v0-7 branch, I will merge back to default later today.

As far as I could tell, the hang when cleaning up was local to GroupBox and was not duplicated in other widgets that also make use of content panes, though of course let me know if something comes up that I missed

CE.

[BrightBit]

Thank you CrazyEddie.

The cleanup seems to work and the exceptions due to the "layoutOnWrite" attribute are gone, too. However, the order in which I define the AutoWindows in the looknfeel file seems to be important now. Is this the intention?

For my custom GroupBox I created two AutoWindow ProgressBars that should not be contained in the content pane (instead they are right next to it). If I define the ProgressBars first, they will be invisible (causing no errors, just invisible). If I define them after the content pane, they will be visible but they will also be contained in the content pane.


Greetings
BrightBit

[CrazyEddie]

Ok, the issue with ordering is likely related to the names you are giving the widgets. In v0-7, the name suffix needs to contain "__auto_" to be flagged as an auto window within the rest of the system, so setting the names to, for example: "__auto_progress1__" and "__auto_progress2__" or some such thing, should resolve that issue.

Now I've said that, I'm going to tell you that you can opt not to use the current GroupBox, and instead use a new technique instead. Basically, the existing GroupBox class is not really needed, and so we are deprecating it in the development code. What you can do instead - and this works in v0-7 too - is create something that behaves the same way via XML alone (but hopefully without all the issues). Effectively this means taking the current <Child> definition for __auto_contentpane__ and turning it into a <NamedArea> where the area is named "inner_rect". For your other auto windows, you should then set the property "NonClient" to "True" to ensure they appear outside the content area. In the scheme, simply update the GroupBox mappings to use the targetType of "DefaultWindow" instead of "CEGUI/GroupBox" and of course remove any casts or whatever you may have in your code.

This changeset shows these changes in a more concrete way: http://crayzedsgui.hg.sourceforge.net/h ... 82e9f2205c

HTH

CE.

[BrightBit]

Thanks again.

The NamedArea technique did work except for the fact that this area isn't updated if I change properties that are flaged with redrawOnWrite or layoutOnWrite (or both). Here's a small video to illustrate what I mean:

https://www.youtube.com/watch?v=0jg3-XJg7Fc

On the top left corner you should see a tiny rectangle (actually this is a button). It doesn't move even if I change the layout (illustrated by the blending in and blending out of the progressbars). The video was done quick and dirty, so ignore the low qualitiy, please.

[CrazyEddie]

Hi,

I just wanted to say that I've recreated this issue, and am looking at possible solutions. I may not get a fix in for a couple of days or maybe more, as I need to ensure the fix is the best it can be. So far I have found that there are many solutions that are either only partial solutions or are completely inelegant - addressing symptoms rather than the root cause of the issue.

I will advise when a fix has been made.

CE.

[CrazyEddie]

Hi,

I have made a fix for the issue in your video. The slightly bad news is that I could not make the fix in a way that allowed me to commit it to the v0-7 stable branch. But rather than just make the fix in our unstable development code, I created a new v0-7-incompatible-fixes branch that now contains this fix. While the name looks a little scary, for the most part you should be fine with just a recompile of CEGUI and then your own code - the current single exception to that is if you have custom Window classes that have overridden the Window::performChildWindowLayout function, if you do, you need to update the signature of those to take two bools.

HTH

CE.

[BrightBit]

Thanks in heaps. It works.

I hope this is no silly question but I am not that familiar with mercurial's branching model. Will future fixes of the 0-7 branch be integrated into the incompatible-fixes branch, too?


Greetings and thank you very much
BrightBit

[CrazyEddie]

I'm glad the fix has worked for you.

Will future fixes of the 0-7 branch be integrated into the incompatible-fixes branch, too?

I will merge any future fixes from v0-7 into the v0-7-incompatible-fixes branch, so you should be safe if you stay on that branch.

CE.

[BrightBit]

Great. You've been amazingly helpful. Thank you so much CrazzyEddie.