I have been looking into OgreBites' ApplicationContextBase. There are pairs of virtual methods createWindow() and _destroyWindow(). There is a non-virtual destroyWindow()... it seems it is designed to destroy a window using its "name".
This is not the contents of this post, but I do not see a good reason for using a window name... looking for a window using its name sounds a lot non-portable to me. Since createWindow() returns a NativeWindowPair, even the non-developer should not use the window name, but the "token" returned by "createWindow()". There should be no "destroy window from its name", IMO. But probably people will not agree with that, and if they do, it would be only for Ogre 15.
The problem I want to address is that those methods are not acting in "pairs". One does not undo what the other does. SDL and Android descendants (Qt is implemented right) do not call ApplicationContextBase::createWindow. But they all call ApplicationContextBase::_destroyWindow (Qt and Android do not override).
This is a little odd, because createWindow / _destroyWindow are supposed to work in pairs, in my opinion. There are at least two consequences:
- The code inside the base virtual createWindow is replicated inside each new createWindow (for example -- twisted example continuation).
- The code inside the base needs to be aware of the code inside its child.
We can easily fix this. We should separate the roles of the base class and the derived class. The derived class is responsible for setting up the "native window" and the miscParams. The base class is responsible for using those two and setting up Ogre's render system.
Code: Select all
/* derived createWindow pseudo-code -- just like in Qt */
create native window;
setup miscParams;
ApplicationContextBase::createWindow(..., miscParams);
Notice that not doing this is the reason for this (unecessary) hack:
https://github.com/OGRECave/ogre/blob/8 ... #L414-L417
There should be no "#ifdef" there. Either this line should not be in the base createWindow(), or it should always be in destroyWindow(). The problem is that those two children (SDL and Android) fail to call "WindowEventUtilities::addRenderWindow()", while Qt does (because it calls the base createWindow()). Probably SDL and Android were implemented a long ago and got "out of sync" with the replicated code in the base class.
Shall I submit a PR?
PS: A side effect is that "WindowEventUtilities::_addRenderWindow()" will be called for SDL and Adroid. Is this okay? Otherwise, we have to call it inside Qt's createWindow(), and remove in Qt's _destroyWindow().