[Bug 52508] New: EnumDisplayDevices() fails in obscure ways on 32-bit arch when 64-bit Vulkan is present but 32-bit is missing
https://bugs.winehq.org/show_bug.cgi?id=52508 Bug ID: 52508 Summary: EnumDisplayDevices() fails in obscure ways on 32-bit arch when 64-bit Vulkan is present but 32-bit is missing Product: Wine Version: 7.1 Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: user32 Assignee: wine-bugs(a)winehq.org Reporter: z.figura12(a)gmail.com CC: jacek(a)codeweavers.com, zzhang(a)codeweavers.com Distribution: --- FWIW, this is easy to trigger by running 32-bit ddraw:ddraw1. I'm fine with calling this case "user error", but the problem is, it's not that hard to be missing Vulkan drivers for one architecture, while debugging this took way too long. It'd be nice to at least have some way of printing a message. The basic problem is that when Vulkan is present we retrieve the PCI IDs from it, but when it's absent we set them to zero. As a result we end up creating two different sets of registry entries, but only the 64-bit one gets populated via initialize_display_settings(). EnumDisplayDevices() tries to access it and finds the GPU entries empty. This does lead me to ask: can we avoid querying GPUs in each process? Can we just let explorer.exe manage them and get our information from there? CC'ing Jacek and Zhiyi as both having worked closely with this code. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #1 from Zhiyi Zhang <zzhang(a)codeweavers.com> --- We can't just let explorer.exe manage them since there are console applications that call EnumDisplayDevices() but don't launch explorer.exe, as Alexandre mentioned. I think with the PE work going on, we are moving in the direction of initializing these registries only on the 64-bit side. Jacek knows more about this. And obviously, some error prompt when Vulkan is missing wouldn't hurt. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #2 from Zhiyi Zhang <zzhang(a)codeweavers.com> --- It seems that wine_vk_init() already prints an error when loading Vulkan fails. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #3 from Zeb Figura <z.figura12(a)gmail.com> --- (In reply to Zhiyi Zhang from comment #1)
We can't just let explorer.exe manage them since there are console applications that call EnumDisplayDevices() but don't launch explorer.exe, as Alexandre mentioned.
Do you have a link to this conversation?
I think with the PE work going on, we are moving in the direction of initializing these registries only on the 64-bit side. Jacek knows more about this. And obviously, some error prompt when Vulkan is missing wouldn't hurt.
Are we planning to make the WoW64 path required? That seems questionable to me, but... Could we initialize and query monitor information from some other centralized location? wineserver, perhaps (I know you had patches for this), or just let the first process initialize it while others query? Maybe I've had a particularly rough time of it, but it just seems too easy for different processes to get different results, in one way or another. (In reply to Zhiyi Zhang from comment #2)
It seems that wine_vk_init() already prints an error when loading Vulkan fails.
It prints an error when loading libvulkan fails, but there are other failure modes that don't (libvulkan isn't present at compile time; loading libvulkan succeeds but initializing a Vulkan device fails, etc). I'm not sure that all of these deserve a winediag error in the general case (there are still plenty of environments without a Vulkan driver), but it's unfortunate that it can result in subtle bugs like this... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #4 from Zhiyi Zhang <zzhang(a)codeweavers.com> --- (In reply to Zeb Figura from comment #3)
(In reply to Zhiyi Zhang from comment #1)
We can't just let explorer.exe manage them since there are console applications that call EnumDisplayDevices() but don't launch explorer.exe, as Alexandre mentioned.
Do you have a link to this conversation?
Eh, I can't find it. I vaguely remember it was during an IRC conversation.
I think with the PE work going on, we are moving in the direction of initializing these registries only on the 64-bit side. Jacek knows more about this. And obviously, some error prompt when Vulkan is missing wouldn't hurt.
Are we planning to make the WoW64 path required? That seems questionable to me, but...
Could we initialize and query monitor information from some other centralized location? wineserver, perhaps (I know you had patches for this), or just let the first process initialize it while others query? Maybe I've had a particularly rough time of it, but it just seems too easy for different processes to get different results, in one way or another.
I think monitors are probably initialized in monitor.sys in kernel mode at startup. Since wine doesn't have a kernel mode, wineserver is probably the way to go. My wineserver patches only move part of the monitor information, most of them are and should still be in SetupAPI. Actually, there is a mechanism to let only the first process initialize monitors. That's what the force parameter in X11DRV_DisplayDevices_Init() is for. Maybe recent win32u work changed its behavior or the first process to create windows isn't always 64-bit. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #5 from Zeb Figura <z.figura12(a)gmail.com> --- (In reply to Zhiyi Zhang from comment #4)
Actually, there is a mechanism to let only the first process initialize monitors. That's what the force parameter in X11DRV_DisplayDevices_Init() is for. Maybe recent win32u work changed its behavior or the first process to create windows isn't always 64-bit.
As far as I can tell that function ends up getting called with force == TRUE once for each process that calls GetDesktopWindow() (plus once for explorer.exe itself). The call chain goes: GetDesktopWindow() -> retrieve top_window from server, call user_driver->pCreateDesktopWindow() -> X11DRV_CreateDesktopWindow() -> root_window is not yet initialized; call X11DRV_init_desktop() -> set virtual desktop handler, call X11DRV_DisplayDevices_Init( TRUE ) The code organization here is too unclear for me to tell where the fault lies. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #6 from Zhiyi Zhang <zzhang(a)codeweavers.com> --- (In reply to Zeb Figura from comment #5)
(In reply to Zhiyi Zhang from comment #4)
Actually, there is a mechanism to let only the first process initialize monitors. That's what the force parameter in X11DRV_DisplayDevices_Init() is for. Maybe recent win32u work changed its behavior or the first process to create windows isn't always 64-bit.
As far as I can tell that function ends up getting called with force == TRUE once for each process that calls GetDesktopWindow() (plus once for explorer.exe itself). The call chain goes:
GetDesktopWindow() -> retrieve top_window from server, call user_driver->pCreateDesktopWindow() -> X11DRV_CreateDesktopWindow() -> root_window is not yet initialized; call X11DRV_init_desktop()
I don't think this trace is correct. root_window is initialized in winex11.drv process_attach(). X11DRV_init_desktop should only be called when using virtual desktop. Are you using virtual desktop? I tried running explorer and then notepad and traced X11DRV_DisplayDevices_Init(). No X11DRV_DisplayDevices_Init(TRUE) calls are made for new processes.
-> set virtual desktop handler, call X11DRV_DisplayDevices_Init( TRUE )
The code organization here is too unclear for me to tell where the fault lies.
-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=52508 --- Comment #7 from Zeb Figura <z.figura12(a)gmail.com> --- (In reply to Zhiyi Zhang from comment #6)
(In reply to Zeb Figura from comment #5)
(In reply to Zhiyi Zhang from comment #4)
Actually, there is a mechanism to let only the first process initialize monitors. That's what the force parameter in X11DRV_DisplayDevices_Init() is for. Maybe recent win32u work changed its behavior or the first process to create windows isn't always 64-bit.
As far as I can tell that function ends up getting called with force == TRUE once for each process that calls GetDesktopWindow() (plus once for explorer.exe itself). The call chain goes:
GetDesktopWindow() -> retrieve top_window from server, call user_driver->pCreateDesktopWindow() -> X11DRV_CreateDesktopWindow() -> root_window is not yet initialized; call X11DRV_init_desktop()
I don't think this trace is correct. root_window is initialized in winex11.drv process_attach(). X11DRV_init_desktop should only be called when using virtual desktop. Are you using virtual desktop? I tried running explorer and then notepad and traced X11DRV_DisplayDevices_Init(). No X11DRV_DisplayDevices_Init(TRUE) calls are made for new processes.
I'm sorry, I neglected to mention that I am using the virtual desktop. In that case root_window is indeed initialized, but it's the wrong window, i.e. I guess it points to the X root window instead of the virtual desktop window. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla