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

WGInput: Work around crash with Steam overlay. #11038

Conversation

AdmiralCurtiss
Copy link
Contributor

@shuffle2 This fixes the crash. As far as I can tell this is a bug with the Steam overlay but I have no idea how to verify that or how to report this.

The code is just the existing TryGetBatteryReport() function from the winrt headers rolled out and with a null check in the right place, which is, for reference:

#define WINRT_IMPL_SHIM(...) (*(abi_t<__VA_ARGS__>**)&static_cast<__VA_ARGS__ const&>(static_cast<D const&>(*this)))

    template <typename D> WINRT_IMPL_AUTO(winrt::Windows::Devices::Power::BatteryReport) consume_Windows_Gaming_Input_IGameControllerBatteryInfo<D>::TryGetBatteryReport() const
    {
        void* value{};
        check_hresult(WINRT_IMPL_SHIM(winrt::Windows::Gaming::Input::IGameControllerBatteryInfo)->TryGetBatteryReport(&value));
        return winrt::Windows::Devices::Power::BatteryReport{ value, take_ownership_from_abi };
    }

Note: There's an extra deadlock that happens after this when the Steam overlay is running, but this fixes the crash at least.

@shuffle2
Copy link
Contributor

shuffle2 commented Sep 7, 2022

Thanks for looking into it!

Does something like this work:

  bool UpdateBatteryLevel()
  {
    try
    {
      // Workaround for Steam. If Steam's GameOverlayRenderer64.dll is loaded, battery_info is null.
      auto battery_info = m_raw_controller.try_as<WGI::IGameControllerBatteryInfo>();
      if (!battery_info)
        return false;
      const winrt::Windows::Devices::Power::BatteryReport report =
          battery_info.TryGetBatteryReport();
      if (!report)
        return false;

might just be cleaner looking.

@AdmiralCurtiss
Copy link
Contributor Author

Yes, that does work.

@shuffle2
Copy link
Contributor

shuffle2 commented Sep 7, 2022

Note: There's an extra deadlock that happens after this when the Steam overlay is running, but this fixes the crash at least.

ah... :( That I do wonder if is SDL-related (there were SDL joystick locking changes recently, and it apparently had to do with steam).

To clarify: the recent sdl update in dolphin was to fix sdl device add/remove notifications, which was fixed by SDL commits changing locking behavior in SDL. Apparently the locking in SDL had been like that because Steam was deadlocking. So, maybe by fixing add/remove notifications, they regressed the steam fix.

related sdl stuff is here:
libsdl-org/SDL@92d3fc4
libsdl-org/SDL#6063

also, since it doesn't occur for me on non-xinput controllers, I guess it is related to whatever steam is doing with xinput, specifically.

@shuffle2
Copy link
Contributor

shuffle2 commented Sep 8, 2022

this pr looks good to merge to me fwiw

@AdmiralCurtiss AdmiralCurtiss merged commit 7102103 into dolphin-emu:master Sep 8, 2022
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the steam-overlay-crash-fix-wgi branch September 8, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants