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

ciface/win32 and WGInput bug fixes #11009

Merged
merged 3 commits into from Sep 1, 2022
Merged

Conversation

shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Aug 23, 2022

this is mostly just modernization / cleanup. fixes https://bugs.dolphin-emu.org/issues/13009

adds a comment about the memory leak described in https://bugs.dolphin-emu.org/issues/13006 . I haven't added the hack-fix; it should be fixed in future windows release and tbh I don't think it's a big enough deal to have such a workaround in the meantime.

I have fleshed out the c++/winrt changes to add proper error handling and take care of the TODOs.

libsdl-org/SDL#6145 should also fix the issue of SDL seeing joysticks twice.

@shuffle2 shuffle2 changed the title ciface/win32: use CM_Register_Notification instead of wnd msgs ciface/win32 and WGInput bug fixes Aug 27, 2022
@lioncash lioncash merged commit 50550cf into dolphin-emu:master Sep 1, 2022
11 checks passed
@NicholasNRG
Copy link

This specific update broke Steam shortcuts to Dolphin games for me. Had to go back to 5.0-17260 for them to work again. Just a heads up

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 2, 2022

You'll need to explain the problem better.

@NicholasNRG
Copy link

NicholasNRG commented Sep 2, 2022

A lot of people use Steam to add shortcuts to all their roms which they can browse and open with just a game controller, which is a huge quality of life for playing on TV without getting up to use the keyboard/mouse.

Usually this involves adding the emulator .exe as a shortcut and adding the rom path to the target as a command line argument, along with other commands like --exec, --batch etc. to hide the main Dolphin window and just show the game.

After this update these shortcuts stopped working and Dolphin just opens and closes without loading the game or the Dolphin UI. It doesn't even load any specific game controller profiles before closing. It just shows a white screen and then closes. In one instance it froze on the white screen and I had to end task.

I tried dragging the rom onto the Dolphin.exe and that worked, so I'm not sure what's going on, I just know that going back to the version preceding this one fixed it. I hope this is enough information and it can be fixed, these shortcuts are extremely convenient for those who play on TV with controller!

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 2, 2022

Sorry, that doesn't really help diagnose the issue. I'm aware how people use steam with dolphin, but I don't have it set up and need some sort of technical description of the problem.

What version of windows are you running? (look in "system > about" in settings app)

Have you tried all 3 of these builds individually?
https://dolphin-emu.org/download/dev/50550cf978f3712e333953362e5bb78d1d686d34/
https://dolphin-emu.org/download/dev/31efd16e16dd60b580cd9f470113277e00b69729/
https://dolphin-emu.org/download/dev/11281b5cef410976606482fb1591eaf1c88bb2af/
Which does the problem occur with?

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 2, 2022

It would be good to test this commit by itself as well: https://dl.dolphin-emu.org/prs/7d/5f/pr-11024-dolphin-latest-x64.7z

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

fwiw i just tried launching current master build of dolphin via steam and it works fine here, so I can't reproduce.

I've mentioned the other builds because I'd more likely suspect it's an SDL issue. But again, doesn't repro here.

@NicholasNRG
Copy link

Sorry I haven't gotten the chance to respond

I am running Windows 10.0.19044 / Windows 10 Education Version 21H2, OS build 19044.1949 with Feature Experience Pack 120.2212.4170.0

To reproduce the problem:

  1. On Steam click "Add a Game" and select the Dolphin executable (with a build newer than 5.0-17260)
  2. Right-click Dolphin on the Steam Library and click Properties
  3. Scroll down to "Launch Options" and paste a file path to a game that opens with Dolphin, making sure to put it between quotes (eg: "C:\Dolphin Games\Dolphin_Game.fileextension")
  4. Connect a game controller to the PC via Bluetooth, and make sure Steam detects the controller (you can check by going to Steam>Settings>Controller>General Controller Settings) In my case I'm using a PS4 and PS5 controller.
  5. Try launching Dolphin via Steam.

Expected behavior: Dolphin opens and begins launching the game
Instead: Dolphin does not open at all, or the Dolphin window opens with a white screen and then closes itself

One thing I noticed from the 4 builds you sent is that they launched properly until I realized I didn't have a controller connected. Once I connected a controller it once again stopped launching correctly unless I was using 5.0-17260 or lower. So I also suspect it might be an SDL issue. It is worth mentioning that Steam has its own input wrapper so that any controller is treated as an Xinput controller when launching a game via Steam. That could possibly be conflicting with SDL. There have also been cases where I open Dolphin games via Steam and the game won't respond to my inputs, so I have to close and reopen the game one or more times until it does respond. For the record I have my Dolphin configured so that all controllers are Xinput.

Please let me know if you need any additional info

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

Does it only happen with Bluetooth or also usb? I was only trying usb

@AdmiralCurtiss
Copy link
Contributor

I can reproduce this with an Xbox One controller that's plugged in over USB.

It's a nullpointer deref. Stack:

>	DolphinD.exe!winrt::impl::consume_Windows_Gaming_Input_IGameControllerBatteryInfo<winrt::Windows::Gaming::Input::RawGameController>::TryGetBatteryReport() Line 194	C++
 	DolphinD.exe!ciface::WGInput::Device::UpdateBatteryLevel() Line 547	C++
 	DolphinD.exe!ciface::WGInput::Device::Device(std::string name, winrt::Windows::Gaming::Input::RawGameController raw_controller, winrt::Windows::Gaming::Input::Gamepad gamepad) Line 183	C++
 	DolphinD.exe!std::_Construct_in_place<ciface::WGInput::Device,std::string,winrt::Windows::Gaming::Input::RawGameController const &,winrt::Windows::Gaming::Input::Gamepad &>(ciface::WGInput::Device & _Obj, std::string && <_Args_0>, const winrt::Windows::Gaming::Input::RawGameController & <_Args_1>, winrt::Windows::Gaming::Input::Gamepad & <_Args_2>) Line 151	C++
 	DolphinD.exe!std::_Ref_count_obj2<ciface::WGInput::Device>::_Ref_count_obj2<ciface::WGInput::Device><std::string,winrt::Windows::Gaming::Input::RawGameController const &,winrt::Windows::Gaming::Input::Gamepad &>(std::string && <_Args_0>, const winrt::Windows::Gaming::Input::RawGameController & <_Args_1>, winrt::Windows::Gaming::Input::Gamepad & <_Args_2>) Line 2018	C++
 	DolphinD.exe!std::make_shared<ciface::WGInput::Device,std::string,winrt::Windows::Gaming::Input::RawGameController const &,winrt::Windows::Gaming::Input::Gamepad &>(std::string && <_Args_0>, const winrt::Windows::Gaming::Input::RawGameController & <_Args_1>, winrt::Windows::Gaming::Input::Gamepad & <_Args_2>) Line 2730	C++
 	DolphinD.exe!ciface::WGInput::AddDevice(const winrt::Windows::Gaming::Input::RawGameController & raw_game_controller) Line 636	C++
 	DolphinD.exe!ciface::WGInput::PopulateDevices() Line 728	C++
 	DolphinD.exe!ciface::Win32::PopulateDevices(void * hwnd) Line 80	C++
 	DolphinD.exe!ControllerInterface::RefreshDevices(ControllerInterface::RefreshReason reason) Line 185	C++
 	DolphinD.exe!ControllerInterface::Initialize(const WindowSystemInfo & wsi) Line 89	C++
 	DolphinD.exe!UICommon::InitControllers(const WindowSystemInfo & wsi) Line 144	C++
 	DolphinD.exe!MainWindow::InitControllers() Line 326	C++
 	DolphinD.exe!MainWindow::MainWindow(std::unique_ptr<BootParameters,std::default_delete<BootParameters>> boot_parameters, const std::string & movie_path) Line 219	C++
 	DolphinD.exe!app_main(int argc, char * * argv) Line 244	C++
 	DolphinD.exe!wWinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, wchar_t * __formal, int __formal) Line 306	C++
 	DolphinD.exe!invoke_main() Line 123	C++
 	DolphinD.exe!__scrt_common_main_seh() Line 288	C++
 	DolphinD.exe!__scrt_common_main() Line 331	C++
 	DolphinD.exe!wWinMainCRTStartup(void * __formal) Line 17	C++
 	kernel32.dll!00007ffbe5547034()	Unknown
 	ntdll.dll!00007ffbe5682651()	Unknown

Since this only happens via Steam I'm guessing Steam is doing something wrong when injecting itself into the process, but shrug. I can get you a crash dump if you give me a Dolphin executable you have symbols for.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

It might be easiest to update sdl to head and see if it still happens before looking into it further. I’m not sure I’ll be able to reproduce if it requires native xinput controller- only have ps4/ps5 controller handy.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

yea i've tried all the controllers i have and i can't get it to crash. i made a pr that updates sdl here you could try: #11036 (they have some recent commits mentioning xinput, no clue if it's related or not)

@AdmiralCurtiss
Copy link
Contributor

I don't think this is SDL related given the callstack touches our WGInput code but yeah I'll try.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

Otherwise (if you can confirm what @NicholasNRG said - that it started with SDL update in 31efd16 ), I'd say report a bug to SDL or Steam.

@AdmiralCurtiss
Copy link
Contributor

They actually said:

Once I connected a controller it once again stopped launching correctly unless I was using 5.0-17260 or lower.

Which means 17260 or lower works, 17264 or higher does not -- which would be this PR, not SDL.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

nvm. hrm, still annoying as it's probably sdl or steam bug. 🤷

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

I wonder if you've actually gotten the updated OS binaries for Windows.Gaming.Input and their patch for the memleak actually introduced a bug (lol)

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

It seems like something that should be reported to Steam? Is there even a way to do that? Or is there a bug in my code? :)

@NicholasNRG
Copy link

I’m not sure I’ll be able to reproduce if it requires native xinput controller- only have ps4/ps5 controller handy.

Keep in mind that it's happening to me with a PS4 and PS5 controller but I have Dolphin configured to use Xinput controllers (since I use all sorts of controllers and Steam changes them to Xinput for me). Maybe it doesn't happen if Dolphin isn't configured with an Xinput controller. I didn't get to test that.

Are your Dolphin controller settings configured for Dinput? Also, do you happen to be using DS4Windows? I'm not using DS4Windows on my end so I can't say whether it happens with that either.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

When launching via steam (or not), the ps4/ps5 controllers do not appear as xinput devices for me.

@AdmiralCurtiss
Copy link
Contributor

You have to enable that. Settings -> Controller -> General Controller Settings -> PlayStation Configuration Support.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Sep 7, 2022

Even without enabling that, GameOverlayRenderer64.dll was being loaded, which is all I thought was required.
After enabling it, starting dolphin via steam, then selecting WGInput/0/Xbox 360 Controller for Windows (which is my ps5 controller), then restarting dolphin, it finally crashes here (actually, it hangs in my case, but whatever).

@shuffle2 shuffle2 deleted the device-notify branch October 21, 2022 22:44
@NicholasNRG
Copy link

Just wanted to report that I'm still having this problem as of 5.0-17813. I think I have no choice but to stick to 5.0-17260 for now. The quality of life benefits Steam Input provides is just too much to give up.

@AdmiralCurtiss
Copy link
Contributor

...Or you could give us a stacktrace and/or crash dump so we can figure out what causes the crash?

@NicholasNRG
Copy link

...Or you could give us a stacktrace and/or crash dump so we can figure out what causes the crash?

How can I do that? Willing to help in any way I can to solve this problem

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Nov 6, 2022

There's a couple ways, I think the easiest is to make a registry key?

https://learn.microsoft.com/en-us/windows/win32/wer/collecting-user-mode-dumps

eg:
crash

Make sure that folder you configured exists, then just make a program crash and it should create a dump in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants