Page 1 of 1

Suggestion: get rid of default context

Posted: Wed Feb 06, 2019 19:26
by niello
Hi all.

I'm not sure why it was added but it rather makes things harder than helps. When creating CEGUI you must already know its properties, and you have to create this context when CEGUI is initialized, not when you need it. For the hypothetical case of using CEGUI for multiple windows (no doubt a pretty rare case but a revealing one) the logic will be like this:

    Init render system
    Create swap chain (viewport)
    Create CEGUI with default context sized as your viewport
    Use this context for your first window
    For each subsequent window create a new target and context

The worst thing here is that this code requires different handling for the first and subsequent windows. Also it requires to initialize CEGUI after the rendering system, but it is usually the case anyway.

I would propose to remove the entire concept of "default context" and create one when you need it. So the logic becomes:

    Init render system, create CEGUI system (in any order)
    Load whatever resources and configs you want
    Then for every window:
    - Create viewport target (which just means an externally set render target) or a CEGUI-created texture target
    - Create context from this target, setup defaults from config inside CEGUI or leave it to the application

So the system becomes more clean, more flexible and more well-designed for multi-context usage at once. I think this is The way to go but I don't want to make any major local changes which will not be the part of main CEGUI.

Re: Suggestion: get rid of default context

Posted: Wed Feb 06, 2019 20:17
by Ident
It was added because of render-to-texture capabilities and that's why we can't remove it.
I just realised I misread your intent while skimming your post.

I think you have a good point, but I do not know what the implications of the proposed changes would be.

Do you know how much work it would be so we could see how it looks and fares? Do you wanna make a PR?

Re: Suggestion: get rid of default context

Posted: Thu Feb 07, 2019 07:45
by niello
The default context is implemented as d_guiContexts[0]. It is used in Config_xmlHandler.cpp to set default font, tooltip and cursor, which is better done for each context at its creation. It is used as fallback case in Window::getGUIContext() with the comment like "should we return nullptr instead?", and yes we should, at least because explicit is better than implicit.
Otherwise it is not used anywhere and can be safely removed. I will make a PR for a review in a week or two.

Re: Suggestion: get rid of default context

Posted: Thu Feb 07, 2019 09:09
by Ident
niello wrote:The default context is implemented as d_guiContexts[0]. It is used in Config_xmlHandler.cpp to set default font, tooltip and cursor, which is better done for each context at its creation. It is used as fallback case in Window::getGUIContext() with the comment like "should we return nullptr instead?", and yes we should, at least because explicit is better than implicit.
Otherwise it is not used anywhere and can be safely removed. I will make a PR for a review in a week or two.

Ok thanks for the information. This all sounds very reasonable.

niello wrote:because explicit is better than implicit.

I wholeheartedly agree with this way of thinking :D

The implicit stuff just causes confusion, mistakes and prevents people from properly understanding what is going on in case they don't already know exactly what's going on. So yes, I think this should be part of CEGUI default branch. Unfortunately we can't change this in v0-8 because it would break the existing applications of all people who depend on this, right?

Re: Suggestion: get rid of default context

Posted: Thu Feb 07, 2019 13:01
by niello
Definitely. I'll try to migrate to default, and if it causes too much pain / time waste, I will make default-based PR for you and patch my local version accordingly until I'm ready to update major version.

Re: Suggestion: get rid of default context

Posted: Sun Feb 10, 2019 09:52
by Ident
Sounds good!

Re: Suggestion: get rid of default context

Posted: Sun Feb 17, 2019 18:14
by niello
The main part is done. But the presense of default viewport target still prevents a proper usage of contexts in a multi-window scenario. We need a viewport target per a viewport, not one per application. I am working on a solution, but it may require to change Renderer interface slightly. To be specific, I plan to remove getDefaultRenderTarget() and to allow createViewportTarget() instead.

Upd:
Or maybe I will just add createViewportTarget() without deleting getDefaultRenderTarget(), because it seems too hard for me to build local infrastructure to test all CEGUI renderers.

Re: Suggestion: get rid of default context

Posted: Tue Feb 19, 2019 19:12
by niello
Migrating to 'default' I noticed that it has no 'drawmode-devel' merged into it. Can you or me merge drawmode-devel? My code relies on it.

Re: Suggestion: get rid of default context

Posted: Sun Feb 24, 2019 18:07
by Ident
I merged it all into default. I hope I did not miss anything, some file histories did not involve a "move" operation due to a change someone once made that is hard to undo, so there is now always a chance of loss, and also there was a bunch to merge now (which shall be a reminder again that i should merge to default immediately after accepting a PR).

Does it work now?

Re: Suggestion: get rid of default context

Posted: Tue Feb 26, 2019 19:37
by niello
ScrollablePane.cpp:248 - need to change from container->setMouseCursor(getMouseCursor()); to container->setCursor(getCursor());
Also drawMode was not merged. No trace of it in code. Looks like drawmode-devel became drawmode-devel-v0 and was not merged to v0-8 or v0 yet. So merging v0-8 -> v0 -> default didn't help.

Re: Suggestion: get rid of default context

Posted: Sun Mar 03, 2019 20:52
by Ident
niello wrote:ScrollablePane.cpp:248 - need to change from container->setMouseCursor(getMouseCursor()); to container->setCursor(getCursor());

I fixed it when i ran the samples but forgot to submit. Submitting it now.
niello wrote:Also drawMode was not merged. No trace of it in code. Looks like drawmode-devel became drawmode-devel-v0 and was not merged to v0-8 or v0 yet. So merging v0-8 -> v0 -> default didn't help.

No idea what that means, can you elaborate?

Re: Suggestion: get rid of default context

Posted: Mon Mar 04, 2019 06:21
by niello
There is a branch named 'drawmode-devel-v0'. It contains a feature of rendering some parts of GUI separately. For example as opaque, allowing it to write to Z-buffer. It would be nice to merge it into the default branch if possible.

Re: Suggestion: get rid of default context

Posted: Fri Mar 29, 2019 13:16
by niello
Any news here?

Re: Suggestion: get rid of default context

Posted: Mon Apr 29, 2019 12:44
by niello
Default context is removed from the 'default' branch.
Please note that using Renderer::getDefaultRenderTarget is now highly discouraged (I would even prefer to say prohibited) in a core CEGUI code. It can be left for convenience and used in user code though.