October 2016 - Wine-Devel - list.winehq.org

Beating the Wineconf drum

by Jeremy White

Hey folks, A reminder, it's now just 6 weeks until Wineconf. We still have funding available, so write to me privately if you'd like to come, but don't have the funds. We could also use topics for conversation; you can email the wineconf mailing list, or edit the wiki page directly; either way is fine. Cheers, Jeremy p.s. And this is going to be my point of pain: https://www.winehq.org/~jwhite/latest.html

Oct. 3, 2016

Re: [3/3] winebus.sys: Add implementation for device removal. (v9)

by Sebastian Lackner

On 03.10.2016 17:27, Aric Stewart wrote: > > On 10/3/16 6:29 AM, Sebastian Lackner wrote: >> > From: Aric Stewart <aric(a)codeweavers.com> >> > >> > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> >> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> >> > --- >> > >> > Second part of 126774. Besides splitting, I've also done the following changes: >> > >> > * Rename find_hid_device -> bus_find_hid_device, for consistency with other bus_*_hid_*() functions. >> > * Do not append timestamp to the device names. To make the code more robust, I've modified >> > bus_remove_hid_device() to defer the release of the pnp_device pointer. If there are more >> > problems I would prefer to fix them properly, adding the timestamp seems more like a hack to me. >> > * Some style improvements. >> > >> > @Aric: I'm not sure if you are aware of it, but there is currently still a bug somewhere which >> > causes a crash during RemovalRelations when trying to remove a driver which was never added properly. >> > I feel like the right place to fix that is somewhere in ntoskrnl. Are you aware of this issue? >> > > Yes, that bug is fixed by patch 126650, which has not been reviewed or committed yet. The patch does not help here. It also sounds like you are trying to fix a different problem because the issue I have can be reproduced without winehid/hidclass. Nevertheless, one way to improve 126650 would be to remove the unnecessary "break;" after the return. Regards, Sebastian

Oct. 3, 2016

RE: [PATCH] make_unicode: Change handling of Turkish i to match Windows

by Daniel Lehman

>Are you sure about that? If I understood the original patch description correctly, Wine currently has the following mappings: > İ -> i > i -> I > > I -> i > ı -> I > >While the correct (Windows) mappings would be: > > with LCMAP_LINGUISTIC_CASING and tr or az locale: > İ <-> i > I <-> ı > > without LCMAP_LINGUISTIC_CASING: > I <-> i > İ -> İ > ı -> ı right >I think doing this properly would need to take the data from https://urldefense.proofpoint.com/v2/url?u=http-3A__www.unicode.org_Public_…>v0AaA02WDVZTGQo77gZQ&m=ojKq6NWxBIZ_S4HX7WhgcDIxqKLoSwppiNx8q7fScrw&s=j6sJ7DQkj8fKnFLo3i4DPMNpiKklnG9qGITUzV9Nq4M&e= into account. https://urldefense.proofpoint.com/v2/url?u=http->3A__www.unicode.org_reports_tr44_-23Casemapping&d=DQIFaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=0->v0AaA02WDVZTGQo77gZQ&m=ojKq6NWxBIZ_S4HX7WhgcDIxqKLoSwppiNx8q7fScrw&s=HlbjMHfc_H_7HQHRfjtzip3RyjOaoF0WSCQPmQNP8Zc&e= seems relevant. I'll take a look Thanks daniel

Oct. 3, 2016

Re: [3/3] winebus.sys: Add implementation for device removal. (v9)

by Aric Stewart

On 10/3/16 6:29 AM, Sebastian Lackner wrote: > From: Aric Stewart <aric(a)codeweavers.com> > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Second part of 126774. Besides splitting, I've also done the following changes: > > * Rename find_hid_device -> bus_find_hid_device, for consistency with other bus_*_hid_*() functions. > * Do not append timestamp to the device names. To make the code more robust, I've modified > bus_remove_hid_device() to defer the release of the pnp_device pointer. If there are more > problems I would prefer to fix them properly, adding the timestamp seems more like a hack to me. > * Some style improvements. > > @Aric: I'm not sure if you are aware of it, but there is currently still a bug somewhere which > causes a crash during RemovalRelations when trying to remove a driver which was never added properly. > I feel like the right place to fix that is somewhere in ntoskrnl. Are you aware of this issue? > Yes, that bug is fixed by patch 126650, which has not been reviewed or committed yet. -aric > dlls/winebus.sys/bus.h | 16 +++++++-- > dlls/winebus.sys/bus_udev.c | 38 ++++++++++++++++++++-- > dlls/winebus.sys/main.c | 74 ++++++++++++++++++++++++++++++++++++++------ > 3 files changed, 111 insertions(+), 17 deletions(-) > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index dcb50f2..099558b 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -19,8 +19,18 @@ > /* Busses */ > NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN; > > +/* Native device function table */ > +typedef struct > +{ > + int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev); > +} platform_vtbl; > + > +void *get_platform_private(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > + > /* HID Plug and Play Bus */ > NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > -DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, void *native, WORD vid, > - WORD pid, DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad, > - const GUID *class) DECLSPEC_HIDDEN; > +DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, WORD vid, WORD pid, > + DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad, > + const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN; > +DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; > +void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index b1d6c3f..d798c53 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -61,6 +61,16 @@ static const WCHAR hidraw_busidW[] = {'H','I','D','R','A','W',0}; > #include "initguid.h" > DEFINE_GUID(GUID_DEVCLASS_HIDRAW, 0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81); > > +struct platform_private > +{ > + struct udev_device *udev_device; > +}; > + > +static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *device) > +{ > + return (struct platform_private *)get_platform_private(device); > +} > + > static DWORD get_sysattr_dword(struct udev_device *dev, const char *sysattr, int base) > { > const char *attr = udev_device_get_sysattr_value(dev, sysattr); > @@ -88,6 +98,18 @@ static WCHAR *get_sysattr_string(struct udev_device *dev, const char *sysattr) > return dst; > } > > +static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > +{ > + struct udev_device *dev1 = impl_from_DEVICE_OBJECT(device)->udev_device; > + struct udev_device *dev2 = platform_dev; > + return strcmp(udev_device_get_syspath(dev1), udev_device_get_syspath(dev2)); > +} > + > +static const platform_vtbl hidraw_vtbl = > +{ > + compare_platform_device, > +}; > + > static void try_add_device(struct udev_device *dev) > { > DWORD vid = 0, pid = 0, version = 0; > @@ -123,12 +145,15 @@ static void try_add_device(struct udev_device *dev) > subsystem = udev_device_get_subsystem(dev); > if (strcmp(subsystem, "hidraw") == 0) > { > - device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, dev, vid, pid, > - version, 0, serial, FALSE, &GUID_DEVCLASS_HIDRAW); > + 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)); > } > > if (device) > - udev_device_ref(dev); > + { > + impl_from_DEVICE_OBJECT(device)->udev_device = udev_device_ref(dev); > + IoInvalidateDeviceRelations(device, BusRelations); > + } > else > WARN("Ignoring device %s with subsystem %s\n", debugstr_a(devnode), subsystem); > > @@ -137,7 +162,12 @@ static void try_add_device(struct udev_device *dev) > > static void try_remove_device(struct udev_device *dev) > { > - /* FIXME */ > + DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev); > + if (!device) return; > + > + dev = impl_from_DEVICE_OBJECT(device)->udev_device; > + bus_remove_hid_device(device); > + udev_device_unref(dev); > } > > static void build_initial_deviceset(void) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 1364eab..09a59d6 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -47,13 +47,16 @@ struct pnp_device > > struct device_extension > { > - void *native; /* Must be the first member of the structure */ > + struct pnp_device *pnp_device; > > WORD vid, pid; > DWORD uid, version, index; > BOOL is_gamepad; > WCHAR *serial; > const WCHAR *busid; /* Expected to be a static constant */ > + > + const platform_vtbl *vtbl; > + BYTE platform_private[1]; > }; > > static CRITICAL_SECTION device_list_cs; > @@ -80,6 +83,12 @@ static inline WCHAR *strdupW(const WCHAR *src) > return dst; > } > > +void *get_platform_private(DEVICE_OBJECT *device) > +{ > + struct device_extension *ext = (struct device_extension *)device->DeviceExtension; > + return ext->platform_private; > +} > + > static DWORD get_vidpid_index(WORD vid, WORD pid) > { > struct pnp_device *ptr; > @@ -157,9 +166,9 @@ static WCHAR *get_compatible_ids(DEVICE_OBJECT *device) > return dst; > } > > -DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, void *native, WORD vid, > - WORD pid, DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad, > - const GUID *class) > +DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, WORD vid, WORD pid, > + DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad, > + const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) > { > static const WCHAR device_name_fmtW[] = {'\\','D','e','v','i','c','e','\\','%','s','#','%','p',0}; > struct device_extension *ext; > @@ -169,16 +178,18 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > WCHAR dev_name[256]; > HDEVINFO devinfo; > NTSTATUS status; > + DWORD length; > > - TRACE("(%p, %s, %p, %04x, %04x, %u, %u, %s, %u, %s)\n", driver, debugstr_w(busidW), native, > - vid, pid, version, uid, debugstr_w(serialW), is_gamepad, debugstr_guid(class)); > + TRACE("(%p, %s, %04x, %04x, %u, %u, %s, %u, %s, %p, %u)\n", driver, debugstr_w(busidW), vid, pid, > + version, uid, debugstr_w(serialW), is_gamepad, debugstr_guid(class), vtbl, platform_data_size); > > if (!(pnp_dev = HeapAlloc(GetProcessHeap(), 0, sizeof(*pnp_dev)))) > return NULL; > > - sprintfW(dev_name, device_name_fmtW, busidW, native); > + sprintfW(dev_name, device_name_fmtW, busidW, pnp_dev); > RtlInitUnicodeString(&nameW, dev_name); > - status = IoCreateDevice(driver, sizeof(*ext), &nameW, 0, 0, FALSE, &device); > + length = FIELD_OFFSET(struct device_extension, platform_private[platform_data_size]); > + status = IoCreateDevice(driver, length, &nameW, 0, 0, FALSE, &device); > if (status) > { > FIXME("failed to create device error %x\n", status); > @@ -190,7 +201,7 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > > /* fill out device_extension struct */ > ext = (struct device_extension *)device->DeviceExtension; > - ext->native = native; > + ext->pnp_device = pnp_dev; > ext->vid = vid; > ext->pid = pid; > ext->uid = uid; > @@ -199,6 +210,7 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > ext->is_gamepad = is_gamepad; > ext->serial = strdupW(serialW); > ext->busid = busidW; > + ext->vtbl = vtbl; > > /* add to list of pnp devices */ > pnp_dev->device = device; > @@ -226,10 +238,52 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > else > ERR("failed to get ClassDevs: %x\n", GetLastError()); > > - IoInvalidateDeviceRelations(device, BusRelations); > return device; > } > > +DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) > +{ > + struct pnp_device *dev; > + DEVICE_OBJECT *ret = NULL; > + > + TRACE("(%p, %p)\n", vtbl, platform_dev); > + > + EnterCriticalSection(&device_list_cs); > + LIST_FOR_EACH_ENTRY(dev, &pnp_devset, struct pnp_device, entry) > + { > + struct device_extension *ext = (struct device_extension *)dev->device->DeviceExtension; > + if (ext->vtbl != vtbl) continue; > + if (ext->vtbl->compare_platform_device(dev->device, platform_dev) == 0) > + { > + ret = dev->device; > + break; > + } > + } > + LeaveCriticalSection(&device_list_cs); > + > + TRACE("returning %p\n", ret); > + return ret; > +} > + > +void bus_remove_hid_device(DEVICE_OBJECT *device) > +{ > + struct device_extension *ext = (struct device_extension *)device->DeviceExtension; > + struct pnp_device *pnp_device = ext->pnp_device; > + > + TRACE("(%p)\n", device); > + > + EnterCriticalSection(&device_list_cs); > + list_remove(&pnp_device->entry); > + LeaveCriticalSection(&device_list_cs); > + > + IoInvalidateDeviceRelations(device, RemovalRelations); > + HeapFree(GetProcessHeap(), 0, ext->serial); > + IoDeleteDevice(device); > + > + /* pnp_device must be released after the device is gone */ > + HeapFree(GetProcessHeap(), 0, pnp_device); > +} > + > static NTSTATUS handle_IRP_MN_QUERY_ID(DEVICE_OBJECT *device, IRP *irp) > { > NTSTATUS status = irp->IoStatus.u.Status; >

Oct. 3, 2016

Re: [PATCH 4/7] wined3d: Avoid using destroyed context.

by Józef Kucia

03.10.2016 3:47 PM "Henri Verbeet" <hverbeet(a)gmail.com>: > I think it may be harder to fix than this. If state_unbind_resources() > can end up destroying the (primary) swapchain, it's not safe to call > context_acquire() afterwards. Which means it's unsafe to unload > resources or even unload a different SRV. The reason destroying the > RTVs is currently safe, is that we unloaded resources earlier, so > releasing the references to the resources for those views shouldn't > acquire a context to destroy e.g. the GL texture again, but that's > already more fragile than I'd like. Perhaps we should just delay > destroying the context until leaving the last level. I think this fix is correct. The device keeps a reference to the implicit swapchain. The implicit swapchain is destroyed just before leaving from wined3d_device_uninit_3d() so it should be safe to call context_acquire(). This problem occurs only for additional swapchains and that's why it happens only in test_swapchain_flip().

Oct. 3, 2016

Re: [PATCH v2] d2d1: Initial implementation of HWND render target

by Nikolay Sivov

On 03.10.2016 16:45, Henri Verbeet wrote: > On 1 October 2016 at 18:12, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: >> +void d2d_d3d_render_target_set_pixel_size(ID2D1RenderTarget *iface, UINT32 width, UINT32 height) >> +{ >> + struct d2d_d3d_render_target *render_target = impl_from_ID2D1RenderTarget(iface); >> + render_target->pixel_size.width = width; >> + render_target->pixel_size.height = height; >> +} >> + > I think d2d_d3d_render_target_create_rtv() already does this. > > Right, most likely a leftover from earlier version.

Oct. 3, 2016

Re: [PATCH 4/7] wined3d: Avoid using destroyed context.

by Henri Verbeet

On 3 October 2016 at 00:38, Józef Kucia <jkucia(a)codeweavers.com> wrote: > The swapchain can be destroyed by state_unbind_resources() > if the only reference to a swapchain is kept by a shader resource view. > This happens in d3d10core and d3d11 tests when a device is destroyed > in test_swapchain_flip(). > I think it may be harder to fix than this. If state_unbind_resources() can end up destroying the (primary) swapchain, it's not safe to call context_acquire() afterwards. Which means it's unsafe to unload resources or even unload a different SRV. The reason destroying the RTVs is currently safe, is that we unloaded resources earlier, so releasing the references to the resources for those views shouldn't acquire a context to destroy e.g. the GL texture again, but that's already more fragile than I'd like. Perhaps we should just delay destroying the context until leaving the last level.

Oct. 3, 2016

Re: [PATCH 3/7] wined3d: Bind dummy textures to all texture image units.

by Henri Verbeet

On 3 October 2016 at 00:38, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/wined3d/context.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Not that there's anything wrong with this, but what's the idea behind it?

Oct. 3, 2016

Re: [PATCH 1/7] wined3d: Use MAX_TEXTURES instead of magic number.

by Henri Verbeet

On 3 October 2016 at 00:38, Józef Kucia <jkucia(a)codeweavers.com> wrote: > diff --git a/dlls/wined3d/ati_fragment_shader.c b/dlls/wined3d/ati_fragment_shader.c > index cd32009..7a04f76 100644 > --- a/dlls/wined3d/ati_fragment_shader.c > +++ b/dlls/wined3d/ati_fragment_shader.c > @@ -1307,7 +1307,7 @@ static void atifs_get_caps(const struct wined3d_gl_info *gl_info, struct fragmen > * The proper fix for this is not to use GL_ATI_fragment_shader on cards newer than the > * r200 series and use an ARB or GLSL shader instead > */ > - caps->MaxTextureBlendStages = 8; > + caps->MaxTextureBlendStages = MAX_TEXTURES; For what it's worth, note that ati_fragment_shader.c in particular has a couple of other places with hard 8's as well.

Oct. 3, 2016

Re: [PATCH] d3d11: Add stub implementation for deferred context.

by Henri Verbeet

Other than some minor comments below, the patch itself is mostly ok. Unfortunately, the issue is that it's not clear that this is the right direction for implementing deferred contexts. In particular, it seems likely that we'll handle the difference between immediate and deferred contexts inside wined3d, and only have a single implementation in d3d11. Of course that's one of those things where you won't know how well it works until you try it. On 2 October 2016 at 08:39, Kimmo Myllyvirta <kimmo.myllyvirta(a)gmail.com> wrote: > -»······view.c > +»······view.c \ > +»······deferred.c It's pretty minor, but please keep these ordered. > + if (context) > + { Is it valid for context to be NULL here? The test doesn't cover it. > + *context = (ID3D11DeviceContext *)&object->ID3D11DeviceContext_iface; That cast seems unnecessary.

Oct. 3, 2016