June 2022 - Wine-Devel - list.winehq.org

Re: [PATCH 2/2] user32: Use registry APIs to enumerate raw input devices.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=116624 Your paranoid android. === debian11 (32 bit report) === user32: menu.c:2324: Test failed: test 25

June 10, 2022

Re: [PATCH 1/2] user32: Store the device path as a simple string pointer.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=116623 Your paranoid android. === debian11 (64 bit WoW report) === user32: msg.c:6881: Test failed: SetFocus(hwnd) on a button: 5: the msg 0x0138 was expected, but got msg 0x030f instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 8: the msg 0x8000 was expected, but got msg 0x0086 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 9: the msg sequence is not complete: expected 0000 - actual 0006

June 10, 2022

Re: [PATCH v2] shell32: Implement SHBindToFolderIDListParent.

by Mohamad Al-Jaf

On Thu, Jun 9, 2022 at 11:57 AM Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: > + TRACE_(shell)("-- psf=%p pidl=%p ret=0x%08lx\n", *ppv, ppidlLast ? *ppidlLast : NULL, hr); The trace says psf=%p but then outputs ppv. What application do you have that calls this function? I submitted a patch for this on May 3 for IE11 with pretty much the same code but didn't add tests. You should also add the prototype in include/shlobj.h. -- Kind regards, Mohamad

June 10, 2022

[PATCH 2/2] user32: Use registry APIs to enumerate raw input devices.

by Zebediah Figura

From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/user32/rawinput.c | 119 +++++++++++++++++++++++++++++------------ 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index 3750332c0cf..ab542956f11 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -30,18 +30,13 @@ #include "winnls.h" #include "winreg.h" #include "winuser.h" -#include "setupapi.h" +#include "ddk/hidclass.h" #include "wine/debug.h" #include "wine/server.h" #include "wine/hid.h" #include "user_private.h" -#include "initguid.h" -#include "ddk/hidclass.h" -#include "ntddmou.h" -#include "ntddkbd.h" - WINE_DEFAULT_DEBUG_CHANNEL(rawinput); struct device @@ -92,40 +87,47 @@ static BOOL array_reserve(void **elements, unsigned int *capacity, unsigned int return TRUE; } -static struct device *add_device( HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface, DWORD type ) +static ULONG query_reg_value( HKEY hkey, const WCHAR *name, + KEY_VALUE_PARTIAL_INFORMATION *info, ULONG size ) { + unsigned int name_size = name ? lstrlenW( name ) * sizeof(WCHAR) : 0; + UNICODE_STRING nameW = { name_size, name_size, (WCHAR *)name }; + + if (NtQueryValueKey( hkey, &nameW, KeyValuePartialInformation, + info, size, &size )) + return 0; + + return size - FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data); +} + +static struct device *add_device( HKEY key, DWORD type ) +{ + static const WCHAR symbolic_linkW[] = {'S','y','m','b','o','l','i','c','L','i','n','k',0}; + char value_buffer[4096]; + KEY_VALUE_PARTIAL_INFORMATION *value = (KEY_VALUE_PARTIAL_INFORMATION *)value_buffer; static const RID_DEVICE_INFO_KEYBOARD keyboard_info = {0, 0, 1, 12, 3, 101}; static const RID_DEVICE_INFO_MOUSE mouse_info = {1, 5, 0, FALSE}; - SP_DEVINFO_DATA device_data = {sizeof(device_data)}; struct hid_preparsed_data *preparsed = NULL; - SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail; HID_COLLECTION_INFORMATION hid_info; struct device *device = NULL; RID_DEVICE_INFO info; IO_STATUS_BLOCK io; WCHAR *path, *pos; NTSTATUS status; + unsigned int i; UINT32 handle; - DWORD i, size; HANDLE file; - SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, &device_data); - if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) + if (!query_reg_value( key, symbolic_linkW, value, sizeof(value_buffer) )) { - ERR("Failed to get device path, error %#lx.\n", GetLastError()); - return FALSE; + ERR( "failed to get symbolic link value\n" ); + return NULL; } - if (!(detail = malloc(size))) - { - ERR("Failed to allocate memory.\n"); - return FALSE; - } - detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); - SetupDiGetDeviceInterfaceDetailW(set, iface, detail, size, NULL, NULL); - path = wcsdup( detail->DevicePath ); - free( detail ); - if (!path) return NULL; + if (!(path = malloc( value->DataLength + sizeof(WCHAR) ))) + return NULL; + memcpy( path, value->Data, value->DataLength ); + path[value->DataLength / sizeof(WCHAR)] = 0; /* upper case everything but the GUID */ for (pos = path; *pos && *pos != '{'; pos++) *pos = towupper(*pos); @@ -234,18 +236,65 @@ fail: return NULL; } -static void enumerate_devices( DWORD type, const GUID *guid ) +static HKEY reg_open_key( HKEY root, const WCHAR *name, ULONG name_len ) { - SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; - HDEVINFO set; - DWORD idx; + UNICODE_STRING nameW = { name_len, name_len, (WCHAR *)name }; + OBJECT_ATTRIBUTES attr; + HANDLE ret; + + attr.Length = sizeof(attr); + attr.RootDirectory = root; + attr.ObjectName = &nameW; + attr.Attributes = 0; + attr.SecurityDescriptor = NULL; + attr.SecurityQualityOfService = NULL; + + if (NtOpenKeyEx( &ret, MAXIMUM_ALLOWED, &attr, 0 )) return 0; + return ret; +} - set = SetupDiGetClassDevsW( guid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT ); +static const WCHAR device_classesW[] = L"\\Registry\\Machine\\System\\CurrentControlSet\\Control\\DeviceClasses\\"; +static const WCHAR guid_devinterface_hidW[] = L"{4d1e55b2-f16f-11cf-88cb-001111000030}"; +static const WCHAR guid_devinterface_keyboardW[] = L"{884b96c3-56ef-11d1-bc8c-00a0c91405dd}"; +static const WCHAR guid_devinterface_mouseW[] = L"{378de44c-56ef-11d1-bc8c-00a0c91405dd}"; - for (idx = 0; SetupDiEnumDeviceInterfaces( set, NULL, &GUID_DEVINTERFACE_HID, idx, &iface); ++idx ) - add_device( set, &iface, type ); +static void enumerate_devices( DWORD type, const WCHAR *class ) +{ + WCHAR buffer[1024]; + KEY_NODE_INFORMATION *subkey_info = (void *)buffer; + HKEY class_key, device_key, iface_key; + unsigned int i, j; + DWORD size; + + wcscpy( buffer, device_classesW ); + wcscat( buffer, class ); + if (!(class_key = reg_open_key( NULL, buffer, wcslen( buffer ) * sizeof(WCHAR) ))) + return; + + for (i = 0; !NtEnumerateKey( class_key, i, KeyNodeInformation, buffer, sizeof(buffer), &size ); ++i) + { + if (!(device_key = reg_open_key( class_key, subkey_info->Name, subkey_info->NameLength ))) + { + ERR( "failed to open %s\n", debugstr_wn(subkey_info->Name, subkey_info->NameLength / sizeof(WCHAR)) ); + continue; + } + + for (j = 0; !NtEnumerateKey( device_key, j, KeyNodeInformation, buffer, sizeof(buffer), &size ); ++j) + { + if (!(iface_key = reg_open_key( device_key, subkey_info->Name, subkey_info->NameLength ))) + { + ERR( "failed to open %s\n", debugstr_wn(subkey_info->Name, subkey_info->NameLength / sizeof(WCHAR)) ); + continue; + } + + add_device( iface_key, type ); + NtClose( iface_key ); + } + + NtClose( device_key ); + } - SetupDiDestroyDeviceInfoList( set ); + NtClose( class_key ); } void rawinput_update_device_list(void) @@ -265,9 +314,9 @@ void rawinput_update_device_list(void) } rawinput_devices_count = 0; - enumerate_devices( RIM_TYPEHID, &GUID_DEVINTERFACE_HID ); - enumerate_devices( RIM_TYPEMOUSE, &GUID_DEVINTERFACE_MOUSE ); - enumerate_devices( RIM_TYPEKEYBOARD, &GUID_DEVINTERFACE_KEYBOARD ); + enumerate_devices( RIM_TYPEHID, guid_devinterface_hidW ); + enumerate_devices( RIM_TYPEMOUSE, guid_devinterface_mouseW ); + enumerate_devices( RIM_TYPEKEYBOARD, guid_devinterface_keyboardW ); LeaveCriticalSection(&rawinput_devices_cs); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/219

June 10, 2022

[PATCH 1/2] user32: Store the device path as a simple string pointer.

by Zebediah Figura

From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/user32/rawinput.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index e7ff854e3b5..3750332c0cf 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -46,7 +46,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(rawinput); struct device { - SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail; + WCHAR *path; HANDLE file; HANDLE handle; RID_DEVICE_INFO info; @@ -103,11 +103,11 @@ static struct device *add_device( HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface, struct device *device = NULL; RID_DEVICE_INFO info; IO_STATUS_BLOCK io; + WCHAR *path, *pos; NTSTATUS status; UINT32 handle; DWORD i, size; HANDLE file; - WCHAR *pos; SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, &device_data); if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) @@ -123,16 +123,19 @@ static struct device *add_device( HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface, } detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); SetupDiGetDeviceInterfaceDetailW(set, iface, detail, size, NULL, NULL); + path = wcsdup( detail->DevicePath ); + free( detail ); + if (!path) return NULL; /* upper case everything but the GUID */ - for (pos = detail->DevicePath; *pos && *pos != '{'; pos++) *pos = towupper(*pos); + for (pos = path; *pos && *pos != '{'; pos++) *pos = towupper(*pos); - file = CreateFileW(detail->DevicePath, GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0); + file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0 ); if (file == INVALID_HANDLE_VALUE) { - ERR("Failed to open device file %s, error %lu.\n", debugstr_w(detail->DevicePath), GetLastError()); - free(detail); + ERR( "Failed to open device file %s, error %lu.\n", debugstr_w(path), GetLastError() ); + free( path ); return NULL; } @@ -199,16 +202,16 @@ static struct device *add_device( HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface, if (device) { - TRACE("Updating device %x / %s.\n", handle, debugstr_w(detail->DevicePath)); + TRACE( "Updating device %#x / %s.\n", handle, debugstr_w(path) ); free(device->data); CloseHandle(device->file); - free(device->detail); + free( device->path ); } else if (array_reserve((void **)&rawinput_devices, &rawinput_devices_max, rawinput_devices_count + 1, sizeof(*rawinput_devices))) { device = &rawinput_devices[rawinput_devices_count++]; - TRACE("Adding device %x / %s.\n", handle, debugstr_w(detail->DevicePath)); + TRACE( "Adding device %#x / %s.\n", handle, debugstr_w(path) ); } else { @@ -216,7 +219,7 @@ static struct device *add_device( HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface, goto fail; } - device->detail = detail; + device->path = path; device->file = file; device->handle = ULongToHandle(handle); device->info = info; @@ -227,7 +230,7 @@ static struct device *add_device( HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface, fail: free( preparsed ); CloseHandle( file ); - free( detail ); + free( path ); return NULL; } @@ -258,7 +261,7 @@ void rawinput_update_device_list(void) { free(rawinput_devices[idx].data); CloseHandle(rawinput_devices[idx].file); - free(rawinput_devices[idx].detail); + free( rawinput_devices[idx].path ); } rawinput_devices_count = 0; @@ -762,8 +765,8 @@ UINT WINAPI GetRawInputDeviceInfoW(HANDLE handle, UINT command, void *data, UINT switch (command) { case RIDI_DEVICENAME: - if ((len = wcslen(device->detail->DevicePath) + 1) <= data_len && data) - memcpy(data, device->detail->DevicePath, len * sizeof(WCHAR)); + if ((len = wcslen( device->path ) + 1) <= data_len && data) + memcpy( data, device->path, len * sizeof(WCHAR) ); *data_size = len; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/219

June 10, 2022

[PATCH 0/2] MR219: user32: Avoid use of setupapi in raw input device enumeration.

by Zebediah Figura (＠zfigura)

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/219

June 10, 2022

Re: [PATCH 0/7] MR212: d3d11: Create dxgi resource objects for textures and buffers.

by Zebediah Figura

This patch series looks sensible to me, given the current structure of the code, but I'm not familiar with all of the considerations here. Henri, do you have any thoughts?

June 9, 2022

Re: HLSL offsetting

by Zebediah Figura

On 6/9/22 17:38, Matteo Bruni wrote: >> My current vision of SMxIR is that it should be a one-to-one >> representation of actual instructions, writable without any lowering >> passes (and hence any passes that are done on it should be optimization >> only, with the *possible* exception of RA.) In a sense, it's what we >> have already with sm4_instruction and such, except that we'd be storing >> it and doing passes on it rather than just writing it directly. > > Right, I agree with the general idea. In practice it might turn out to > be useful to relax the 1:1 requirements a bit and introduce some > "extra" instructions (that would be quickly lowered to real ones) if > that makes the HIR->SMxIR conversion easier. > > Something related we already discussed: SM1IR and SM4IR are going to > be different, obviously, but we want to try and architect them so that > the optimization passes machinery and most of the actual passes can be > shared between the two. Hmm, I don't remember that discussion. My take is that if there's only two backends it probably makes more sense not to (over-)engineer any common framework, but rather just make them look similar enough that we can copy-paste things from one to the other. (It's an unpopular opinion, perhaps, but I think that code duplication is an important and often the best method of code deduplication.) Of course, I don't know what the sm6 backend will look like, when and if we add it.

June 9, 2022

Re: HLSL offsetting

by Matteo Bruni

On Thu, Jun 9, 2022 at 9:17 PM Zebediah Figura <zfigura(a)codeweavers.com> wrote: > > On 6/9/22 04:04, Matteo Bruni wrote: > >> The ugliness that we've run into is: how do we emit IR for the following > >> variable load? > >> > >> struct apple > >> { > >> int a; > >> struct > >> { > >> Texture2D b; > >> int c; > >> } s; > >> } a; > >> > >> /* in some expression */ > >> func(a.s); > >> > >> Unlike the SM1 example above, the register numbers don't match up. > >> Separately, it's kind of ugly that backend-specific details regarding > >> register size and alignment are leaking into the frontend so much. > > > > I think most of that can be hidden or contained with some proper > > abstraction. And generous handwaving. > > But basically, that probably could be represented in the IR as copying > > around individual fields of the structure separately, rather than a > > single "struct deref". Clearly it can become more complex depending on > > the type of the variable but I think it should be doable. > > Yeah, it could. Like I said it's not prohibitive. I'm just not sure it's > the best option at this point. > > It's worth pointing out that, at parse time, we want and need for load > instructions (and therefore probably also store instructions) to have > larger-than-vector types—that is, load instructions can produce structs, > and store instructions can consume them. But we don't want that for > SMxIR, and I believe we don't want that for the "final form" of HLSL IR > either. That's the way the code is currently arranged and I see no > reason not to keep it that way. Right, and agreed, but that's currently taken care of by split_struct_copies() and I don't think that's a (large) problem for the register offset approach. > >> Similarly, the amount of code that has to deal with matrix majority is > >> unfortunate. > > > > That personally seems more annoying. Although it's not clear to me > > that handling matrix majority at a later stage is necessarily any > > better. > > The main idea is that we could handle it something closer to once (well, > once per backend), at HLSL -> SMx translation. > > That doesn't necessarily mean requiring that all matrix loads and stores > are done on a single scalar—after all, we could translate a single > vector load to multiple MOV instructions if it can't actually be > represented by one. > > It does potentially mean doing vectorization passes on SMxIR, though. > Hard to tell this far in advance, and it's also hard to tell if that's > something we're going to need anyway. Yeah, that's the sort of possible snag that I was thinking about WRT moving the matrix majority handling "down". > >> The former problem can potentially be solved by embedding multiple > >> register offsets into hlsl_deref (one per register type). Neither this > >> nor the latter problem are prohibitive, and I was at one point in favour > >> of continuing to use register offsets everywhere, but at this point my > >> feeling has changed, and I think using register offsets is looking more > >> ugly than the alternatives. I get the impression that Francisco > >> disagrees, though, which is why we should probably hash this out now. > > > > As I mention below, I currently see two options as the most appealing. > > This one (multiple register offsets) sits somewhat in the middle and > > it feels like it would be best to go to one of the extremes instead. > > It's also possible that this middle ground solution would end up being > > nicer in practice. At any rate, I certainly wouldn't flat out discount > > it. > > > >> Nor do I think we should use both register offsets and component offsets > >> (either in the same node type, or in different node types). That just > >> makes the IR way more complicated. Rather, I think we should be doing > >> everything in *just* component offsets until translation from HLSL IR to > >> SMx IR. > > > > I touched on this earlier and I agree that the additional complexity > > is unlikely to be worth it. Admittedly we're in a limbo right now > > where SMxIR isn't quite there yet, which makes reasoning on some of > > these details a bit fuzzy. > > > >> In order to deal with the problem of translating dynamic offsets from > >> components to registers, I see three options: > >> > >> (a) emit code at runtime, or do some sophisticated lowering, > >> > >> (b) use special offsetof and sizeof nodes, > >> > >> (c) introduce a structured deref type, much like [1]. Francisco was > >> actually proposing something like this, although with an array instead > >> of a recursive structure, which strikes me as an improvement. > >> > >> My guess is that (a) is very hard. I haven't really tried to reason it > >> out, though. > >> > >> Given a choice between (b) and (c), I'm more inclined to pick (c). It > >> makes the IR structure more restrictive, and those restrictions > >> fundamentally match the structured nature of the language we're working > >> with, both things I tend to like. > > > > After giving it some thought I think that's certainly fine *for the > > higher level IR*. At the same time it seems to me that, if we go that > > route, eventually we also want to have real SMxIR with register > > offsets, and make sure that we can optimize constant offsets (thus > > expressions) at that level. > > > > As I see it (as of current time and date, can't guarantee that I won't > > change my mind again...) we either push the backend-specific info up > > (register offsets all the way) or down (component offsets with > > structured deref / type info in the generic IR, transformation into > > register offsets in the SMxIR). I think either option works and it's > > mostly a matter of preference and which one fits / feels better with > > the rest of the compiler. > > Yeah, that general approach makes sense to me. And yes, of course the > SMxIR should deal entirely in register offsets. > > My current vision of SMxIR is that it should be a one-to-one > representation of actual instructions, writable without any lowering > passes (and hence any passes that are done on it should be optimization > only, with the *possible* exception of RA.) In a sense, it's what we > have already with sm4_instruction and such, except that we'd be storing > it and doing passes on it rather than just writing it directly. Right, I agree with the general idea. In practice it might turn out to be useful to relax the 1:1 requirements a bit and introduce some "extra" instructions (that would be quickly lowered to real ones) if that makes the HIR->SMxIR conversion easier. Something related we already discussed: SM1IR and SM4IR are going to be different, obviously, but we want to try and architect them so that the optimization passes machinery and most of the actual passes can be shared between the two. > Between those two extremes—well, what we currently have basically *is* > the first extreme, with register offsets pushed all the way up to parse > time. It's just causing some friction that makes me think the latter > extreme is probably going to be pretty. It is, but it sort of happened by accident. My guess is that an explicit effort to "clean up" the compiler around register offsets from the top might lessen some of that friction. It's fair to say that, at this point, we probably have more visibility into the latter, which arguably makes that option more compelling. > >> Note that either way we're going to need specialized functions to > >> resolve deref offsets in one step. I also think that should depend on > >> the domain—e.g. for copy-prop we'll actually want to do everything in > >> component counts, but when translating to SMxIR we'll evaluate given the > >> register alignment constraints of the shader model. In the case of (b) > >> it's not going to be as simple as running the existing constant folding > >> pass, because we can't actually fold the sizeof/offsetof constants > >> (unless we dup the node list, evaluate, and then fold, which seems very > >> hairy and more work than the alternative). > > > > Right, each option will have different tradeoffs WRT optimization > > passes. But e.g. copy-prop should be doable even with register > > offsets, we "just" need to make sure to always map the component > > offsets to their respective register offsets. > > Quite, in fact we're already doing it that way. But it's probably better > to work with components, since we (a) don't waste space tracking padding > [not very important], and (b) don't have to deal with multiple register > sets [more important]. Yeah. I guess my point would have been better served by pointing out passes that become easier or more powerful with register offsets, like probably vectorization. > >> I invite thoughts—especially from Matteo, since we discussed this sort > >> of problem ages ago. > > > > Yep, hope that my comments make sense. I want to hear from the others too. > > > >> > >> ἔρρωσθε, > >> Zeb > >> > >> > >> [1] https://www.winehq.org/pipermail/wine-devel/2020-April/164399.html > >> > >> [2] https://www.winehq.org/pipermail/wine-devel/2020-April/165493.html > >>

June 9, 2022

Re: [PATCH v3 5/5] winegstreamer: Support zero-copy in wg_transform_push_data.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=116614 Your paranoid android. === debian11 (32 bit German report) === mf: Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x7d6d62f9). === debian11 (32 bit Hindi:India report) === mf: Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x7d4082f9).

June 9, 2022