logic/joystick: fix multiple bugs

- the enumerator and the handle list are now static; joysticks were
erroneously destructed during refresh
- setup dinput only once; creating LPDIRECTINPUT8 many times causes
Release() to invalidate all handles
- don't create joystick handles many times; same thing applies
- refresh joy list on failed acquire; usually joystick is unplugged at
that moment leading to endless stderr spam
- use a static mutex as to avoid dinput races

Issue: #315

3 files changed, 180 insertions, 118 deletions

diff --git a/opentrack-logic/keybinding-worker.cpp b/opentrack-logic/keybinding-worker.cpp
index a0c7178c..eba026a5 100644
--- a/opentrack-logic/keybinding-worker.cpp
+++ b/opentrack-logic/keybinding-worker.cpp
@@ -55,7 +55,7 @@ KeybindingWorker::KeybindingWorker() :
         din = 0;
         return;
     }
-    
+
     if (dinkeyboard->SetCooperativeLevel((HWND) fake_main_window.winId(), DISCL_NONEXCLUSIVE | DISCL_BACKGROUND) != DI_OK) {
         dinkeyboard->Release();
         din->Release();
@@ -165,7 +165,7 @@ KeybindingWorker::fun* KeybindingWorker::_add_receiver(fun& receiver)
     QMutexLocker l(&mtx);
     receivers.push_back(std::unique_ptr<fun>(new fun(receiver)));
     fun* f = receivers[receivers.size() - 1].get();
-    qDebug() << "add receiver" << (long) f;
+    //qDebug() << "add receiver" << (long) f;
     joy_ctx.refresh();
     return f;
 }
@@ -175,12 +175,15 @@ void KeybindingWorker::remove_receiver(KeybindingWorker::fun* pos)
     QMutexLocker l(&mtx);
     bool ok = false;
 
-    for (int i = receivers.size() - 1; i >= 0; i--)
+    using s = int;
+
+    for (int i = s(receivers.size()) - 1; i >= 0; i--)
     {
-        if (receivers[i].get() == pos)
+        using u = unsigned;
+        if (receivers[u(i)].get() == pos)
         {
             ok = true;
-            qDebug() << "remove receiver" << (long) pos;
+            //qDebug() << "remove receiver" << (long) pos;
             receivers.erase(receivers.begin() + i);
             break;
         }
diff --git a/opentrack-logic/win32-joystick.cpp b/opentrack-logic/win32-joystick.cpp
index bf919f4a..02b8c6d1 100644
--- a/opentrack-logic/win32-joystick.cpp
+++ b/opentrack-logic/win32-joystick.cpp
@@ -1,14 +1,21 @@
+#ifdef _WIN32
+
 #undef NDEBUG
 #include <cassert>
+#include <algorithm>
 #include "win32-joystick.hpp"
+#include "opentrack-compat/sleep.hpp"
 
-#ifdef _WIN32
+QMutex win32_joy_ctx::enum_state::mtx;
+win32_joy_ctx::enum_state win32_joy_ctx::enumerator;
 
 void win32_joy_ctx::poll(fn f)
 {
     //refresh(false);
 
-    QMutexLocker l(&mtx);
+    QMutexLocker l(&enumerator.mtx);
+
+    auto& joys = enumerator.get_joys();
 
     for (auto& j : joys)
     {
@@ -16,100 +23,115 @@ void win32_joy_ctx::poll(fn f)
     }
 }
 
-bool win32_joy_ctx::poll_axis(const QString &guid, int axes[])
+bool win32_joy_ctx::poll_axis(const QString &guid, int* axes)
 {
-    //refresh(false);
+    QMutexLocker l(&enumerator.mtx);
 
-    QMutexLocker l(&mtx);
-
-    auto iter = joys.find(guid);
+    for (int k = 0; k < 10; k++)
+    {
+        if (k > 0)
+            enumerator.refresh();
 
-    if (iter == joys.end())
-        return false;
+        const joys_t& joys = enumerator.get_joys();
+        auto iter = joys.find(guid);
 
-    auto& j = iter->second;
+        if (iter == joys.end())
+            return false;
 
-    auto& joy_handle = j->joy_handle;
-    bool ok = false;
-    HRESULT hr;
+        auto& j = iter->second;
 
-    (void) joy_handle->Acquire();
-
-    if (!FAILED(hr = joy_handle->Poll()))
-        ok = true;
+        auto& joy_handle = j->joy_handle;
+        bool ok = false;
+        HRESULT hr;
 
-    if (!ok)
-    {
-        qDebug() << "joy acquire failed" << guid << hr;
-        (void) joy_handle->Unacquire();
-        return false;
-    }
+        if (!FAILED(hr = joy_handle->Poll()))
+        {
+            ok = true;
+        }
 
-    DIJOYSTATE2 js;
-    memset(&js, 0, sizeof(js));
+        if (!ok && FAILED(hr = joy_handle->Acquire()))
+        {
+            //qDebug() << "joy acquire failed" << hr;
+        }
 
-    if (FAILED(hr = joy_handle->GetDeviceState(sizeof(js), &js)))
-    {
-        qDebug() << "joy get state failed" << guid << hr;
-        return false;
-    }
+        if (!ok)
+        {
+            portable::sleep(25);
+            (void) joy_handle->Unacquire();
+            continue;
+        }
 
-    const int values[] = {
-        js.lX,
-        js.lY,
-        js.lZ,
-        js.lRx,
-        js.lRy,
-        js.lRz,
-        js.rglSlider[0],
-        js.rglSlider[1]
-    };
+        DIJOYSTATE2 js;
+        memset(&js, 0, sizeof(js));
 
-    for (int i = 0; i < 8; i++)
-        axes[i] = values[i];
+        if (FAILED(hr = joy_handle->GetDeviceState(sizeof(js), &js)))
+        {
+            qDebug() << "joy get state failed" << guid << hr;
+            continue;
+        }
 
-    return true;
-}
+        const int values[] =
+        {
+            js.lX,
+            js.lY,
+            js.lZ,
+            js.lRx,
+            js.lRy,
+            js.lRz,
+            js.rglSlider[0],
+            js.rglSlider[1]
+        };
+
+        for (int i = 0; i < 8; i++)
+            axes[i] = values[i];
+
+        return true;
+    }
 
-win32_joy_ctx::~win32_joy_ctx()
-{
-    release();
+    return false;
 }
 
 std::vector<win32_joy_ctx::joy_info> win32_joy_ctx::get_joy_info()
 {
-    QMutexLocker l(&mtx);
-
     std::vector<joy_info> ret;
+    QMutexLocker l(&enumerator.mtx);
+    auto& joys = enumerator.get_joys();
+    ret.reserve(joys.size());
 
     for (auto& j : joys)
         ret.push_back(joy_info { j.second->name, j.first });
 
+    std::sort(ret.begin(), ret.end(), [&](const joy_info& fst, const joy_info& snd) -> bool { return fst.name < snd.name; });
+
     return ret;
 }
 
-win32_joy_ctx::win32_joy_ctx()
+win32_joy_ctx::di_t& win32_joy_ctx::make_di()
 {
-    if (DirectInput8Create(GetModuleHandle(NULL), DIRECTINPUT_VERSION, IID_IDirectInput8, (void**)&di, NULL) != DI_OK) {
-        qDebug() << "setup DirectInput8 Creation failed!" << GetLastError();
-        assert(!"direct input handle can't be created");
+    static LPDIRECTINPUT8 di_ = nullptr;
+    if (di_ == nullptr)
+    {
+        if (SUCCEEDED(DirectInput8Create(GetModuleHandle(NULL), DIRECTINPUT_VERSION, IID_IDirectInput8, (void**)&di_, NULL)))
+        {
+            return di_;
+        }
+        else
+        {
+            return di_ = nullptr;
+        }
     }
-    refresh();
+    return di_;
 }
 
-void win32_joy_ctx::release()
+win32_joy_ctx::win32_joy_ctx()
 {
-    joys = std::unordered_map<QString, std::shared_ptr<joy>>();
-    di->Release();
-    di = nullptr;
+    refresh();
 }
 
 void win32_joy_ctx::refresh()
 {
-    QMutexLocker l(&mtx);
-
-    qDebug() << "joy list refresh";
-    enum_state st(joys, fake_main_window, di);
+    QMutexLocker l(&enumerator.mtx);
+    enumerator.refresh();
 }
 
 QString win32_joy_ctx::guid_to_string(const GUID guid)
@@ -177,13 +199,36 @@ bool win32_joy_ctx::joy::poll(fn f)
     return true;
 }
 
-win32_joy_ctx::enum_state::enum_state(std::unordered_map<QString, std::shared_ptr<joy>> &joys_,
-                                      QMainWindow &fake_main_window,
-                                      LPDIRECTINPUT8 di) :
-    fake_main_window(fake_main_window),
-    di(di)
+win32_joy_ctx::enum_state::enum_state()
 {
-    joys = joys_;
+}
+
+win32_joy_ctx::enum_state::~enum_state()
+{
+    QMutexLocker l(&mtx);
+
+    di_t& di = make_di();
+    if (!di)
+    {
+        qDebug() << "can't create dinput";
+        return;
+    }
+
+    joys = std::unordered_map<QString, std::shared_ptr<joy>>();
+    di->Release();
+    di = nullptr;
+}
+
+void win32_joy_ctx::enum_state::refresh()
+{
+    all.clear();
+
+    di_t di = make_di();
+    if (!di)
+    {
+        qDebug() << "can't create dinput";
+        return;
+    }
 
     HRESULT hr;
 
@@ -198,31 +243,41 @@ win32_joy_ctx::enum_state::enum_state(std::unordered_map<QString, std::shared_pt
 
     for (auto it = joys.begin(); it != joys.end(); )
     {
-        if (std::find_if(all.cbegin(), all.cend(), [&](const QString& guid2) -> bool { return it->second->guid == guid2; }) == all.cend())
+        if (std::find_if(all.cbegin(), all.cend(), [&](const QString& guid2) -> bool { return it->second->guid == guid2; }) == all.end())
+        {
             it = joys.erase(it);
+        }
         else
-            it++;
+        {
+            ++it;
+        }
     }
-
-    joys_ = joys;
 }
 
-win32_joy_ctx::enum_state::EnumJoysticksCallback(const DIDEVICEINSTANCE *pdidInstance, void *pContext)
+BOOL CALLBACK win32_joy_ctx::enum_state::EnumJoysticksCallback(const DIDEVICEINSTANCE *pdidInstance, void *pContext)
 {
+    di_t di = make_di();
+    if (!di)
+    {
+        qDebug() << "can't create dinput";
+        return DIENUM_STOP;
+    }
+
     enum_state& state = *reinterpret_cast<enum_state*>(pContext);
     const QString guid = guid_to_string(pdidInstance->guidInstance);
     const QString name = QString(pdidInstance->tszInstanceName);
 
-    auto it = state.joys.find(guid);
-    const bool exists = it != state.joys.end();
+    const bool exists = state.joys.find(guid) != state.joys.end();
 
     state.all.push_back(guid);
 
-    if (!exists)
+    if (exists)
+        goto end;
+
     {
         HRESULT hr;
         LPDIRECTINPUTDEVICE8 h;
-        if (FAILED(hr = state.di->CreateDevice(pdidInstance->guidInstance, &h, nullptr)))
+        if (FAILED(hr = di->CreateDevice(pdidInstance->guidInstance, &h, nullptr)))
         {
             qDebug() << "createdevice" << guid << hr;
             goto end;
@@ -234,7 +289,10 @@ win32_joy_ctx::enum_state::EnumJoysticksCallback(const DIDEVICEINSTANCE *pdidIns
             goto end;
         }
 
-        if (FAILED(h->SetCooperativeLevel((HWND) state.fake_main_window.winId(), DISCL_NONEXCLUSIVE | DISCL_BACKGROUND)))
+        // not a static member - need main() to run for some time first
+        static const QWidget fake_window;
+
+        if (FAILED(h->SetCooperativeLevel(reinterpret_cast<HWND>(fake_window.winId()), DISCL_NONEXCLUSIVE | DISCL_BACKGROUND)))
         {
             qDebug() << "coop";
             h->Release();
@@ -247,14 +305,13 @@ win32_joy_ctx::enum_state::EnumJoysticksCallback(const DIDEVICEINSTANCE *pdidIns
             goto end;
         }
 
-        qDebug() << "add joy" << guid;
         state.joys[guid] = std::make_shared<joy>(h, guid, name);
     }
 end:
     return DIENUM_CONTINUE;
 }
 
-win32_joy_ctx::enum_state::EnumObjectsCallback(const DIDEVICEOBJECTINSTANCE *pdidoi, void *ctx)
+BOOL CALLBACK win32_joy_ctx::enum_state::EnumObjectsCallback(const DIDEVICEOBJECTINSTANCE *pdidoi, void *ctx)
 {
     if (pdidoi->dwType & DIDFT_AXIS)
     {
@@ -282,7 +339,7 @@ win32_joy_ctx::enum_state::EnumObjectsCallback(const DIDEVICEOBJECTINSTANCE *pdi
 win32_joy_ctx::joy::joy(LPDIRECTINPUTDEVICE8 handle, const QString &guid, const QString &name)
     : joy_handle(handle), guid(guid), name(name)
 {
-    qDebug() << "got joy" << guid;
+    qDebug() << "make joy" << guid << name << joy_handle;
     for (int i = 0; i < 128; i++)
         pressed[i] = false;
     memset(&js_old, 0, sizeof(js_old));
diff --git a/opentrack-logic/win32-joystick.hpp b/opentrack-logic/win32-joystick.hpp
index d9b62e45..ac592a54 100644
--- a/opentrack-logic/win32-joystick.hpp
+++ b/opentrack-logic/win32-joystick.hpp
@@ -20,7 +20,7 @@
 #include <QDebug>
 #include <QMutex>
 #include <QMutexLocker>
-#include <QMainWindow>
+#include <QWidget>
 
 namespace std {
 template<>
@@ -35,33 +35,9 @@ struct hash<QString>
 
 struct OPENTRACK_LOGIC_EXPORT win32_joy_ctx
 {
+    using di_t = LPDIRECTINPUT8;
     using fn = std::function<void(const QString& guid, int btn, bool held)>;
 
-    enum { joy_axis_size = 65535 };
-
-    struct joy_info
-    {
-        QString name, guid;
-    };
-
-    void poll(fn f);
-    bool poll_axis(const QString& guid, int axes[8]);
-    std::vector<joy_info> get_joy_info();
-
-    win32_joy_ctx(const win32_joy_ctx&) = delete;
-    win32_joy_ctx& operator=(const win32_joy_ctx&) = delete;
-
-    win32_joy_ctx();
-    ~win32_joy_ctx();
-    void refresh();
-private:
-    QMutex mtx;
-    QMainWindow fake_main_window;
-    LPDIRECTINPUT8 di;
-
-    static QString guid_to_string(const GUID guid);
-    void release();
-
     struct joy
     {
         LPDIRECTINPUTDEVICE8 joy_handle;
@@ -77,20 +53,46 @@ private:
         bool poll(fn f);
     };
 
-    std::unordered_map<QString, std::shared_ptr<joy>> joys;
+    using joys_t = std::unordered_map<QString, std::shared_ptr<joy>>;
+
+    static constexpr int joy_axis_size = 65535;
 
-    class enum_state
+    struct joy_info
     {
-        std::unordered_map<QString, std::shared_ptr<joy>> joys;
-        QMainWindow& fake_main_window;
-        LPDIRECTINPUT8 di;
+        QString name, guid;
+    };
+
+    void poll(fn f);
+    bool poll_axis(const QString& guid, int* axes);
+    std::vector<joy_info> get_joy_info();
 
+    win32_joy_ctx(const win32_joy_ctx&) = delete;
+    win32_joy_ctx& operator=(const win32_joy_ctx&) = delete;
+
+    win32_joy_ctx();
+    void refresh();
+    static di_t& make_di();
+private:
+    static QString guid_to_string(const GUID guid);
+
+    class enum_state final
+    {
         std::vector<QString> all;
+        joys_t joys;
+
         static BOOL CALLBACK EnumJoysticksCallback(const DIDEVICEINSTANCE* pdidInstance, VOID* pContext);
         static BOOL CALLBACK EnumObjectsCallback(const DIDEVICEOBJECTINSTANCE* pdidoi, VOID* ctx);
+
     public:
-        enum_state(std::unordered_map<QString, std::shared_ptr<joy>>& joys, QMainWindow& fake_main_window, LPDIRECTINPUT8 di);
+        static QMutex mtx;
+
+        enum_state();
+        ~enum_state();
+        void refresh();
+        const joys_t& get_joys() const { return joys; }
     };
+
+    static enum_state enumerator;
 };
 
 #endif