CommonUI Input Routing - Widget Visibility causes unintended routing

CommonUIRouterBase tries to find the active root during its Tick so that it can find the root object displayed on top of others and mark it as the `ActiveRoot` so that any input can be routed to the widgets within that Root. However, to figure out which `Root` is the topmost, it relies on `LayerId` variable which is the layer that root widget was drawn at during its `last` render frame. When the root becomes invisible, this `LayerId` becomes frozen and if enough widgets are removed/deactivated/became invisible underneath the real root, its `LayerId` can become less than the `LayerId` of a widget root that was hidden, resulting wrong root being marked as the active root and receiving input unintentionally. While checking whether the root widget is active or not, visibility of it should also play a role and most likely eliminate the root from receiving input, or at least invisible roots should contibute to the `LayerId` calculation and have their layer ids updated.

CommonUIVisibility.zip(82.1 KB)

Steps to Reproduce

  • Open the attached CommonUIVisibility project in the editor and load BugMap level and start a PIE session.
  • When you send a `Back` input to the UI (Shift + Esc by default), you will see the input being passed to the MenuHUD widget as expected as it is the topmost active widget
  • Toggle off `InWorldHUD` and `GameHUD` stacks so that the only `Visible` stack is the `MenuHUD`.
  • When you send a `Back` input to the UI again, you will notice `GameHUD` receiving the event this time, instead of the topmost active widget `MenuHUD`

Hi,

I haven’t been able to repro this with your test project (I see MenuHUD receive the input in both cases) but based on your description, updating FActivatableTreeNode::GetLastPaintLayer should fix this. Could you give the following a try and let us know how it goes?

int32 FActivatableTreeNode::GetLastPaintLayer() const
{
	if (TSharedPtr<SWidget> CachedSlate = RepresentedWidget.IsValid() ? RepresentedWidget->GetCachedWidget() : nullptr)
	{
		if (!CachedSlate->GetVisibility().IsVisible())
		{
			return INDEX_NONE;
		}
		return CachedSlate->GetPersistentState().LayerId;
	}
	return INDEX_NONE;
}

Best,

Cody

Hi,

Good callout, we should check visibility up the hierarchy. That should be a simple enough change:

int32 FActivatableTreeNode::GetLastPaintLayer() const
{
	if (TSharedPtr<SWidget> CachedSlate = RepresentedWidget.IsValid() ? RepresentedWidget->GetCachedWidget() : nullptr)
	{
 
		// See if the widget is actually visible
		TSharedPtr<SWidget> WidgetToCheck = CachedSlate;
		while (WidgetToCheck)
		{
			if (!WidgetToCheck->GetVisibility().IsVisible())
			{
				return INDEX_NONE;
			}
			WidgetToCheck = WidgetToCheck->GetParentWidget();
		}
		return CachedSlate->GetPersistentState().LayerId;
	}
	return INDEX_NONE;
}

Your setup should stress test this a bit better than Lyra’s relatively flat structure, does that change work for you? I can see about getting that checked in, though it will likely not make it into the 5.8 release.

Best,

Cody

Hey Cody,

It is interesting you were not able to replicate the issue, I’ve created the attached project in Vanilla 5.7.4 and tested with it before sharing it here. If you want to see it in action I can share a video of it with you.

The change you’ve shared above is the exact fix I’ve implemented in our codebase and I can confirm that it is working. However, even though I think it would be expected, this would have a “side effect” of active but hidden topmost ui root to reject inputs. In my personal view it would be a safe assumption to accept, I just want to bring it up in case you hear from some other licensee that their activatable widget that is kept hidden is no longer receiving input :smiley:

Ah, I’ve got the repro now. GameHUD needs to be toggled off before InWorldHUD, which makes sense since it will have the higher LayerID and needs to be frozen at that state. I think the change of behavior with invisible widgets is expected but we’ll keep an eye out in case it causes problems elsewhere; I’ve checked the fix in at CL# 53635848 if you want to grab it.

Thanks Cody, I will be changing our codebase to match exactly. Have a great day.

Hey Cody,

I was working on hiding all game layers and noticed another issue with this implementation, the visibility we are checking against is specific to the visibility of the widget itself, so if the widget itself is not hidden but its parent is, we are still routing the input to the wrong widget through this flow. So, we need to calculate the visibility in hierarchy to decide whether to ignore the widget or not in this case. With that, I want to give you a small explanation of how UI is layered [not sharing all layers] so that you can have an idea where the problem is arising from and it might be useful for you to guide us if we should make any changes or while you’re making changes in the future. The structure was borrowed a bit from ParrotGame sample and built further upon it;

  • GameStack (Overlay)
    • GameScreen (CommonActivatableWidgetStack)
    • GameOverlay (CommonActivatableWidgetStack)
    • GameMenu (CommonActivatableWidgetStack)
  • ApplicationStack (Overlay)
    • AppMenu (CommonActivatableWidgetStack)
    • AppModal (CommonActivatableWidgetStack)

Within this structure, in this example I was just trying to update the visibility of GameStack to make all `Game` UI invisible, but this caused the input to be routed to some widget within the GameStack instead of the ApplicationStack because of the bad layer id values.