inexorgame / vulkan-renderer

A new 3D game engine for Linux and Windows using C++20 and Vulkan API 1.3, in very early but ongoing development
https://inexor.org
MIT License
790 stars 34 forks source link

ALT+PRINTSCREEN freezes the window #410

Closed westernheld closed 3 years ago

westernheld commented 3 years ago

Describe the bug Pressing the keycombo ALT+PRINTSCREEN freezes the window without a way to make it work again without restarting

To Reproduce Steps to reproduce the behavior: Press ALT+PRINTSCREEN

Desktop (please complete the following information): Windows 10 21H1

westernheld commented 3 years ago

Tried to dig a little to that. For me and Visual Studio it seems like it got stuck on /input/keyboard_mouse_data.cpp on line 17

IAmNotHanni commented 3 years ago

This could be a deadlock! It is possible the std::shared_mutex is used incorrectly.

IceflowRE commented 3 years ago

https://github.com/inexorgame/vulkan-renderer/blob/master/src/vulkan-renderer/input/keyboard_mouse_data.cpp#L80-L82

This is not covered by any lock. By i don't think the reason is our input class.

IAmNotHanni commented 3 years ago

Do other key combinations trigger this bug?

westernheld commented 3 years ago

I tried several, but this was the only one I was able to find causing this issue.

yeetari commented 3 years ago

I think we've confirmed that the issue is caused by something in keyboard_mouse_data.cpp since reverting commit 301a1e9d34617a691c9976841db8af009bd6f9f3 seems to fix the issue.

IAmNotHanni commented 3 years ago

I will look into this.

IAmNotHanni commented 3 years ago

I have this issue as well on Windows!

IAmNotHanni commented 3 years ago

I fixed the problem @IceflowRE found:

std::array<double, 2> KeyboardMouseInputData::calculate_cursor_position_delta() {
    std::scoped_lock lock(m_input_mutex);
    // Calculate the change in cursor position in x- and y-axis.
    const std::array m_cursor_pos_delta{
        static_cast<double>(m_current_cursor_pos[0]) - static_cast<double>(m_previous_cursor_pos[0]),
        static_cast<double>(m_current_cursor_pos[1]) - static_cast<double>(m_previous_cursor_pos[1])};

    m_previous_cursor_pos = m_current_cursor_pos;

    return m_cursor_pos_delta;
}

but it's still freezing after I fixed it.

IAmNotHanni commented 3 years ago

What the... When pressing ALT + Printscreen, Windows tells us the key -1 is being pressed, which is then used as key for the std::unordered_map.

grafik

I will fix this quickly. I will just ignore it if it's negative.

IAmNotHanni commented 3 years ago

This fixed it:

void KeyboardMouseInputData::press_key(const std::int32_t key) {
    if (key < 0 || key > GLFW_KEY_LAST) {
        return;
    }

    std::scoped_lock lock(m_input_mutex);
    m_pressed_keys[key] = true;
    m_keyboard_updated = true;
}

I added this also for other methods. Pull request incoming.