Fix issue with ImGui popup/modal windows not being able to be closed …

[yash3ahuja]

…in transparent mouse input mode. The issue was caused by relying on ImGui::IsWindowHovered to determine whether an ImGui window was being hovered over, which doesn't work as expected with popups and modals. The ImGui documentation instead recommends using ImGuiIO::WantCaptureMouse to determine mouse input forwarding, so now SImGuiWidget relies on this value to update it's transparent mouse input state.

[yash3ahuja]

Hello,

Thanks for the awesome plugin! I integrated it directly into the engine recently and noticed there was a bug with popup windows when using transparent mouse input. I pulled vanilla UE4.24 and tested that the same behavior happens on the base plugin. Based upon the ImGui FAQ and docs, IsWindowHovered shouldn't be used for checking whether the mouse input needs to be consumed by ImGui.

I also have some other changes to remove the reliance on monolithic headers (UBT complains when it's brought in as an Engine module as opposed to a plugin). If you'd like a PR for that as well, please let me know.

Thanks.

[segross]

Thanks, I'm glad you like it :)

Sorry for so late response but I'm in the middle of changing continents and couldn't really focus on this project lately. I still need to bring over my PC or by a worthy laptop as right now I only have a Chromebook ;) I will try to look at this in my new office once I have a chance, but first I need to set-up here, so please bear with me.

Your change looks good, though, and I will try merging it as soon as I can.

When it comes to monolithic headers, I already have a change on my old PC switching to IWYU and fixing the usage of monolithic headers. Unfortunately, I didn't manage to push it before moving. Once I get access to that repository, I will recheck it and push it.

Best regards and thanks for your update.

[segross]

Thank you for the great fix. I completely missed that issue.

Sorry that it took so long but I had a bit on my plate recently (many people probably do). Anyway, it is already merged.

If I may, I would ask one thing. Is there any strong reason to keep bWantsMouseCapture = IO.WantCaptureMouse; inside the BeginFrame() function? I would be tempted to either move it to the Tick() function, where I update other IO parameters or move those to the BeginFrame(). It is a minor thing, but essentially they feel like parts of the same group and for better visibility, I would tend to update them in the same scope.

[yash3ahuja]

No worries on the delay -- I hope you had a safe move, especially with all that's going on in the world right now.

It's been a while, but IIRC the reason was that the ImGui docs recommend checking the value after calling ImGui::NewFrame as it's an output value from ImGui (refer to the second note on this page).

I imagine moving it into Tick before BeginFrame (with the other IO parameters) would at most introduce a 1-frame latency on updating whether to use transparent mouse, which doesn't seem like a big deal. I tested moving it locally, and things seemed to work as expected. Alternatively, perhaps moving it into Tick but after BeginFrame (with a comment on why) would be best?

[segross]

Anyhow, exactly as you suggested I was thinking to move it after the BeginFrame() call to maintain the right order. The flow would be as follow: copy some stuff that needs to be copied before starting the new frame, begin the new frame, copy some stuff that needs to be copied after. It is a minor thing, so I will update whenever I decide to make a next clean-up. I might even remove bIsMouseHoveringAnyWindow as I think it might be not used anymore.

Cheers and thanks.