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

wine 1.9.5 shared wow64 compile error
by Olivier F. R. Dierick March 6, 2016

March 6, 2016

Hello, I've updated my git tree with 1.9.5 and now I have this error while compiling 32bit Wine with the --with-wine64 option (in a shared wow64 setup): ---begin make output--- ... rm -f fonts && ln -s ../1.9.5_wow_64/fonts fonts rm: impossible de supprimer « fonts »: est un dossier make: *** [fonts] Erreur 1 ... ---end make output--- It says fonts is a directory and can't be removed. I do out-of-tree builds. The 32bit directory doesn't exists when I run configure. After running configure, the 32bit build tree is there and contains a fonts directory with a single Makefile in it. I had no such error with Wine 1.9.4 and I don't have it when compiling stand-alone 32bit Wine or the 64bit shared wow part. I did a regression test and found this commit to be the culprit: ---begin git bisect output--- dcc16c7cac1cbf93096b072fe2f7b32ad08d1fa5 is the first bad commit commit dcc16c7cac1cbf93096b072fe2f7b32ad08d1fa5 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Feb 29 15:06:42 2016 +0900 makefiles: Don't generate global rules in disabled directories. Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> ... ---end git bisect output--- I look forward to hearing from you soon -- Olivier F. R. Dierick o.dierick(a)piezo-forte.be

2 2

Wine-1.9.5 source code Cppcheck 1.69 scanning
by Игорь Чебаненко March 4, 2016

March 4, 2016

Source code scanned with Cppcheck 1.69. Yes,I know that in this scanning report I have messages called "noise messages",but I hope,that you will see real bugs,and will ingnore this type of messages.

1 0

Re: [PATCH 07/10] d3d10_1: Implement D3D10CreateDeviceAndSwapChain1().
by Henri Verbeet March 4, 2016

March 4, 2016

On 4 March 2016 at 11:43, Józef Kucia <jkucia(a)codeweavers.com> wrote: > It doesn't seem to be easy to declare D3D10CreateDeviceAndSwapChain1() > without using cpp_quote() because D3D10_DRIVER_TYPE is unknown to idl > (it's declared in d3d10misc.h). My knowledge about idl is limited > so maybe someone knows better. The easiest thing to do would be to use "enum D3D10_DRIVER_TYPE" instead of "D3D10_DRIVER_TYPE". I wouldn't be opposed to just using idl for d3d10misc.h either though. > +cpp_quote("HRESULT WINAPI D3D10CreateDeviceAndSwapChain1(IDXGIAdapter *,") > +cpp_quote(" D3D10_DRIVER_TYPE, HMODULE, UINT, D3D10_FEATURE_LEVEL1,") > +cpp_quote(" UINT, DXGI_SWAP_CHAIN_DESC *, IDXGISwapChain **, ID3D10Device1 **);") I'd prefer this with parameter names either way.

1 0

Re: [PATCH 1/2] user32: Store pointer to timer callback in SetTimer function instead of treating it as window procedure
by Alexandre Julliard March 4, 2016

March 4, 2016

Piotr Caban <piotr(a)codeweavers.com> writes: > I decided to leave 16-bit version of SetTimer as is for now. I haven't > seen any application running out of window procedures in this > case. This patch will change behavior of following 16-bit application: > SetTimer(hWnd, TIMER_ID, 1, tfunc); > GetMessage32(&msg, 0, 0, 0, TRUE); //get's WM_TIMER message > DispatchMessage32(&msg, TRUE); > Now this code will no longer call tfunc callback. According to my test > new behavior is correct. The more likely case is a 16-bit app retrieving a 32-bit timer and calling it through DispatchMessage16. How is this going to work with your proposed changes? -- Alexandre Julliard julliard(a)winehq.org

2 4

Re: [PATCH 2/2 v2] d3drm: Implement AddDestroyCallback/DeleteDestroyCallback for a viewport
by Henri Verbeet March 4, 2016

March 4, 2016

On 3 March 2016 at 09:10, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +HRESULT d3drm_object_delete_destroy_callback(struct d3drm_object *object, D3DRMOBJECTCALLBACK cb, void *ctx) > +{ > + struct destroy_callback *callback, *callback2; > + > + if (!cb) > + return D3DRMERR_BADVALUE; > + > + LIST_FOR_EACH_ENTRY_SAFE(callback, callback2, &object->destroy_callbacks, struct destroy_callback, entry) > + { > + if (callback->cb == cb && callback->ctx == ctx) > + { > + list_remove(&callback->entry); > + HeapFree(GetProcessHeap(), 0, callback); > + } > + } > + > + return D3DRM_OK; > +} I won't hold it against the patch (much), but is that correct? E.g. suppose you do something like the following: add cb1 add cb2 add cb3 add cb2 delete cb2 What would the sequence be that ends up getting called?

2 1

Re: [v3 6/7] d3dx9: implement state save and restore in effect.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- For your next submissions, can you please write a short changelog of the differences with the previous version(s) of the same patch? Ideally that goes after the '---'. > @@ -3493,18 +3501,34 @@ static HRESULT WINAPI ID3DXEffectImpl_Begin(ID3DXEffect *iface, UINT *passes, DW > if (flags & ~(D3DXFX_DONOTSAVESTATE | D3DXFX_DONOTSAVESAMPLERSTATE | D3DXFX_DONOTSAVESHADERSTATE)) > WARN("Invalid flags (%#x) specified.\n", flags); > > - if (This->manager || flags & D3DXFX_DONOTSAVESTATE) > + if (effect->manager || flags & D3DXFX_DONOTSAVESTATE) > { > TRACE("State capturing disabled.\n"); > } > else > { > - FIXME("State capturing not supported, yet!\n"); > + HRESULT hr; > + unsigned int i; > + > + if (!technique->saved_state) > + { > + hr = IDirect3DDevice9_BeginStateBlock(effect->device); > + if (FAILED(hr)) > + ERR("BeginStateBlock failed, hr %#x.\n", hr); > + for (i = 0; i < technique->pass_count; i++) > + d3dx9_apply_pass_states(effect, &technique->passes[i]); This is likely correct but testing it with a technique with at least 2 passes would be nice.

1 0

Re: [v3 5/7] d3dx9: implement sampler and texture effect states application.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/effect.c | 41 +++++++++++++++++++++++++++++++++++++++-- > dlls/d3dx9_36/tests/effect.c | 2 +- > 2 files changed, 40 insertions(+), 3 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index e1f01f2..6813c79 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -2681,7 +2681,8 @@ HRESULT d3dx_set_shader_const_state(IDirect3DDevice9 *device, enum SHADER_CONSTA > return D3D_OK; > } > > -static HRESULT d3dx9_apply_state(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass, struct d3dx_state *state) > +static HRESULT d3dx9_apply_state(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass, > + struct d3dx_state *state, int parent_index) I would have used an unsigned int here, flagging "none" with ~0u.

1 0

Re: [v3 4/7] d3dx9: implement shader constants effect states application.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > + UINT nelem; "unsigned int element_count;" would be better.

1 0

Re: [v3 3/7] d3dx9: implement light and material effect states application.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > + light_tbl[] = > + { > + {FIELD_OFFSET(D3DLIGHT9, Type), "LC_TYPE"}, > + {FIELD_OFFSET(D3DLIGHT9, Diffuse), "LT_DIFFUSE"}, > + {FIELD_OFFSET(D3DLIGHT9, Specular), "LT_SPECULAR"}, > + {FIELD_OFFSET(D3DLIGHT9, Ambient), "LT_AMBIENT"}, > + {FIELD_OFFSET(D3DLIGHT9, Position), "LT_POSITION"}, > + {FIELD_OFFSET(D3DLIGHT9, Direction), "LT_DIRECTION"}, > + {FIELD_OFFSET(D3DLIGHT9, Range), "LT_RANGE"}, > + {FIELD_OFFSET(D3DLIGHT9, Falloff), "LT_FALLOFF"}, > + {FIELD_OFFSET(D3DLIGHT9, Attenuation0), "LT_ATTENUATION0"}, > + {FIELD_OFFSET(D3DLIGHT9, Attenuation1), "LT_ATTENUATION1"}, > + {FIELD_OFFSET(D3DLIGHT9, Attenuation2), "LT_ATTENUATION2"}, > + {FIELD_OFFSET(D3DLIGHT9, Theta), "LT_THETA"}, > + {FIELD_OFFSET(D3DLIGHT9, Phi), "LT_PHI"} > + }; This is misindented. > + switch (op) > + { > + case LT_TYPE: > + TRACE("LT_TYPE %u.\n", *(D3DLIGHTTYPE *)value); > + light->Type = *(D3DLIGHTTYPE *)value; > + break; > + case LT_DIFFUSE: > + case LT_SPECULAR: > + case LT_AMBIENT: > + { > + D3DCOLORVALUE c = *(D3DCOLORVALUE *)value; > + > + TRACE("%s (%f %f %f %f).\n", light_tbl[op].name, c.r, c.g, c.b, c.a); > + *(D3DCOLORVALUE *)((char *)light + light_tbl[op].offset) = c; It's probably a bit nicer to cast to (BYTE *) instead of (char *). > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 81a7d12..3bcba72 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -2910,11 +2910,11 @@ static void test_effect_states(IDirect3DDevice9 *device) > } > > hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > if (hr == D3D_OK) > ok(bval, "Got result %u, expected TRUE.\n", bval); > hr = IDirect3DDevice9_GetLight(device, 2, &light); > - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > if (hr == D3D_OK) > ok(light.Position.x == 4.0f && light.Position.y == 5.0f && light.Position.z == 6.0f, > "Got unexpected light position (%f, %f, %f).\n", light.Position.x, light.Position.y, light.Position.z); > @@ -2950,9 +2950,9 @@ static void test_effect_states(IDirect3DDevice9 *device) > todo_wine ok(!memcmp(mat.m, test_mat.m, sizeof(mat)), "World matrix not restored.\n"); > > hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > if (hr == D3D_OK) > - ok(!bval, "Got result %u, expected 0.\n", bval); > + todo_wine ok(!bval, "Got result %u, expected 0.\n", bval); It would be nice if the test also checked the material.

1 0

Re: [v3 1/7] d3dx9: add test for basic effect state management.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > + > + if (effect) > + effect->lpVtbl->Release(effect); > +} The if is unnecessary, the effect creation should never fail (and if it failed you would have already crashed long before that).

1 0