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

Game input not registering correctly #79

Open
Godnoken opened this issue Feb 24, 2023 · 13 comments
Open

Game input not registering correctly #79

Godnoken opened this issue Feb 24, 2023 · 13 comments

Comments

@Godnoken
Copy link
Contributor

There's an issue with game input that pops up in some games, seemingly primarily DX9 games, although I have had limited possibilities to try lots of games with the other renderers.
Basically, input sometimes registers fine, and other times not.

The "WndProc called before hook was set" log from the imgui_wnd_proc function always show up in the games where this happens.
I'm not sure what the cause of this is yet. I'm going to investigate further and I'd be happy if anyone knows anything that'd help.

@Godnoken
Copy link
Contributor Author

Is this something you noticed when you built the DX9 renderer @FrankvdStam?
I saw that you mentioned double rendering issues here 4241d0b. Is that something you completely fixed, and if not, could it be related to this..?

@FrankvdStam
Copy link
Collaborator

FrankvdStam commented Feb 25, 2023

Double rending was fixed. Only issue I know of that I didn't fix has to do with scaling/mouse positions. What you're having seems more like a race condition, where the WndProc hook is not always installed at the right time. My guess would be to inspect the initialization steps carefully, maybe add some logging to steps to figure out if something is crashing and burning, causing the hook to not be installed sometimes.

@Godnoken
Copy link
Contributor Author

Godnoken commented Feb 26, 2023

Hmm, okay, interesting. I will start there. However, just to be clear - Game input isn't completely dead, it's just that sometimes input doesn't reach the game, and these messages always log in the same games. This happens with the example hooks too, so it's not on my end at the very least, unless it is hardware related.
But as said previously, this is not something unique for the DX9 renderer, I have at least one case of it happening in DX11 with the game Niche. It just seems (as far as I've tested) that it is more prominent with DX9 games, and it feels likely that this is something that could exist in all three of Hudhooks DirectX renderers.

I'm starting to understand how all of this works but I'm still miles from being able to make good educated guesses, so really - any input is super valuable!

@Godnoken
Copy link
Contributor Author

Oh. Here's an interesting one.. Disabling Steam overlay fixes the game input on Shelter2 & Car Mechanic Simulator 2015 (both DX9), while Niche (DX11) still breaks.
It seems that Steam overlay creates its own device in some games and interferes with our DX9 device.. Possibly we could check the return address and only call if it is our device..? Not sure if that makes sense.

@veeenu
Copy link
Owner

veeenu commented Feb 27, 2023

That log message shouldn't really be an error -- we make no assumptions over whether the game should run the wnd proc or the hooked function first, the global mutexes are just there for synchronization and I think this should be expected to some degree. With that being said, this could either be a race condition or some mistake in our wnd proc code that eats up inputs instead of forwarding them to the game.

We should probably gather a collection of easily accessible games and build tests for them, but right now I don't really have the bandwidth to take care of it -- it's been a pretty hectic couple months, hopefully it'll get easier soonish.

@camas
Copy link

camas commented Oct 7, 2023

Had this same issue.

This is basically correct #104 (comment)
wndproc message comes in on a different thread for some reason -> lock fails -> DefWindowProcW is called instead of imgui_wnd_proc_impl (which then calls CallWindowProcW) -> the underlying application and hudhook/imgui never receive the message

Made some hacky changes that work for my use-case https://github.com/camas/hudhook/tree/input-issue-hack
Stores the previous wndproc in a OnceLock kinda like the next comment mentioned and uses that if possible every time. new thread used to handle messages for imgui instead
Downsides are hudhook can't return anything for those messages or stop them from being passed to the hooked app

@veeenu
Copy link
Owner

veeenu commented Oct 8, 2023

That's great news! Thank you for reporting this. Considering it's a synchronization issue, we at least now know where to intervene.

I think the current code is too mutex heavy and we could use lighter weight synchronization which should hopefully solve the issue.

@camas
Copy link

camas commented Jan 30, 2024

After #143 render and wnd_proc are on different threads so can try locking at the same time. wnd_proc quicker so more likely to fail, but when render does it crashes. Doesn't seem to be happening for you though so maybe I'm missing something

Still not sure why it wasn't originally working either. Can't remember if wnd_proc actually was coming from a different thread like I mentioned before. From what I can tell it should always be the main one.

personal todo list: check why the older version didn't work. see if raw input or attachthreadinput could be useful

@veeenu
Copy link
Owner

veeenu commented Jan 31, 2024

GWYF works for me without crashing. I tried to run your project, though, and with it I'm getting crashes as well immediately, both with your branch and main here.

Could you try checking out event viewer to confirm it's an issue with hudhook? Afaik, there is no direct path to crashes without a panic stack trace so it might be a wrong memory access on your end.

immagine

@camas
Copy link

camas commented Jan 31, 2024

Haha, yeah it breaks every time gwyf updates and I've been lazy. To test before I was skipping any initialisation and just showing the imgui demo window.

Reproduced again today with the simple_hook example and the first dll injector google gave me. Takes a while for the render lock to fail. Seems more likely to happen when cpu is maxed. Getting wnd_proc fails before render failed too. Clicks not going through etc.

Put a breakpoint on where try_lock fails internally and you can see the two threads.

breakpoint hit when wnd_proc lock fails

image

other thread paused in render

image

@veeenu
Copy link
Owner

veeenu commented Jan 31, 2024

Mmm, I'm a bit hesitant to use channels and process events out of band because I'm afraid that might introduce weird undebuggable input issues.

It's peculiar that the breakpoint stopped the other thread right in the middle of render... I wonder if we could change the new RENDER_LOCK to a CondVar; that way we can tell if a render is underway and thus is assumed that all the mutexes will be unlocked soon all together and it's worth waiting in the wnd_proc. Then again, I'm not sure if waiting in the wnd_proc is something that should ever be done.

@camas
Copy link

camas commented Feb 7, 2024

Still not sure why it wasn't originally working either

I was being dumb. Same issue. Render and WndProc have always been on two different threads.

Agree that wnd_proc probably shouldn't hang.

Created an example using channels to queue input changes which are then resolved before rendering. Seems to work without crashing or missing input.
camas@e8e22d3
Does wait in wnd_proc for renderer lock when resizing because handling that in the middle of a render seemed wrong.
Also removed on_wnd_proc because I couldn't think of a good way to handle it

@veeenu
Copy link
Owner

veeenu commented Feb 7, 2024

Created an example using channels to queue input changes which are then resolved before rendering. Seems to work without crashing or missing input.

Can you try spamming keyup/keydown very fast with autohotkey a given number of times while in a textbox and see whether the correct number of characters is typed? I think not because if more than one keyup-keydown pair gets consumed at the same time updating imgui's input will show only the last "event" (a misnomer as it only has last frame state, really) as the ones in the middle get overridden.

Does wait in wnd_proc for renderer lock when resizing because handling that in the middle of a render seemed wrong.

I think if the rendering is happening on a separate thread anyway you'd most likely want to properly synchronize the resize and have it happen before the render is started, otherwise there could be frames in flight on the gpu and the command queue would most likely complain.

Also removed on_wnd_proc because I couldn't think of a good way to handle it

I think that should definitely stay, as a simple call at the beginning of the function. If anything, it's the user's responsibility to not hog the proc. if the user doesn't implement it then it'll get inlined as a noop anyway. I don't think it's necessary to introduce a breaking change for that. An acceptable breaking change could be to listen for a return value to choose the next action (return lresult, call DefWindowProcW, call old wnd proc).

This was referenced Feb 24, 2024
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

No branches or pull requests

4 participants