February 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 3/6] ddraw: Keep a ddraw object reference in d3d7 vertex buffers.
by Henri Verbeet Feb. 8, 2017

Feb. 8, 2017

On 7 February 2017 at 22:08, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > +static void test_vb_refcount(void) > +{ > + ULONG prev_d3d_refcount, prev_device_refcount; > + ULONG cur_d3d_refcount, cur_device_refcount; > + D3DVERTEXBUFFERDESC vb_desc; > + IDirect3DVertexBuffer *vb; > + IDirect3DDevice3 *device; > + IDirect3D3 *d3d; > + HWND window; > + HRESULT hr; > + > + window = CreateWindowA("static", "d3d3_test", WS_OVERLAPPEDWINDOW, > + 0, 0, 640, 480, 0, 0, 0, 0); > + if (!(device = create_device(window, DDSCL_NORMAL))) > + { > + skip("Failed to create a 3D device, skipping test.\n"); > + DestroyWindow(window); > + return; > + } > + > + hr = IDirect3DDevice3_GetDirect3D(device, &d3d); > + ok(SUCCEEDED(hr), "Failed to get Direct3D3 interface, hr %#x.\n", hr); > + > + IDirect3D3_AddRef(d3d); > + prev_d3d_refcount = IDirect3D3_Release(d3d); Why not get_refcount()? > + hr = IDirect3D3_CreateVertexBuffer(d3d, &vb_desc, &vb, 0, NULL); > + ok(SUCCEEDED(hr), "Failed to create vertex buffer, hr %#x.\n", hr); > + > + IDirect3D3_AddRef(d3d); > + cur_d3d_refcount = IDirect3D3_Release(d3d); > + IDirect3DDevice3_AddRef(device); > + cur_device_refcount = IDirect3DDevice3_Release(device); > + ok(cur_d3d_refcount == prev_d3d_refcount, "D3D object refcount changed from %u to %u.\n", > + prev_d3d_refcount, cur_d3d_refcount); > + ok(cur_device_refcount == prev_device_refcount, "D3D device refcount changed from %u to %u.\n", > + prev_device_refcount, cur_device_refcount); > + > + IDirect3DVertexBuffer_Release(vb); It might not be bad to check the reference count after release as well. > diff --git a/dlls/ddraw/vertexbuffer.c b/dlls/ddraw/vertexbuffer.c > index 797afe2..7914805e 100644 > --- a/dlls/ddraw/vertexbuffer.c > +++ b/dlls/ddraw/vertexbuffer.c > @@ -126,16 +126,26 @@ static ULONG WINAPI d3d_vertex_buffer1_AddRef(IDirect3DVertexBuffer *iface) > return d3d_vertex_buffer7_AddRef(&buffer->IDirect3DVertexBuffer7_iface); > } > > +static void d3d_vertex_buffer_destroy(struct d3d_vertex_buffer *buffer) > +{ > + struct wined3d_buffer *vb = NULL; > + UINT offset, stride; > + > + /* D3D7 vertex buffers don't stay bound in the device, they are passed > + * as a parameter to DrawPrimitiveVB. DrawPrimitiveVB sets them as the > + * stream source in wined3d and they should get unset there before > + * they are destroyed. */ > + wined3d_mutex_lock(); > + wined3d_device_get_stream_source(buffer->ddraw->wined3d_device, > + 0, &vb, &offset, &stride); > + if (vb == buffer->wined3d_buffer) > + wined3d_device_set_stream_source(buffer->ddraw->wined3d_device, 0, NULL, 0, 0); > + > + wined3d_vertex_declaration_decref(buffer->wined3d_declaration); > + wined3d_buffer_decref(buffer->wined3d_buffer); > + wined3d_mutex_unlock(); > +} We'd typically call this kind of function d3d_vertex_buffer_cleanup(), the difference being that _destroy() deallocates the object itself, while _cleanup() only releases references, allocations, etc. inside the object. > static ULONG WINAPI d3d_vertex_buffer1_Release(IDirect3DVertexBuffer *iface) > { > struct d3d_vertex_buffer *buffer = impl_from_IDirect3DVertexBuffer(iface); > + ULONG ref = InterlockedDecrement(&buffer->ref); > > - TRACE("iface %p.\n", iface); > + TRACE("%p decreasing refcount to %u.\n", buffer, ref); > > - return d3d_vertex_buffer7_Release(&buffer->IDirect3DVertexBuffer7_iface); > + if (!ref) > + { > + d3d_vertex_buffer_destroy(buffer); > + > + HeapFree(GetProcessHeap(), 0, buffer); > + } > + return ref; > } Does this imply that if you query an IDirect3DVertexBuffer interface from an IDirect3DVertexBuffer7, and that ends up being the last reference to be released, the ddraw reference should be released? (And in the reverse case, that you'd release a reference you don't have.) For surfaces and palettes, we use the "ifaceToRelease" mechanism for this kind of thing.

1 0

Re: mshtml/tests: A couple of spelling fixes in ok() messages.
by Marvin Feb. 8, 2017

Feb. 8, 2017

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=28408 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2670: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [PATCH 1/2] localspl: Clear LastError on success in AddMonitor [v2]
by Huw Davies Feb. 8, 2017

Feb. 8, 2017

Hi Detlef, On Tue, Feb 07, 2017 at 11:22:35PM +0100, Detlef Riekenberg wrote: > dlls/localspl/provider.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/dlls/localspl/provider.c b/dlls/localspl/provider.c > index f359b73..3a912b1 100644 > --- a/dlls/localspl/provider.c > +++ b/dlls/localspl/provider.c > @@ -1507,6 +1507,8 @@ static BOOL WINAPI fpAddMonitor(LPWSTR pName, DWORD Level, LPBYTE pMonitors) > } > monitor_unload(pm); > > + SetLastError(ERROR_SUCCESS); /* Monitor installer depend on this */ I've sent version 3 of this series in your name that fixes the typo in this comment. Thanks, Huw.

1 0

Re: [PATCH] kernel32: fix command line argument escaping in build_command_line
by Vincent Povirk Feb. 8, 2017

Feb. 8, 2017

Is it really necessary to rewrite this loop? It'd be better to fix the existing code, if possible.

3 2

Re: [PATCH] dbghelp: Always return a dos path from SymGetLineFromAddr
by Huw Davies Feb. 8, 2017

Feb. 8, 2017

On Mon, Jan 30, 2017 at 07:04:51AM +0000, Alistair Leslie-Hughes wrote: > Fixes: https://bugs.winehq.org/show_bug.cgi?id=34687 > > Bases off the information in MSDN. > > https://msdn.microsoft.com/en-us/library/windows/desktop/ms681330%28v=vs.85… > "This function returns a pointer to a buffer that may be reused by another function. > Therefore, be sure to copy the data returned to another buffer immediately." > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/dbghelp/symbol.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c > index 08ea834..1638a1e 100644 > --- a/dlls/dbghelp/symbol.c > +++ b/dlls/dbghelp/symbol.c > @@ -1435,9 +1435,11 @@ BOOL WINAPI SymGetSymFromName(HANDLE hProcess, PCSTR Name, PIMAGEHLP_SYMBOL Symb > BOOL symt_fill_func_line_info(const struct module* module, const struct symt_function* func, > DWORD64 addr, IMAGEHLP_LINE64* line) > { > + static char path[MAX_PATH]; > struct line_info* dli = NULL; > BOOL found = FALSE; > int i; > + WCHAR *dos_path; > > assert(func->symt.tag == SymTagFunction); > > @@ -1455,7 +1457,10 @@ BOOL symt_fill_func_line_info(const struct module* module, const struct symt_fun > } > if (found) > { > - line->FileName = (char*)source_get(module, dli->u.source_file); > + dos_path = wine_get_dos_file_name((char*)source_get(module, dli->u.source_file)); > + WideCharToMultiByte(CP_ACP, 0, dos_path, -1, path, MAX_PATH, NULL, NULL); > + HeapFree( GetProcessHeap(), 0, dos_path ); > + line->FileName = path; > return TRUE; > } > } This has implications for winedbg that at least need to be discussed. For example, with this patch, backtraces contain Windows-like paths. I imagine there will be more serious issues too. For the patch itself, you now have a Unicode string, so it would make sense to have symt_fill_func_line_info() return that, then switch the dependencies so that SymGetLineFromAddr64() calls SymGetLineFromAddrW64(). To create the ANSI buffer, you probably want to use fetch_buffer() rather than a static array. Huw.

2 1

Re: [v3 2/6] user32: Add tests for windows when first shown with SetWindowPos
by Alexandre Julliard Feb. 7, 2017

Feb. 7, 2017

Fabian Maurer <dark.shadow4(a)web.de> writes: > These tests need the fix for WM_WINDOWPOSCHANGING and > WM_WINDOWPOSCHANGED to not fail. Then you should mark them as todo, and remove the todos in the patch that fixes them. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [v3 1/6] user32: Add tests for windows when first shown with WS_VISIBLE or RedrawWindow
by Fabian Maurer Feb. 7, 2017

Feb. 7, 2017

I just noted that this patch, and the second from the same series got marked as "Test failure". I don't really understand why though, is it because my patches introduce temporary test errors? I could use wine_todo, but since the wiki said to "send the tests as a separate patch before the fix" and I did fix the test failures right away I didn't think that's necessary. Or is it something else that's wrong? Regards, Fabian Maurer

1 0

Re: [resend 1/2] winex11: Don't send fake caption click to windows without a caption.
by Alexandre Julliard Feb. 7, 2017

Feb. 7, 2017

Grazvydas Ignotas <notasas(a)gmail.com> writes: > Confuses TFM/VGM Music Maker, see bug 41647. > > The problem triggers at the time the window is activated as a result of > SetWindowPos(), then WM_MOUSEACTIVATE arrives which definitely doesn't > happen on Windows. Probably should somehow get rid the fake click > completely, but it has been around for a very long time and this patch > is enough to fix the app. That doesn't seem right. Probably what you want is to skip the fake click if the window is already active or something along those lines. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v2 PATCH 1/4] shell32: Added IShellImageData stub
by Alex Henrie Feb. 7, 2017

Feb. 7, 2017

+static inline WCHAR *strdupW(const WCHAR *src) +{ + WCHAR *dest; + if (!src) return NULL; + dest = HeapAlloc(GetProcessHeap(), 0, (lstrlenW(src) + 1) * sizeof(*dest)); + if (dest) + lstrcpyW(dest, src); + return dest; +} This looks better, but it would still be nice to consistently indent or not indent after the if statements. -Alex

1 0

Re: winex11: Fix alpha blending in X11DRV_UpdateLayeredWindow.
by Alexandre Julliard Feb. 7, 2017

Feb. 7, 2017

Sebastian Lackner <sebastian(a)fds-team.de> writes: > @@ -2627,14 +2627,12 @@ BOOL CDECL X11DRV_UpdateLayeredWindow( HWND hwnd, const UPDATELAYEREDWINDOWINFO > { > IntersectRect( &rect, &rect, info->prcDirty ); > memcpy( src_bits, dst_bits, bmi->bmiHeader.biSizeImage ); > - PatBlt( hdc, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, BLACKNESS ); > - } > - ret = GdiAlphaBlend( hdc, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, > - info->hdcSrc, > - rect.left + (info->pptSrc ? info->pptSrc->x : 0), > - rect.top + (info->pptSrc ? info->pptSrc->y : 0), > - rect.right - rect.left, rect.bottom - rect.top, > - (info->dwFlags & ULW_ALPHA) ? *info->pblend : blend ); > + } > + ret = BitBlt( hdc, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, > + info->hdcSrc, > + rect.left + (info->pptSrc ? info->pptSrc->x : 0), > + rect.top + (info->pptSrc ? info->pptSrc->y : 0), > + SRCCOPY ); BitBlt is not guaranteed to set a valid alpha channel. -- Alexandre Julliard julliard(a)winehq.org

2 2