February 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 3/5] winebus.sys: Process device reports for linux event devices

by Sebastian Lackner

On 27.02.2017 16:09, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_udev.c | 163 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 162 insertions(+), 1 deletion(-) > > > > 0003-winebus.sys-Process-device-reports-for-linux-event-dev.txt > > > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index e2a475ccd8..2f198c7d4f 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -242,6 +242,9 @@ struct wine_input_absinfo { > struct wine_input_private { > struct platform_private base; > > + int buffer_length; > + BYTE *report_buffer; > + > int report_descriptor_size; > BYTE *report_descriptor; > > @@ -340,6 +343,87 @@ static const BYTE* what_am_I(struct udev_device *dev) > return Unknown; > } > > +static BOOL set_button_value(struct wine_input_private *ext, int code, int value) > +{ > + int index = ext->button_map[code]; > + int bindex = index / 8; > + int b = index % 8; > + BYTE mask; > + > + mask = 1<<b; > + if (value) > + ext->report_buffer[bindex] = ext->report_buffer[bindex] | mask; > + else > + { > + mask = ~mask; > + ext->report_buffer[bindex] = ext->report_buffer[bindex] & mask; > + } > + return TRUE; > +} > + > +static BOOL set_abs_axis_value(struct wine_input_private *ext, int code, int value) > +{ > + int index; > + /* check for hatswitches */ > + if (code <= ABS_HAT3Y && code >= ABS_HAT0X) > + { > + index = code - ABS_HAT0X; > + ext->hat_values[index] = value; > + if ((code - ABS_HAT0X) % 2) > + index--; > + if (ext->hat_values[index] == 0) > + { > + if (ext->hat_values[index+1] == 0) > + value = 8; > + else if (ext->hat_values[index+1] < 0) > + value = 0; > + else > + value = 4; > + } > + else if (ext->hat_values[index] > 0) > + { > + if (ext->hat_values[index+1] == 0) > + value = 2; > + else if (ext->hat_values[index+1] < 0) > + value = 1; > + else > + value = 3; > + } > + else > + { > + if (ext->hat_values[index+1] == 0) > + value = 6; > + else if (ext->hat_values[index+1] < 0) > + value = 7; > + else > + value = 5; > + } > + ext->report_buffer[ext->hat_map[index]] = value; > + return TRUE; > + } > + else if (code < HID_ABS_MAX && ABS_to_HID_map[code][0] != 0) > + { > + index = ext->abs_map[code].report_index; > + *((WORD*)&ext->report_buffer[index]) = MulDiv(value, 0xff, ext->abs_map[code].info.maximum - ext->abs_map[code].info.minimum); > + return TRUE; > + } > + return FALSE; > +} > + > +static BOOL set_rel_axis_value(struct wine_input_private *ext, int code, int value) > +{ > + int index; > + if (code < HID_REL_MAX && REL_to_HID_map[code][0] != 0) > + { > + index = ext->rel_map[code]; > + if (value > 127) value = 127; > + if (value < -127) value = -127; > + ext->report_buffer[index] = value; > + return TRUE; > + } > + return FALSE; > +} > + > static VOID build_report_descriptor(struct wine_input_private *ext, struct udev_device *dev) > { > int abs_pages[TOP_ABS_PAGE][HID_ABS_MAX+1]; > @@ -510,6 +594,37 @@ static VOID build_report_descriptor(struct wine_input_private *ext, struct udev_ > memcpy(report_ptr, report_tail, sizeof(report_tail)); > > ext->report_descriptor_size = descript_size; > + ext->buffer_length = report_size; > + ext->report_buffer = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, report_size); > + > + /* Initialize axis in the report */ > + for (i = 0; i < HID_ABS_MAX; i++) > + if (test_bit(absbits, i)) > + set_abs_axis_value(ext, i, ext->abs_map[i].info.value); > +} > + > +static BOOL set_report_from_event(struct wine_input_private *ext, struct input_event *ie) > +{ > + switch(ie->type) > + { > +#ifdef EV_SYN > + case EV_SYN: > + return FALSE; > +#endif > +#ifdef EV_MSC > + case EV_MSC: > + return FALSE; > +#endif > + case EV_KEY: > + return set_button_value(ext, ie->code, ie->value); > + case EV_ABS: > + return set_abs_axis_value(ext, ie->code, ie->value); > + case EV_REL: > + return set_rel_axis_value(ext, ie->code, ie->value); > + default: > + ERR("TODO: Process Report (%i, %i)\n",ie->type, ie->code); > + return FALSE; > + } > } > #endif > > @@ -846,9 +961,52 @@ static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buff > return STATUS_NOT_IMPLEMENTED; > } > > +static DWORD CALLBACK lnxev_device_report_thread(void *args) > +{ > + DEVICE_OBJECT *device = (DEVICE_OBJECT*)args; > + struct wine_input_private *private = input_impl_from_DEVICE_OBJECT(device); > + struct pollfd plfds[2]; > + > + plfds[0].fd = private->base.device_fd; > + plfds[0].events = POLLIN; > + plfds[0].revents = 0; > + plfds[1].fd = private->base.control_pipe[0]; > + plfds[1].events = POLLIN; > + plfds[1].revents = 0; > + > + while (1) > + { > + int size; > + struct input_event ie; > + > + if (poll(plfds, 2, -1) <= 0) continue; > + if (plfds[1].revents || !private->report_buffer || private->buffer_length == 0) It would be better to catch memory allocation failures earlier than while processing device reports. > + break; > + size = read(plfds[0].fd, &ie, sizeof(ie)); > + if (size == -1) > + TRACE_(hid_report)("Read failed. Likely an unplugged device\n"); > + else if (size == 0) > + TRACE_(hid_report)("Failed to read report\n"); > + else if (set_report_from_event(private, &ie)) > + process_hid_report(device, private->report_buffer, private->buffer_length); > + } > + return 0; > +} > + > static NTSTATUS lnxev_begin_report_processing(DEVICE_OBJECT *device) > { > - return STATUS_NOT_IMPLEMENTED; > + struct wine_input_private *private = input_impl_from_DEVICE_OBJECT(device); > + You have to check that the thread was not started yet. > + pipe(private->base.control_pipe); Some error handling wouldn't hurt. > + private->base.report_thread = CreateThread(NULL, 0, lnxev_device_report_thread, device, 0, NULL); > + if (!private->base.report_thread) > + { > + ERR("Unable to create device report thread\n"); > + close(private->base.control_pipe[0]); > + close(private->base.control_pipe[1]); > + return STATUS_UNSUCCESSFUL; > + } > + return STATUS_SUCCESS; > } > > static NTSTATUS lnxev_set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > @@ -990,6 +1148,9 @@ static void try_remove_device(struct udev_device *dev) > close(private->control_pipe[0]); > close(private->control_pipe[1]); > CloseHandle(private->report_thread); > +#ifdef HAS_PROPER_INPUT_HEADER > + HeapFree(GetProcessHeap(), 0, ((struct wine_input_private*)private)->report_buffer); It looks like this will attempt to release an invalid pointer for hidraw devices. > +#endif > } > > dev = private->udev_device; > > >

Feb. 27, 2017

Re: [2/4] dinput: Handle username in EnumDevicesBySemantics.

by Andrew Eikum

On Thu, Feb 23, 2017 at 01:22:19AM +0200, Jetro Jormalainen wrote: > @@ -955,6 +998,14 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > > didevi.dwSize = sizeof(didevi); > > + if (ptszUserName) > + { > + username_size = MultiByteToWideChar(CP_ACP, 0, ptszUserName, -1, 0, 0); > + MultiByteToWideChar(CP_ACP, 0, ptszUserName, -1, username_w, username_size); Normally, we use the return from MultiByteToWideChar to allocate the destination buffer. You could switch to that. I'm not opposed to doing it on the stack like this, but then there's no need for username_size. You can just pass the size of the buffer in WCHARs as the last param of MultiByteToWideChar. Andrew

Feb. 27, 2017

Re: [1/4] dinput: Keep username same between device objects.

by Andrew Eikum

On Thu, Feb 23, 2017 at 01:22:18AM +0200, Jetro Jormalainen wrote: > @@ -1308,11 +1308,24 @@ HRESULT WINAPI IDirectInputDevice2WImpl_GetProperty(LPDIRECTINPUTDEVICE8W iface, > case (DWORD_PTR) DIPROP_USERNAME: > { > LPDIPROPSTRING ps = (LPDIPROPSTRING)pdiph; > + struct DevicePlayer *device_player; > > if (pdiph->dwSize != sizeof(DIPROPSTRING)) return DIERR_INVALIDPARAM; > > - lstrcpynW(ps->wsz, This->username, sizeof(ps->wsz)/sizeof(WCHAR)); > - break; > + LIST_FOR_EACH_ENTRY(device_player, &This->dinput->device_players, > + struct DevicePlayer, entry) > + { > + if (IsEqualGUID(&device_player->guid, &This->guid)) > + { > + if (lstrlenW(device_player->username)) Please use *username instead of strlen in places where we don't actually care about the string length. Same in the other patch. > @@ -1390,10 +1403,32 @@ HRESULT WINAPI IDirectInputDevice2WImpl_SetProperty( > case (DWORD_PTR) DIPROP_USERNAME: > { > LPCDIPROPSTRING ps = (LPCDIPROPSTRING)pdiph; > + struct DevicePlayer *device_player; > + unsigned char found = 0; Please use BOOL, TRUE, and FALSE for booleans. Some people use those symbols for their static analyzers. Same in the other patch. > @@ -56,6 +57,12 @@ struct dinput_device { > HRESULT (*create_device)(IDirectInputImpl *dinput, REFGUID rguid, REFIID riid, LPVOID *pdev, int unicode); > }; > > +struct DevicePlayer { > + GUID guid; "guid" is a little vague. This refers to the specific device instance, right? Maybe device_guid or instance_guid? Andrew

Feb. 27, 2017

Re: [PATCH 2/5] winebus.sys: Build device reports for linux event devices

by Sebastian Lackner

On 27.02.2017 16:09, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_udev.c | 437 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 435 insertions(+), 2 deletions(-) > > > > 0002-winebus.sys-Build-device-reports-for-linux-event-devic.txt > > > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 2dcc7fb442..e2a475ccd8 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -63,6 +63,10 @@ > #include "wine/debug.h" > #include "wine/unicode.h" > > +#ifdef HAS_PROPER_INPUT_HEADER > +# include "hidusage.h" > +#endif > + > #include "bus.h" > > WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > @@ -97,6 +101,418 @@ static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *de > return (struct platform_private *)get_platform_private(device); > } > > +#ifdef HAS_PROPER_INPUT_HEADER > +static const BYTE report_header[] = { > + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > + 0x09, 0x00, /* USAGE (??) */ > + 0xa1, 0x01, /* COLLECTION (Application) */ > + 0x09, 0x01, /* USAGE () */ > + 0xa1, 0x00, /* COLLECTION (Physical) */ > +}; > + > +#define IDX_HEADER_PAGE 1 > +#define IDX_HEADER_USAGE 3 > + > +static const BYTE report_buttons[] = { > + 0x05, 0x09, /* USAGE_PAGE (Button) */ > + 0x19, 0x01, /* USAGE_MINIMUM (Button 1) */ > + 0x29, 0x03, /* USAGE_MAXIMUM (Button 3) */ > + 0x15, 0x00, /* LOGICAL_MINIMUM (0) */ > + 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */ > + 0x35, 0x00, /* LOGICAL_MINIMUM (0) */ > + 0x45, 0x01, /* LOGICAL_MAXIMUM (1) */ > + 0x95, 0x03, /* REPORT_COUNT (3) */ > + 0x75, 0x01, /* REPORT_SIZE (1) */ > + 0x81, 0x02, /* INPUT (Data,Var,Abs) */ > +}; > +#define IDX_BUTTON_MIN_USAGE 3 > +#define IDX_BUTTON_MAX_USAGE 5 > +#define IDX_BUTTON_COUNT 11 > + > +static const BYTE report_padding[] = { > + 0x95, 0x03, /* REPORT_COUNT (3) */ > + 0x75, 0x01, /* REPORT_SIZE (1) */ > + 0x81, 0x03, /* INPUT (Cnst,Var,Abs) */ > +}; > +#define IDX_PADDING_BIT_COUNT 1 > + > +static const BYTE report_axis_header[] = { > + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > +}; > +#define IDX_AXIS_PAGE 1 > + > + > +static const BYTE report_axis_usage[] = { > + 0x09, 0x30, /* USAGE (X) */ > +}; > +#define IDX_AXIS_USAGE 1 > + > +static const BYTE report_abs_axis_tail[] = { > + 0x16, 0x00, 0x00, /* LOGICAL_MINIMUM (0) */ > + 0x26, 0xff, 0x00, /* LOGICAL_MAXIMUM (0xff) */ > + 0x35, 0x00, /* PHYSICAL_MINIMUM (0) */ > + 0x46, 0xff, 0x00, /* PHYSICAL_MAXIMUM (256) */ > + 0x75, 0x10, /* REPORT_SIZE (16) */ > + 0x95, 0x00, /* REPORT_COUNT (2) */ > + 0x81, 0x02, /* INPUT (Data,Var,Abs) */ > +}; > +#define IDX_ABS_AXIS_COUNT 14 > + > +static const BYTE report_rel_axis_tail[] = { > + 0x15, 0x81, /* LOGICAL_MINIMUM (0) */ > + 0x25, 0x7f, /* LOGICAL_MAXIMUM (0xffff) */ > + 0x75, 0x08, /* REPORT_SIZE (16) */ > + 0x95, 0x02, /* REPORT_COUNT (2) */ > + 0x81, 0x06, /* INPUT (Data,Var,Rel) */ > +}; > +#define IDX_REL_AXIS_COUNT 7 > + > +static const BYTE report_hatswitch[] = { > + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > + 0x09, 0x39, /* USAGE (Hatswitch) */ > + 0x15, 0x00, /* LOGICAL_MINIMUM (0) */ > + 0x25, 0x08, /* LOGICAL_MAXIMUM (0x08) */ > + 0x35, 0x00, /* PHYSICAL_MINIMUM (0) */ > + 0x45, 0x08, /* PHYSICAL_MAXIMUM (8) */ > + 0x75, 0x08, /* REPORT_SIZE (8) */ > + 0x95, 0x01, /* REPORT_COUNT (1) */ > + 0x81, 0x02, /* INPUT (Data,Var,Abs) */ > +}; > +#define IDX_HATSWITCH_COUNT 15 > + > +static const BYTE report_tail[] = { > + 0xc0, /* END_COLLECTION */ > + 0xc0 /* END_COLLECTION */ > +}; > + > +static const BYTE ABS_to_HID_map[][2] = { > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_X}, /*ABS_X*/ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_Y}, /*ABS_Y*/ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_Z}, /*ABS_Z*/ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_RX}, /*ABS_RX*/ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_RY}, /*ABS_RY*/ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_RZ}, /*ABS_RZ*/ > + {HID_USAGE_PAGE_SIMULATION, HID_USAGE_SIMULATION_THROTTLE}, /*ABS_THROTTLE*/ > + {HID_USAGE_PAGE_SIMULATION, HID_USAGE_SIMULATION_RUDDER}, /*ABS_RUDDER*/ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_WHEEL}, /*ABS_WHEEL*/ > + {HID_USAGE_PAGE_SIMULATION, 0xC4}, /*ABS_GAS*/ > + {HID_USAGE_PAGE_SIMULATION, 0xC5}, /*ABS_BRAKE*/ > + {0,0}, /*ABS_HAT0X*/ > + {0,0}, /*ABS_HAT0Y*/ > + {0,0}, /*ABS_HAT1X*/ > + {0,0}, /*ABS_HAT1Y*/ > + {0,0}, /*ABS_HAT2X*/ > + {0,0}, /*ABS_HAT2Y*/ > + {0,0}, /*ABS_HAT3X*/ > + {0,0}, /*ABS_HAT3Y*/ > + {HID_USAGE_PAGE_DIGITIZER, 0x30}, /*ABS_PRESSURE*/ > + {0, 0}, /*ABS_DISTANCE*/ > + {HID_USAGE_PAGE_DIGITIZER, 0x3D}, /*ABS_TILT_X*/ > + {HID_USAGE_PAGE_DIGITIZER, 0x3F}, /*ABS_TILT_Y*/ > + {0, 0}, /*ABS_TOOL_WIDTH*/ > + {0, 0}, > + {0, 0}, > + {0, 0}, > + {HID_USAGE_PAGE_CONSUMER, 0xE0} /*ABS_VOLUME*/ > +}; > +#define HID_ABS_MAX (ABS_VOLUME+1) > +#define TOP_ABS_PAGE (HID_USAGE_PAGE_DIGITIZER+1) > + > +static const BYTE REL_to_HID_map[][2] = { > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_X}, /* REL_X */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_Y}, /* REL_Y */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_Z}, /* REL_Z */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_RX}, /* REL_RX */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_RY}, /* REL_RY */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_RZ}, /* REL_RZ */ > + {0, 0}, /* REL_HWHEEL */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_DIAL}, /* REL_DIAL */ > + {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_WHEEL}, /* REL_WHEEL */ > + {0, 0} /* REL_MISC */ > +}; > + > +#define HID_REL_MAX (REL_MISC+1) > +#define TOP_REL_PAGE (HID_USAGE_PAGE_CONSUMER+1) > + > +struct wine_input_absinfo { > + struct input_absinfo info; > + BYTE report_index; > +}; > + > +struct wine_input_private { > + struct platform_private base; > + > + int report_descriptor_size; > + BYTE *report_descriptor; > + > + BYTE button_map[KEY_MAX]; > + BYTE rel_map[HID_REL_MAX]; > + BYTE hat_map[8]; > + int hat_values[8]; > + struct wine_input_absinfo abs_map[HID_ABS_MAX]; > +}; > + > +#define test_bit(arr,bit) (((BYTE*)(arr))[(bit)>>3]&(1<<((bit)&7))) > + > +static BYTE *add_button_block(BYTE* report_ptr, BYTE usage_min, BYTE usage_max) > +{ > + memcpy(report_ptr, report_buttons, sizeof(report_buttons)); > + report_ptr[IDX_BUTTON_MIN_USAGE] = usage_min; > + report_ptr[IDX_BUTTON_MAX_USAGE] = usage_max; > + report_ptr[IDX_BUTTON_COUNT] = (usage_max - usage_min) + 1; > + return report_ptr + sizeof(report_buttons); > +} > + > +static BYTE *add_axis_block(BYTE *report_ptr, BYTE count, BYTE page, BYTE *usages, BOOL absolute) > +{ > + int i; > + memcpy(report_ptr, report_axis_header, sizeof(report_axis_header)); > + report_ptr[IDX_AXIS_PAGE] = page; > + report_ptr += sizeof(report_axis_header); > + for (i = 0; i < count; i++) > + { > + memcpy(report_ptr, report_axis_usage, sizeof(report_axis_usage)); > + report_ptr[IDX_AXIS_USAGE] = usages[i]; > + report_ptr += sizeof(report_axis_usage); > + } > + if (absolute) > + { > + memcpy(report_ptr, report_abs_axis_tail, sizeof(report_abs_axis_tail)); > + report_ptr[IDX_ABS_AXIS_COUNT] = count; > + report_ptr += sizeof(report_abs_axis_tail); > + } > + else > + { > + memcpy(report_ptr, report_rel_axis_tail, sizeof(report_rel_axis_tail)); > + report_ptr[IDX_REL_AXIS_COUNT] = count; > + report_ptr += sizeof(report_rel_axis_tail); > + } > + return report_ptr; > +} > + > +static BYTE *add_padding_block(BYTE *report_ptr, BYTE bitcount) > +{ > + memcpy(report_ptr, report_padding, sizeof(report_padding)); > + report_ptr[IDX_PADDING_BIT_COUNT] = bitcount; > + return report_ptr + sizeof(report_padding); > +} > + > +static BYTE *add_hatswitch(BYTE *report_ptr, INT count) > +{ > + memcpy(report_ptr, report_hatswitch, sizeof(report_hatswitch)); > + report_ptr[IDX_HATSWITCH_COUNT] = count; > + return report_ptr + sizeof(report_hatswitch); > +} > + > +static const BYTE* what_am_I(struct udev_device *dev) > +{ > + static const BYTE Unknown[2] = {HID_USAGE_PAGE_GENERIC, 0}; > + static const BYTE Mouse[2] = {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_MOUSE}; > + static const BYTE Keyboard[2] = {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_KEYBOARD}; > + static const BYTE Gamepad[2] = {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_GAMEPAD}; > + static const BYTE Keypad[2] = {HID_USAGE_PAGE_GENERIC, HID_USAGE_GENERIC_KEYPAD}; > + static const BYTE Tablet[2] = {HID_USAGE_PAGE_DIGITIZER, 0x2}; > + static const BYTE Touchscreen[2] = {HID_USAGE_PAGE_DIGITIZER, 0x4}; > + static const BYTE Touchpad[2] = {HID_USAGE_PAGE_DIGITIZER, 0x5}; > + > + struct udev_device *parent = dev; > + > + /* Look to the parents until we get a clue */ > + while (parent) > + { > + if (udev_device_get_property_value(parent, "ID_INPUT_MOUSE")) > + return Mouse; > + else if (udev_device_get_property_value(parent, "ID_INPUT_KEYBOARD")) > + return Keyboard; > + else if (udev_device_get_property_value(parent, "ID_INPUT_JOYSTICK")) > + return Gamepad; > + else if (udev_device_get_property_value(parent, "ID_INPUT_KEY")) > + return Keypad; > + else if (udev_device_get_property_value(parent, "ID_INPUT_TOUCHPAD")) > + return Touchpad; > + else if (udev_device_get_property_value(parent, "ID_INPUT_TOUCHSCREEN")) > + return Touchscreen; > + else if (udev_device_get_property_value(parent, "ID_INPUT_TABLET")) > + return Tablet; > + > + parent = udev_device_get_parent_with_subsystem_devtype(parent, "input", NULL); > + } > + return Unknown; > +} > + > +static VOID build_report_descriptor(struct wine_input_private *ext, struct udev_device *dev) > +{ > + int abs_pages[TOP_ABS_PAGE][HID_ABS_MAX+1]; > + int rel_pages[TOP_REL_PAGE][HID_REL_MAX+1]; > + BYTE absbits[(ABS_MAX+7)/8]; > + BYTE relbits[(REL_MAX+7)/8]; > + BYTE keybits[(KEY_MAX+7)/8]; > + BYTE *report_ptr; > + INT i, descript_size; > + INT report_size; > + INT button_count, abs_count, rel_count, hat_count; > + const BYTE *device_usage = what_am_I(dev); > + > + if (ioctl(ext->base.device_fd, EVIOCGBIT(EV_REL, sizeof(relbits)), relbits) == -1) > + { > + WARN("ioctl(EVIOCGBIT, EV_REL) failed: %d %s\n", errno, strerror(errno)); > + return; > + } > + if (ioctl(ext->base.device_fd, EVIOCGBIT(EV_ABS, sizeof(absbits)), absbits) == -1) > + { > + WARN("ioctl(EVIOCGBIT, EV_ABS) failed: %d %s\n", errno, strerror(errno)); > + return; > + } > + if (ioctl(ext->base.device_fd, EVIOCGBIT(EV_KEY, sizeof(keybits)), keybits) == -1) > + { > + WARN("ioctl(EVIOCGBIT, EV_KEY) failed: %d %s\n", errno, strerror(errno)); > + return; > + } > + > + descript_size = sizeof(report_header) + sizeof(report_tail); > + report_size = 0; > + > + /* For now lump all buttons just into incremental usages, Ignore Keys */ > + button_count = 0; > + for (i = BTN_MISC; i < KEY_MAX; i++) > + { > + if (test_bit(keybits, i)) > + { > + ext->button_map[i] = button_count; > + button_count++; > + } > + } > + if (button_count) > + { > + descript_size += sizeof(report_buttons); > + if (button_count % 8) > + descript_size += sizeof(report_padding); > + report_size = (button_count + 7) / 8; > + } > + > + abs_count = 0; > + memset(abs_pages, 0, sizeof(abs_pages)); > + for (i = 0; i < HID_ABS_MAX; i++) > + if (test_bit(absbits, i)) > + { > + abs_pages[ABS_to_HID_map[i][0]][0]++; > + abs_pages[ABS_to_HID_map[i][0]][abs_pages[ABS_to_HID_map[i][0]][0]] = i; > + > + ioctl(ext->base.device_fd, EVIOCGABS(i), &(ext->abs_map[i])); > + if (abs_pages[ABS_to_HID_map[i][0]][0] == 1) > + { > + descript_size += sizeof(report_axis_header); > + descript_size += sizeof(report_abs_axis_tail); > + } > + } > + /* Skip page 0, aka HID_USAGE_PAGE_UNDEFINED */ > + for (i = 1; i < TOP_ABS_PAGE; i++) > + if (abs_pages[i][0] > 0) > + { > + int j; > + descript_size += sizeof(report_axis_usage) * abs_pages[i][0]; > + for (j = 1; j <= abs_pages[i][0]; j++) > + { > + ext->abs_map[abs_pages[i][j]].report_index = report_size; > + report_size+=2; > + } > + abs_count++; > + } > + > + rel_count = 0; > + memset(rel_pages, 0, sizeof(rel_pages)); > + for (i = 0; i < HID_REL_MAX; i++) > + if (test_bit(relbits, i)) > + { > + rel_pages[REL_to_HID_map[i][0]][0]++; > + rel_pages[REL_to_HID_map[i][0]][rel_pages[REL_to_HID_map[i][0]][0]] = i; > + if (rel_pages[REL_to_HID_map[i][0]][0] == 1) > + { > + descript_size += sizeof(report_axis_header); > + descript_size += sizeof(report_rel_axis_tail); > + } > + } > + /* Skip page 0, aka HID_USAGE_PAGE_UNDEFINED */ > + for (i = 1; i < TOP_REL_PAGE; i++) > + if (rel_pages[i][0] > 0) > + { > + int j; > + descript_size += sizeof(report_axis_usage) * rel_pages[i][0]; > + for (j = 1; j <= rel_pages[i][0]; j++) > + { > + ext->rel_map[rel_pages[i][j]] = report_size; > + report_size++; > + } > + rel_count++; > + } > + > + hat_count = 0; > + for (i = ABS_HAT0X; i <=ABS_HAT3X; i+=2) > + if (test_bit(absbits, i)) > + { > + ext->hat_map[i - ABS_HAT0X] = report_size; > + ext->hat_values[i - ABS_HAT0X] = 0; > + ext->hat_values[i - ABS_HAT0X + 1] = 0; > + report_size++; > + hat_count++; > + } > + > + TRACE("Report Descriptor will be %i bytes\n", descript_size); > + TRACE("Report will be %i bytes\n", report_size); > + > + ext->report_descriptor = HeapAlloc(GetProcessHeap(), 0, descript_size); > + report_ptr = ext->report_descriptor; > + > + memcpy(report_ptr, report_header, sizeof(report_header)); > + report_ptr[IDX_HEADER_PAGE] = device_usage[0]; > + report_ptr[IDX_HEADER_USAGE] = device_usage[1]; > + report_ptr += sizeof(report_header); > + if (button_count) > + { > + report_ptr = add_button_block(report_ptr, 1, button_count); > + if (button_count % 8) > + { > + BYTE padding = 8 - (button_count % 8); > + report_ptr = add_padding_block(report_ptr, padding); > + } > + } > + if (abs_count) > + { > + for (i = 1; i < TOP_ABS_PAGE; i++) > + { > + if (abs_pages[i][0]) > + { > + BYTE usages[HID_ABS_MAX]; > + int j; > + for (j = 0; j < abs_pages[i][0]; j++) > + usages[j] = ABS_to_HID_map[abs_pages[i][j+1]][1]; > + report_ptr = add_axis_block(report_ptr, abs_pages[i][0], i, usages, TRUE); > + } > + } > + } > + if (rel_count) > + { > + for (i = 1; i < TOP_REL_PAGE; i++) > + { > + if (rel_pages[i][0]) > + { > + BYTE usages[HID_REL_MAX]; > + int j; > + for (j = 0; j < rel_pages[i][0]; j++) > + usages[j] = REL_to_HID_map[rel_pages[i][j+1]][1]; > + report_ptr = add_axis_block(report_ptr, rel_pages[i][0], i, usages, FALSE); > + } > + } > + } > + if (hat_count) > + report_ptr = add_hatswitch(report_ptr, hat_count); > + > + memcpy(report_ptr, report_tail, sizeof(report_tail)); > + > + ext->report_descriptor_size = descript_size; > +} > +#endif > + > static inline WCHAR *strdupAtoW(const char *src) > { > WCHAR *dst; > @@ -406,9 +822,23 @@ static const platform_vtbl hidraw_vtbl = > > #ifdef HAS_PROPER_INPUT_HEADER > > +static inline struct wine_input_private *input_impl_from_DEVICE_OBJECT(DEVICE_OBJECT *device) > +{ > + return (struct wine_input_private*)get_platform_private(device); > +} > + > static NTSTATUS lnxev_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > { > - return STATUS_NOT_IMPLEMENTED; > + struct wine_input_private *ext = input_impl_from_DEVICE_OBJECT(device); > + > + *out_length = ext->report_descriptor_size; > + > + if (length < ext->report_descriptor_size) > + return STATUS_BUFFER_TOO_SMALL; > + > + memcpy(buffer, ext->report_descriptor, ext->report_descriptor_size); > + > + return STATUS_SUCCESS; > } > > static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > @@ -511,7 +941,7 @@ static void try_add_device(struct udev_device *dev) > #ifdef HAS_PROPER_INPUT_HEADER > else if (strcmp(subsystem, "input") == 0) > { > - device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version, 0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl, sizeof(struct platform_private)); > + device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version, 0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl, sizeof(struct wine_input_private)); > } > #endif > > @@ -520,6 +950,9 @@ static void try_add_device(struct udev_device *dev) > struct platform_private *private = impl_from_DEVICE_OBJECT(device); > private->udev_device = udev_device_ref(dev); > private->device_fd = fd; > +#ifdef HAS_PROPER_INPUT_HEADER > + build_report_descriptor((struct wine_input_private*)private, dev); I don't think you want this code to be executed for hidraw devices. > +#endif > IoInvalidateDeviceRelations(device, BusRelations); > } > else > > >

Feb. 27, 2017

Re: [PATCH 1/5] winebus.sys: Handle linux input event device via udev

by Sebastian Lackner

On 27.02.2017 16:09, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 3 + > dlls/winebus.sys/bus_udev.c | 150 ++++++++++++++++++++++++++++++++++++++++++-- > dlls/winebus.sys/main.c | 26 ++++++++ > 3 files changed, 173 insertions(+), 6 deletions(-) > I assume this is meant as a fallback, if we don't have access to hidraw, right? If you plan to use this as a replacement for dinput, it would be better to implement it without fixed dependency on udev. > > > 0001-winebus.sys-Handle-linux-input-event-device-via-udev.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index effbd4afd0..fed35e8c1e 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -43,3 +43,6 @@ DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) DECLSPEC_HIDDEN; > + > +/* General Bus Functions */ > +DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 415bd3db08..2dcc7fb442 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -42,6 +42,14 @@ > # include <sys/ioctl.h> > #endif > > +#ifdef HAVE_LINUX_INPUT_H > +# include <linux/input.h> > +# undef SW_MAX > +# if defined(EVIOCGBIT) && defined(EV_ABS) && defined(BTN_PINKIE) > +# define HAS_PROPER_INPUT_HEADER > +# endif > +#endif > + > #define NONAMELESSUNION > > #include "ntstatus.h" > @@ -65,11 +73,15 @@ WINE_DECLARE_DEBUG_CHANNEL(hid_report); > > static struct udev *udev_context = NULL; > static DRIVER_OBJECT *udev_driver_obj = NULL; > +static DWORD disable_hidraw = 0; > +static DWORD disable_input = 0; > > static const WCHAR hidraw_busidW[] = {'H','I','D','R','A','W',0}; > +static const WCHAR lnxev_busidW[] = {'L','N','X','E','V',0}; > > #include "initguid.h" > DEFINE_GUID(GUID_DEVCLASS_HIDRAW, 0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81); > +DEFINE_GUID(GUID_DEVCLASS_LINUXEVENT, 0x1b932c0d,0xfea7,0x42cd,0x8e,0xaa,0x0e,0x48,0x79,0xb6,0x9e,0xaa); > > struct platform_private > { > @@ -392,6 +404,52 @@ static const platform_vtbl hidraw_vtbl = > hidraw_set_feature_report, > }; > > +#ifdef HAS_PROPER_INPUT_HEADER > + > +static NTSTATUS lnxev_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_begin_report_processing(DEVICE_OBJECT *device) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *read) > +{ > + *read = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static const platform_vtbl lnxev_vtbl = { > + compare_platform_device, > + lnxev_get_reportdescriptor, > + lnxev_get_string, > + lnxev_begin_report_processing, > + lnxev_set_output_report, > + lnxev_get_feature_report, > + lnxev_set_feature_report, > +}; > +#endif > + > static void try_add_device(struct udev_device *dev) > { > DWORD vid = 0, pid = 0, version = 0; > @@ -400,6 +458,7 @@ static void try_add_device(struct udev_device *dev) > const char *subsystem; > const char *devnode; > WCHAR *serial = NULL; > + const char* gamepad = NULL; > int fd; > > if (!(devnode = udev_device_get_devnode(dev))) > @@ -419,6 +478,26 @@ static void try_add_device(struct udev_device *dev) > version = get_sysattr_dword(usbdev, "version", 10); > serial = get_sysattr_string(usbdev, "serial"); > } > +#ifdef HAS_PROPER_INPUT_HEADER > + else > + { > + struct input_id device_id = {0}; > + char device_uid[255] = {0}; > + > + if (ioctl(fd, EVIOCGID, &device_id) == -1) > + WARN("ioctl(EVIOCGID) failed: %d %s\n", errno, strerror(errno)); > + if (ioctl(fd, EVIOCGUNIQ(254), device_uid) && device_uid[0]) > + MultiByteToWideChar(CP_UNIXCP, 0, device_uid, -1, serial, 255); I don't think this will work as expected. serial is a NULL pointer, and the check if ioctl() was successful is also done wrong. On error a value < 0 is returned. > + > + gamepad = udev_device_get_property_value(dev, "ID_INPUT_JOYSTICK"); > + vid = device_id.vendor; > + pid = device_id.product; > + version = device_id.version; > + } > +#else > + else > + WARN("Could not get device to query VID, PID, Version and Serial\n"); > +#endif > > TRACE("Found udev device %s (vid %04x, pid %04x, version %u, serial %s)\n", > debugstr_a(devnode), vid, pid, version, debugstr_w(serial)); > @@ -429,6 +508,12 @@ static void try_add_device(struct udev_device *dev) > device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, vid, pid, version, 0, serial, FALSE, > &GUID_DEVCLASS_HIDRAW, &hidraw_vtbl, sizeof(struct platform_private)); > } > +#ifdef HAS_PROPER_INPUT_HEADER > + else if (strcmp(subsystem, "input") == 0) > + { > + device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version, 0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl, sizeof(struct platform_private)); Some linebreaks wouldn't hurt. > + } > +#endif > > if (device) > { > @@ -448,8 +533,17 @@ static void try_add_device(struct udev_device *dev) > > static void try_remove_device(struct udev_device *dev) > { > - DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev); > - struct platform_private *private; > + DEVICE_OBJECT *device = NULL; > + const char* subsystem; > + struct platform_private* private; > + > + subsystem = udev_device_get_subsystem(dev); > + if (strcmp(subsystem, "hidraw") == 0) > + device = bus_find_hid_device(&hidraw_vtbl, dev); > +#ifdef HAS_PROPER_INPUT_HEADER > + else if (strcmp(subsystem, "input") == 0) > + device = bus_find_hid_device(&lnxev_vtbl, dev); > +#endif It might be easier to just check both after each other, like: device = bus_find_hid_device(...); if (!device) device = bus_find_hid_device(...); > if (!device) return; > > IoInvalidateDeviceRelations(device, RemovalRelations); > @@ -483,8 +577,18 @@ static void build_initial_deviceset(void) > return; > } > > - if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) > - WARN("Failed to add subsystem 'hidraw' to enumeration\n"); > + if (!disable_hidraw) > + if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) > + WARN("Failed to add subsystem 'hidraw' to enumeration\n"); > +#ifdef HAS_PROPER_INPUT_HEADER > + if (!disable_input) > + { > + if (udev_enumerate_add_match_subsystem(enumerate, "event") < 0) > + WARN("Failed to add subsystem 'event' to enumeration\n"); Why do you add this match when results are not processed anywhere? > + if (udev_enumerate_add_match_subsystem(enumerate, "input") < 0) > + WARN("Failed to add subsystem 'input' to enumeration\n"); > + } > +#endif > > if (udev_enumerate_scan_devices(enumerate) < 0) > WARN("Enumeration scan failed\n"); > @@ -509,6 +613,7 @@ static void build_initial_deviceset(void) > static struct udev_monitor *create_monitor(struct pollfd *pfd) > { > struct udev_monitor *monitor; > + int systems = 0; > > monitor = udev_monitor_new_from_netlink(udev_context, "udev"); > if (!monitor) > @@ -517,8 +622,27 @@ static struct udev_monitor *create_monitor(struct pollfd *pfd) > return NULL; > } > > - if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0) > - WARN("Failed to add subsystem 'hidraw' to monitor\n"); > + if (!disable_hidraw) > + { > + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0) > + WARN("Failed to add 'hidraw' subsystem to monitor\n"); > + else > + systems++; > + } > +#ifdef HAS_PROPER_INPUT_HEADER > + if (!disable_input) > + { > + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "input", NULL) < 0) > + WARN("Failed to add 'input' subsystem to monitor\n"); > + else > + systems++; > + } > +#endif > + if (systems == 0) > + { > + WARN("No subsystems added to monitor\n"); > + goto error; > + } > > if (udev_monitor_enable_receiving(monitor) < 0) > goto error; > @@ -589,6 +713,10 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry > { > HANDLE events[2]; > DWORD result; > + static const WCHAR hidraw_disabledW[] = {'D','i','s','a','b','l','e',' ','h','i','d','r','a','w',0}; > + static const UNICODE_STRING hidraw_disabled = {sizeof(hidraw_disabledW) - sizeof(WCHAR), sizeof(hidraw_disabledW), (WCHAR*)hidraw_disabledW}; > + static const WCHAR input_disabledW[] = {'D','i','s','a','b','l','e',' ','i','n','p','u','t',0}; > + static const UNICODE_STRING input_disabled = {sizeof(input_disabledW) - sizeof(WCHAR), sizeof(input_disabledW), (WCHAR*)input_disabledW}; > > TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer)); > > @@ -602,6 +730,16 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry > driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; > driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; > > + disable_hidraw = check_bus_option(registry_path, &hidraw_disabled); > + if (disable_hidraw) > + TRACE("UDEV hidraw devices disabled in registry\n"); > + > +#ifdef HAS_PROPER_INPUT_HEADER > + disable_input = check_bus_option(registry_path, &input_disabled); > + if (disable_input) > + TRACE("UDEV input devices disabled in registry\n"); > +#endif Does it hurt when both hidraw and input event devices are found? If yes, it would be better to filter duplicates somewhere else, instead of using registry keys. > + > if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) > goto error; > if (!(events[1] = CreateThread(NULL, 0, deviceloop_thread, events[0], 0, NULL))) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 978540bb17..0388f0263c 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -625,6 +625,32 @@ void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) > LeaveCriticalSection(&ext->report_cs); > } > > +DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option) > +{ > + OBJECT_ATTRIBUTES attr; > + HANDLE key; > + DWORD output = 0; > + > + InitializeObjectAttributes(&attr, registry_path, OBJ_CASE_INSENSITIVE | OBJ_KERNEL_HANDLE, NULL, NULL); > + if (NtOpenKey(&key, KEY_ALL_ACCESS, &attr) == STATUS_SUCCESS) > + { > + DWORD size; > + char buffer[FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data[sizeof(DWORD)])]; > + > + KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION*)buffer; > + > + if (NtQueryValueKey(key, option, KeyValuePartialInformation, info, sizeof(buffer), &size) == STATUS_SUCCESS) > + { > + if (info->Type == REG_DWORD) > + output = *(DWORD*)info->Data; > + } > + > + NtClose(key); > + } > + > + return output; > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

Feb. 27, 2017

Re: [PATCH v2 2/2] wined3d: Add quirk for Nvidia viewport subpixel bits bug.

by Józef Kucia

On Mon, Feb 27, 2017 at 3:12 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > On 27.02.2017 14:47, Józef Kucia wrote: >> The offsets used by 1bed10abee7d2439a5c7358596c860c454f06b53 are more >> correct, e.g. because of filling conventions. I don't think using >> different offsets is a correct thing to do. > > I'm not suggesting to change it in general, but the question is if it > is reliable enough to be used as a quirk on Nvidia drivers. It is sufficient > to pass all the Wine tests at least. If you think manual fixup is better, > thats also fine ofc. I know, but I think that we do not want to use different offsets just because the Wine tests pass when these offsets are used. The fractional viewport handling seems to be quite clearly broken in Nvidia drivers. Anyway, the final decision is up to Henri.

Feb. 27, 2017

Re: winmm: Fix some spec file entries

by Andrew Eikum

On Wed, Feb 22, 2017 at 11:23:08PM +0100, André Hentschel wrote: > -@ stdcall PlaySound(ptr long long) PlaySoundA > -@ stdcall PlaySoundA(ptr long long) > -@ stdcall PlaySoundW(ptr long long) > +@ stdcall PlaySound(str long long) PlaySoundA > +@ stdcall PlaySoundA(str long long) > +@ stdcall PlaySoundW(wstr long long) The first parameter is not always a string, sometimes it's a resource identifier or even a raw pointer: https://msdn.microsoft.com/en-us/library/windows/desktop/dd743679(v=vs.85).… Andrew

Feb. 27, 2017

Re: [PATCH v2 2/2] wined3d: Add quirk for Nvidia viewport subpixel bits bug.

by Sebastian Lackner

On 27.02.2017 14:47, Józef Kucia wrote: > On Mon, Feb 27, 2017 at 1:49 PM, Sebastian Lackner > <sebastian(a)fds-team.de> wrote: >> As you might be aware, the problem on Nvidia binary can also be worked >> around by using slightly different offsets (basically reverting >> 1bed10abee7d2439a5c7358596c860c454f06b53). Do you think its better to >> disable it, or should we just use different pixel center offsets? > > The offsets used by 1bed10abee7d2439a5c7358596c860c454f06b53 are more > correct, e.g. because of filling conventions. I don't think using > different offsets is a correct thing to do. I'm not suggesting to change it in general, but the question is if it is reliable enough to be used as a quirk on Nvidia drivers. It is sufficient to pass all the Wine tests at least. If you think manual fixup is better, thats also fine ofc. > We also have a code path > which handles the pixel offsets correctly when ARB_clip_control is not > available. On the other hand, we may want to reconsider the > ARB_viewport_array detection logic when we'll add support for multiple > viewports in wined3d. >

Feb. 27, 2017

Re: [PATCH v2 2/2] wined3d: Add quirk for Nvidia viewport subpixel bits bug.

by Józef Kucia

On Mon, Feb 27, 2017 at 1:49 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > As you might be aware, the problem on Nvidia binary can also be worked > around by using slightly different offsets (basically reverting > 1bed10abee7d2439a5c7358596c860c454f06b53). Do you think its better to > disable it, or should we just use different pixel center offsets? The offsets used by 1bed10abee7d2439a5c7358596c860c454f06b53 are more correct, e.g. because of filling conventions. I don't think using different offsets is a correct thing to do. We also have a code path which handles the pixel offsets correctly when ARB_clip_control is not available. On the other hand, we may want to reconsider the ARB_viewport_array detection logic when we'll add support for multiple viewports in wined3d.

Feb. 27, 2017

Re: [PATCH] oledb32: Correct length calculation in DataConvert for DBTYPE_STR

by Huw Davies

On Fri, Feb 24, 2017 at 06:45:16AM +0000, Alistair Leslie-Hughes wrote: > v2 - Correct failing test under wine > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/oledb32/convert.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/dlls/oledb32/convert.c b/dlls/oledb32/convert.c > index ac462f2..20ef917 100644 > --- a/dlls/oledb32/convert.c > +++ b/dlls/oledb32/convert.c > @@ -816,23 +816,23 @@ static HRESULT WINAPI convert_DataConvert(IDataConvert* iface, > case DBTYPE_STR: > { > BSTR b; > - DBLENGTH bstr_len; > + DBLENGTH length; > INT bytes_to_copy; > - hr = IDataConvert_DataConvert(iface, src_type, DBTYPE_BSTR, src_len, &bstr_len, > + hr = IDataConvert_DataConvert(iface, src_type, DBTYPE_BSTR, src_len, &length, > src, &b, sizeof(BSTR), src_status, dst_status, > precision, scale, flags); > if(hr != S_OK) return hr; > - bstr_len = SysStringLen(b); > - *dst_len = bstr_len * sizeof(char); /* Doesn't include size for '\0' */ > + length = WideCharToMultiByte(CP_ACP, 0, b, -1, NULL, 0, NULL, NULL); > + *dst_len = SysStringLen(b); /* Doesn't include size for '\0' */ This obviously can't be right. As I said, you want to pass SysStringLen(b) as the wstr_len to WideCharToMultiByte(). Then whatever that returns will be the length of the ansi string excluding the '\0'. That should go into *dst_len. > *dst_status = DBSTATUS_S_OK; > - bytes_to_copy = min(*dst_len + sizeof(char), dst_max_len); > + bytes_to_copy = min(length, dst_max_len); bytes_to_copy should be min(length + 1, dst_max_len), so unchanged, but let's get rid of sizeof(char). > if(dst) > { > if(bytes_to_copy >= sizeof(char)) > { > - WideCharToMultiByte(CP_ACP, 0, b, bytes_to_copy - sizeof(char), dst, dst_max_len, NULL, NULL); > + WideCharToMultiByte(CP_ACP, 0, b, -1, dst, dst_max_len, NULL, NULL); Again SysStringLen(b) here for the wstr_len. > *((char *)dst + bytes_to_copy / sizeof(char) - 1) = 0; This is correct, but again let's get rid of the sizeof(char). > - if(bytes_to_copy < *dst_len + sizeof(char)) > + if(bytes_to_copy < length) should be length + 1. You probably want to add a test for a STR -> STR conversion of "test\0ed" and pass in interesting src lengths like 6, 7 and 8, then check the returned dst (using memcmp, not strcmp). Huw.

Feb. 27, 2017