Isn't mousethread somewhat pointless?

[dtugend]

Let's assume it was working correctly (without the bugs in thread synchronization):

• mousethread uses GetCursorPos to accumulate a delta of the mousemovement
• mouse position is not reset to the center in the mouse thread, only IN_MouseMove or IN_Accumulate using IN_ResetMouse
• mouse thread status is queried in IN_MouseMove, where normal mouse input would be queried without mouse thread too

How could this be different from the delta that we get normally (no mousethread, m_rawinput 0) from GetCursorPos, that is as said above when we assume it wouldn't have those thread synchronization issues?

Yes it can be different in the following situation:

• The mouse hits the cursor clip rect (a border of the window)

In that situation for mousethread the movement bellow the border would be dropped and potential backwards movement would be accumulated. That (in my opinion) is even worse than just the mouse not moving further.

If we assume a) border of the window is not hit (cursor stays within clip rect) between calls to IN_ResetMouse b) there would be no thread synchronization bugs

Then the result with mousethread should be the same as without!

Now you may argue that a) may not hold valid in some situations: However usually fast movements of the user maintain the same direction, meaning the backward movement won't occur, meaning the result will still be the same. Even if we assume backward movement does occur, then isn't the accumulated result in that case even more awkward?

My conclusion: Either I overlooked s.th. or mousethread in it's current form (not resetting mouse) is pointless and bugged.

Thus I'd vote for removing it and telling the user as reason, that it wasn't effective compared to normal input with m_rawinput 0.

Another option of course would be of course to:

• fix the synchronization issues (difficult to get that right)
• reset the mouse in the mouse thread, so there is actually really more room to move for usual user movement compared to normal operation

The (easiest) example for synchronization issues:

• What if the mousethread uses GetCursorPos between L361 and L362
• There are probably other potential issues with the way the interlocked exchange is done, but they are not as easy to spot

[dtugend]

Summary of what I am trying to say:

• Mousethread would only be useful if it was resetting the mouse position (which it does not atm) and preferably if it hadn't thread synchronization issues (which it has atm)
• even if these two things were fixed, it would only be useful in the case of very fast mouse movements using up the window/screen space (meaning mouse movements of more than 300 pixels in 10 ms (assuming 800x600 as resolution and 100 FPS, actually it's more pixels because IN_ResetMouse is called several times per frame due to IN_Accumulate))

I don't have stats about mouse movement with modern high DPI gaming mice, so I can't say if it's worth fixing mousethread for people using very high DPI settings. As I already said it's not easy to get the thread synchronization right :-(

[dtugend]

Also there is a potential problem with resetting the mouse in the mousethread (SetCursorPos):

According to several comments on the internet calling SetCursorPos will lead to a WM_MOUSEMOVE message.

The big question is: Are those messages accumulated or is it generated for each call?

If it's the latter, then there would be a problem: the mousethread could flood the main HL thread with WM_MOUSEMOVE messages.

I will try to investigate the problem on Windows XP when I have time.

[dtugend]

I investigated the Wine 1.7.28 code first:

• See SetCursorPos in wine-1.7.28/​dlls/​user32/​input.c
• See DECL_HANDLER(set_cursor), set_cursor_pos, queue_hardware_message and merge_message in wine-1.7.28/​server/​queue.c

Conclusion: At least in wine-1.7.28 WM_MOUSEMOVE messages sent from SetCursorPos will be merged. Meaning there will be no flooding of the message queue in that Wine version. Also the X driver will only be called when the new cursor pos is different from the old one.

Still got to verify on Windows though :-(

[dtugend]

Turns out I am a stupid human:

I forgot that we can just limit the SetCursorPos calls in the mousethread to only occur when the mouse position changed. That way the flooding would be limited by the mouse update rate and even more by the user's mouse movment, which shouldn't be a problem on nowadays PCs :-)

Still the question remains: Is it worth the effort of re-coding the mousethread to have the properties as said in this comment?

[djdallmann]

@mikela-valve, might be worth reviewing this one, it seems pretty important as it impacts user input and experience. Tagging @dtugend to see if he's still around to comment.

[dtugend]

While I personally prefer fixing things I am not sure if it's worth fixing this after 5 years:
People that had problems with it don't use it and people that use it might have gotten used to the bugs / problems, meaning it could potentially annoy the latter if it's fixed.

I haven't looked into it further than what I already posted in this issue so far.

[dtugend]

I forgot I made a pull request related to that: https://github.com/ValveSoftware/halflife/pull/1900