This question was created in reference to: [5.5.4 slate global invalidation not updating hittest grid when collapsing parent widget in a retainer [Content removed]
Hi, we’re looking into upgrading to 5.6.1 (from 5.5.4) now that the hotfix has been released, but unfortunately it seems that the referenced slate global invalidation regression has not yet been addressed. Having to disable global invalidation in 5.5 was a pretty substantial setback on our game thread and, given our focus has now shifted towards optimization, we are very much interested in re-enabling it as soon as we can. Has there been any movement on this issue in main/5.7? The manual workaround of removing/readding the offending widgets is not really viable for a team of our size due to time required to identify all the affected areas of our game.
No movement on this issue, yet, it looks like the biggest hurdle is that we don’t have an isolated repro case to test against. I’ve tried a few different configurations to try and replicate something similar, but I haven’t had any luck so far. We still think it’s related to visibility changes on SObjectWidget, but collapsing an SObjectWidget in my test didn’t cause any issues with the hit test grid, so it seems I’m missing something.
If you’re seeing this on any widgets that you’re able to extract into a sample project, I can dig into it a bit and see if we can track down a fix. It may also be interesting to enable Slate.InvalidationRoot.VerifyHittestGrid and see if that triggers to get a callstack of the moment that the grid goes stale.
I had issues with hit test grid previously that you guys fixed when inside retainer widgets and the simplest test was to add a retainer widget to the lyra front end main menu list, might be worth trying it there
I’ve tried a repro within Lyra (especially in the options menu which uses a widget switcher), but I still haven’t had any luck. The retainer issue was quite a bit easier to reproduce, but this looks to be a separate problem causing the hit test grid to go stale. Definitely let us know if you’re able to find somewhere in Lyra that we can reproduce this issue with global invalidation enabled, as that could be a huge help in getting a fix in for this sooner.
That seems like a good lead, and would explain why we’re seeing it so inconsistently. Let me dig into a few things and get back to you, perhaps we can come up with a speculative fix for you to try since you have a consistent repro in your own project.
I think the expectation here is that the visibility would propagate to the newly created SWidget in TakeWidget as a result of the call to SynchronizeProperties. Do the broken widgets share a base class that may have overridden SynchronizeProperties and not called the Super? That should be where we ensure the SWidget and UWidget don’t fall out of sync, so that may be a good spot to investigate.
Ok, after some more investigation, I have another update. To begin with, unfortunately I believe the null SafeWidget was actually a red herring. Somehow everything does seem to by synchronized eventually after the fact (which is desired, obviously). It just occurs later in the frame so I wasn’t seeing it in the debugger when I (incorrectly) expected to, leading to my incorrect assessment of the situation.
That said, I think I may have come closer to tracking down the core of the problem. I managed to confirm that the widgets that were not being removed from the hittest grid are running through SWidget::Prepass_ChildLoop and calling MarkPrepassAsDirty() when their parent widget is collapsed. However, as far as I can tell, the code never does anything with that flag after it is set there? It remains “bNeedsPrepass = true” indefinitely.
I was actually able to “fix” the original issue entirely by adding “Child.Invalidate(EInvalidateWidgetReason::Visibility);” immediately after as such:
{
// If the child widget is collapsed, we need to store the new layout scale it will have when
// it is finally visible and invalidate it's prepass so that it gets that when its visibility
// is finally invalidated.
Child.MarkPrepassAsDirty();
Child.Invalidate(EInvalidateWidgetReason::Visibility); // <--
Child.PrepassLayoutScaleMultiplierValue = Self->bHasRelativeLayoutScale
? InLayoutScaleMultiplier * Self->GetRelativeLayoutScale(ChildIndex, InLayoutScaleMultiplier)
: InLayoutScaleMultiplier;
Child.bPrepassLayoutScaleMultiplierSet = true;
}
Now, I have no idea if this is an appropriate modification given that I don’t know what the consequences are of performing an invalidation here. The comment immediately above seems to imply something else will invalidate the child widget, but it seems reasonable from my (very) limited understanding of what’s going on here that, if the child is dirtied as a result of collapsed visibility, it should always have it’s visibility invalidated? At the very least, this seems to suggest that when global invalidation is enabled, the widgets do not propagate their invalidation to their children in the same way that they do when global invalidation is disabled (potentially by design?) even though they do propagate their “bNeedsPrepass” flag. Perhaps there’s another mechanism that’s supposed to be reading that flag that’s skipping the child widgets incorrectly?
Do you have any thoughts regarding this information?
Can you try enabling “Verify Visibility” in the Runtime Validation section of the widget reflector, then reproducing the issue? If we’re lucky, that will trip an error that may point us in the right direction. If not, you could try enabling the other validation settings there to see if anything else is flagged. I agree that it seems something else should be calling invalidate (so doing it here might be heavy-handed), though I’m curious if that unblocks you from using global invalidation as a last resort.
In the original post linked above the repro scenario had a visible widget appearing in the hit test grid despite it’s ancestor SObjectWidget being collapsed. It sounds like this scenario is a bit different, where the child itself is also collapsed? An updated picture of the hierarchy might be useful, so we can get a better idea of where the invalidate call should be originating from (and why it isn’t hitting the widget who needs to be removed from the grid).
I wanted to give an update here, as we had this pop up internally and were able to make some headway in investigating the issue. We now know the root cause of the bad hittest grid. Any time a widget’s child order is invalidated in the same frame that one of it’s children’s visibility is invalidated, the child never has a chance to remove itself from the hittest grid. We process the child order invalidation in FSlateInvalidationRoot::ProcessPreUpdate and since our invalidation proxies need to be removed/rearranged, we clear those children from the PreUpdate list.
I’ve got an experimental change where we process those visibility changes earlier, though it’ll need quite a bit of testing and may have performance implications. In the meantime, now that we have a more explicit idea of the cause, you might be able to work around the issue so you can reenable invalidation in your project. You’ll still need to fix up every widget where you’re changing visibility on the same frame as things are being added/removed from a parent, but hopefully that gives you a path forward if you think it’s worth the performance gain from global invalidation.
Sounds good, I’ve made a note to try and revisit it as well once I have some cycles. We’re pretty confident that there is something which needs fixing here, but aren’t seeing it as widespread as you are (so the one-off workaround was sufficient). If you see issues throughout the project, perhaps there’s some common thread that we can use to get a minimal repro case. Let me know if you have any luck in the future and we’ll dive back into it, I’d love to unblock global invalidation for you if possible!
Good news, after sitting down and spending a day just chipping away at this issue, I think I see what’s going wrong.
UWidget::SetVisibilityInternal will try to get a reference to the “SafeWidget” via GetCachedWidget() and, if that TSharedPtr is valid, it will update the visibility of the slate widget. Normally that succeeds just fine.
However, there appears to be a race condition here with creating widgets (that are initially visible/hittestable) and immediately (on the same frame) setting their visibility to collapsed while global invalidation is enabled. In this scenario, if the race condition is triggered, GetCachedWidget() does not return a valid SafeWidget in time for the validity check, causing the UWidget to believe it’s collapsed (in the widget reflector and visually in game) but (later) the SWidget is created with the original visibility state, therefore incorrectly resulting in the widget being added to the hittest grid when it’s not supposed to be there.
Further evidence to suggest that it’s a race condition is that running with -stompmalloc results in the function calls taking long enough that the issue is masked (as in, the SafeWidget is always valid by the time the IsValid check occurs). Unfortunately, because of this, I’m unable to reproduce this in a clean project at all. Something with the specific timings of our widget initialization consistently fails to create a valid SWidget in time.
That said, it seems that the solution is to ensure that the SWidget is synchronized with the UWidget’s visibility under all scenarios, even if the visibility has changed before GetCachedWidget() has a valid SWidget to return. How to best accomplish that… I’m not sure.
As far as I can tell, they all seem to call the Super SynchronizeProperties. Perhaps I can step through there and see if something is failing in that function.
Sorry, I think I wasn’t quite exact in my wording of that. The widget that remains on the hit test grid is still set to “visible” but it is contained in a parent widget that is collapsed (see screenshot).[Image Removed]
That said, I did run with Verify Visibility checked, but nothing happened. Ran with all invalidation verification checkboxes checked and hit an ensure “The widget ‘WidgetName_C [OfflineText]’ is in the update list and it still has a Invalidation Reason.” OfflineText is an SRichTextBlock.
Continuing, I then hit:
“The SecondarySort of widget ‘…/Overlay_0/OfflineGroup/ScaleBox_1/OfflineText/RichTextBlockImageDecorator.cpp(161)’ doesn’t match the SecondarySort inside the HittestGrid.”
The SecondarySort of widget ‘…/ScaleBox_1/OfflineText/RichTextBlockImageDecorator.cpp(161)/RichTextBlockImageDecorator.cpp(54)/RichTextBlockImageDecorator.cpp(58)’ doesn’t match the SecondarySort inside the HittestGrid.
The SecondarySort of widget ‘…/OfflineText/RichTextBlockImageDecorator.cpp(161)/RichTextBlockImageDecorator.cpp(54)/RichTextBlockImageDecorator.cpp(58)/RichTextBlockImageDecorator.cpp(63)’ doesn’t match the SecondarySort inside the HittestGrid.
That sounds very suspicious. Perhaps this other widget is somehow “corrupting” the invalidation list?
Glad to hear some more has been uncovered about this issue! That does sound like it could be related to what we’re seeing due to the rich text block we observed having a child widget embedded in it. If there’s a CL you’d like me to try out, I’d be happy to cherry pick it to see if it makes a difference in our case.
Another update just to keep this thread alive, I was able to make a super minimal repro case and confirm that my fix works, but it’ll take some more time to verify it doesn’t cause bigger issues. I’ll get back to you early next week.
Looks like my approach introduced some other issues, so the “real” fix here is going to be a bit trickier. We’ve got a game plan now for how we should be able to fix this without affecting performance too much, but it may be some time before that fix makes it into the engine.
We’ll probably have to shelve this for now since it’s going to be a pretty substantial rework to handle this properly, so we’ll have to stick with the workaround of avoiding visibility and child order invalidations in the same frame for the time being. Hopefully we’ll be able to revisit this before too long so that you can reenable global invalidation.
