March 2016 - Wine-devel - list.winehq.org

Re: [v6] shell32: fix two bugs in IQueryAssocations
by Theodore Dubois 16 Mar '16

16 Mar '16

OK. I'll resend with the changes suggested. ~Theodore On Mar 15, 2016, at 07:03 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: Theodore Dubois <tblodt(a)icloud.com> writes: The reason I need the macro is: If I wrote ok(getstring_test(whatever)), first the ok macro would call winetest_set_location with the current line number, and then it would call getstring_test. The ok macros in getstring_test would call winetest_set_location and overwrite the location of the actual test. Then if getstring_test returns with a failure, the error message has the wrong line number. The only way to fix this is to call getstring_test on one line and call ok on the next line. There's no reason to call ok on the getstring_test result, since getstring_test would have already reported the error. If you want to know which caller triggered it, you can pass a line number or some string description to getstring_test. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v6] shell32: fix two bugs in IQueryAssocations
by Alexandre Julliard 16 Mar '16

16 Mar '16

Theodore Dubois <tblodt(a)icloud.com> writes: > + > + /* This macro is necessary because of what the ok macro expands to. Failures would report the wrong line number. */ > +#define TEST_GET_STRING(a,p,s,es) \ > + getstring_test_result = getstring_test(a,p,s,es);\ > + ok(getstring_test_result, "\n"); It would be better to avoid the macro and write the tests explicitly, with meaningful error messages. Also, any chance to split the patch? -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH 2/2] d3drm: Partially Implement IDirect3DRM*::LoadTexture.
by Aaryaman Vasishta 15 Mar '16

15 Mar '16

I forgot to remove some trailing whitespaces - sorry about that! I will send another patch without them if needed.

1 1

Re: [PATCH 2/2] d3drm: Partially Implement IDirect3DRM*::LoadTexture.
by Marvin 15 Mar '16

15 Mar '16

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=21458 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [v3 resend 3/6] d3dx9: implement fxlc constants (expressions) in effect.
by Matteo Bruni 15 Mar '16

15 Mar '16

2016-03-10 19:50 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > +static float regstore_get_float(struct d3dx_regstore *rs, unsigned int table, unsigned int offset) > +{ > + BYTE *p; > + > + p = (BYTE *)rs->tables[table] + table_info[table].value_size * offset; > + switch (table_info[table].type) > + { > + case PRES_VT_FLOAT : return *(float *)p; > + case PRES_VT_DOUBLE: return *(double *)p; > + case PRES_VT_INT : return *(int *)p; > + case PRES_VT_BOOL : return *(BOOL *)p ? 1.0f : -0.0f; I guess that -0.0f comes from some test? > + /* no EOL in WARN to dump arg in the same line */ > + WARN("Using uninitialized input "); > + dump_arg(rs, opr, comp); > + TRACE(".\n"); > + dump_ins(rs, ins); You can probably do without the comment, it's pretty clear anyway. > +#define ARGS_ARRAY_SIZE 8 > +static HRESULT execute_preshader(struct d3dx_preshader *pres) > +{ > + unsigned int i, j, k; > + float args[ARGS_ARRAY_SIZE]; > + float res; > + > + for (i = 0; i < pres->nins; i++) > + { > + struct d3dx_pres_ins *ins; > + struct op_info *oi; > + > + ins = &pres->ins[i]; > + oi = &pres_op_info[ins->op]; > + if (oi->func_all_comps) > + { > + if (ins->ninp_args * ins->ncomps > ARGS_ARRAY_SIZE) > + { > + FIXME("Too much arguments (%u) for one instruction.\n", ins->ninp_args * ins->ncomps); That should be "Too many". > +HRESULT d3dx_param_eval_set_shader_constants(struct IDirect3DDevice9 *device, struct d3dx_param_eval *peval) This function is unused in this patch. Please move it to the first patch requiring it (I think that's patch 6/6). BTW, splitting patch 6/6 would be nice although I don't have any suggestions on how to do that in a sensible manner... > +HRESULT d3dx_param_eval_get_shader_parameters(struct d3dx_param_eval *peval, struct d3dx_parameter ***param, > + unsigned int *nparam, D3DXCONSTANT_DESC **cdesc) > +{ > + *nparam = peval->shader_inputs.ninputs; > + *param = peval->shader_inputs.inputs_param; > + *cdesc = peval->shader_inputs.inputs; > + return D3D_OK; > +} You don't use this function in this patch either. Also it doesn't seem useful, it isn't an improvement to just assigning those three pointers directly in the caller IMO.

2 2

Re: [v3 1/6] d3dx9: add test for preshader in effect.
by Matteo Bruni 15 Mar '16

15 Mar '16

2016-03-10 15:23 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- Looks much better, thanks. I still have a few comments though. > + for (i = 0; i < 256; i++) > + { > + hr = IDirect3DDevice9_SetVertexShaderConstantF(device, i, > + (const float *)&fvect_empty, 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + } > + for (i = 0; i < TEST_EFFECT_PRES_NFLOATP; i++) > + { > + hr = IDirect3DDevice9_SetPixelShaderConstantF(device, i, > + (const float *)&fvect_empty, 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + } I know it was me who suggested to clear all the 256 float shader constants but it looks a bit weird if the same doesn't happen for the other constant types. It's also not very useful if those additional constants aren't checked later on. I would change the initialization of the other constant types in the same way (maybe with a define or something instead of the plain number?) and then check that those are not modified by BeginPass(). > + bval = FALSE; > + for (i = 0; i < TEST_EFFECT_PRES_NBOOL; i++) > + { > + hr = IDirect3DDevice9_SetPixelShaderConstantB(device, i, &bval, 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + } > + > + hr = effect->lpVtbl->Begin(effect, &npasses, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + par = effect->lpVtbl->GetParameterByName(effect, NULL, "g_Pos2"); > + ok(par != NULL, "GetParameterByName failed.\n"); > + > + hr = effect->lpVtbl->SetVector(effect, par, &fvect1); > + ok(hr == D3D_OK, "SetFloatArray failed, hr %#x.\n", hr); This message is still wrong. > + > + hr = effect->lpVtbl->BeginPass(effect, 0); > + todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShaderConstantF(device, 0, (float *)fdata, TEST_EFFECT_PRES_NFLOATV); We usually avoid the cast in these cases with something like "&fdata->x". > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + todo_wine ok(!memcmp(fdata, test_effect_preshader_fconstsv, sizeof(test_effect_preshader_fconstsv)), > + "Vertex shader float constants do not match.\n"); > + hr = IDirect3DDevice9_GetPixelShaderConstantF(device, 0, (float *)fdata, TEST_EFFECT_PRES_NFLOATP); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + todo_wine ok(!memcmp(fdata, test_effect_preshader_fconstsp, sizeof(test_effect_preshader_fconstsp)), > + "Pixel shader float constants do not match.\n"); > + > + hr = IDirect3DDevice9_GetPixelShaderConstantB(device, 0, bdata, TEST_EFFECT_PRES_NBOOL); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + for (i = 0; i < TEST_EFFECT_PRES_NBOOL; i++) > + todo_wine_if(!bdata[i] != !test_effect_preshader_bconsts[i]) > + ok(!bdata[i] == !test_effect_preshader_bconsts[i], > + "Pixel shader boolean constants do not match.\n"); > + > + for (i = 0; i < TEST_EFFECT_PRES_NOPTESTS; i++) > + { > + unsigned int *v; > + > + hr = IDirect3DDevice9_GetLight(device, i % 8, &light); > + v = i < 8 ? (unsigned int *)&light.Diffuse : (unsigned int *)&light.Ambient; > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + for (j = 0; j < 4; j++) > + todo_wine_if(test_effect_preshader_op_results[i].todo[j] & 1) > + ok(v[j] == test_effect_preshader_op_results[i].result[j] || > + ((test_effect_preshader_op_results[i].todo[j] & 2) && > + broken(v[j] != test_effect_preshader_op_results[i].result[j])), > + "Operation %s, component %u, expected %#x, got %#x.\n", > + test_effect_preshader_op_results[i].comment, j, > + test_effect_preshader_op_results[i].result[j], v[j]); The indentation is pretty weird here. Even if you keep the ok on the same column of the todo_wine_if (to make its removal a few patches later nicer) the continuations of the ok condition should be indented, matching the other continuation lines. FWIW in newer d3d code we usually put the operators at the start of the next line (instead of at the end of the previous line) in line continuations. Also we prefer preincrement to postincrement of typical 'for' induction variables. > + ok(!!vshader, "Got NULL vshader.\n"); This double negation is unnecessary. > + > + hr = IDirect3DVertexShader9_GetFunction(vshader, NULL, &byte_code_size); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + byte_code = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, byte_code_size); > + hr = IDirect3DVertexShader9_GetFunction(vshader, byte_code, &byte_code_size); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + ok(byte_code_size > 1, "Got unexpected byte code size %u.\n", byte_code_size); > + todo_wine ok(!memcmp(byte_code, > + &test_effect_preshader_effect_blob[TEST_EFFECT_PRESHADER_VSHADER_POS + > + 1 * TEST_EFFECT_PRESHADER_VSHADER_LEN], byte_code_size), > + "Incorrect shader selected.\n"); > + HeapFree(GetProcessHeap(), 0, byte_code); > + IDirect3DVertexShader9_Release(vshader); > + > + hr = IDirect3DDevice9_SetVertexShader(device, NULL); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetVector(effect, par, &fvect1); > + ok(hr == D3D_OK, "SetFloatArray failed, hr %#x.\n", hr); > + hr = effect->lpVtbl->CommitChanges(effect); > + todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + ok(!vshader, "Incorrect shader selected.\n"); > + if (vshader) > + IDirect3DVertexShader9_Release(vshader); I think you can remove this release too.

2 2

Re: [PATCH v2 08/12] wined3d: Implement stencil clear in wined3d_device_clear_rendertarget_view().
by Henri Verbeet 15 Mar '16

15 Mar '16

On 15 March 2016 at 10:52, Józef Kucia <jkucia(a)codeweavers.com> wrote: > static HRESULT cpu_blit_depth_fill(struct wined3d_device *device, > - struct wined3d_rendertarget_view *view, const RECT *rect, float depth) > + struct wined3d_rendertarget_view *view, const RECT *rect, DWORD clear_flags, Minor typo.

1 0

Re: [PATCH 2/3] winex11.drv: Fix handling of CF_DIBV5 clipboard data.
by Alexandre Julliard 15 Mar '16

15 Mar '16

Charles Davis <cdavis5x(a)gmail.com> writes: > @@ -1149,9 +1154,7 @@ static BOOL X11DRV_CLIPBOARD_RenderSynthesizedBitmap(Display *display) > lpbmih = GlobalLock(lpSource->hData); > if (lpbmih) > { > - offset = sizeof(BITMAPINFOHEADER) > - + ((lpbmih->biBitCount <= 8) ? (sizeof(RGBQUAD) * > - (1 << lpbmih->biBitCount)) : 0); > + offset = lpbmih->biSize - lpbmih->biSizeImage; This doesn't look right. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: dinput: Support IUnknown in the IDirectInputEffect COM object. (2nd try)
by Huw Davies 15 Mar '16

15 Mar '16

On Tue, Mar 15, 2016 at 11:43:58AM +0100, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> > --- > v2: Use IsEqualIID consistently. Thanks Huw! > > And of course again not even compile tested as I don't have a Mac. Yes, > I need to set me up with a cross compile for that. > gcc -m32 -c -o joystick_osx.o joystick_osx.c -I. -I../../include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wstrict-prototypes \ -Wwrite-strings -Wpointer-arith -gdwarf-2 -fno-omit-frame-pointer -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 joystick_osx.c: In function ‘effect_QueryInterface’: joystick_osx.c:1477: error: ‘riid’ undeclared (first use in this function) joystick_osx.c:1477: error: (Each undeclared identifier is reported only once joystick_osx.c:1477: error: for each function it appears in.) make: *** [joystick_osx.o] Error 1 > > dlls/dinput/joystick_osx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/dinput/joystick_osx.c b/dlls/dinput/joystick_osx.c > index 6a8926d..19ca070 100644 > --- a/dlls/dinput/joystick_osx.c > +++ b/dlls/dinput/joystick_osx.c > @@ -1474,7 +1474,7 @@ static HRESULT WINAPI effect_QueryInterface(IDirectInputEffect *iface, > > TRACE("%p %s %p\n", This, debugstr_guid(guid), out); > > - if(IsEqualIID(guid, &IID_IDirectInputEffect)){ > + if(IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(guid, &IID_IDirectInputEffect)){ > *out = iface; > IDirectInputEffect_AddRef(iface); > return S_OK; > -- > 2.4.3 > >

1 0

Re: dinput: Support IUnknown in the IDirectInputEffect COM object.
by Huw Davies 15 Mar '16

15 Mar '16

On Tue, Mar 15, 2016 at 10:37:16AM +0100, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> > --- > dlls/dinput/joystick_osx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/dinput/joystick_osx.c b/dlls/dinput/joystick_osx.c > index 6a8926d..19ca070 100644 > --- a/dlls/dinput/joystick_osx.c > +++ b/dlls/dinput/joystick_osx.c > @@ -1474,7 +1474,7 @@ static HRESULT WINAPI effect_QueryInterface(IDirectInputEffect *iface, > > TRACE("%p %s %p\n", This, debugstr_guid(guid), out); > > - if(IsEqualIID(guid, &IID_IDirectInputEffect)){ > + if(IsEqualGUID(riid, &IID_IUnknown) || IsEqualIID(guid, &IID_IDirectInputEffect)){ It looks a bit strange to mix IsEqualGUID and IsEqualIID. Huw.

2 3