[Patch] properly support Overlapped non-root widgets

Follow-up to viewtopic.php?f=17&t=30284

Changes:
- Removed strange _forcePick call on mouse click.
- upLayerItem will use the root LayerNode rather than the deepest LayerNode.

Since I have not found any uses of Overlapped non-root widgets in MyGUI demos/tools/unit tests, the changes shouldn't break anything.

Index: MyGUIEngine/src/MyGUI_InputManager.cpp
===================================================================
--- MyGUIEngine/src/MyGUI_InputManager.cpp (revision 5304)
+++ MyGUIEngine/src/MyGUI_InputManager.cpp (working copy)
@@ -268,20 +268,6 @@
{
// поднимаем виджет, надо подумать что делать если поменялся фокус клавы
LayerManager::getInstance().upLayerItem(mWidgetMouseFocus);
-
- // поднимаем пикинг Overlapped окон
- Widget* pick = mWidgetMouseFocus;
- do
- {
- // если оверлаппед, то поднимаем пикинг
- if (pick->getWidgetStyle() == WidgetStyle::Overlapped)
- {
- if (pick->getParent()) pick->getParent()->_forcePick(pick);
- }
-
- pick = pick->getParent();
- }
- while (pick);
}
}

Index: MyGUIEngine/src/MyGUI_LayerItem.cpp
===================================================================
--- MyGUIEngine/src/MyGUI_LayerItem.cpp (revision 5304)
+++ MyGUIEngine/src/MyGUI_LayerItem.cpp (working copy)
@@ -131,8 +131,16 @@

void LayerItem::upLayerItem()
{
- if (mLayerNode)
- mLayerNode->getLayer()->upChildItemNode(mLayerNode);
+ if (!mLayerNode)
+ return;
+
+ MyGUI::ILayerNode* node = mLayerNode;
+ while (node->getParent())
+ {
+ node = node->getParent();
+ }
+ if (node)
+ node->getLayer()->upChildItemNode(node);
}

void LayerItem::attachToLayerItemNode(ILayerNode* _item, bool _deep)

Wasn't sure if you prefer patch or github pull request, so here's Pull request as well.

Use case

Here you see a map window with 5 different "layers", always rendered in specific order:
- Map
- Yellow markers (covered by fog)
- Fog
- Red markers (visible through fog)
- Compass arrow



This is achieved by creating an Overlapped widget for each "layer", then creating the individual components as children of the correct "layer". The patch is necessary to make Overlapped widgets receive mouse input properly.

I tried your patch on Demo_Gui and in both cases input is wrong.
Before patch input have inverted order of picking for overlapped child widgets.
After patch latest created widgets alway have input even if it is behind.

Making Widget::_forcePick move widget to the end of mWidgetChild seems to fix all issues (without your changes applied), but I need to check other places where _forcePick is used.

- upLayerItem will use the root LayerNode rather than the deepest LayerNode.Actually all layer nodes in hierarchy need to be upped, because overlapping widgets should get on top relatively to other childs, but parent widget need to get on top of other widgets as well, like root window.

Notice, that after clicking on "ColourDemo Demo" window two widgets changed their rendering order: ColourDemo itself, but also it's parent widget "Editor Window":

overlapping widgets should get on top relatively to other childs

I see, but can we add an option (on Widget level) to disable this? I do not want widget order to change on mouse clicks in my use case. I want to have it set beforehand and the user should not be able to change it.

WidgetInput::mRaiseOnMouseClick?

I will update patch to restore original behaviour (except fixed _forcePick), but add Widget option to disable it.

I already fixed and committed Overlapping widgets issues.

And what about your specific case, I don't see why you used Overlapped widgets in the first place. You have fixed order of rendering, while overlapped widgets tend to overlap each other and selected one goes on top (like overlapping windows).

You can use same 5 "layers" as widgets, where each next "layer" is child widget to previous widget in reverse order. No overlapping property is used.
So hierarchy would be something like:
Parent window
Compass_arrow_layer_widget
Red_layer_markers_widget
Fog_layer_widget
Yellow_markers_layer_widget
Map_widget
map_item1
map_item2
yellow_marker1
yellow_marker2
Fog_widget
red_marker1
red_marker2
compass_arrowChild widgets would have counter-intuitive order, but this would allow you to freely add and remove widgets.

Oh, and don't forget to make layer widgets themselves not receive mouse input.

this would allow you to freely add and remove widgets

No, that does not work at all. Let's say I add a new widget to Yellow_markers_layer_widget (after I created everything else), then its DrawItem is added to the end of the LayerNode and will be rendered on top, even though it is on a "layer" that should be below. The problem here is that all the widgets are on the same LayerNode. That's why I tried to use Overlapping widgets, because they are created on a separate LayerNode and creating or changing widgets on it will not mess with drawing order.


I tried exactly what you say, it doesn't work.

Code

MyGUI::Widget* compassLayer = createLayer(mLocalMap);
mCustomMarkerLayer = createLayer(compassLayer);
mFogLayer = createLayer(mCustomMarkerLayer);
mMarkerLayer = createLayer(mFogLayer);
mMapLayer = createLayer(mMarkerLayer);

MyGUI::Widget* LocalMapBase::createLayer(MyGUI::Widget* parent)
{
MyGUI::Widget* w = parent->createWidget<MyGUI::Widget>(MyGUI::WidgetStyle::Child, "",
MyGUI::IntCoord(0,0,mLocalMap->getCanvasSize().width,mLocalMap->getCanvasSize().height), MyGUI::Align::Stretch);
w->setNeedMouseFocus(false);
w->setInheritsPick(true);
return w;
}


Result


Fog is gone, because I updated Map texture after creating Fog

Unless you have any other ideas, then I would think adding a RaiseOnMouseClick option and using Overlapped widgets would be the best solution.

my.name implemented "Depth" property (can be accessed as widget property in layout editor) or through setDepth method. It used simple integer value for rendering order. By default all child widgets have depth 0, and widgets with higher values will be rendered below widgets with lower values.

Interesting idea, but I don't think it is the right approach. There are too many usage problems with it.

1. LayerItem reinitializations (e.g. ImageBox::setImageTexture) will still move the RenderItem to the end of the LayerNode, ie always rendering on top. [/*:m]
   
2. If two widgets (with different Depth values) use the same texture, they could potentially share same batch and can not be rendered in proper order.[/*:m]
   
3. Depth property is not inherited, so I need to set it on each child widget. This is only a minor annoyance in some cases, but in others it is very bad (e.g. a Widget creating its own internal widgets, and Depth of them can not be controlled).[/*:m]
   
4. I still can not get a button rendering on top of ScrollView with this property. Do I need to set Depth property of the ScrollView's Client widget? Let's test:[/*:m]
   
5. When trying to use Depth property on a skin widget, a critical "widget not found" exception is thrown. Callstack:
   0 __cxa_throw /usr/lib/x86_64-linux-gnu/libstdc++.so.6 0x7ffff3f6ea30
1 MyGUI::Widget::_unlinkChildWidget MyGUI_Widget.cpp 1056 0x7ffff46c1853
2 MyGUI::Widget::setDepth MyGUI_Widget.cpp 1354 0x7ffff46c2b26
3 MyGUI::Widget::setPropertyOverride MyGUI_Widget.cpp 1164 0x7ffff46c3770
4 MyGUI::Widget::setProperty MyGUI_Widget.cpp 1089 0x7ffff46c50a7
5 MyGUI::Widget::setSkinProperty MyGUI_Widget.cpp 1072 0x7ffff46c58e0
6 MyGUI::Widget::_initialise MyGUI_Widget.cpp 116 0x7ffff46c5a12
7 MyGUI::WidgetManager::createWidget MyGUI_WidgetManager.cpp 110 0x7ffff46ca8a7
8 MyGUI::Widget::baseCreateWidget MyGUI_Widget.cpp 293 0x7ffff46c0b1d
9 MyGUI::Widget::initialiseWidgetSkinBase MyGUI_Widget.cpp 240 0x7ffff46c0dcf
10 MyGUI::Widget::_initialise MyGUI_Widget.cpp 96 0x7ffff46c59cc
11 MyGUI::WidgetManager::createWidget MyGUI_WidgetManager.cpp 110 0x7ffff46ca8a7
12 MyGUI::Widget::baseCreateWidget MyGUI_Widget.cpp 306 0x7ffff46c0c42
13 MyGUI::Widget::baseCreateWidget MyGUI_Widget.cpp 300 0x7ffff46c0bca
14 MyGUI::Widget::createWidgetT MyGUI_Widget.cpp 929 0x7ffff46c1264
15 MyGUI::ResourceLayout::createWidget MyGUI_ResourceLayout.cpp 144 0x7ffff465af24
16 MyGUI::ResourceLayout::createWidget MyGUI_ResourceLayout.cpp 160 0x7ffff465a7d8
17 MyGUI::ResourceLayout::createLayout MyGUI_ResourceLayout.cpp 107 0x7ffff465c779
18 MyGUI::LayoutManager::loadLayout MyGUI_LayoutManager.cpp 72 0x7ffff460e8fe
19 OEngine::GUI::Layout::initialise layout.hpp 56 0xd13bbc
20 OEngine::GUI::Layout::Layout layout.hpp 17 0xd13a3d
... <More>

<MyGUI type="Skin">
<Skin name="DepthTest" size="516 516">
<Property key="Depth" value="1"/>
</Skin>

<Skin name="MW_MapView" size="516 516">
<Child type="Widget" skin="DepthTest" offset="0 0 516 516" align="Stretch" name="Client"/>

<!-- invisible scroll bars, needed for setting the view offset -->
<Child type="MWScrollBar" skin="" offset="0 0 0 0" align="Default" name="VScroll"/>
<Child type="MWScrollBar" skin="" offset="0 0 0 0" align="Default" name="HScroll"/>
</Skin>
</MyGUI>
[/*:m][/list:u]

Fix suggestions:
- When creating a widget, copy the parent's Depth value.
- Create a new LayerNode if child depth doesn't match parent depth.

LayerItem reinitializations (e.g. ImageBox::setImageTexture) will still move the RenderItem to the end of the LayerNode, ie always rendering on top.
I guess this would work if you will force properly reorder widgets. Try calling _updateChilds for parent window. Need to think about proper fix, but want to be sure, that otherwise it works correctly.
If two widgets (with different Depth values) use the same texture, they could potentially share same batch and can not be rendered in proper order.
I see no problem with this: if widgets properly ordered they will get in one batch, but one on top of another. Whole Windows with lots of child widgets and complex hierarchy are correctly rendered in one batch, when only one texture is used.
Depth property is not inherited, so I need to set it on each child widget. This is only a minor annoyance in some cases, but in others it is very bad (e.g. a Widget creating its own internal widgets, and Depth of them can not be controlled).
Currently it control order for widgets with same parent, so you need to remove all "layer" widgets and create those you need to control on the same parent and set depth for each.
Making inherited depths can screw you in many ways. Such system is quiet complex. Current "Depth" is most simple solution, that solves given task.
I still can not get a button rendering on top of ScrollView with this property. Do I need to set Depth property of the ScrollView's Client widget? Let's test:
When trying to use Depth property on a skin widget, a critical "widget not found" exception is thrown.Crash will be fixed, but you should use Depth property only for common widgets, not for skins. Skins have fixed set of widgets whose render order is order if creation and depth is not needed there.

I still need a solution for being able to draw a widget on top of a scroll view. You said that Depth only works for ordering child widgets with the same parent, so I can not use it.

This is what I need:


The map is child of ScrollView's Client widget. The Button is sibling to the ScrollView, because it's not supposed to scroll with it. How can I get it rendering on top? Currently whenever I change map textures they will draw on top of the Button.

Whenever map changes you should call _updateChilds for parent window, this should redraw widgets to their initial order. If this works properly then we can try to catch this moment and properly reorder items when texture changes.

If this works properly then we can try to catch this moment and properly reorder items when texture changes.
Yes, it works. I'm not sure if it's the best idea efficiency wise, since you're essentially redrawing the whole widget hierarchy every time a leaf changes. But having something that works is a good first step I guess.