Child windows again

Started by netrick, 13 July 2013, 10:46:28

netrick

Okay, after some profiling with a lot of objects I found manually keeping child windows in place is a little bottleneck and it's not very clean solution to iterate through thousands of gui objects every frame to check them type in my code.

Simple solution:
- child window has property like enum keepInPlace - inWindow, inPanel, dontKeep (default).
- when child window is forced to change its position, it makes sure it's in place using my simple code which actually works

Maybe not the cleanest possible solution, but it won't hurt and will be so damn useful for me.

If you agree I can submit a simple patch later today. It will be just optional feature and default it will be turned off.

Edit:
There is a possibility that object doesn't have acces to window - in that case, it will just keep itself in its container. I need your feedback here.

texus

Is the inWindow really necessary? Isn't an inParent and dontKeep enough?

QuoteIf you agree I can submit a simple patch later today. It will be just optional feature and default it will be turned off.
I agree, but I might want to make some small changes on the patch before it gets merged.
If you are going to make a pull request on github then maybe it is better to first post the diff file here so that I can see what you changed exactly and perhaps change a few things. That way I can just merge the pull request immediately.

netrick

I will do only inParent and dontKeep. Of course naming could be better because sometimes it's hard to think of a good name in english.

I will write here when I finish it.

netrick

Well it's almost done except one thing that I can't solve - tgui::Container doesn't have its size. And dynamic casting to check if it's a tgui::ContainerWidget or a tgui::Gui isn't a good solution... Do you have any idea?

And if we can't solve it, then all I can do is to make it keepInWindow only (but, I'm not sure if Widget has access to the window).

texus

There is a getDisplaySize function in Container, which was added for exactly this reason.

Widget can't access the window, this is why keepInWindow can't be easily done (and I don't really see a use case for it anyway).

netrick

(I think) it's working. Now I only need to find out how to make a diff file.

texus

Or you could send me the changed files and then I'll make the diff myself.

On linux this is super-easy and I only have to type this in the terminal:
diff old_tgui_folder tgui_folder_with_changes > changes.diff

I'm sure there is a similar tool on windows too, but the command prompt isn't really something you open every day.

netrick

I'm working on linux so I can easily make it. I will send it you as soon as possible but I encountered a very strange bug and I'm looking into it. (for some reason it ignores my "if" and it always keeps in parent, despite debugger saying the value is set to false as it should). I will do a short break and then look into it.

netrick

#8
I hope the .diff file is good, because it shows some strange changes that I didn't make. (mostly formatting and in one place it uses ! instead of +). Tell me if it works, if not I will just upload the changed files.

Also I removed virtual keyword from Transformable::setPosition, because if you decided to use it as a virtual function my patch wouldn't work (because I explicitly call Transformable::setPosition in a few places).

texus

setPosition has to be virtual for other objects.
But when you call Transformable::setPosition explicitly then the virtual won't bother you, only the code in Transformable will be executed.

I should have asked for a different way to patch, because this diff file is a bit hard to read (its even worse because of the changed whitespace, this is because you used tabs instead of spaces).
Could you perhaps run this?
git clone https://github.com/texus/TGUI.git
cd TGUI
cp -R changed_tgui_folder .
git diff > changes.diff


That should give a better patch that I can even immediately apply.

netrick

It doesn't work.

cp -R changed_tgui_folder . -- it gives the error than it's the same file

git diff produces empty file.


netrick

Here is the current git snapshot with changed ChildWindow files. Create patch yourself :P

TGUI-patch.tar.gz

texus

Good idea.

Looks good (except for the tabs everywhere but that was easily fixed), but I'll still change a few lines.
I'll do that later today.

If I changed the lines, do I need to send you the changes so that you can write a pull request (and have your name next to the commit) or do I just add it to git myself (and add your name in the commit message)?

netrick

Just add my name in the commit message. This patch is so small that there is no point to bother with making github account :P

netrick

Post here when you apply the patch so I can remove child window checking in downstream code.