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

Re: [3/4] vcomp: Implement 8-bit atomic instructions.
by Alexandre Julliard July 18, 2016

July 18, 2016

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > dlls/vcomp/main.c | 109 ++++++++++++++++++++++++++++++++++++++++++++ > dlls/vcomp/vcomp.spec | 22 ++++---- > dlls/vcomp100/vcomp100.spec | 22 ++++---- > dlls/vcomp110/vcomp110.spec | 22 ++++---- > dlls/vcomp120/vcomp120.spec | 22 ++++---- > dlls/vcomp90/vcomp90.spec | 22 ++++---- > 6 files changed, 164 insertions(+), 55 deletions(-) It doesn't compile here (gcc 5.4.0): gcc -m32 -c -o main.o main.c -I. -I../../include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer -Werror -g -O2 -fno-diagnostics-show-caret main.c: Assembler messages: main.c:246: Error: bad register name `%sil' main.c:246: Error: bad register name `%dil' main.c:246: Error: bad register name `%dil' main.c:246: Error: bad register name `%sil' main.c:246: Error: bad register name `%sil' main.c:246: Error: bad register name `%sil' main.c:246: Error: bad register name `%sil' main.c:246: Error: bad register name `%sil' main.c:246: Error: bad register name `%sil' Makefile:163: recipe for target 'main.o' failed make[1]: *** [main.o] Error 1 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: shell32: add SHGetPropertyStoreForWindow stub
by Alex Henrie July 18, 2016

July 18, 2016

@ stdcall SHGetPropertyStoreFromParsingName(wstr ptr long ptr ptr) +@ stdcall SHGetPropertyStoreForWindow(long ptr ptr) Switch these two lines to get alphabetical order. -Alex

1 0

Re: [PATCH] wined3d: Test and enable ARB_gpu_shader5 in shaders.
by Józef Kucia July 18, 2016

July 18, 2016

As it was already mentioned the plan was to use GLSL 4.30 for shader model 5. However, it shouldn't be much of a burden to also support SM5 bitwise opcodes with ARB_gpu_shader5. Apart from that the other issue is that adding newer OpenGL extensions is generally blocked by bug 38648 [1]. The main reason is that it cannot be tested in the current version of Wine with drivers which do not support compatibility profiles. On Fri, Jul 15, 2016 at 2:25 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > + if (gl_info->supported[ARB_GPU_SHADER5]) > + shader_addline(buffer, "#extension GL_ARB_gpu_shader5 : enable\n"); > if (gl_info->supported[ARB_SHADER_BIT_ENCODING]) > shader_addline(buffer, "#extension GL_ARB_shader_bit_encoding : enable\n"); > if (gl_info->supported[ARB_SHADER_TEXTURE_LOD]) These should be sorted alphabetically. I guess the ARB_gpu_shader5 extension should also be added to the "core_extensions" table. [1] - https://bugs.winehq.org/show_bug.cgi?id=38648

3 5

Re: [PATCH] vcruntime140: Forward set_unexpected
by Piotr Caban July 18, 2016

July 18, 2016

On 07/18/16 12:11, Alistair Leslie-Hughes wrote: > dlls/ucrtbase/ucrtbase.spec | 2 +- > dlls/vcruntime140/vcruntime140.spec | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Please also update api-ms-win-crt-private-l1-1-0.spec file. Thanks, Piotr

1 0

Re: [PATCH] wined3d: Implement SM5 bfrev opcode in glsl.
by Józef Kucia July 18, 2016

July 18, 2016

On Fri, Jul 15, 2016 at 1:25 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/glsl_shader.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c > index 5e23920..376d110 100644 > --- a/dlls/wined3d/glsl_shader.c > +++ b/dlls/wined3d/glsl_shader.c > @@ -3538,6 +3538,7 @@ static void shader_glsl_map2gl(const struct wined3d_shader_instruction *ins) > case WINED3DSIH_ROUND_Z: instruction = "trunc"; break; > case WINED3DSIH_SQRT: instruction = "sqrt"; break; > case WINED3DSIH_UMAX: instruction = "max"; break; > + case WINED3DSIH_BFREV: instruction = "bitfieldReverse"; break; It looks like this should be in alphabetical order.

1 0

Re: [PATCH] wined3d: Implement SM5 f32tof16 opcode in glsl.
by Józef Kucia July 18, 2016

July 18, 2016

A test would be nice to have. On Fri, Jul 15, 2016 at 1:37 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/arb_program_shader.c | 1 + > dlls/wined3d/glsl_shader.c | 38 ++++++++++++++++++++++++++++++++++++++ > dlls/wined3d/shader_sm4.c | 2 ++ > dlls/wined3d/wined3d_private.h | 1 + > 4 files changed, 42 insertions(+) > > diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c > index 24c1194..9aa4252 100644 > --- a/dlls/wined3d/arb_program_shader.c > +++ b/dlls/wined3d/arb_program_shader.c > @@ -5286,6 +5286,7 @@ static const SHADER_HANDLER shader_arb_instruction_handler_table[WINED3DSIH_TABL > /* WINED3DSIH_EQ */ NULL, > /* WINED3DSIH_EXP */ shader_hw_scalar_op, > /* WINED3DSIH_EXPP */ shader_hw_scalar_op, > + /* WINED3DSIH_F32TOF16 */ NULL, > /* WINED3DSIH_FRC */ shader_hw_map2gl, > /* WINED3DSIH_FTOI */ NULL, > /* WINED3DSIH_FTOU */ NULL, You need to add WINED3DSIH_F32TOF16 in other places as well, e.g. in shader.c. > + shader_addline(ins->ctx->buffer, "unpackHalf2x16(floatBitsToUint(%s)).x", src_param.param_str); You shouldn't need floatBitsToUint. shader_glsl_add_src_param() takes care about it when you set a proper type for the parameter. > + *dst_mask_ptr++ = dst_masks[i]; > + } else > + shader_addline(ins->ctx->buffer, "0"); > + > + if(i != NB_COMPS - 1) > + shader_addline(ins->ctx->buffer, ","); > + shader_addline(ins->ctx->buffer, "\n"); > + } > + > + *dst_mask_ptr++ = '\0'; You shouldn't need to construct dst_mask manually. > + shader_addline(ins->ctx->buffer, ")%s);\n", dst_mask); > +#undef NB_COMPS > +} > + > static void shader_glsl_else(const struct wined3d_shader_instruction *ins) > { > shader_addline(ins->ctx->buffer, "} else {\n"); > @@ -8704,6 +8741,7 @@ static const SHADER_HANDLER shader_glsl_instruction_handler_table[WINED3DSIH_TAB > /* WINED3DSIH_EQ */ shader_glsl_relop, > /* WINED3DSIH_EXP */ shader_glsl_scalar_op, > /* WINED3DSIH_EXPP */ shader_glsl_expp, > + /* WINED3DSIH_F32TOF16 */ shader_glsl_f32tof16, > /* WINED3DSIH_FRC */ shader_glsl_map2gl, > /* WINED3DSIH_FTOI */ shader_glsl_to_int, > /* WINED3DSIH_FTOU */ shader_glsl_to_uint, > diff --git a/dlls/wined3d/shader_sm4.c b/dlls/wined3d/shader_sm4.c > index c8fdb32..6c15751 100644 > --- a/dlls/wined3d/shader_sm4.c > +++ b/dlls/wined3d/shader_sm4.c > @@ -215,6 +215,7 @@ enum wined3d_sm4_opcode > WINED3D_SM5_OP_DERIV_RTY_COARSE = 0x7c, > WINED3D_SM5_OP_DERIV_RTY_FINE = 0x7d, > WINED3D_SM5_OP_GATHER4_C = 0x7e, > + WINED3D_SM5_OP_F32TOF16 = 0x82, > WINED3D_SM5_OP_BFI = 0x8c, > WINED3D_SM5_OP_BFREV = 0x8d, > WINED3D_SM5_OP_SWAPC = 0x8e, > @@ -811,6 +812,7 @@ static const struct wined3d_sm4_opcode_info opcode_table[] = > {WINED3D_SM5_OP_DERIV_RTY_COARSE, WINED3DSIH_DSY_COARSE, "f", "f"}, > {WINED3D_SM5_OP_DERIV_RTY_FINE, WINED3DSIH_DSY_FINE, "f", "f"}, > {WINED3D_SM5_OP_GATHER4_C, WINED3DSIH_GATHER4_C, "f", "fRSf"}, > + {WINED3D_SM5_OP_F32TOF16, WINED3DSIH_F32TOF16, "f", "f"}, You should use "u" for the source parameters. Similar comments as to other patches also applies.

1 0

Re: [PATCH 1/2] wined3d: Implement SM5 bfi opcode in glsl.
by Józef Kucia July 18, 2016

July 18, 2016

On Fri, Jul 15, 2016 at 1:53 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/glsl_shader.c | 72 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 1 deletion(-) > > diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c > index d8edfdc..b0d3a91 100644 > --- a/dlls/wined3d/glsl_shader.c > +++ b/dlls/wined3d/glsl_shader.c > @@ -4380,6 +4380,76 @@ static void shader_glsl_f32tof16(const struct wined3d_shader_instruction *ins) > #undef NB_COMPS > } This patch requires 124405 so it should be in the same series. > > + > +static void shader_glsl_bitfield_op(const struct wined3d_shader_instruction *ins) > +{ > +#define NB_COMPS 4 > + const DWORD write_masks[NB_COMPS] = { > + WINED3DSP_WRITEMASK_0, > + WINED3DSP_WRITEMASK_1, > + WINED3DSP_WRITEMASK_2, > + WINED3DSP_WRITEMASK_3, > + }; You shouldn't need this array. You can assume that WINED3DSP_WRITE_MASK_i = WINED3DSP_WRITE_MASK_0 << i. > + const char dst_masks[NB_COMPS] = { 'x', 'y', 'z', 'w' }; > + const char *prefix; > + const char *suffix; > + int nb_params; For consistency, this should be named "param_count" and should be unsigned int. > + struct glsl_src_param src_param; > + DWORD write_mask; > + char dst_mask[6]; > + char *dst_mask_ptr = dst_mask; > + > + switch(ins->handler_idx) > + { > + case WINED3DSIH_BFI: > + prefix = "bitfieldInsert("; > + suffix = ")"; > + nb_params = 4; > + break; > + > + default: > + prefix = "<unhandled opcode>"; > + suffix = ""; > + nb_params = 0; > + FIXME("Can't handle opcode %s.\n", debug_d3dshaderinstructionhandler(ins->handler_idx)); > + } > + > + write_mask = shader_glsl_append_dst(ins->ctx->buffer, ins); > + > + *dst_mask_ptr++ = '.'; > + shader_addline(ins->ctx->buffer, "uvec4(\n"); > + > + for (int i = 0; i < NB_COMPS; i++) This requires C99. > + { > + if (write_mask & write_masks[i]) > + { > + shader_addline(ins->ctx->buffer, "%s", prefix); > + for(int j = nb_params - 1; j >= 0; j--) > + { > + shader_glsl_add_src_param(ins, &ins->src[j], write_masks[i], &src_param); > + if(j <= 1) > + shader_addline(ins->ctx->buffer, "int(%s)", src_param.param_str); You shouldn't need this cast. It's better to adjust the parameter types and let shader_glsl_add_src_param() handle this. > + else > + shader_addline(ins->ctx->buffer, "%s", src_param.param_str); > + > + if(j) > + shader_addline(ins->ctx->buffer, ", "); > + } > + shader_addline(ins->ctx->buffer, "%s", suffix); > + *dst_mask_ptr++ = dst_masks[i]; > + } else > + shader_addline(ins->ctx->buffer, "0"); > + > + if(i != NB_COMPS - 1) > + shader_addline(ins->ctx->buffer, ","); > + shader_addline(ins->ctx->buffer, "\n"); > + } > + > + *dst_mask_ptr++ = '\0'; You shouldn't need to build dst_mask manually. > + shader_addline(ins->ctx->buffer, ")%s);\n", dst_mask); > +#undef NB_COMPS I don't like this duplicated enums. Generally, this implementation seems overly complicated. You should be able to do something similar to: for (i = 0; i < 4; ++i) { write_mask = WINED3DSP_WRITEMASK_0 << i; dst.write_mask = ins->dst[0].write_mask & write_mask; if (!(write_mask = shader_glsl_append_dst_ext(ins->ctx->buffer, ins, &dst, dst.reg.data_type))) continue; shader_glsl_add_src_param(ins, &ins->src[0], write_mask, &bits); shader_glsl_add_src_param(ins, &ins->src[1], write_mask, &offset); shader_glsl_add_src_param(ins, &ins->src[2], write_mask, &insert); shader_glsl_add_src_param(ins, &ins->src[3], write_mask, &base); shader_addline(ins->ctx->buffer, "bitfieldInsert(%s, %s, %s, %s));\n", base.param_str, insert.param_str, offset.param_str, bits.param_str); }

1 0

Re: [4/5] msvcirt: Implement istream::getline
by Piotr Caban July 18, 2016

July 18, 2016

On 07/18/16 10:13, Iván Matellanes wrote: > DEFINE_THISCALL_WRAPPER(istream_getline, 16) > istream* __thiscall istream_getline(istream *this, char *str, int count, char delim) > { > - FIXME("(%p %p %d %c) stub\n", this, str, count, delim); > - return this; > + this->extract_delim++; > + return istream_get_str_delim(this, str, count, (unsigned char) delim); > } You probably need to call ios_lock before updating extract_delim. Thanks, Piotr

1 0

Re: [v1 1/1] dinput: Sort virtual joysticks by name on the Mac
by Ken Thomases July 18, 2016

July 18, 2016

On Jul 16, 2016, at 3:15 AM, David Lawrie <david.dljunk(a)gmail.com> wrote: > > Tested on OS X 10.10.5. > > Signed-off-by: David Lawrie <david.dljunk(a)gmail.com> > --- > dlls/dinput/joystick_osx.c | 35 ++++++++++++++++++++++++++++++++--- > 1 file changed, 32 insertions(+), 3 deletions(-) > > diff --git a/dlls/dinput/joystick_osx.c b/dlls/dinput/joystick_osx.c > index 637692b..22bdc4f 100644 > --- a/dlls/dinput/joystick_osx.c > +++ b/dlls/dinput/joystick_osx.c > @@ -193,6 +193,26 @@ static long get_device_property_long(IOHIDDeviceRef device, CFStringRef key) > return result; > } > > +static CFStringRef get_device_name(IOHIDDeviceRef device) > +{ > + CFTypeRef ref; > + CFStringRef name = CFSTR("Default Name"); You can probably do better for a fallback. For example, you can format the vendor ID and product ID into a string. For an extreme take on this, see the Copy_DeviceName() function in Apple's HID Calibrator sample code: https://developer.apple.com/library/mac/samplecode/HID_Calibrator/Listings/… > + > + if(device) > + { > + assert(IOHIDDeviceGetTypeID() == CFGetTypeID(device)); > + > + ref = IOHIDDeviceGetProperty(device, CFSTR(kIOHIDProductKey)); > + > + if (ref && CFStringGetTypeID() == CFGetTypeID(ref)) > + name = ref; > + else if (ref) > + name = CFCopyDescription(ref); This introduces a memory-management problem, a.k.a. a leak. This copy is never released. If you follow my suggestion above for formatting a string as a fallback if the product name isn't available, you'd also have to release that. So, you would change this function's name to copy_device_name() and make sure all code paths return an owned reference to the caller, which the caller has to release. Basically, in the case where IOHIDDeviceGetProperty() gives you a string, you'd have to copy it to match the other code paths. Of course, you'd have to make sure the callers do, in fact, release the references. > + } > + > + return name; > +} > + > static long get_device_location_ID(IOHIDDeviceRef device) > { > return get_device_property_long(device, CFSTR(kIOHIDLocationIDKey)); > @@ -203,7 +223,15 @@ static void copy_set_to_array(const void *value, void *context) > CFArrayAppendValue(context, value); > } > > -static CFComparisonResult device_location_comparator(const void *val1, const void *val2, void *context) > +static CFComparisonResult device_name_comparator(IOHIDDeviceRef device1, IOHIDDeviceRef device2) > +{ > + CFStringRef name1 = get_device_name(device1), name2 = get_device_name(device2); > + CFStringCompareFlags compareOptions = kCFCompareForcedOrdering | kCFCompareNumerically; Any reason you've added this separate variable instead of just inlining the options in the call? > + > + return CFStringCompare(name1, name2, compareOptions); > +} -Ken

2 3

Re: [PATCH] winex11.drv: Use XRRSetScreenSize with XRRSetCrtcConfig
by Henri Verbeet July 17, 2016

July 17, 2016

On 16 July 2016 at 00:48, Gabriel Corona <gabriel.corona(a)enst-bretagne.fr> wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=33290 > > The nvidia driver is not so happy about using XRRSetCrtcConfig alone > and generates a panning configuration. We set the screen size > accordingly. Moreover we disable the CRT during the transition. > Updating the screen size isn't necessarily wrong, although avoiding panning is the wrong reason. If you don't want panning, make sure it doesn't get enabled in the first place. The main problem with this patch though, is that it's almost certainly wrong for multi-head configurations.

1 0

Re: [v1 1/1] winejoystick.drv: Sort virtual joysticks by name on the Mac
by Ken Thomases July 17, 2016

July 17, 2016

On Jul 16, 2016, at 3:15 AM, David Lawrie <david.dljunk(a)gmail.com> wrote: > > Tested on OS X 10.10.5. > > Signed-off-by: David Lawrie <david.dljunk(a)gmail.com> > --- > dlls/winejoystick.drv/joystick_osx.c | 42 +++++++++++++++++++++++++++++++----- > 1 file changed, 37 insertions(+), 5 deletions(-) Same comments as for the dinput version. -Ken

1 0

Re: [PATCH] winegstreamer: Fixes for video flipping
by Andrew Eikum July 15, 2016

July 15, 2016

Thanks very much for working on this, I'm glad to have more eyes looking at it. Comments in-line below. On Sat, Jul 16, 2016 at 02:51:10AM +1000, Jan Schmidt wrote: > The videoflip element doesn't handle all formats, > so some formats like Intel Indeo 3 will fail. > Add videoconvert to do format conversion when it's > needed. > Shouldn't the decodebin handle this during format negotiation when the videoflip is connected to it? > Fix some refcount issues too - add a new element > to a bin takes ownership of the floating reference, > so there's no need to keep pointers around to unref > later. > Good catch. Could you separate this out into a different, stand-alone patch from the other changes? > Fix state setting of new elements - always just > set them to PAUSED. gst_element_sync_state_with_parent() > can be problematic, and we only need these in paused > or playing anyway. > I'm not sure about this part. It makes sense to me for all members of a container to be in the same state. Why is it better for these to be PAUSED while others are PLAYING? If it's needed, this should be its own patch, too. > @@ -846,13 +855,16 @@ static void init_new_decoded_pad(GstElement *bin, GstPad *pad, GSTImpl *This) > > gst_util_set_object_arg(G_OBJECT(pin->flipfilter), "method", "vertical-flip"); > > - gst_bin_add(GST_BIN(This->container), pin->flipfilter); > - gst_element_sync_state_with_parent(pin->flipfilter); > + gst_element_set_state (vconv, GST_STATE_PAUSED); > + gst_bin_add(GST_BIN(This->container), vconv); /* bin takes ownership */ > + gst_element_set_state (pin->flipfilter, GST_STATE_PAUSED); > + gst_bin_add(GST_BIN(This->container), pin->flipfilter); /* bin takes ownership */ > > - pin->flip_sink = gst_element_get_static_pad(pin->flipfilter, "sink"); > + gst_element_link (vconv, pin->flipfilter); > + > + pin->flip_sink = gst_element_get_static_pad(vconv, "sink"); > if(!pin->flip_sink){ > WARN("Couldn't find sink on flip filter\n"); > - gst_object_unref(pin->flipfilter); I believe this leaves a dangling element in the container. Should these lines be replaced with calls to gst_bin_remove() instead? Andrew

2 2

Re: [PATCH 2/9] d3drm: Implement IDirect3DRMDevice*::InitFromD3D. (v2)
by Henri Verbeet July 15, 2016

July 15, 2016

On 15 July 2016 at 12:32, Aaryaman Vasishta <jem456.vasishta(a)gmail.com> wrote: > @@ -3984,6 +3985,8 @@ static void test_create_device_from_d3d1(void) > ok(ref2 > ref1, "expected ref2 > ref1, got ref1 = %u , ref2 = %u.\n", ref1, ref2); > device_ref2 = get_refcount((IUnknown *)d3ddevice1); > ok(device_ref2 > device_ref1, "Expected device_ref2 > device_ref1, got device_ref1 = %u, device_ref2 = %u.\n", device_ref1, device_ref2); > + d3d_ref2 = get_refcount((IUnknown *)d3d1); > + ok(d3d_ref2 > d3d_ref1, "Expected d3d_ref2 > d3d_ref1, got d3d_ref1 = %u, d3d_ref2 = %u.\n", d3d_ref1, d3d_ref2); > ret_val = IDirect3DRMDevice_GetWidth(device1); > ok(ret_val == rc.right, "Expected device width = 300, got %u.\n", ret_val); > ret_val = IDirect3DRMDevice_GetHeight(device1); > @@ -4053,6 +4056,133 @@ static void test_create_device_from_d3d1(void) > device_ref2 = get_refcount((IUnknown *)d3ddevice1); > ok(device_ref2 == device_ref1, "Expected device_ref2 == device_ref1, got device_ref1 = %u, device_ref2 = %u.\n", device_ref1, device_ref2); > > + /* InitFromD3D tests */ > + hr = IDirect3DRM_CreateObject(d3drm1, &CLSID_CDirect3DRMDevice, NULL, &IID_IDirect3DRMDevice, (void **)&device1); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DRMDevice interface (hr = %#x).\n", hr); > + > + hr = IDirect3DRMDevice_InitFromD3D(device1, NULL, d3ddevice1); > + ok(hr == D3DRMERR_BADVALUE, "Expected hr == D3DRMERR_BADVALUE, got %#x.\n", hr); > + hr = IDirect3DRMDevice_InitFromD3D(device1, d3d1, NULL); > + ok(hr == D3DRMERR_BADVALUE, "Expected hr == D3DRMERR_BADVALUE, got %#x.\n", hr); > + > + hr = IDirect3DRMDevice_InitFromD3D(device1, d3d1, d3ddevice1); > + ok(SUCCEEDED(hr), "Failed to initialise IDirect3DRMDevice interface (hr = %#x)\n", hr); > + ref2 = get_refcount((IUnknown *)d3drm1); > + ok(ref2 > ref1, "expected ref2 > ref1, got ref1 = %u , ref2 = %u.\n", ref1, ref2); > + device_ref2 = get_refcount((IUnknown *)d3ddevice1); > + ok(device_ref2 > device_ref1, "Expected device_ref2 > device_ref1, got device_ref1 = %u, device_ref2 = %u.\n", device_ref1, device_ref2); I've just fixed some these up in the past, and let some others slide, but please watch your line length.

2 1

Fwd: Re: [PATCH] wined3d: Implement SM5 bfrev opcode in glsl.
by Guillaume Charifi July 15, 2016

July 15, 2016

Right, I was sure to forget to git add something... Do I need to resend everything, or is it ok if I send a patch that only enables the required extension, and will need to be committed before the others? Le 15/07/2016 13:59, Józef Kucia a écrit : > On Fri, Jul 15, 2016 at 1:25 PM, Guillaume Charifi > <guillaume.charifi(a)sfr.fr> wrote: >> Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> >> --- >> dlls/wined3d/glsl_shader.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c >> index 5e23920..376d110 100644 >> --- a/dlls/wined3d/glsl_shader.c >> +++ b/dlls/wined3d/glsl_shader.c >> @@ -3538,6 +3538,7 @@ static void shader_glsl_map2gl(const struct wined3d_shader_instruction *ins) >> case WINED3DSIH_ROUND_Z: instruction = "trunc"; break; >> case WINED3DSIH_SQRT: instruction = "sqrt"; break; >> case WINED3DSIH_UMAX: instruction = "max"; break; >> + case WINED3DSIH_BFREV: instruction = "bitfieldReverse"; break; > I think this requires you to enable appriopriate extension in GLSL > (GL_ARB_gpu_shader5, in this case). I haven't looked closely at the > other patches but they seem to have the same issue.

1 0

Re: [PATCH] wined3d: Implement SM5 bfrev opcode in glsl.
by Józef Kucia July 15, 2016

July 15, 2016

On Fri, Jul 15, 2016 at 1:25 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/glsl_shader.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c > index 5e23920..376d110 100644 > --- a/dlls/wined3d/glsl_shader.c > +++ b/dlls/wined3d/glsl_shader.c > @@ -3538,6 +3538,7 @@ static void shader_glsl_map2gl(const struct wined3d_shader_instruction *ins) > case WINED3DSIH_ROUND_Z: instruction = "trunc"; break; > case WINED3DSIH_SQRT: instruction = "sqrt"; break; > case WINED3DSIH_UMAX: instruction = "max"; break; > + case WINED3DSIH_BFREV: instruction = "bitfieldReverse"; break; I think this requires you to enable appriopriate extension in GLSL (GL_ARB_gpu_shader5, in this case). I haven't looked closely at the other patches but they seem to have the same issue.

2 1

Re: [PATCH 4/9] d3drm: Implement IDirect3DRMViewport*::Init. (v5)
by Henri Verbeet July 15, 2016

July 15, 2016

On 15 July 2016 at 12:32, Aaryaman Vasishta <jem456.vasishta(a)gmail.com> wrote: > diff --git a/dlls/d3drm/viewport.c b/dlls/d3drm/viewport.c > index 9fd627d..743fb75 100644 > --- a/dlls/d3drm/viewport.c > +++ b/dlls/d3drm/viewport.c > @@ -270,19 +279,126 @@ static HRESULT WINAPI d3drm_viewport1_GetClassName(IDirect3DRMViewport *iface, D > static HRESULT WINAPI d3drm_viewport2_Init(IDirect3DRMViewport2 *iface, IDirect3DRMDevice3 *device, > IDirect3DRMFrame3 *camera, DWORD x, DWORD y, DWORD width, DWORD height) > { > - FIXME("iface %p, device %p, camera %p, x %u, y %u, width %u, height %u stub!\n", > + struct d3drm_viewport *viewport = impl_from_IDirect3DRMViewport2(iface); > + D3DVIEWPORT vp; > + D3DVALUE scale; > + IDirect3D *d3d1 = NULL; > + D3DCOLOR color; > + IDirect3DDevice *d3d_device = NULL; > + IDirect3DMaterial *material = NULL; > + D3DMATERIAL mat; > + D3DMATERIALHANDLE hmat; > + HRESULT hr = D3DRM_OK; > + ... > + color = IDirect3DRMFrame3_GetSceneBackground(camera); > + /* Create material (ambient/diffuse/emissive?), set material */ > + if (FAILED(hr = IDirect3D_CreateMaterial(d3d1, &material, NULL))) > + goto cleanup; > + > + memset(&mat, 0, sizeof(mat)); > + memcpy(&mat.diffuse, &color, sizeof(color)); That doesn't look right.

1 0

Re: [PATCH] msscript: Initial support for hosting script engines
by Jacek Caban July 15, 2016

July 15, 2016

Hi Nikolay, On 14.07.2016 16:35, Nikolay Sivov wrote: > +static HRESULT init_script_host(const CLSID *clsid, ScriptHost **ret) > +{ > + IObjectSafety *objsafety; > + ScriptHost *host; > + HRESULT hr; > + > + *ret = NULL; > + > + host = heap_alloc(sizeof(*host)); > + if (!host) > + return E_OUTOFMEMORY; > + > + host->IActiveScriptSite_iface.lpVtbl = &ActiveScriptSiteVtbl; > + host->ref = 1; > + host->script = NULL; > + host->parse = NULL; > + host->clsid = *clsid; > + > + hr = CoCreateInstance(&host->clsid, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, > + &IID_IActiveScript, (void**)&host->script); > + if (FAILED(hr)) { > + WARN("Failed to create an instance for %s, %#x\n", debugstr_guid(clsid), hr); > + goto failed; > + } > + > + hr = IActiveScript_QueryInterface(host->script, &IID_IObjectSafety, (void**)&objsafety); > + if (FAILED(hr)) { > + FIXME("Could not get IObjectSafety, %#x\n", hr); > + goto failed; > + } > + > + hr = IObjectSafety_SetInterfaceSafetyOptions(objsafety, &IID_IActiveScriptParse, INTERFACESAFE_FOR_UNTRUSTED_DATA, 0); > + if (FAILED(hr)) > + FIXME("SetInterfaceSafetyOptions failed, %#x\n", hr); This failure is important, please handle it well. > + IObjectSafety_Release(objsafety); > + > + hr = IActiveScript_SetScriptSite(host->script, &host->IActiveScriptSite_iface); > + if (FAILED(hr)) { > + WARN("SetScriptSite failed, %#x\n", hr); > + goto failed; > + } > + > + hr = IActiveScript_QueryInterface(host->script, &IID_IActiveScriptParse, (void**)&host->parse); > + if (FAILED(hr)) { > + WARN("Failed to get IActiveScriptParse, %#x\n", hr); > + goto failed; > + } This and... > + hr = IActiveScriptParse_InitNew(host->parse); > + if (FAILED(hr)) { > + WARN("InitNew failed, %#x\n", hr); > + goto failed; > + } ...this failure will cause ref leak. You have already set script site, so engine holds a reference to ScriptHost, but you only release one reference on failure. Using release_script_engine on failure should fix that. > + > + *ret = host; > + return S_OK; > + > +failed: > + IActiveScriptSite_Release(&host->IActiveScriptSite_iface); > + return hr; > +} > + > static HRESULT WINAPI ScriptControl_QueryInterface(IScriptControl *iface, REFIID riid, void **ppv) > { > ScriptControl *This = impl_from_IScriptControl(iface); > @@ -257,6 +498,10 @@ static ULONG WINAPI ScriptControl_Release(IScriptControl *iface) > if(!ref) { > if (This->site) > IOleClientSite_Release(This->site); > + if (This->host) { > + release_script_engine(This->host); > + IActiveScriptSite_Release(&This->host->IActiveScriptSite_iface); > + } > heap_free(This); > } > > @@ -321,15 +566,47 @@ static HRESULT WINAPI ScriptControl_Invoke(IScriptControl *iface, DISPID dispIdM > static HRESULT WINAPI ScriptControl_get_Language(IScriptControl *iface, BSTR *p) > { > ScriptControl *This = impl_from_IScriptControl(iface); > - FIXME("(%p)->(%p)\n", This, p); > - return E_NOTIMPL; > + LPOLESTR progidW; > + HRESULT hr; > + > + TRACE("(%p)->(%p)\n", This, p); > + > + if (!p) > + return E_POINTER; > + > + *p = NULL; > + > + if (!This->host) > + return S_OK; > + > + hr = ProgIDFromCLSID(&This->host->clsid, &progidW); > + if (FAILED(hr)) > + return hr; > + > + *p = SysAllocStringLen(progidW, lstrlenW(progidW)); How about SysAllocString instead? > + CoTaskMemFree(progidW); > + return *p ? S_OK : E_OUTOFMEMORY; > } > > static HRESULT WINAPI ScriptControl_put_Language(IScriptControl *iface, BSTR language) > { > ScriptControl *This = impl_from_IScriptControl(iface); > - FIXME("(%p)->(%s)\n", This, debugstr_w(language)); > - return E_NOTIMPL; > + CLSID clsid; > + > + TRACE("(%p)->(%s)\n", This, debugstr_w(language)); > + > + if (language && FAILED(CLSIDFromProgID(language, &clsid))) > + return CTL_E_INVALIDPROPERTYVALUE; > + > + if (This->host) { > + IActiveScriptSite_Release(&This->host->IActiveScriptSite_iface); > + This->host = NULL; > + } This will leave zombie script host and engine. You should probably use release_script_engine here as well. > + > + if (!language) > + return S_OK; > + > + return init_script_host(&clsid, &This->host); > } > > static HRESULT WINAPI ScriptControl_get_State(IScriptControl *iface, ScriptControlStates *p) > @@ -1379,6 +1656,7 @@ static HRESULT WINAPI ScriptControl_CreateInstance(IClassFactory *iface, IUnknow > script_control->ref = 1; > script_control->site = NULL; > script_control->cp_list = NULL; > + script_control->host = NULL; > > ConnectionPoint_Init(&script_control->cp_scsource, script_control, &DIID_DScriptControlSource); > ConnectionPoint_Init(&script_control->cp_propnotif, script_control, &IID_IPropertyNotifySink); > Thanks, Jacek

1 0

Re: [PATCH 4/9] d3drm: Implement IDirect3DRMViewport*::Init. (v5)
by Aaryaman Vasishta July 15, 2016

July 15, 2016

On Fri, Jul 15, 2016 at 4:02 PM, Aaryaman Vasishta < jem456.vasishta(a)gmail.com> wrote: > struct d3drm_viewport > { > struct d3drm_object obj; > + IDirect3DRMFrame *camera; > IDirect3DRMViewport IDirect3DRMViewport_iface; > IDirect3DRMViewport2 IDirect3DRMViewport2_iface; > + IDirect3DViewport *d3d_viewport; > + IDirect3DMaterial *material; > IDirect3DRM *d3drm; > D3DVALUE back; > D3DVALUE front; > I've decided to keep the interface instead of using the struct directly, since it'll be easier to use the interface functions for transformations and other paramaters. Referencing the interface via the object for every function call seems a bit redundant. Also I think refcounts should be handled seamlessly this way. A reference to the background material created internally is also kept here. As Stefan had pointed out, the material is not AddRef'd since its handle is used instead, so it's probably safer to keep the reference to ourselves and release it when the object is destroyed. Cheers, Aaryaman

2 2

Re: [PATCH 4/4] d3drm: Implement IDirect3DRMDevice*::Add/DeleteDestroyCallback. (v2)
by Aaryaman Vasishta July 14, 2016

July 14, 2016

CC'ing to wine-devel again. On Mon, Jul 11, 2016 at 1:10 PM, Aaryaman Vasishta < jem456.vasishta(a)gmail.com> wrote: > > > On Mon, Jul 11, 2016 at 1:06 PM, Aaryaman Vasishta < > jem456.vasishta(a)gmail.com> wrote: > >> >> >> On Mon, Jul 11, 2016 at 12:05 PM, Stefan Dösinger < >> stefandoesinger(a)gmail.com> wrote: >> >>> -----BEGIN PGP SIGNED MESSAGE----- >>> Hash: SHA256 >>> >>> Am 2016-07-10 um 23:14 schrieb Aaryaman Vasishta: >>> > Anyways, the vtable qi tests tests written so far show that >>> > querying IDirect3DRMObject from a particular interface will return >>> > version 1 of that interface. So technically it all comes down to >>> > calling >>> > >>> IDirect3DRM<Interface_name>::AddDestroyCallback(&object->version1_vtable, >>> > ...);. In our implementation of this method (see d3drm_main.c), we >>> > have passed version 1 of the relevant interface while calling the >>> > destroy callbacks within d3drm_object_destroy. The tests seem to >>> > agree with that implementation so far. >>> >> Also, I may have written the example incorrectly. You're right about the > fact that calling device3->AddDestroyCallback will pass device3 into this > function. But internally this interface will be QI'd to IDirect3DRMObject, > and then passed as the first parameter to the destroy callback itself. > >

2 2

Re: [PATCH 05/12] d3drm: Implement IDirect3DRMDevice*::InitFromD3D.
by Stefan Dösinger July 14, 2016

July 14, 2016

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Hi, Patches 1-4 look good to me, once the fixes you mentioned and we talked about on IRC are implemented. Am 2016-07-07 um 14:52 schrieb Aaryaman Vasishta: > struct d3drm_frame > { > + struct d3drm_object obj; This seems unrelated. > + if (device->ddraw) > + hr = D3DRMERR_BADOBJECT; I guess you're not returning here right away because you want to addref d3drm later. Wouldn't it be better to move those AddRefs in front in this case? The QueryInterface / GetRenderTarget code below is unlikely to fail, if it does something is severely wrong (A d3d device without a render target???) and getting the reference leak right is the last of your worries. > device->width = desc.dwWidth; > device->height = desc.dwHeight; If I read the diff right this code will set width and height to 0 on an already initialized device. > static HRESULT WINAPI d3drm_device3_GetDirect3DDevice2(IDirect3DRMDevice3 *iface, IDirect3DDevice2 **d3d_device) > { > struct d3drm_device *device = impl_from_IDirect3DRMDevice3(iface); > + HRESULT hr; > > TRACE("iface %p, d3d_device %p.\n", iface, d3d_device); > > - IDirect3DDevice_QueryInterface(device->device, &IID_IDirect3DDevice2, (void**)d3d_device); > + if (FAILED(hr = IDirect3DDevice_QueryInterface(device->device, &IID_IDirect3DDevice2, (void**)d3d_device))) > + hr = D3DRMERR_BADOBJECT; > > - return D3DRM_OK; > + return hr; > } I think this change (like the device2_GetDirect3DDevice2 one) can be moved to a separate patch. > + hr = IDirect3DRMDevice_InitFromD3D(device1, d3d1, d3ddevice1); > + ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > + ref3 = get_refcount((IUnknown *)d3ddevice1); > + ok(ref3 > device_ref2, "Expected ref3 > device_ref2, got ref3 = %u, device_ref2 = %u.\n", ref3, device_ref2); > + ref3 = get_refcount((IUnknown *)d3d1); > + ok(ref3 > d3d_ref2, "Expected ref3 > d3d_ref2, got ref3 = %u, d3d_ref2 = %u.\n", ref3, d3d_ref2); You forgot to test the reference of d3drm1. > + hr = IDirect3DRMDevice_QueryInterface(device1, &IID_IDirect3DRMDevice2, (void **)&device2); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DRMDevice2 Interface (hr = %x).\n", hr); > + hr = IDirect3DRMDevice2_GetDirect3DDevice2(device2, &d3ddevice2); > + ok(SUCCEEDED(hr), "Expected hr == D3DRM_OK, got %#x.\n", hr); > + ok(d3ddevice2 == NULL, "Expected d3ddevice2 == NULL, got %p.\n", d3ddevice2); > + IDirect3DRMDevice2_Release(device2); You forgot d3ddevice2 = 0xdeadbeef here. > + hr = IDirect3DRM2_CreateObject(d3drm2, &CLSID_CDirect3DRMDevice, NULL, &IID_IDirect3DRMDevice2, (void **)&device2); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DRMDevice2 interface (hr = %#x).\n", hr); > + > + hr = IDirectDraw_QueryInterface(ddraw1, &IID_IDirect3D, (void **)&d3d1); > + ok(SUCCEEDED(hr), "Cannot get IDirect3D interface (hr = %x).\n", hr); > + if (SUCCEEDED(hr = IDirect3DDevice2_QueryInterface(d3ddevice2, &IID_IDirect3DDevice, (void **)&d3ddevice1))) > + { > + hr = IDirect3DRMDevice2_InitFromD3D(device2, d3d1, d3ddevice1); > + ok(hr == E_NOINTERFACE, "Expected hr == E_NOINTERFACE, got %#x.\n", hr); What happens if you QI IDirect3DRMDevice1 from IDirect3DRMDevice2 and call InitFromD3D on it? I would expect it to succeed because common decency suggests that CreateObject doesn't leave a history of which interface was requested and there's no CLSID_CDirect3DRMDevice2 or CLSID_CDirect3DRMDevice3. Still worth a check though I think. Cheers, Stefan -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXgmxrAAoJEN0/YqbEcdMw9dQP/RbovFFjwIdo5ho2BcxWi1Sm 68U7n+sX+HAPmHPc2W/fOK1scbPaQz84e3cHj+dsFClCUKNLUpZ1eGdJDu1u9Fgh ZJVC9hgw8rzC+IDGr5vdmuzzOj65mI7KslJX9CfD9tnXigLtlvPf3me8jnhjL51b Eq8N1rStZTh805gDBEnmr/oqgQFkRoHKgs5lLIWq2inAfF3dq0BiThBGpC+eq19H YcihqOJWgSXrbXQYtTPjFtjp3jqqFa32+/ZQufCEAELcYUjM2gG23+O/rT5sa4d2 QuQOIhdUnTR9G6WnoZXgxKwxtJTvTu02GKlk5gaIkpWFJ2O+mPJuiBmylLoucQ6V BkYkhORuT2WYKainbTx3qNJaxBPSa5/y4HWTpQhzYH0LryFBLkL8MiVjO+pGk7QM 4wHTeL4ZXmjtyYOs6qfuxNIEJyfDs0hU5850lWdVD6MLaj3JwZYRLedCJ9TwV7Jb Qz3JQIK6YPOgTSqehIGTwzxgcbvjjiLP2zalEV4HOIqKeEkodcBxVS0QiXSDbuAj gLhbqwlRMshT5Dtt1Umi2KUoGljFjh1rbe42cyVxXcoa72D+g9LQUb4d0hUP/Ppt 6lNhfq+Vf2W+FmdB2Hv/WWof7vBV59r/TwjjExrN00yRGCLNLF3BaK8lzjeJprGO H6J9OAsscEyzMNpB+l0W =exYP -----END PGP SIGNATURE-----

2 3

Re: Testbot 3D tests broken
by Patrick Rudolph July 14, 2016

July 14, 2016

On 2016-07-13 09:53 PM, Matteo Bruni wrote: > 2016-07-12 17:51 GMT+02:00 Patrick Rudolph <siro(a)das-labor.org>: >> Hi, >> can you have a look at the testbot, it doesn't run d3d8 and d3d9 tests. >> Other 3D tests might be affected, too. >> As you can see here it reports no test failures, but it skipped all >> tests: >> https://testbot.winehq.org/JobDetails.pl?Key=24151 >> https://testbot.winehq.org/JobDetails.pl?Key=24150 > > Not sure if you already got an answer on IRC or elsewhere. Anyway, the > tests are actually executed on OS versions with the WARP device > available, i.e. Win8 and newer. WARP device doesn't entirely match > "usual" hardware devices WRT test results and since we really only > care to match hardware behavior (but we still want the tests to pass > on the testbot) there are a number of hacks in the tests. > Should I don't care about pre Win8 test results ? Both, d3d8 and d3d9 games, are likely to be used with WinXp or Win7. Does it make sense to test them on Win8 or newer ? As likely every wine user is running wined3d on real hardware, shouldn't there be test systems that have real GPUs ? >> Regards, >> Patrick

2 1

Re: kernel32: Overhaul the handling of argv in set_process_name().
by Sebastian Lackner July 13, 2016

July 13, 2016

On 12.07.2016 06:44, Ken Thomases wrote: > This fixes several problems with the code: > > * The code had been assuming that the argument strings pointed to by the argv > array are contiguous iff certain process-name-setting functions are available. > This doesn't seem reliable. Instead, test if it's true and shift the strings > if so. Thanks for working on this, looks indeed like a nice improvement. > > However, setproctitle() is specifically documented as a preferred alternative > to the technique of overwriting the arg strings, so don't shift the strings > if that's available. Wouldn't it make sense to remove the directory component also in this case, to be consistent? By moving the code to the bottom all functions would be called in the same way. > > * Use the last path component, recognizing backslash as a path separator, for > setprogname() in addition to prctl(). First, setprogname() is documented as > searching for the last component itself, but it doesn't understand Windows- > style paths, so we need to help it. Second, on some platforms (e.g. macOS), > setprogname(), like prctl(), has a fairly small internal length limit (e.g. > 32 characters). So, concentrate on the most meaningful part of the path. > > * Remove argv[0] from argv whether or not there are any process-name-setting > functions available. This is necessary for the proper functioning of Wine, > so it must be done on all platforms. This part of the logic was lost with > commit 5a4576ee0. > > * Call all available process-name-setting functions instead of treating them > as mutually exclusive alternatives. This is also logic that was lost with > commit 5a4576ee0. > > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > --- > dlls/kernel32/process.c | 63 +++++++++++++++++++++++++++++-------------------- > 1 file changed, 37 insertions(+), 26 deletions(-) >

2 9

Re: [2/2] kernel32/tests: Add tests for GetThreadSelectorEntry.
by Marvin July 13, 2016

July 13, 2016

Hi, While running your changed tests on Windows, 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=24204 Your paranoid android. === w1064 (32 bit thread) === thread.c:1086: Test failed: got 0 with 5 (expected FALSE with ERROR_GEN_FAILURE)

2 1

Testbot 3D tests broken
by Patrick Rudolph July 13, 2016

July 13, 2016

Hi, can you have a look at the testbot, it doesn't run d3d8 and d3d9 tests. Other 3D tests might be affected, too. As you can see here it reports no test failures, but it skipped all tests: https://testbot.winehq.org/JobDetails.pl?Key=24151 https://testbot.winehq.org/JobDetails.pl?Key=24150 Regards, Patrick

2 1

Re: [PATCH v2 2/3] winex11.drv: avoid passing NULL to memcpy (clang)
by Alexandre Julliard July 13, 2016

July 13, 2016

Kirill Smirnov <kirill.k.smirnov(a)gmail.com> writes: > @@ -92,7 +92,8 @@ static void X11DRV_ImmSetInternalString(DWORD dwOffset, > ptr_new = CompositionString + byte_offset; > memmove(ptr_new + byte_length, ptr_new + byte_selection, > dwCompStringLength - byte_offset - byte_selection); > - memcpy(ptr_new, lpComp, byte_length); > + /* Do not pass NULL even if the length == 0 */ > + if (lpComp) memcpy(ptr_new, lpComp, byte_length); > dwCompStringLength += byte_expansion; Why would that be a problem? -- Alexandre Julliard julliard(a)winehq.org

3 3