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

Re: [v3 1/6] d3dx9: add test for preshader in effect.
by Matteo Bruni March 15, 2016

March 15, 2016

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 March 15, 2016

March 15, 2016

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 March 15, 2016

March 15, 2016

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 March 15, 2016

March 15, 2016

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 March 15, 2016

March 15, 2016

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

GSoC Idea: Tools - Winetest Scripting Interface
by Zhenbo Li March 15, 2016

March 15, 2016

Hello, I'm Zhenbo Li. I participated in GSoC twice. This year, I want to work on "Tools - Winetest Scripting Interface". After "make test", checking the output in stdout manually is horrible. As ok() macro is used quite widely, my patches should cause no surprise. That is to say, other developers need to read neither my patches nor the changelog to continue their jobs, although ok() macro and other tools for test have been changed. I think there will be two steps for the goal: 1. ok() macro ===================== int winetest_vok( int condition, const char *msg, __winetest_va_list args ) { tls_data* data=get_tls_data(); if (data->todo_level){...} else { if (!condition) { printf( "%s:%d: Test failed: ", data->current_file, data->current_line ); vprintf(msg, args); InterlockedIncrement(&failures); return 0; } else{.....} } } Instead of printing all the data to stderr, I think we can write a wrapper static void test_print(const char *s) { if(want_print_to_file) fprintf(file_handle, s); printf(s); } It needs some work to make sure that the file_handle is opened/closed successfully. With that, we can record the failed tests to a file, and developers don't need to find them in stdout anymore. 2. compare the results ======================= On the idea page, it says that any scripting language is acceptable. I prefer Python3. To make the code simple, I think editing runtest script is the best solution. Currently, our logic in Makefile is htmllocation.ok: ../../../tools/runtest $(RUNTESTFLAGS) -T ../../.. -M mshtml.dll -p mshtml_test.exe.so htmllocation && touch $@ And in runtest, we have exec $WINETEST_WRAPPER "$topobjdir/wine" "$program" "$@" I don't want to touch Makefile. Instead, I'm going to add some options for runtest. For example, one can use ../../../tools/runtest $(RUNTESTFLAGS) -new_option expected_errors_generated before -T ../../.. -M mshtml.dll -p mshtml_test.exe.so htmllocation And in runtest script, we can expected_errors_generated and new_errors_got, then tell that whether the test result has been changed. It should be easy for developers to write a script for git bisect or other circumstance. Problems ========= This approach seems to be quite simple, and there are some conditions can't be handled. For example void test_foo(sturct Data *d){ ret = foo(d); ok(ret, "foo(%s) is %d\n", data_to_str(d), ret); } struct Data list[]={a,b,c,d,e}; void test(){ for(i in list) test_foo(i); } I think we should not do too much "smart deduce" here, and don't hide what's happening to other developers. Maybe we can add another option for runtest script: --scrict=false This is my humble idea for Wine's GSoC 2016. If this project goes well, working on wine test suit will be much more satisfying(although still boring). I appreciate for your comment. With you a happy weekend. -- Have a nice day! Zhenbo Li

6 15

Re: winemac: Call the EnumDisplayMonitors callback using a wrapper.
by Henri Verbeet March 15, 2016

March 15, 2016

On 15 March 2016 at 05:50, Ken Thomases <ken(a)codeweavers.com> wrote: > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > --- > For <https://bugs.winehq.org/show_bug.cgi?id=40297>. > > dlls/winemac.drv/display.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > Is this really specific to winemac, or would it be more appropriate to add the wrapper in user32?

1 0

Re: winemac: Call the EnumDisplayMonitors callback using a wrapper.
by Ken Thomases March 15, 2016

March 15, 2016

On Mar 14, 2016, at 11:50 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > --- > For <https://bugs.winehq.org/show_bug.cgi?id=40297>. > > dlls/winemac.drv/display.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) Dmitry pointed out that this version misaligns the stack, so ignore it. I'll submit another try tomorrow. -Ken

1 0

Re: [PATCH] crypt32/tests: Accept Windows 10 results.
by Alexandre Julliard March 15, 2016

March 15, 2016

Detlef Riekenberg <wine.dev(a)web.de> writes: > @@ -336,11 +336,13 @@ static void testStringToBinaryA(void) > /* Bad strings */ > for (i = 0; i < sizeof(badStrings) / sizeof(badStrings[0]); i++) > { > - bufLen = 0; > + bufLen = 0xdeadbeef; > + SetLastError(0xdeadbeef); > + /* This succeeds on Windows 10 with our CRYPT_STRING_BASE64 test data */ > ret = pCryptStringToBinaryA(badStrings[i].str, 0, badStrings[i].format, > NULL, &bufLen, NULL, NULL); > - ok(!ret && GetLastError() == ERROR_INVALID_DATA, > - "Expected ERROR_INVALID_DATA, got ret=%d le=%u\n", ret, GetLastError()); > + ok((!ret && GetLastError() == ERROR_INVALID_DATA) || broken(ret), > + "%d: Expected FALSE with ERROR_INVALID_DATA, got ret=%d le=%u len=%d\n", i, ret, GetLastError(), bufLen); I'm not convinced this should be considered broken. It would be worth testing some other truly bad strings, and the CRYPT_STRING_STRICT flag also. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: msrle32: Use the correct number of colors when biClrUsed = 0 (try 2)
by Alexandre Julliard March 15, 2016

March 15, 2016

Bruno Jesus <00cpxxx(a)gmail.com> writes: > @@ -1647,27 +1655,33 @@ static LRESULT DecompressBegin(CodecInfo *pi, LPCBITMAPINFOHEADER lpbiIn, > > if (lpbiIn->biCompression != BI_RGB) > { > + int colors; > + > + if (lpbiIn->biClrUsed == 0) > + colors = 1 << lpbiIn->biBitCount; > + else > + colors = lpbiIn->biClrUsed; I don't think you want to do that for biBitCount > 8. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH] winex11: Don't create a temporary context each time X11DRV_wglBindTexImageARB is called.
by Matteo Bruni March 15, 2016

March 15, 2016

2016-03-12 14:57 GMT+01:00 Miklós Máté <mtmkls(a)gmail.com>: > Theoretically wglBindTexImageARB is supposed to be a fast > render-to-texture method, and creating a new context is anything but fast. > > This fixes horrible performance in Star Wars > Knights of the Old Republic when post-process effects are enabled. > > v2: fix context caching, fix destructor, fix comments > v3: better handling of context delete > > Signed-off-by: Miklós Máté <mtmkls(a)gmail.com> Thank you for following up after the review comments. Unfortunately I think there is one more (and hopefully last) bit which still needs addressing: > @@ -2301,6 +2314,7 @@ static struct wgl_pbuffer *X11DRV_wglCreatePbufferARB( HDC hdc, int iPixelFormat > SetLastError(ERROR_NO_SYSTEM_RESOURCES); > goto create_failed; /* unexpected error */ > } > + list_add_head( &pbuffer_list, &object->entry ); > TRACE("->(%p)\n", object); > return object; This needs to be done inside the critical section, so please put EnterCriticalSection() + LeaveCriticalSection() around it. FTR the wglCreateContext and wglCreateContextAttribsARB implementation functions indirectly acquire and release the critical section via get_gl_drawable() / release_gl_drawable(). It looks good to me otherwise.

1 0

Re: [v2 6/6] d3dx9: imlement setting named shader constants in effect.
by Matteo Bruni March 15, 2016

March 15, 2016

2016-03-10 14:51 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > + unsigned i, j; We usually avoid the bare "unsigned". > + for (i = 0; i < parameters_count; i++) > + if (params[i] && params[i]->class == D3DXPC_OBJECT && params[i]->type == D3DXPT_SAMPLER) > + { > + struct d3dx_sampler *sampler; > + > + sampler = (struct d3dx_sampler *)params[i]->data; > + TRACE("sampler %s, register index %u, state count %u.\n", params[i]->name, > + cdesc[i].RegisterIndex, sampler->state_count); > + for (j = 0; j < sampler->state_count; j++) > + { > + hr = d3dx9_apply_state(effect, pass, &sampler->states[j], > + cdesc[i].RegisterIndex + (vs ? D3DVERTEXTEXTURESAMPLER0 : 0)); > + if (FAILED(hr)) > + ret = hr; > + } > + } It's better to add brackets to the for, for greater clarity.

1 0

Re: [2/2] user32: Skip menu loop for empty popup menus.
by Marvin March 14, 2016

March 14, 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=21431 Your paranoid android. === w864 (32 bit msg) === msg.c:6417: Test failed: Update region shouldn't be empty msg.c:6651: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6653: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different === w1064 (32 bit msg) === msg.c:6417: Test failed: Update region shouldn't be empty msg.c:6651: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6653: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different === w1064 (64 bit msg) === msg.c:6417: Test failed: Update region shouldn't be empty msg.c:6651: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6653: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different

1 0

Re: [PATCH 2/2] d3d11/tests: Port swapchain_flip to d3d11.
by Henri Verbeet March 14, 2016

March 14, 2016

On 13 March 2016 at 19:08, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > --- > > I am not using create_test_context in this test because it would introduce > asymetry in how back buffer 0 is created that would make the test more > difficult to read in my opinion. It would probably be worth it, mostly because you also get to use draw_quad() that way. We could also consider making the "backbuffer" and "backbuffer_rtv" members arrays. For the moment it's probably more important that it matches the d3d10core test though. > > Signed-off-by: Stefan Dösinger <stefandoesinger(a)gmx.at> If you put the Signed-off-by here, it won't be put in the commit message.

2 2

Re: [PATCH 1/2] windowscodecs: Stub IWICMetadataQueryReader
by Vincent Povirk March 14, 2016

March 14, 2016

You're referencing the block reader in MetadataQueryReader_CreateInstance but not releasing it in mqr_Release.

1 0

Re: [PATCH 1/8] wined3d: Introduce wined3d_rendertarget_view_clear().
by Henri Verbeet March 14, 2016

March 14, 2016

On 14 March 2016 at 00:03, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/wined3d/device.c | 18 +----------------- > dlls/wined3d/surface.c | 22 ++-------------------- > dlls/wined3d/view.c | 40 ++++++++++++++++++++++++++++++++++++++++ > dlls/wined3d/wined3d_private.h | 3 +++ > 4 files changed, 46 insertions(+), 37 deletions(-) > I'm not sure how I feel about this change. There doesn't seem to be a particularly compelling reason to change this, although I suppose there's no strong reason not to either. I guess it depends on where you're going with this.

2 2

Re: [PATCH 2/2] ole32: Fix refcounting of IObjContext per-thread instance
by Huw Davies March 14, 2016

March 14, 2016

On Mon, Mar 14, 2016 at 08:10:00AM +0300, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/ole32/compobj.c | 57 +++++++++++++++++++--------------------------- > dlls/ole32/tests/compobj.c | 46 ++++++++++++++++++++++--------------- > 2 files changed, 51 insertions(+), 52 deletions(-) > > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c > index b095aeb..1081f98 100644 > --- a/dlls/ole32/compobj.c > +++ b/dlls/ole32/compobj.c > @@ -4683,10 +4683,13 @@ static ULONG Context_AddRef(Context *This) > > static ULONG Context_Release(Context *This) > { > - ULONG refs = InterlockedDecrement(&This->refs); > - if (!refs) > + if (!This->refs) > + { Since this is non-standard it needs a comment. Probably just referring to the constructor below. > HeapFree(GetProcessHeap(), 0, This); > - return refs; > + return 0; > + } > + > + return InterlockedDecrement(&This->refs); > } > > static HRESULT WINAPI Context_CTI_QueryInterface(IComThreadingInfo *iface, REFIID riid, LPVOID *ppv) > @@ -4924,39 +4927,19 @@ static const IObjContextVtbl Context_Object_Vtbl = > */ > HRESULT WINAPI CoGetObjectContext(REFIID riid, void **ppv) > { > - APARTMENT *apt = COM_CurrentApt(); > - Context *context; > + IObjContext *context; > HRESULT hr; > > TRACE("(%s, %p)\n", debugstr_guid(riid), ppv); > > *ppv = NULL; > - if (!apt) > - { > - if (!(apt = apartment_find_multi_threaded())) > - { > - ERR("apartment not initialised\n"); > - return CO_E_NOTINITIALIZED; > - } > - apartment_release(apt); > - } > - > - context = HeapAlloc(GetProcessHeap(), 0, sizeof(*context)); > - if (!context) > - return E_OUTOFMEMORY; > - > - context->IComThreadingInfo_iface.lpVtbl = &Context_Threading_Vtbl; > - context->IContextCallback_iface.lpVtbl = &Context_Callback_Vtbl; > - context->IObjContext_iface.lpVtbl = &Context_Object_Vtbl; > - context->refs = 1; > - > - hr = IComThreadingInfo_QueryInterface(&context->IComThreadingInfo_iface, riid, ppv); > - IComThreadingInfo_Release(&context->IComThreadingInfo_iface); > + hr = CoGetContextToken((ULONG_PTR*)&context); > + if (FAILED(hr)) > + return hr; > > - return hr; > + return IObjContext_QueryInterface(context, riid, ppv); > } > > - > /*********************************************************************** > * CoGetContextToken [OLE32.@] > */ > @@ -4985,16 +4968,22 @@ HRESULT WINAPI CoGetContextToken( ULONG_PTR *token ) > > if (!info->context_token) > { > - HRESULT hr; > - IObjContext *ctx; > + Context *context; > + > + context = HeapAlloc(GetProcessHeap(), 0, sizeof(*context)); > + if (!context) > + return E_OUTOFMEMORY; > + > + context->IComThreadingInfo_iface.lpVtbl = &Context_Threading_Vtbl; > + context->IContextCallback_iface.lpVtbl = &Context_Callback_Vtbl; > + context->IObjContext_iface.lpVtbl = &Context_Object_Vtbl; Add a comment here stating that context_token does not take a ref. > + context->refs = 0; > > - hr = CoGetObjectContext(&IID_IObjContext, (void **)&ctx); > - if (FAILED(hr)) return hr; > - info->context_token = ctx; > + info->context_token = &context->IObjContext_iface; > } > > *token = (ULONG_PTR)info->context_token; > - TRACE("apt->context_token=%p\n", info->context_token); > + TRACE("context_token=%p\n", info->context_token); > > return S_OK; > } I'm wondering whether this patch could be split since it's doing rather more than fixing a ref counting issue. Could you move the constructor to CoGetContextToken first and then fix the refs, or would that mess up the tests? Huw.

2 2

Re: [PATCH 6/8] d3d10core/tests: Add test for ClearDepthStencilView().
by Henri Verbeet March 14, 2016

March 14, 2016

On 14 March 2016 at 00:03, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +static float get_readback_float(struct texture_readback *rb, unsigned int x, unsigned int y) > +{ > + return rb->texture > + ? ((float *)rb->mapped_texture.pData)[rb->mapped_texture.RowPitch * y / sizeof(float) + x] : NAN; > +} Can the texture ever legitimately be NULL?

2 1

Re: [PATCH 2/8] wined3d: Implement depth clear in wined3d_device_clear_rendertarget_view().
by Henri Verbeet March 14, 2016

March 14, 2016

On 14 March 2016 at 00:03, Józef Kucia <jkucia(a)codeweavers.com> wrote: > @@ -4100,7 +4110,13 @@ HRESULT CDECL wined3d_device_clear_rendertarget_view(struct wined3d_device *devi > rect = &r; > } > > - return wined3d_rendertarget_view_clear(view, rect, WINED3DCLEAR_TARGET, color, 0.0f, 0); > + if (!(flags & WINED3DCLEAR_TARGET) == !(flags & (WINED3DCLEAR_ZBUFFER | WINED3DCLEAR_STENCIL))) > + { > + ERR("Color and depth/stencil clears are mutually exclusive.\n"); > + return WINED3DERR_INVALIDCALL; > + } You may not need this. The documentation suggests depth/stencil views can only be created on depth formats, and I'd be inclined to believe it. IIRC it doesn't say much about creating rendertarget views on depth/stencil formats, but I wouldn't be all that surprised if that's not allowed either. In that case you'd just need to check the flags against the format capabilities. > +const char *debug_color(const struct wined3d_color *color) > +{ > + if (!color) > + return "(null)"; > + return wine_dbg_sprintf("{%.8e, %.8e, %.8e, %.8e}", > + color->r, color->g, color->b, color->a); > +} This is fine, but might as well be a separate change. There are a couple of other places where we trace wined3d_color values as well, the easiest way to find them is probably to look for the format string.

2 1

Re: d3drm: Use debugstr_a to print a string that can be NULL.
by Henri Verbeet March 14, 2016

March 14, 2016

On 14 March 2016 at 10:36, Michael Stefaniuc <mstefani(a)redhat.de> wrote: > - TRACE("Mesh name is '%s'\n", This->name ? This->name : ""); > + TRACE("Mesh name is '%s'\n", debugstr_a(This->name)); You should remove the quotes, since debugstr_a() also adds quotes. Note that just being external data is enough reason to use debugstr_a(), it doesn't matter much if it can be NULL or not in that regard.

2 1

Re: [PATCH] Read operation's type explicitly as QStringList
by Nikolay Sivov March 14, 2016

March 14, 2016

On 13.03.2016 19:12, Łukasz Wojniłowicz wrote: > Fixes bug #360129 > --- > kmymoney/plugins/csvimport/investprocessing.cpp | 22 ++++++++-------------- > 1 file changed, 8 insertions(+), 14 deletions(-) > Hi, Łukasz. Just pointing out so this patch is not lost by accident - wrong mailing list. You aimed for https://mail.kde.org/pipermail/kmymoney-devel/ probably or some list next to it.

1 0

Re: [PATCH] kernel32/tests: Skip ResolveDelayLoadedAPI tests on Win10+
by Nikolay Sivov March 13, 2016

March 13, 2016

On 13.03.2016 19:42, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > dlls/kernel32/tests/loader.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c > index c35cf25..30cc2e4 100644 > --- a/dlls/kernel32/tests/loader.c > +++ b/dlls/kernel32/tests/loader.c > @@ -2728,6 +2728,11 @@ static void test_ResolveDelayLoadedAPI(void) > > cb_count = 0; > ret = pResolveDelayLoadedAPI(hlib, delaydir, failuredllhook, NULL, &itda[i], 0); > + if (!ret && td[i].succeeds) > + { > + win_skip("Skipping broken ResolveDelayLoadedAPI implementation\n"); > + goto cleanup; > + } > if (td[i].succeeds) > { > ok(ret != NULL, "Test %u: ResolveDelayLoadedAPI failed\n", i); > @@ -2745,6 +2750,7 @@ static void test_ResolveDelayLoadedAPI(void) > delaydir++; > } > > +cleanup: > FreeLibrary(hlib); > trace("deleting %s\n", dll_name); > DeleteFileA(dll_name); > So what's exactly wrong with it on Win10? Silencing failures does not seem useful if actual issue was not investigated.

1 0

Re: [PATCH] d3d9/tests Accept Windows 10 result
by Stefan Dösinger March 13, 2016

March 13, 2016

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-03-13 um 17:04 schrieb Detlef Riekenberg: > Windows 10 is returning D3D_OK, so the recent behavior seems to be > better as before. That's why I used D3D_OK as expected result, > marked the old S_PRESENT_MODE_CHANGED as broken and removed the > wine_todo I think the new behavior is more broken than the old one. "It needs a present call to pick up a mode change" - If my memory serves me right the issue was that CheckDeviceState would incorrectly return D3D_OK instead of S_PRESENT_MODE_CHANGED when the mode was changed. Now it's always wrong, even with a preceding Present call. Of course in light of the Windows 10 results we may have to redefine "wrong". -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJW5aMEAAoJEN0/YqbEcdMweH8P/2OQymbb9EhNw4YqaXw0lcVN ozmH/7MYrxM4V1zzk+1+7XBR7nlIwDFRBGYw6LaZCiCmJYDfb1CTpYbSoVokHzQo UW2aCJCVRNWnZ5L72aZ12AoFq5sIuUX0FCAWhnpT4rNU7/bv35T/Zz0h439vn4RN +USZIHayRpmsJbhSbSn9VejSyin7GhBZ3VD7Qcd+7MqBxqOATX5p62cgFg+KUPkN Wi1HelDEHNsybHE4U60zYcZ2uxZxh7alfP6FjOtffOOxYilmG5eGEpGQX/dRIIv7 wxAfkjy/lgZqV9T4kEVyVO4tiwf9Vj1ccijkBNOttP7cFNGA4IM5W/urSWxL0uPd QEzP3QUv03NPN9Mtgq7K+DOyZmMUeedU+dHcCTwPJj/GTuXGxs2KHpLURXiAWXjD they7Iog2TEBFWuDskx58RHJyevbzKfWqfeM6/nJew5ter19W62Y1Sn01hfI0Ym0 aaTdzj+/tnf5pdc5Mgu6AtSRbApziwtYdfn4xGyEImw7J4pu7BnSmzHk8FMowtHm Dy2+GKE0FlmpAExwo0FAY2z6fCvMtIQMYzinU95LYaWvIggjsUo3wII3PxLtjKY0 8gXqBk+6WUUeDrAKqRqdkMCdzLBsVrlmxlj37KhfmPNCvG9gAlnMRegNW9eZkE2F aqvHGFSUrJzmk9CPtwtt =+25F -----END PGP SIGNATURE-----

1 0

Re: [PATCH] kernel32/tests: Skip ResolveDelayLoadedAPI tests on Win10+
by Marvin March 13, 2016

March 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=21399 Your paranoid android. === w864 (32 bit loader) === loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 === w1064 (32 bit loader) === loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 === w864 (64 bit loader) === loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 === w1064 (64 bit loader) === loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195

1 0

Builtin DLL View
by Keno Fischer March 12, 2016

March 12, 2016

Hello everyone, out of curiosity, I was tracking down why a piece of code in julia [1] threw an assertion when running under wine, but not on windows proper. The problem in this piece of code turned out to be that VirtualQuery is claiming a region of memory is free, when it is actually occupied by part of one of the builtin dlls. If I understand correctly what's happening here is that builtin dlls are regular shared objects on the host system, and get loaded as such, but contain the respective windows dll inside of them (modulo header, which gets created in `map_dll` by mapping over a couple of pages in the middle). Later, after going through the load callback, `virtual_create_builtin_view` records the appropriate information in the view map. However, it only records what it knows about the inner dll, not the shared object wrapper around it. I was thinking of preparing a patch, but I am not sure how to go about it. Options I could see: - Expand the mapping in `virtual_create_builtin_view`. This seems most straightforward, but I don't quite know what the proper way to pass this information through to here is. Or maybe we can parse the wrapper's header right in the function, if we assume that it's base will always be 64K before the PE header base? - Unmap the parts of the shared library that do not constitute the inner dll. I would imagine that at some point wine stops relying on the native dynamic linker, at which point it would probably be safe to just unmap the extra pages such that view agrees with what is actually mapped. I'd be happy to put together a patch, but I'll need some guidance on how best to do this, since I haven't looked at the Wine codebase before today. [1] https://github.com/JuliaLang/julia/blob/01dd5ec0b25fc5fa8cf1fbae6bb00ef5874…

1 0