Page 1 of 1

Windows Update

Posted: Fri Nov 10, 2006 15:28
by Whirly
Hi,

I am currently using CEGUI v0.4.1 and I stumble across a problem, then I checked the source code from v0.5 and it is still there so I just wanted to share my experience thus preventing some pain for CEGUI Users.

The window::update code looks like this :

Code: Select all

   // update child windows
   size_t child_count = getChildCount();

   for (size_t i = 0; i < child_count; ++i)
   {
      d_children[i]->update(elapsed);
   }


The problem is there is one case where a update can kill a window, it is if you use Tooltip. In that case if the tooltip is displayed and something make it disappear it will execute this code (in Tooltip::switchToInactivateWindowState ):

Code: Select all

        if (d_parent)
            d_parent->removeChildWindow(this);


So the tooltip remove itself from the window and the update loop of the window can simply crash as it tries to access an out of bound element.

I simply reverse counted the update loop going from childCount - 1 to 0 and then it works.

Stéphane[/code][/quote]

Posted: Mon Nov 13, 2006 10:00
by CrazyEddie
Hi,

Thanks a lot for the heads-up on this. I'll make an appropriate fix to the code in SVN :)

CE.

Posted: Mon Nov 13, 2006 15:05
by vasmann
CrazyEddie wrote:Hi,

Thanks a lot for the heads-up on this. I'll make an appropriate fix to the code in SVN :)

CE.


IMHO it should be like:

System singleton has std::map<Window*, Window*> of items to be removed, call it m_WindowsToDestroy
system singlten has function DestroyWindow(Window* wnd) (like CreateWindow) which only do is adds this window to this list
And in renderGUI shoul look like this
void renderGUI()
{
// .. do render stuff and updating of windows
// clear m_WindowsToDestroy //do not forget to free each Window* //instance
}

I guess this solution will be better than just reorder counter, because you do not make any suggestions about user defined code - user want to remove window please remove it. But when you change order of index (not bad solution) but you suggest that user can remove window - and it is not bug fix but just bug hide - mine opinion.

And another argument is:
if some child window wants to remove not only self but two or more child windows of same parent we will have such situation - for example:
child_count = 10
i = 9
and this 9th window removes 2 windows in nex iteration we have child_count = 8 and i = 8 but 8 not less then 8 so left windows will not have update event.
So if you wish to destroy window - mine solution is good and another fature - to avoid some problems - user can not only remove child windows but also add them, so base class Window will have to maps -
std::map<Window*, Window> - m_WindowsToAdd and m_WindowsToRemove (from childs) and when it is being updated code will look like

update
{
// m_Childs.insert(m_WindowsToAdd.first(), m_WindowsToAdd.end())
//m_WindowsToAdd.clear();
// m_Childs.erase(m_WindowsToRemove.end(), m_WidnowsToRemove.end()) update stuff
//m_WindowsToRemove.clear() - possible destroy these windows

// update stuff

}

Also methods should be updated - getChild()
{
//seek in m_WindowsToAdd
//if not find
//{ Seek in m_Childs
}

if you are intrested - I will make some coding home and post here or send on email.


Forgive me for my bad english
Thank you.
Hope I was helpfull.

Posted: Mon Nov 13, 2006 19:11
by vasmann
continue the idea it could be done more gracefully:

we have base class Window


Code: Select all


//window.h
//add abstract user command
//all it could do is process;

struct IUserCommand
{
  virtual void process() = 0;
};


class Widnow
{
//...
protected:
  std::vector<IUserCommand*> m_UserCommands;
protected:
  void updateUserCommands();
public:
  void directRemoveChild(Window* child); //this method removes directly from m_Childs
  void addChild(Window* wnd);
  void removeChild(Window* wnd);

  void update(/*params you need like time*/);
};

//window.cpp

class CmdRemoveChildWindow: public IUserCommand
{
private:
  Window* m_Parent;
  Window* m_Child;
public:
  CmdRemoveChildWindow(Window* parent, Window* child):m_Parent(parent), m_Child(child)
  {
  }
  virtual void process()
  {
    m_Parent->directRemoveChild(m_Child);
  }
};


void Window::updateUserCommands()
{
  while (m_UserCommands.size() > 0)
  {
   
     IUserCommand* cmd = m_UserCommands[0];
     m_UserCommands.erase(m_UserCommands.begin());
     cmd->process();
     delete cmd;
  }
}

void Window::removeChild(Window* child)
{
  m_UserCommands.push_back(new CmdRemoveChildWindow(this, child));
}

void Window::update(/*params*/)
{
  updateUserCommands();
  //do your stuff
}

//... Other methods make in such way



As I mentioned before - this is only my opinion - if you wish I could implement such thing in you code and than post it to you and you decide - need you or not this solution, or if you wish make it by yourself

Thank you.

Posted: Tue Nov 14, 2006 09:16
by CrazyEddie
Thanks for the suggested implementation - we already have a dead-pool for window destructions.

One thing that needs to be maintained is that once a window is 'removed' or 'destroyed', it must appear that this has occurred - we can't really have those operations waiting until the end of the frame, since the user could be relying on such behaviour (like remove a child from one window, add it to another, and then check to see if it's a child... oops! it's not there ;) ).

The other consideration is that whatever the solution, it ideally needs to be no slower than what it is now - we already have enough speed issues :P

I have this on my task list for today, so no need for anyone else implement anything ;)

CE.

Posted: Tue Nov 14, 2006 12:08
by CrazyEddie
After looking at the situation, this is one of those issues where the more I look at it the more it seems the 'simple' solutions are not going to cut the mustard. Oh well... ;)