Prevent event loop pause while a caption button is pressed on Windows (eventloop-2.0)

[aloucks]

• Tested on all platforms changed

Fixes an issue where the event loop would freeze while a user presses (but does not release) the minimize, maximize, restore, or close buttons on Windows.

[Osspial]

This fixes the problem you've outlined, but it introduces another problem: this solution changes the behavior of the minimize/maximize/close buttons in a manner that's visible to the user. On eventloop-2.0, try clicking on a button, moving onto a different button, and releasing the button. You'll notice that the event is cancelled. If you attempt to that on this branch, it'll just perform the action assigned to the newly moused-over button, which is inconsistent with how the rest of Windows works.

Instead of trying to re-implement how exactly the windows shell handles this (since there's a bunch of nuance and it may be prone to change anyway), it would be better to enable modal loop mode on the event loop runner, which allows the event loop to continue running even in an internal Windows API loop. Look at the in_modal_loop variable for how we handle that with window resizing, which also triggers a modal loop. You should be able to re-use that mechanism for this purpose.

[aloucks]

it would be better to enable modal loop mode on the event loop runner, which allows the event loop to continue running even in an internal Windows API loop

That's what I tried first but was unable to get it to work. I was able to get a message into the modal mode loop but the re-queuing behavior wasn't working. I'll take another look tomorrow or later in the week.

[Osspial]

Hmm.

I did some testing, and it seems like this is a pretty common bug to run into - my text editor (Sublime Text 3) does it, MS Office programs do it, and even Windows Explorer does it.

[felixrabe]

Could be the same on macOS btw – Sublime Text 3 stops blinking cursor while pressing and holding one of the top-left color circles.

[aloucks]

@Osspial The latest commit addresses and fixes the issue of the user cancelling the operation.

I was able to get a message into the modal mode loop but the re-queuing behavior wasn't working.

I was mistaken here. That was while trying to deal with the right button click that opens the menu box (WM_ENTERMENULOOP). I have a solution for that too, but it uses WM_TIMER and I'm not a big fan of it.

I haven't been able to get the modal loop to pick up any events while pressing a caption button if the default subclass proc is used. Re-implementing the behavior has been the only way I've been able to prevent the freezing.

[Osspial]

Still, this fix impacts a user's interaction with the window frame; see this video for an example of how exactly it does that. Point is, there are a bunch of subtleties to the interaction, there may be additional subtleties that users currently rely on, and Microsoft may introduce more subtleties down the line. I'm fine with Winit having weird hacks that work (see the entire WM_PAINT modal event loop structure), but I'm not comfortable with weird hacks that affect the established design language of how users interact with their applications, which this does. If there isn't any way to fix this that meets that requirement, then this is a ubiquitous and harmless enough bug that I'd rather just leave it as it is.

[aloucks]

The merge conflicts have been resolved but currently failing due to rustfmt -- waiting on the outcome of the rustfmt conversation in #913 before formatting again.

this is a ubiquitous and harmless enough bug that I'd rather just leave it as it is.

I agree with your sentiment in regards to impacting usability, but I disagree with the assessment of the bug.

Leaving the bug as-is allows for the end-user to willfully pause the event loop, which could lead to bugs or exploits in user code. Imagine a game loop where the user can pause all game logic by pressing and holding the minimize button.

The most obvious solution is to encourage the user to move rendering and other logic into a secondary thread. This is the stance that GLFW has taken and I think it's a fine design decision.

However, a significant amount of effort (by you none-the-less!) was put into winit to allow for rendering on the main thread without blocking during window interaction (e.g. resizing). It doesn't make much sense to me to introduce all the complexity to enable continued event processing during resize, while allowing the caption button interaction to block the main thread.

I'm not terribly fond of re-implementing the caption button behavior either but it does work. The only other option that I had any success with was using WM_TIMER. I had something working initially but I hadn't yet worked out rescheduling of the timer messages so they couldn't overlap. I can re-visit that approach if you're open to it.

[Osspial]

@aloucks I'd be interested in looking at the timer approach, yeah. If that works without changing how the window buttons behave, I'd be down with merging it.

[aloucks]

I took a look at this again today and sadly I was mistaken about the timer approach. It does not work for the caption buttons. The experiment I had going was for dealing with WM_ENTERMENULOOP.

From what I can tell, there are zero messages processed inside DefSubclassProc while the caption buttons are pressed and held down.

[Osspial]

@aloucks I did some investigation, and it looks like you're mostly correct.

However, "mostly" is an important qualifier there! Windows still needs to process mouse input events*, and we should be able to use that detail along with a few other APIs to jump-start the event loop during caption button processing without re-implementing the caption button behavior ourselves.

See, Windows exposes a function called SetWindowsHookEx, which lets you install functions that get executed at certain hook points in Windows' various event handlers. Conveniently, one of those hook points is WH_GETMESSAGE, which intercepts all PostMessage and SendMessage calls - including the ones used in the default WM_NCLBUTTONDOWN handler. We should be able to use PeekMessage (filtered to ignore all mouse events, so we don't interrupt the native Windows processing) to pull other events out of the message queue, and with those messages we can restart the windows event loop.

How exactly we go about driving the event loop depends on what ControlFlow is set to. If it's Poll, the solution is fairly simple: when WM_MOUSEMOVE events get received, use PostMessageW to post filler WM_MOUSEMOVE messages to the event queue. If it's Wait**, the solution is even simpler: we just do nothing. WaitUntil is the trickiest, and the only solution I can think of is to spawn a secondary thread whose only job is to wait until the right moment and send filler WM_MOUSEMOVE events to the frozen window. EDIT: the initial solution in this post for Wait and WaitUntil is flawed. I'm going to do more investigation to see if there's a better one.

Once caption button event processing is complete, we can use UnhookWindowsHookEx to get rid of the event-loop-driver hook, so that this hack only runs when it needs to.

We can also use that solution to fix similar freezes caused by right-click-dragging the top bar, which I discovered while researching this solution.

^{* specifically, all events between WM_MOUSEFIRST and WM_MOUSELAST, if ReactOS' reverse engineering can be believed.
** or Exit. The current behavior elsewhere in the code is, if we can't kill the event loop immediately, just wait until we can do so and quit at the earliest possible time.}

[goddessfreya]

Closing for inactivity.