godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.85k stars 20.15k forks source link

Access Violation Exception in wintab_WTOpen(...) #49018

Open ChainedLupine opened 3 years ago

ChainedLupine commented 3 years ago

Godot version: Using tag: 3.3.1-stable, commit 4da4277dc9ff253ec9b23af1355d6f8bcefff5ca

OS/device including version: Windows 10 20H2 (OS Build 19042.985) Huion Tablet driver v14.8.166.1482 Huion Table Model GT-133

Issue description: Occasionally, the Huion driver freaks out and I get a sigsegv deep inside the Huion wintab32.dll replacement. Unfortunately, this outright crashes Godot before it can do anything, even display output on the screen (since it occurs before the hWnd is shown) or write to the Godot console (in debug builds).

sigsegv occurs in platform/windows/os_windows.cpp at line 3705 (at commit 4da4277dc9ff253ec9b23af1355d6f8bcefff5ca) on the wintab_WTOpen() call.

image

Steps to reproduce:

This problem is VERY intermittent and rare, but it does occur once every 20-30 days for me when developing a Godot game.

Now that I know what is happening, I can stop it from occurring by either rebooting or by power-cycling the tablet.

Minimal reproduction project: N/A

The Good, The Bad, And The Ugly

I know this crash isn't Godot's fault. It's a sigsegv. Godot couldn't handle it if wanted to (without complicated signal handling which I dunno would even work on all the platforms that Godot supports). So I don't see a way to actually prevent it from occurring.

Now that brings me to my next point: What if I have a customer/client who has Godot and has this issue? Even knowing that this is a problem is hard because it occurs before logging/window creation, inside of main.cpp::Main::setup(). There's the option --table_driver=<name> and the project setting display/window/tablet_driver. But it doesn't support a "none" or "don't care" as far as I can tell.

What I propose is we add a don't-care option which will skip tablet driver initialization (the call to set_current_tablet_driver() that occurs in main.cpp) so that this can at least be used as a debugging option in the field for weird unexpected crashes in Godot.

I'm willing to write a pull request for this but before I do I want to make sure this is the Godot Way, and if not, how should it be handled?

akien-mga commented 3 years ago

CC @bruvzg

bruvzg commented 3 years ago

Add option to disable it is probably a good idea, and should be easy to do.

4.x:

diff --git a/main/main.cpp b/main/main.cpp
index 2f191b5f63..41efb73219 100644
--- a/main/main.cpp
+++ b/main/main.cpp
@@ -1542,7 +1542,7 @@ Error Main::setup2(Thread::ID p_main_tid_override) {
    {
        GLOBAL_DEF_RST_NOVAL("input_devices/pen_tablet/driver", "");
        GLOBAL_DEF_RST_NOVAL("input_devices/pen_tablet/driver.Windows", "");
-       ProjectSettings::get_singleton()->set_custom_property_info("input_devices/pen_tablet/driver.Windows", PropertyInfo(Variant::STRING, "input_devices/pen_tablet/driver.Windows", PROPERTY_HINT_ENUM, "wintab,winink"));
+       ProjectSettings::get_singleton()->set_custom_property_info("input_devices/pen_tablet/driver.Windows", PropertyInfo(Variant::STRING, "input_devices/pen_tablet/driver.Windows", PROPERTY_HINT_ENUM, "none,wintab,winink"));
    }

    if (tablet_driver == "") { // specified in project.godot
diff --git a/platform/windows/display_server_windows.cpp b/platform/windows/display_server_windows.cpp
index 0dd02b4888..e1d9e5732e 100644
--- a/platform/windows/display_server_windows.cpp
+++ b/platform/windows/display_server_windows.cpp
@@ -3187,6 +3187,8 @@ DisplayServerWindows::DisplayServerWindows(const String &p_rendering_driver, Win
        tablet_drivers.push_back("winink");
    }

+   tablet_drivers.push_back("none");
+
    if (OS::get_singleton()->is_hidpi_allowed()) {
        HMODULE Shcore = LoadLibraryW(L"Shcore.dll");

3.x:

diff --git a/platform/windows/os_windows.cpp b/platform/windows/os_windows.cpp
index 952c83179b..02733c7f1c 100644
--- a/platform/windows/os_windows.cpp
+++ b/platform/windows/os_windows.cpp
@@ -3638,6 +3638,8 @@ OS_Windows::OS_Windows(HINSTANCE _hInstance) {
        tablet_drivers.push_back("winink");
    }

+   tablet_drivers.push_back("none");
+
    hInstance = _hInstance;
    pressrc = 0;
    old_invalid = true;

(not tested).

jitspoe commented 2 years ago

So I just ran into this bug today. Never hit it before after using Godot for years. What's really strange is that it seemed to break the callstack. The crash happens in wintab_WTOpen, but the callstack would jump back to the main.cpp OS::get_singleton()->set_current_tablet_driver(OS::get_singleton()->get_tablet_driver_name(i)); even with the optimizations disabled.

Even though I've only just gotten this crash, I have had a frequent issue of running godot causing pressure sensitivity to stop working in drawing programs, so it might be good to disable the tablet driver by default. To do this, I used @bruvzg's suggestion, but put the tablet_drivers.push_back("none"); ABOVE the other tablet_driver push_back's. This fixed the crash (without having to edit the project.godot file), and I assume will fix the tablet pressure issues in other applications.

Note that the driver crash also crashed other applications. Krita, Gimp, Blender, and Clip Studio Paint all failed to launch. I'm also using the Huion tablet drivers: 12.3.7.1270420. Using with a Monoprice 10x6.25in tablet.

jitspoe commented 1 year ago

Just had this issue happen again, this time in Godot 4. I can't even open the project browser in any version of Godot (3.x or 4.x), but in my custom builds that have disabled the tablet driver, I can.

This is the callstack output from my Godot 4 custom build (downloaded release builds don't give a call stack):

>godot.windows.editor.x86_64.exe

Godot Engine v4.1.dev.custom_build.dddcc2d65 - https://godotengine.org

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.1.dev.custom_build (dddcc2d65d7769391fca201f720fcfa192d42771)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] <couldn't map PC to fn name>
[1] <couldn't map PC to fn name>
[2] DisplayServerWindows::_update_tablet_ctx (E:\Projects\godot\godot_engine_tremble\platform\windows\display_server_windows.cpp:3823)
[3] DisplayServerWindows::tablet_set_current_driver (E:\Projects\godot\godot_engine_tremble\platform\windows\display_server_windows.cpp:4100)
[4] Main::setup2 (E:\Projects\godot\godot_engine_tremble\main\main.cpp:2056)
[5] Main::setup (E:\Projects\godot\godot_engine_tremble\main\main.cpp:1874)
[6] widechar_main (E:\Projects\godot\godot_engine_tremble\platform\windows\godot_windows.cpp:164)
[7] _main (E:\Projects\godot\godot_engine_tremble\platform\windows\godot_windows.cpp:205)
[8] main (E:\Projects\godot\godot_engine_tremble\platform\windows\godot_windows.cpp:217)
[9] __scrt_common_main_seh (d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[10] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================

Ran through the debugger and got the same callstack and confirmed it's happening within wd.wtctx = wintab_WTOpen(wd.hWnd, &wd.wtlc, false);

wintab tends to be pretty flakey, so I would recommend disabling the driver by default. Not only is there this crash issue, but sometimes it breaks pressure sensitivity in drawing applications simply by launching something made in the Godot engine.