Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input code fixes #1559

Closed
wants to merge 2 commits into from
Closed

Input code fixes #1559

wants to merge 2 commits into from

Conversation

dtugend
Copy link
Contributor

@dtugend dtugend commented Oct 13, 2014

I re-submited this as #1900, since I didn't want this to sit on the "master" branch of our repository, which shall reflect the master of ValveSoftware/halflife instead.

Fixes ValveSoftware#1546
- fixed m_rawinput 1 mouse trapped inside a rectangle
- fixed thirdperson camera not working as it should

Fixes ValveSoftware#1558
- recoded mousethread and made it somewhat useful for very fast mouse
  movements (will reset CursorPos now) and thus also fixed some thread
  synchronization issues (hopefully)

Further fixes:
- Fixed IN_JoyMove doing nothing on Linux for ricochet client
- Limited mouse thread sleep to sane values between 0 and 1000
@DanielOaks
Copy link
Contributor

This fixes the mouse getting trapped inside a rectangle issue for me, but for some reason makes my mouse clicks register intermittently. I'll do some more testing and play with the code and see if there's anything weird going on with my environment though

I'm using OSX 10.9, with the following environment info:

Hardware Overview:
  Model Name:   MacBook Pro
  Model Identifier: MacBookPro8,2
  Processor Name:   Intel Core i7
  Processor Speed:  2 GHz
  Number of Processors: 1
  Total Number of Cores:    4
  L2 Cache (per Core):  256 KB
  L3 Cache: 6 MB
  Memory:   16 GB

AMD Radeon HD 6490M:
  Chipset Model:  AMD Radeon HD 6490M
  Type: GPU
  Bus:  PCIe
  PCIe Lane Width:  x8
  VRAM (Total): 256 MB
  Vendor: ATI (0x1002)
  Device ID:  0x6760
  Revision ID:  0x0000
  ROM Revision: 113-C0170H-521
  gMux Version: 1.9.23
  EFI Driver Version: 01.00.521
  Displays:
Color LCD:
  Display Type: LCD
  Resolution: 1440 x 900
  Pixel Depth:  32-Bit Color (ARGB8888)
  Main Display: Yes
  Mirror: Off
  Online: Yes
  Built-In: Yes

But I'll look into this and see if I can get it to work for me.

@dtugend
Copy link
Contributor Author

dtugend commented Dec 15, 2014

Hmm :-(
That's really tricky, I cannot think of any mistake I might have made at the moment.

What do you mean by "intermittently"?
Does it mean the same as delayed randomly? (I assume it means that.)
Or does it mean the clicks re-occur periodically?
(Sorry but my English might not be good enough to understand "intermittently" correctly.)

Can you please check the following:

  1. Does the problem appear in Half-Life itself too (meaning when not using a custom compiled modification)?
  2. Does the problem go away when you turn vsync off (gl_vsync 0 in console)?
  3. Are only mouse button clicks delayed randomly or is the whole mouse input (also movement) delayed?

@dtugend
Copy link
Contributor Author

dtugend commented Dec 15, 2014

Btw I cannot reproduce your problem, neither on Linux nor on Windows :-((
Don't have Mac OS.

@DanielOaks
Copy link
Contributor

Huh, I actually just built the thing again and was able to properly determine what was going on. Sorry, it's not actually a problem with your code.

Basically, I was getting confused because just after spawning with the crowbar, clicking did not actually swing the crowbar until after the first time I stand right next to a surface and make an impact on there.

That same thing happens in HL without your code applied, so it's not an issue. Sorry for the trouble, and thanks for all the awesome work you've been doing on the engine so far!

@dtugend
Copy link
Contributor Author

dtugend commented Dec 15, 2014

Thanks for reporting back.

Btw.: i have to correct myself too a bit: When using primusrun without vblank_mode=0 environment variable set (vblank_mode=0 means vsync forced off), then (mouse) input lag happens, but that is a problem with primusrun afaik and happens in CS:GO (Counter-Strike: Global Offensive) and default Half-Life too.
(Primusrun is used on Linux to use the dedicated graphic chip (NViDIA Geforcet GT 740M in my case) instead of the graphic chip built into the processor.)
In all other cases there is no input lag.

@JoelTroch
Copy link
Contributor

Made a quick test on a clean SDK, nothing wrong to report. Great job !

@dtugend
Copy link
Contributor Author

dtugend commented Jan 3, 2015

Thanks, always good to have someone else test it or look through the code.

I am mostly worried about making mistakes in thread synchronization on Windows builds (with -mousethread launch option and m_rawinput 0), but I hope I didn't introduce new ones and fixed the old ones.

PS:

The reason why I coded InterlockedExchange* variable access despite using a lock mechanism is because the Windows Documentation didn't say anything about how the memory barrier works for the functions WaitForSingleObject and SetEvent (i.e. I can't know from the documentation if it affects all memory access or only interlocked memory access).

The InterlockedExchange functions used in the code make sure there is a memory barrier (which is a lot more than just declaring a variable as volatile does).

I used volatile for the mouse_thread_sleep variable though, because I thought the side effects don't matter there (the user might only observe a very small additional random delay until the changes in that variable are applied to the thread).

PPS:
If anyone would find some good documentation how the memory barrier works in WaitForSingleObject and SetEvent and how to use memory barriers in Visual C++ 2010 properly, that would be cool if I could have it for reading.
(Since I am not sure if the memory-barrier on hardware-level probably caused by WaitForSingleObject and SetEvent and C++ sequence points and volatile variable access alone are guaranteed to already avoid things like Out-Of-Order execution or compiler reordering optimizations - That is why is used InterlockedExchange* Functions additionally to the WaitForSingleObject / SetEvent functions.)

@dtugend
Copy link
Contributor Author

dtugend commented Jan 3, 2015

Okay I found a problem, which is not a problem yet, but will become one as soon as someone uses ISO C++ 11:

In cl_dll/inputw32.cpp line 88

static volatile cvar_t *m_mousethread_sleep;

would work fine in Visual Studio 2010 and lots of earlier versions, however it might not work in ISO C++ 11 anymore, because the struct cannote be copied in one instruction:
http://msdn.microsoft.com/en-us/library/12a04hfd%28v=vs.120%29.aspx

A possible solution would be to instead make the access on the float value volatile, meaning change line 215 to:

int sleepVal = (int)*(volatile float *)&m_mousethread_sleep->value;

But that still would be not standard conform, because the Micrsoft documentation (linked above) says:
The volatile keyword in C++11 ISO Standard code is to be used only for hardware access; do not use it for inter-thread communication. For inter-thread communication, use mechanisms such as std::atomic from the C++ Standard Template Library.

I am very confused now.

Anyone an idea?

@dtugend
Copy link
Contributor Author

dtugend commented Jan 4, 2015

Okay fixed it, see commit above.
Fix is not that elegant, but should be stable.

@YaLTeR
Copy link

YaLTeR commented Dec 24, 2017

Fix for mouse disappearing on tabbing out/back into the game: YaLTeR/OpenAG@8e60ac3

@dtugend dtugend closed this Aug 25, 2018
@dtugend dtugend mentioned this pull request Aug 25, 2018
chinese-soup pushed a commit to chinese-soup/halflife that referenced this pull request Jun 28, 2022
chinese-soup pushed a commit to chinese-soup/halflife that referenced this pull request Jun 28, 2022
YaLTeR pushed a commit to YaLTeR/halflife that referenced this pull request Jun 28, 2022
SmileyAG pushed a commit to SmileyAG/OpenAG that referenced this pull request Dec 14, 2023
SmileyAG pushed a commit to SmileyAG/OpenAG that referenced this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants