Implementation examples link to xinput 1.4 which doesn't work on Windows 7

[codecat]

Version/Branch of Dear ImGui:

Version: 1.72b
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_win32.cpp + imgui_impl_dx11.cpp
Compiler: Visual Studio 2017
Operating System: Windows 10

My Issue/Question:

This issue is in response to #787. As of recently with the added gamepad inputs, Dear ImGui added a pragma link to the source to automatically link to xinput:

#ifdef _MSC_VER
#pragma comment(lib, "xinput")
#endif

This will cause MSVC to link against xinput1_4.dll, which is not available on Windows 7. (It probably will link against 1.3 or the strangely named "9.1.0" if you're building on a Windows 7 machine though.)

Since there are 3 possible versions of xinput, I propose a possible fix where we link with xinput 9.1.0 instead, which is shipped both within the 8.1 SDK and up, and on Windows 7 and up.

#ifdef _MSC_VER
#pragma comment(lib, "Xinput9_1_0")
#endif

It's a bit weird that this is even required, but this will cause MSVC to link against Xinput9_1_0.dll, which seems to be available everywhere.

[ocornut]

Things I can think of:

• If we use #pragma comment(lib, "Xinput9_1_0") how would that behave for applications already linking with xinput.lib ? (which is likely the case in many game inputs).. It seems to be linking properly but I am not sure which one which will be used, and 9.1.0 presumably alter functionalities... that's the main problem. Some testing would be useful (maybe try both in both orders in same file, and try to test at runtime what is happening). Tricky :(

• (easy to fix) Xinput9_1_0 is not available with SDK 7.x which is our minimum test target, but we can easily ifdef based on the WINVER macro.

[codecat]

Windows 7 support will end tomorrow.. maybe this is no longer an issue very soon? :) (Unless you want to support Windows 7 despite Microsoft ending official support, of course!)

[ocornut]

Hello Melissa!

I think we took this from the wrong angle: If you want your app to reliably run on Windows 7 (regardless of its obsolescence) you should use an older Windows SDK. Newer version of Visual Studio allow you to select and change which Windows SDK you are using and you can install old ones.

That said, I am going to push a compile-time option IMGUI_IMPL_WIN32_DISABLE_GAMEPAD right now to disable the XInput code, and add some comments around it.

[codecat]

That's a good solution I suppose! I'm definitely not using any of the gamepad features myself.

[ocornut]

Pushed
IMGUI_IMPL_WIN32_DISABLE_GAMEPAD
IMGUI_IMPL_WIN32_DISABLE_LINKING_XINPUT

[codecat]

Thank you! ❤️ I assume IMGUI_IMPL_WIN32_DISABLE_LINKING_XINPUT is separate so you can manually link with a different version of the lib?

[ocornut]

That's correct! Since it appears that header/api are compatible you could just disable linking if you need.

[CookiePLMonster]

I think we took this from the wrong angle: If you want your app to reliably run on Windows 7 (regardless of its obsolescence) you should use an older Windows SDK. Newer version of Visual Studio allow you to select and change which Windows SDK you are using and you can install old ones.

For what it's worth, I compiled my project with Windows 10 SDK, but I specifically targetted Windows 7 (by defining WINVER=0x0601 and _WIN32_WINNT=0x0601), but the resulting binary still linked against xinput1_4.dll. I did not check the example code yet, but something might still not be handled properly in said example.

[mirh]

Xinput 9.1.0 is the original release and it's truly old and halfassed.
I'm not sure why you can't just use 1.3 instead? I have yet to see anybody caring for these two new features that 1.4 has.

If you don't want to require users to install the dx redist on w10, you can just do something like PCSX2/pcsx2#1237

[CookiePLMonster]

If you don't want to require users to install the dx redist on w10, you can just do something like

Yes, that would be the best way to go around it. It would also allow to gracefully not support gamepad at all in the (very, very unlikely) case where no xinput DLL can be loaded.

In the best case it should try in the following order:

• xinput1_4
• xinput1_3
• xinput9_1_0

[ocornut]

I'd be very happy to take a PR doing that (with minimum amount of cruft and code overhead so it doesn't overwhelm casual readers).

[CookiePLMonster]

I'd be very happy to take a PR doing that (with minimum amount of cruft and code overhead so it doesn't overwhelm casual readers).

I'll have to research this for my own purposes already so I might as well PR it back upstream, sure!

[mirh]

You probably don't want to crash hard if the user hasn't installed the dx redist, and I can't claim to be an expert of xinput, but I don't think you are going to have a good time trying to support the half assed first release of xinput.

It all depends on the features you need. If everything you are doing is just checking a) if the controller is connected and b) the buttons mapping
, then even 9.1.0 is already enough. You don't need 1.3.

You can just use the pcsx2 hack, where you build the thing against the W8/W10 sdk, and then just dynamically load whatever dll is supported on the OS

https://walbourn.github.io/xinput-and-windows-8/
Duh, in fact you should just specify a low _WIN32_WINNT, link against XINPUT9_1_0.LIB and you are done.

[CookiePLMonster]

It all depends on the features you need. If everything you are doing is just checking a) if the controller is connected and b) the buttons mapping , then even 9.1.0 is already enough. You don't need 1.3.

Would Dear Imgui even need more? I've only seen two exports used so it's probably enough.

[mirh]

I made that sentence exactly after having checked wherever xinput was being used to begin with.

[ocornut]

This has been reworked and better solved by #3646
Now loading the XInput DLL dynamically, remove need to link to any of it.
Removed IMGUI_IMPL_WIN32_DISABLE_LINKING_XINPUT.