- wine-devel - winehq.org

Re: [PATCH] msvcrt: Protect onexit table with critical section
by Piotr Caban 11 Sep '17

11 Sep '17

Hi Daniel, Probably you shouldn't call the registered onexit function while holding the lock. Thanks, Piotr

1 0

Re: [PATCH resend] configure: Ensure that $includedir/wine is empty when deleted.
by Alexandre Julliard 11 Sep '17

11 Sep '17

Zebediah Figura <zfigura(a)codeweavers.com> writes: > Fixes a minor bug when running 'make uninstall'. > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > configure.ac | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) This can't work: Makefile:24387: *** target file '__uninstall__' has both : and :: entries. Stop. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH v2 2/3] shell32: Implement FolderItems_Item.
by Alex Henrie 11 Sep '17

11 Sep '17

2017-09-10 22:43 GMT-06:00 Alex Henrie <alexhenrie24(a)gmail.com>: > v2: > - Changed E_OUTOFMEMORY to ERROR_BUFFER_OVERFLOW Forgot to mention: - Started rejecting paths with relative (non-canonical) components -Alex

1 0

Re: [PATCH 2/3] shell32: Implement FolderItems_Item.
by Nikolay Sivov 11 Sep '17

11 Sep '17

On 08.09.2017 8:29, Alex Henrie wrote: > typedef struct { > FolderItems3 FolderItems3_iface; > LONG ref; > + VARIANT dir; > + WCHAR **item_filenames; > + LONG item_count; > } FolderItemsImpl; If 'dir' is always a string maybe store it as such? > static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) > { > - FIXME("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); > + FolderItemsImpl *This = impl_from_FolderItems(iface); > + WCHAR path_str[MAX_PATH]; > + VARIANT path_var; > + HRESULT ret; > + > + TRACE("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); > > *ppid = NULL; > - return E_NOTIMPL; > + > + if (!PathIsDirectoryW(V_BSTR(&This->dir))) > + return S_FALSE; How could it not be a directory? > + TRACE("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); Could add some spaces in argument list? Most of the wine code is doing that I think. > + if (!PathCombineW(path_str, V_BSTR(&This->dir), This->item_filenames[V_I4(&index)])) > + return E_OUTOFMEMORY; Why E_OUTOFMEMORY? > + do > + { > + if (!strcmpW(file_info.cFileName, dot) || !strcmpW(file_info.cFileName, dot_dot)) > + continue; > + > + if (This->item_count >= item_size) > + { > + item_size *= 2; > + filenames = HeapReAlloc(GetProcessHeap(), 0, This->item_filenames, item_size * sizeof(WCHAR*)); > + if (!filenames) > + goto fail; > + This->item_filenames = filenames; > + } > + > + This->item_filenames[This->item_count] = strdupW(file_info.cFileName); > + if (!This->item_filenames[This->item_count]) > + goto fail; > + This->item_count++; > + } > + while (FindNextFileW(first_file, &file_info)); Do we have tests showing that it takes a snapshot like that? For example if new file is created after FolderItems was created, is it accessible with BSTR index for example?

2 1

Re: [PATCH 1/3] shell32/tests: Add tests for FolderItems_Item and FolderItems_get_Count.
by Nikolay Sivov 11 Sep '17

11 Sep '17

On 08.09.2017 8:29, Alex Henrie wrote: > + for (i = 0; i < sizeof(file_defs)/sizeof(file_defs[0]); i++) > + { > + switch (file_defs[i].type) > + { > + case DIRECTORY: > + r = CreateDirectoryA(file_defs[i].name, NULL); > + ok(r, "CreateDirectory failed: %08x\n", GetLastError()); > + PathCombineA(buf, file_defs[i].name, "foo.txt"); > + file = CreateFileA(buf, 0, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + > + case EMPTY: > + file = CreateFileA(file_defs[i].name, 0, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + > + case RANDOM: > + file = CreateFileA(file_defs[i].name, GENERIC_WRITE, 0, NULL, > + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + for (j = 0; j < sizeof(buf); j++) > + buf[j] = rand() % 256; > + r = WriteFile(file, buf, sizeof(buf), &dwcount, NULL); > + ok(r, "WriteFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + } > + } Why would it matter if file is empty or not? And if it does for some reason, why filling it with random content? > + MultiByteToWideChar(CP_ACP, 0, file_defs[0].name, -1, wstr, MAX_PATH); > + V_VT(&var) = VT_BSTR; > + V_BSTR(&var) = SysAllocString(wstr); I understand why you have test names as single byte strings, but this construct is used more than once, so maybe add a helper for that. Could be similar to _bstr_() in msxml test, but without global pointer array. > + for (i = -10; i < 10; i++) > + { > + V_VT(&var) = VT_ERROR; > + V_ERROR(&var) = i; VT_ERROR is used for optional arguments, what does [-10,10) range mean? I think it's enough to test some HRESULT-style failure code. > + ok(!!bstr, "bstr is null\n"); > + ok(!lstrcmpW(bstr, cur_dir), > + "expected %s, got %s\n", wine_dbgstr_w(cur_dir), wine_dbgstr_w(bstr)); First check seems redundant.

2 5

Re: [PATCH v2 6/7] evr: Add EnhancedVideoRenderer coclass
by Nikolay Sivov 10 Sep '17

10 Sep '17

On 10.09.2017 18:16, Fabian Maurer wrote: > +typedef struct > +{ > + BaseFilter basefilter; > + > + IUnknown IUnknown_inner; > + LONG ref; > +} evr_impl; I don't think you need a separate refcount field. Also it's not called "basefilter" in other quartz components. In fact you don't use it at all it seems in patch 6/7, and I think 7/7 is short enough to merge it with 6/7. > + ERR("(%p)->(%s,%p),not found\n", This, debugstr_guid(riid), ppv); > + return E_NOINTERFACE; I think this is usually FIXME or WARN. > ... Style-wise, I think naming could be improved, for example "impl" part does not add anything.

2 1

Re: [PATCH v2 4/7] evr: Add tests
by Marvin 10 Sep '17

10 Sep '17

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=33024 Your paranoid android. === build (build) === Make failed

2 1

Re: [PATCH] test.patch
by Fabian Maurer 10 Sep '17

10 Sep '17

Please ignore this patch, didn't mean to send that. Regards, Fabian Maurer

1 0

Re: [PATCH 4/7] evr: Add tests
by Marvin 10 Sep '17

10 Sep '17

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

1 0

Re: [PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangles
by Henri Verbeet 08 Sep '17

08 Sep '17

On 8 September 2017 at 16:11, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +static BOOL d2d_point_on_segment(const D2D1_POINT_2F *q, const D2D1_POINT_2F *p0, > + const D2D1_POINT_2F *p1, float tolerance) > +{ > + float dot, edge_length, distance; > + D2D1_POINT_2F v_p, v_q, n; > + > + d2d_point_subtract(&v_p, p1, p0); > + if ((edge_length = d2d_point_length(&v_p)) == 0.0f) > + return FALSE; > + > + /* Edge distance. */ > + d2d_point_subtract(&v_q, q, p0); > + > + n.x = -v_p.y; > + n.y = v_p.x; Unfortunately it's not that easy when "transform" is a world transformation instead of a geometry transformation. I.e., when you have for example a shear transformation, the "normal" is no longer necessarily perpendicular to the line segment. Put a different way, you're potentially testing against a parallelogram instead of a rectangle. In a sense you account for that with stroke_widths[], but see below. > + distance = fabsf(d2d_point_dot(&v_q, &n)) / edge_length; > + if (distance >= tolerance) > + return FALSE; > + > + /* Test if normal and edge intersect. */ > + dot = d2d_point_dot(&v_q, &v_p); > + return dot >= 0.0f && dot <= edge_length * edge_length; > +} Likewise, this doesn't do the right thing under all transformations. > + d2d_point_transform(&v_s[0], transform, stroke_width, 0.0f); > + d2d_point_transform(&v_s[1], transform, stroke_width, 0.0f); > + d2d_point_transform(&origin, transform, 0.0f, 0.0f); > + > + d2d_point_subtract(&v_s[0], &v_s[0], &origin); > + d2d_point_subtract(&v_s[1], &v_s[1], &origin); > + > + stroke_widths[0] = d2d_point_length(&v_s[0]); > + stroke_widths[1] = d2d_point_length(&v_s[1]); This ends up with the same value for stroke_widths[0] and stroke_widths[1]. > + d2d_point_subtract(&v, &point, &vertices[i]); > + > + if (d2d_point_dot(&v, &v) < tolerance * tolerance) > + { > + *contains = TRUE; > + break; > + } This tests around the corner of the joins, but note that there's potentially a space between that corner and the start of the line segment that this would miss. > + else > + { > + D2D1_POINT_2F d, s; > + > + s.x = rect->right - rect->left; > + s.y = rect->bottom - rect->top; > + d.x = fabsf((rect->right + rect->left) * 0.5f - point.x); > + d.y = fabsf((rect->bottom + rect->top) * 0.5f - point.y); > + > + /* Inside test. */ > + if (d.x <= (s.x - stroke_width) * 0.5f - tolerance && d.y <= (s.y - stroke_width) * 0.5f - tolerance) > + { > + *contains = FALSE; > + return S_OK; > + } > + > + if (tolerance == 0.0f) > + { > + *contains = d.x < (s.x + stroke_width) * 0.5f && d.y < (s.y + stroke_width) * 0.5f; > + } > + else > + { > + d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f); > + d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f); > + > + *contains = d2d_point_dot(&d, &d) < tolerance * tolerance; > + } > + } Ultimately it's not that important, but I think there's something to be said for handling the "!transform" case first as a special case. I.e., if (!transform) { ... return S_OK; } /* General case. */ > diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c > index f6261295b6..95a010b13e 100644 > --- a/dlls/d2d1/tests/d2d1.c > +++ b/dlls/d2d1/tests/d2d1.c > @@ -76,6 +76,15 @@ struct expected_geometry_figure > const struct geometry_segment *segments; > }; > > +struct contains_point_test > +{ > + D2D1_MATRIX_3X2_F transform; > + D2D1_POINT_2F point; > + float tolerance; > + float stroke_width; > + BOOL contains; > +}; Unless these are reused between tests, we tend to keep them local to the function. > + static const struct contains_point_test stroke_contains[] = > + { > + {{0.0f, 0.0f, 0.0f, 1.0f}, {0.1f, 10.0f}, 0.0f, 1.0f, FALSE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {5.0f, 10.0f}, 5.0f, 1.0f, FALSE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {4.9f, 10.0f}, 5.0f, 1.0f, TRUE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {5.0f, 10.0f}, -5.0f, 1.0f, FALSE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {4.9f, 10.0f}, -5.0f, 1.0f, TRUE}, > + > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.0f, 10.0f}, 0.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.1f, 10.0f}, 0.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.5f, 10.0f}, 0.0f, 1.0f, FALSE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.0f, 10.0f}, 1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.59f, 10.0f}, 1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {-0.59f, 10.0f}, 1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.59f, 10.0f}, -1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {-0.59f, 10.0f}, -1.0f, 1.0f, TRUE}, > + }; Rotations and shears are interesting. > @@ -2766,6 +2793,108 @@ static void test_rectangle_geometry(void) > ok(SUCCEEDED(hr), "FillContainsPoint() failed, hr %#x.\n", hr); > ok(!!contains, "Got wrong hit test result %d.\n", contains); > > + /* Stroked area hittesting. Edge. */ > + contains = FALSE; > + set_point(&point, 0.4f, 10.0f); > + hr = ID2D1RectangleGeometry_StrokeContainsPoint(geometry, point, 1.0f, NULL, NULL, 0.0f, &contains); > + ok(SUCCEEDED(hr), "StrokeContainsPoint() failed, hr %#x.\n", hr); > + ok(!!contains, "Got wrong hit test result %d.\n", contains); We tend to use %#x for BOOLs. And well, you might as well make these array based as well.

2 2

Re: [PATCH 1/2] msvcrt: Back up and restore the FPU state in setjmp/longjmp on arm
by Alexandre Julliard 08 Sep '17

08 Sep '17

Martin Storsjo <martin(a)martin.st> writes: > Signed-off-by: Martin Storsjo <martin(a)martin.st> > --- > dlls/msvcrt/except_arm.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) This doesn't build here with the Android NDK: arm-linux-androideabi-gcc -c -o except_arm.o ../../../wine/dlls/msvcrt/except_arm.c -I. -I../../../wine/dlls/msvcrt \ -I../../include -I../../../wine/include -D__WINESRC__ -D_MT -D_MSVCR_VER=0 -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 -g -O2 -fno-diagnostics-show-caret -D__ANDROID_API__=26 -marm {standard input}: Assembler messages: {standard input}:44: Error: selected processor does not support ARM mode `vmrs r2,fpscr' {standard input}:46: Error: selected processor does not support ARM mode `vstr d8,[r0,#0x30]' {standard input}:47: Error: selected processor does not support ARM mode `vstr d9,[r0,#0x38]' {standard input}:48: Error: selected processor does not support ARM mode `vstr d10,[r0,#0x40]' {standard input}:49: Error: selected processor does not support ARM mode `vstr d11,[r0,#0x48]' {standard input}:50: Error: selected processor does not support ARM mode `vstr d12,[r0,#0x50]' {standard input}:51: Error: selected processor does not support ARM mode `vstr d13,[r0,#0x58]' {standard input}:52: Error: selected processor does not support ARM mode `vstr d14,[r0,#0x60]' {standard input}:53: Error: selected processor does not support ARM mode `vstr d15,[r0,#0x68]' {standard input}:75: Error: selected processor does not support ARM mode `vmsr fpscr,r3' {standard input}:76: Error: selected processor does not support ARM mode `vldr d8,[r0,#0x30]' {standard input}:77: Error: selected processor does not support ARM mode `vldr d9,[r0,#0x38]' {standard input}:78: Error: selected processor does not support ARM mode `vldr d10,[r0,#0x40]' {standard input}:79: Error: selected processor does not support ARM mode `vldr d11,[r0,#0x48]' {standard input}:80: Error: selected processor does not support ARM mode `vldr d12,[r0,#0x50]' {standard input}:81: Error: selected processor does not support ARM mode `vldr d13,[r0,#0x58]' {standard input}:82: Error: selected processor does not support ARM mode `vldr d14,[r0,#0x60]' {standard input}:83: Error: selected processor does not support ARM mode `vldr d15,[r0,#0x68]' Makefile:311: recipe for target 'except_arm.o' failed -- Alexandre Julliard julliard(a)winehq.org

2 4

Strange behaviour of CreateProcess with Linux binary.
by John Found 08 Sep '17

08 Sep '17

I have a program running under WINE that need to start some external programs using CreateProcess API function. Some of these programs are Linux ELF executables, some are Windows PE. But recently, after some updates of my Linux distribution (Manjaro 64bit) some of the Linux executables stopped to start this way. CreateProcess returns 0, GetLastError returns 0C1h (193) == ERROR_BAD_EXE_FORMAT The same executable runs fine when started from the Linux path. One example file that behaves such way is xfce4-terminal (but the problem is not limited to only this file). For now I am wrapping such executables in short bash script in order to workaround: #!/usr/bin/sh /usr/bin/xfce4-terminal "$@" Any ideas about what is the problem and how can be fixed? -- http://fresh.flatassembler.net http://asm32.info John Found <johnfound(a)asm32.info>

2 2

Re: [PATCH] comctl32/listview: Send NM_RELEASEDCAPTURE after the window focus has changed
by Nikolay Sivov 08 Sep '17

08 Sep '17

What use case does this fix? With this patch notification order is more broken than without. On Windows when clicking on item of unfocused control gives roughly: NM_RELEASEDCAPTURE LVN_ITEMCHANGING/LVN_ITEMCHANGED NM_CUSTOMDRAW NM_SETFOCUS NM_CLICK Without this patch we currently do: NM_RELEASEDCAPTURE LVN_ITEMCHANGIN/LVN_ITEMCHANGED NM_SETFOCUS NM_CLICK With the patch: LVN_ITEMCHANGING/LVN_ITEMCHANGED NM_SETFOCUS NM_RELEASECAPTURE NM_CLICK (in both cases on Wine sequence is interleaved with some NM_CUSTOMDRAW)

2 2

Re: webservices: Make alloc_float_text() static.
by Hans Leidekker 08 Sep '17

08 Sep '17

On Fri, 2017-09-08 at 02:24 +0200, Francois Gouget wrote: > dlls/webservices/reader.c | 2 +- > dlls/webservices/webservices_private.h | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) > > It was introduced in 82306e84 on 2017-07-20 and has not been used > outside reader.c since then. Ignore this patch if there are ongoing > plans to use it elsewhere. It will be used in the writer at some point.

1 0

Re: msi: Make MSI_ProcessMessageVerbatim() static.
by Hans Leidekker 08 Sep '17

08 Sep '17

On Fri, 2017-09-08 at 02:12 +0200, Francois Gouget wrote: > diff --git a/dlls/msi/msipriv.h b/dlls/msi/msipriv.h > index f316c0cce74..1bb0a849eed 100644 > --- a/dlls/msi/msipriv.h > +++ b/dlls/msi/msipriv.h > @@ -872,7 +872,6 @@ extern UINT MSI_SetInstallLevel( MSIPACKAGE *package, int iInstallLevel ) DECLSP > extern MSIPACKAGE *MSI_CreatePackage( MSIDATABASE *, LPCWSTR ) DECLSPEC_HIDDEN; > extern UINT MSI_OpenPackageW( LPCWSTR szPackage, MSIPACKAGE **pPackage ) DECLSPEC_HIDDEN; > extern UINT MSI_SetTargetPathW( MSIPACKAGE *, LPCWSTR, LPCWSTR ) DECLSPEC_HIDDEN; > -extern INT MSI_ProcessMessageVerbatim( MSIPACKAGE *, INSTALLMESSAGE, MSIRECORD * ) DECLSPEC_HIDDEN; > extern INT MSI_ProcessMessage( MSIPACKAGE *, INSTALLMESSAGE, MSIRECORD * ) DECLSPEC_HIDDEN; > extern MSICONDITION MSI_EvaluateConditionW( MSIPACKAGE *, LPCWSTR ) DECLSPEC_HIDDEN; > extern UINT MSI_GetComponentStateW( MSIPACKAGE *, LPCWSTR, INSTALLSTATE *, INSTALLSTATE * ) DECLSPEC_HIDDEN; > diff --git a/dlls/msi/package.c b/dlls/msi/package.c > index 47164613209..a88609a3dd8 100644 > --- a/dlls/msi/package.c > +++ b/dlls/msi/package.c > @@ -1444,6 +1444,8 @@ UINT msi_set_original_database_property( MSIDATABASE *db, const WCHAR *package ) > return r; > } > > +static INT MSI_ProcessMessageVerbatim(MSIPACKAGE*,INSTALLMESSAGE,MSIRECORD*); I'd prefer moving this function (and internal_ui_handler) up.

1 0

Re: [PATCH 1/3] d3dx9/tests: Add test for null shaders in effect.
by Matteo Bruni 07 Sep '17

07 Sep '17

2017-09-01 20:19 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +static void test_effect_null_shader(void) > +{ > + D3DPRESENT_PARAMETERS present_parameters = {0}; > + IDirect3DDevice9 *device; > + ID3DXEffect *effect; > + D3DXPASS_DESC desc; > + IDirect3D9 *d3d; > + D3DXHANDLE pass; > + ULONG refcount; > + HWND window; > + HRESULT hr; > + > + /* Creating a fresh device because the existing device can have invalid render states > + from previous tests. If IDirect3DDevice9_ValidateDevice() returns certain error codes, > + native ValidateTechnique() fails. */ We generally want to be sure that each test is independent from the others so creating a separate device for each test function is a good idea regardless. Your point about ValidateDevice() is very interesting though, actually it makes a lot of sense and it probably means that ValidateTechnique() should delegate a good chunk of its work to ValidateDevice() (not surprising but somehow I forgot about it). It would be interesting to extend the d3d9 ValidateDevice() tests at some point.

1 0

Re: [1/2] ntdll: Implement set_cpu_context for ARM64
by Alexandre Julliard 07 Sep '17

07 Sep '17

André Hentschel <nerv(a)dawncrow.de> writes: > + "ldr x30, [x17, #0xf8]\n\t" > + "ldr x16, [x17, #0x100]\n\t" > + "mov sp, x16\n\t" > + "ldr x16, [x17, #0x108]\n\t" > + "br x16\n\t" You'd also need to restore x16 and x17, this is used in signal handlers. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v5 PATCH] wmvcore/tests: add tests for WMReader methods
by Alexandre Julliard 06 Sep '17

06 Sep '17

Jefferson Carpenter <jeffersoncarpenter2(a)gmail.com> writes: > Can somebody explain why this one wasn't committed? I'm not convinced we need a large binary file for this, and it also seems to take a long time to run mainly because it's just a bunch of timeouts at this point. I'd like to see the implementation being fleshed out a bit first. -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: [v2 PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangles
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 12:35, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > -static void d2d_point_normalise(D2D1_POINT_2F *p) > +static float d2d_point_normalise(D2D1_POINT_2F *p) > { > float l; > > if ((l = sqrtf(d2d_point_dot(p, p))) != 0.0f) > d2d_point_scale(p, 1.0f / l); > + > + return l; > } Maybe. I'm not entirely convinced you need this, see below. > +static BOOL d2d_rect_point_on(const D2D1_POINT_2F *point, float tolerance, const D2D1_POINT_2F *points) > +{ > + unsigned int i; > + float dot; > + > + for (i = 0; i < 4; i++) > + { It may improve readability to pull that loop out into the caller, and make this function just about testing whether a point is on a line segment. > + float edge_length, distance; > + D2D1_POINT_2F v, edge; > + > + v.x = point->x - points[i].x; > + v.y = point->y - points[i].y; d2d_point_subtract(&v, point, &points[i]); Or, alternatively, d2d_point_subtract(&v_q, q, p0); > + /* Point to vertex distance. */ > + if (d2d_point_dot(&v, &v) < tolerance * tolerance) > + return TRUE; Maybe. An argument could probably be made for testing joins separately. > + /* Point to edge distance. */ > + edge.x = points[(i + 1) % 4].x - points[i].x; > + edge.y = points[(i + 1) % 4].y - points[i].y; d2d_point_subtract(&edge, &points[(i + 1) % 4], &points[i]); Or, d2d_point_subtract(&v_p, p1, p0); > + if ((edge_length = sqrtf(d2d_point_dot(&edge, &edge))) == 0.0f) > + continue; I guess we'd want to introduce d2d_point_length(). > + distance = fabsf(edge.x * v.y - edge.y * v.x) / edge_length; n.x = -v_p.y; n.y = v_p.x; distance = fabsf(d2d_point_dot(&v_q, &n)) / edge_length; > + d2d_point_normalise(&edge); > + dot = d2d_point_dot(&edge, &v); > + if (dot >= 0.0f && dot <= edge_length) > + return TRUE; If you compare against "edge_length * edge_length", you can drop the normalisation. > +static BOOL d2d_rect_point_inside(const D2D1_POINT_2F *point, const D2D1_POINT_2F *points) > +{ > + D2D1_POINT_2F vec[2]; > + unsigned int i; > + float dot; > + > + for (i = 0; i < 2; i++) > + { > + float l; > + > + vec[0].x = points[i + 1].x - points[i].x; > + vec[0].y = points[i + 1].y - points[i].y; > + vec[1].x = point->x - points[i].x; > + vec[1].y = point->y - points[i].y; > + > + if ((l = d2d_point_normalise(&vec[0])) == 0.0f) > + return FALSE; > + > + if ((dot = d2d_point_dot(&vec[0], &vec[1])) < 0.0f || dot > l) > + return FALSE; > + } > + > + return TRUE; > +} Some of the comments on d2d_rect_point_on() apply here as well. Perhaps more importantly, does this work with shear transformations? > + if (transform) > + { > + D2D1_POINT_2F outer[4], inner[4]; > + > + stroke_width *= 0.5f; > + > + d2d_point_transform(&inner[0], transform, rect->left + stroke_width, rect->top + stroke_width); > + d2d_point_transform(&inner[1], transform, rect->left + stroke_width, rect->bottom - stroke_width); > + d2d_point_transform(&inner[2], transform, rect->right - stroke_width, rect->bottom - stroke_width); > + d2d_point_transform(&inner[3], transform, rect->right - stroke_width, rect->top + stroke_width); > + > + d2d_point_transform(&outer[0], transform, rect->left - stroke_width, rect->top - stroke_width); > + d2d_point_transform(&outer[1], transform, rect->left - stroke_width, rect->bottom + stroke_width); > + d2d_point_transform(&outer[2], transform, rect->right + stroke_width, rect->bottom + stroke_width); > + d2d_point_transform(&outer[3], transform, rect->right + stroke_width, rect->top - stroke_width); This seems suspicious. In general, the stroke width isn't affected by transformations on the geometry. Test coverage for that would be nice in either case.

1 0

Re: [PATCH 2/3] include: Add interfaces IMFMediaTypeHandler, IMFStreamDescriptor
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 09:38, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +[ > + object, > + uuid(e93dcf6c-4b07-4e1e-8123-aa16ed6eadf5), > +] > +interface IMFMediaTypeHandler : IUnknown > +{ ... > +}; Redundant ';'. Unfortunately we have that issue in a bunch of other IDLs as well, and the Microsoft IDLs don't necessarily always get it right either.

1 0

Re: [PATCH 3/5] wined3d: Store FLUSH flag in the query object.
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 01:39, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > So it can be used in the CS thread's query polling. Passing the > appropriate flag to glClientWaitSync() is potentially lighter on > performance than a separate, explicit glFlush(). Maybe. It would be debatable once there are multiple queries in-flight. > @@ -389,8 +389,8 @@ HRESULT CDECL wined3d_query_get_data(struct wined3d_query *query, > } > else if (query->counter_main != query->counter_retrieved) > { > - if (flags & WINED3DGETDATA_FLUSH) > - wined3d_cs_emit_flush(query->device->cs); > + if (flags & WINED3DGETDATA_FLUSH && !(query->flags & WINED3DGETDATA_FLUSH)) > + InterlockedExchange((LONG *)&query->flags, flags); Shouldn't the flag be cleared again when the query is restarted? How about the following: - wined3d_query_get_data() emits a flush when the counters don't match, "flags" contains WINED3DGETDATA_FLUSH, and query->flushed is FALSE. - wined3d_cs_exec_flush() sets query->flushed for all queries on cs->query_poll_list. - query->flushed is cleared when a query is removed from cs->query_poll_list. Would that work? It would leave a small window between wined3d_cs_emit_flush() and wined3d_cs_exec_flush() that could probably be avoided by checking in wined3d_query_get_data() if any queries were issued since the last flush. Not sure whether that's worth it. Alternatively, we could only do the latter. I.e., set cs->queries_flushed in wined3d_cs_emit_flush(), and clear it in wined3d_cs_emit_query_issue(). That wouldn't prevent all redundant flushes, but it would help for applications calling wined3d_query_get_data() in a tight loop.

1 0

Re: [PATCH 1/5] wined3d: Restore support for d3d9 "fake" formats.
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 01:39, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > @@ -4114,6 +4114,7 @@ extern enum wined3d_format_id pixelformat_for_depth(DWORD depth) DECLSPEC_HIDDEN > #define WINED3DFMT_FLAG_DEPTH 0x00000004 > #define WINED3DFMT_FLAG_STENCIL 0x00000008 > #define WINED3DFMT_FLAG_RENDERTARGET 0x00000010 > +#define WINED3DFMT_FLAG_FAKE 0x00000020 Could I suggest WINED3DFMT_FLAG_EXTENSION?

1 0

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest
by Alexandre Julliard 06 Sep '17

06 Sep '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > processorArchitecture should not be empty string I don't think that's correct for assemblies that can be imported. Note that the processorArchitecture is fixed up when creating the fake dll, so it doesn't end up empty in the resources. -- Alexandre Julliard julliard(a)winehq.org

2 5

Re: [PATCH] wined3d: wined3d_check_surface_capability return TRUE for surfaces with WINED3DFMT_FLAG_TEXTURE.
by Henri Verbeet 05 Sep '17

05 Sep '17

On 4 September 2017 at 18:26, lorenzo Skotos <lorenzofersteam(a)live.it> wrote: > - if (format->glInternal) > + if (format->glInternal || format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_TEXTURE) That should fix the regression, but since the ambiguity of WINED3DFMT_FLAG_TEXTURE led to the issue in the first place, I'd rather not simply restore the check. Instead, I think it would be better to explicitly mark these special "extension" formats as such. In theory Matteo has a patch for that.

2 3

Re: [PATCH] ntdll: Make RtlDeregisterWaitEx(handle, INVALID_HANDLE_VALUE) thread safe.
by Stefan Dösinger 05 Sep '17

05 Sep '17

Hi, Is the test I was referring to in the description of the previous patch. It causes seemingly unrelated test failures like https://testbot.winehq.org/JobDetails.pl?Key=32956 (previous iteration of the test that used a different implementation) Stefan Am 2017-09-05 um 17:34 schrieb Stefan Dösinger: > Chromium signals the wait semaphore and calls DeregisterWaitEx with > CompletionHandle = INVALID_HANDLE_VALUE in close succession. Sometimes > the worker thread decides to run the callback, but before it sets > CallbackInProgress RtlDeregisterWaitEx decides that the callback is not > running and returns STATUS_SUCCESS. Chromium then releases resources > that the callback needs to run, resulting in random crashes. > > Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com> > > --- > > The tests show that we're only supposed to return STATUS_PENDING if the > callback is running. Note that the choice between waiting and returning > STATUS_SUCCESS and not waiting and returning STATUS_PENDING is still > racy, but we'll no longer return STATUS_SUCCESS and run the callback > afterwards. > > I tried to extend the tests to show what happens when both events are > available simultanously. The answer is that it is fairly random. > INVALID_HANDLE_VALUE blocks and runs the callback one last time, the > other ways of calling RtlDeregisterWait[Ex] randomly return STATUS_SUCCESS > and do not run the callback, or they return STATUS_PENDING and may or > may not run the callback. However, in no circumstance does the callback > run after STATUS_SUCCESS is returned. > > I am not including the tests because of the random outcome and because > it plays with SuspendThread and makes the random failures in follow up > tests worse. I'll send it to wine-devel for reference. > --- > dlls/ntdll/threadpool.c | 59 +++++++++++++++++++++++++++---------------------- > 1 file changed, 32 insertions(+), 27 deletions(-) > > diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c > index 6063d51d9f..1b0546a781 100644 > --- a/dlls/ntdll/threadpool.c > +++ b/dlls/ntdll/threadpool.c > @@ -541,11 +541,13 @@ static DWORD CALLBACK wait_thread_proc(LPVOID Arg) > break; > } > > - completion_event = wait_work_item->CompletionEvent; > - if (completion_event) NtSetEvent( completion_event, NULL ); > > if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 ) > + { > + completion_event = wait_work_item->CompletionEvent; > delete_wait_work_item( wait_work_item ); > + if (completion_event) NtSetEvent( completion_event, NULL ); > + } > > return 0; > } > @@ -633,44 +635,47 @@ NTSTATUS WINAPI RtlRegisterWait(PHANDLE NewWaitObject, HANDLE Object, > NTSTATUS WINAPI RtlDeregisterWaitEx(HANDLE WaitHandle, HANDLE CompletionEvent) > { > struct wait_work_item *wait_work_item = WaitHandle; > - NTSTATUS status = STATUS_SUCCESS; > + NTSTATUS status; > + HANDLE LocalEvent = NULL; > + BOOLEAN CallbackInProgress; > > - TRACE( "(%p)\n", WaitHandle ); > + TRACE( "(%p %p)\n", WaitHandle, CompletionEvent ); > > if (WaitHandle == NULL) > return STATUS_INVALID_HANDLE; > > - NtSetEvent( wait_work_item->CancelEvent, NULL ); > - if (wait_work_item->CallbackInProgress) > + CallbackInProgress = wait_work_item->CallbackInProgress; > + if (CompletionEvent == INVALID_HANDLE_VALUE || !CallbackInProgress) > { > - if (CompletionEvent != NULL) > - { > - if (CompletionEvent == INVALID_HANDLE_VALUE) > - { > - status = NtCreateEvent( &CompletionEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE ); > - if (status != STATUS_SUCCESS) > - return status; > - interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > - if (wait_work_item->CallbackInProgress) > - NtWaitForSingleObject( CompletionEvent, FALSE, NULL ); > - NtClose( CompletionEvent ); > - } > - else > - { > - interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > - if (wait_work_item->CallbackInProgress) > - status = STATUS_PENDING; > - } > - } > - else > - status = STATUS_PENDING; > + status = NtCreateEvent( &LocalEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE ); > + if (status != STATUS_SUCCESS) > + return status; > + interlocked_xchg_ptr( &wait_work_item->CompletionEvent, LocalEvent ); > + } > + else if (CompletionEvent != NULL) > + { > + interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > } > > + NtSetEvent( wait_work_item->CancelEvent, NULL ); > + > if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 ) > { > status = STATUS_SUCCESS; > delete_wait_work_item( wait_work_item ); > } > + else if (LocalEvent) > + { > + NtWaitForSingleObject( LocalEvent, FALSE, NULL ); > + status = STATUS_SUCCESS; > + } > + else > + { > + status = STATUS_PENDING; > + } > + > + if (LocalEvent) > + NtClose( LocalEvent ); > > return status; > } >

1 0

Re: [PATCH] wbemprox: Fix CreateInstanceEnum/Async entries.
by Hans Leidekker 05 Sep '17

05 Sep '17

On Tue, 2017-09-05 at 15:19 +0300, Andrey Gusev wrote: > @@ -599,7 +599,7 @@ static HRESULT WINAPI wbem_services_CreateInstanceEnum( > > static HRESULT WINAPI wbem_services_CreateInstanceEnumAsync( > IWbemServices *iface, > - const BSTR strFilter, > + const BSTR strClass, > LONG lFlags, > IWbemContext *pCtx, > IWbemObjectSink *pResponseHandler ) I think the original name is fine.

1 0

Re: [2/2] ntdll: Implement RtlUnwind for ARM64
by Stefan Dösinger 05 Sep '17

05 Sep '17

Am 2017-09-04 um 20:46 schrieb André Hentschel: > + frame = NtCurrentTeb()->Tib.ExceptionList; > + while ((frame != (EXCEPTION_REGISTRATION_RECORD*)~0UL) && (frame != endframe)) Unless I am blind this handles only x86-style stack handlers, and not the function tables similar to amd64 that are likely to be present as well on arm64. Though I guess because of the lack of arm64 compilers, documentation and binaries from Microsoft we don't really know how Windows (will) handle arm64 exceptions.

1 0

Testing reading and writing of WMF files
by Jefferson Carpenter 04 Sep '17

04 Sep '17

I'm trying to imagine what would be a good way to test for the successful processing (reading or writing) of a Windows Media Format file. The WMReader interface has to parse the file correctly. I believe it would be impossible to test for this, without having a natural WMF (.wma, .wmv, or .asf) file to read. There are two ways of getting one: include one in the test suite, or create one as part of the test. The first option has certain disadvantages, such as A) the file size and B) inability to tweak the contents, or easily make a variety of similar files, should the need arise. Also, the WMWriter has to write the file correctly. I see only two ways of testing this: compare the output byte-for-byte with a file generated on a Windows PC, or attempt to read the generated file with a known good wmf implementation. The first way would assume the output is always the same byte-for-byte, which it may not be. For the second, we would need a conforming implementation to use. It seems to me, libav would be quite useful, as it can already decode and encode many if not most of the WMF formats. Do you think it's a valid assumption to make, that libav's codecs are a conforming implementation of the WMF formats? Jefferson Carpenter

1 0

Re: Re: [v5 PATCH] wmvcore/tests: add tests for WMReader methods
by Marvin 04 Sep '17

04 Sep '17

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

1 0

Re: PATCH] user32: Allow activating desktop only when clicked directly
by Fabian Maurer 02 Sep '17

02 Sep '17

Hello, just noticed this patch being marked as 'Superseded', but I don't see any new patch addressing this issue. Am I missing something here? Regards, Fabian Maurer

2 2

comctl v6
by Fabian Maurer 02 Sep '17

02 Sep '17

Hello wine-devel, While investigating Bug 43643, I learned that comboboxes behave differently when comctl32 v6 is requested by the program. Basically, when v6 is used, all comboboxes display 30 items maximum, regardless of height. This behavior is unimplemented by wine, so affected programs doesn't show any drop down list at all. This would be the correct behavior when not using v6, but it's wrong in this case. We probably need a second combobox class with different behavior (or one class for both behaviors). I don't know too much on how windows implements comboboxes, in wine the comboboxes are implemented in user32, but we probably need to split them between comctl32 v6 and lower. Do you already have plans on how to do that? Especially, how would we do the separation on the source level but still reuse as much code as possible? Is there something that could be done as a start to bring this into the right direction? Regards, Fabian Maurer

1 0

Re: [PATCH] winecfg: Remove processorArchitecture from manifest
by Fabian Maurer 02 Sep '17

02 Sep '17

It works with that patch, but maybe we want > processorArchitecture="*" instead? Regards, Fabian Maurer

1 0

Re: [v3 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Marvin 01 Sep '17

01 Sep '17

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

2 1

Re: [v3 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Paul Gofman 01 Sep '17

01 Sep '17

Please ignore this patch for now (along with the previous version). I have discovered that there is still the case when object_id can be nonzero, but the object is never created (not initialized array of shaders). So we should keep the creation_failed flag probably. I will send the test for null shaders and the new patch version in the next series. On 09/01/2017 01:56 PM, Paul Gofman wrote: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > v3: > - use object_id != 0 as the indication of non-null shader. > --- > dlls/d3dx9_36/effect.c | 45 +++++++++++++++++++++++++++++++++++++++++--- > dlls/d3dx9_36/tests/effect.c | 3 --- > 2 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index 2cd0c22f64..ef772d99b5 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -3941,11 +3941,50 @@ static D3DXHANDLE WINAPI ID3DXEffectImpl_GetCurrentTechnique(ID3DXEffect *iface) > > static HRESULT WINAPI ID3DXEffectImpl_ValidateTechnique(ID3DXEffect* iface, D3DXHANDLE technique) > { > - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface); > + struct d3dx9_base_effect *base = &effect->base_effect; > + struct d3dx_technique *tech = get_valid_technique(base, technique); > + HRESULT ret = D3D_OK; > + unsigned int i, j; > > - FIXME("(%p)->(%p): stub\n", This, technique); > + FIXME("iface %p, technique %p semi-stub.\n", iface, technique); > > - return D3D_OK; > + if (!tech) > + { > + ret = D3DERR_INVALIDCALL; > + goto done; > + } > + for (i = 0; i < tech->pass_count; ++i) > + { > + struct d3dx_pass *pass = &tech->passes[i]; > + > + for (j = 0; j < pass->state_count; ++j) > + { > + struct d3dx_state *state = &pass->states[j]; > + > + if (state_table[state->operation].class == SC_VERTEXSHADER > + || state_table[state->operation].class == SC_PIXELSHADER) > + { > + struct d3dx_parameter *param; > + void *param_value; > + BOOL param_dirty; > + HRESULT hr; > + > + if (FAILED(hr = d3dx9_get_param_value_ptr(pass, &pass->states[j], &param_value, &param, > + FALSE, &param_dirty))) > + return hr; > + > + if (param->object_id && !*(void **)param->data) > + { > + ret = E_FAIL; > + goto done; > + } > + } > + } > + } > +done: > + TRACE("Returning %#x.\n", ret); > + return ret; > } > > static HRESULT WINAPI ID3DXEffectImpl_FindNextValidTechnique(ID3DXEffect* iface, D3DXHANDLE technique, D3DXHANDLE* next_technique) > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index bb26fe4e51..7260411c39 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -7090,10 +7090,8 @@ static void test_effect_unsupported_shader(void) > ok(hr == D3D_OK, "Got result %#x.\n", hr); > > hr = effect->lpVtbl->ValidateTechnique(effect, "missing_technique"); > - todo_wine > ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech0"); > - todo_wine > ok(hr == E_FAIL, "Got result %#x.\n", hr); > > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > @@ -7101,7 +7099,6 @@ static void test_effect_unsupported_shader(void) > effect->lpVtbl->SetInt(effect, "i", 1); > ok(hr == D3D_OK, "Got result %#x.\n", hr); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > - todo_wine > ok(hr == E_FAIL, "Got result %#x.\n", hr); > effect->lpVtbl->SetInt(effect, "i", 0); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1");

1 0

Re: [v2 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Matteo Bruni 01 Sep '17

01 Sep '17

2017-08-31 20:56 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + if (!param->object_id || base->objects[param->object_id].creation_failed) It should be possible to replace this with a check for !*(void **)param->data. Unless you mean to use the flag for something else later on?

2 1

Re: [PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangle geometry
by Henri Verbeet 01 Sep '17

01 Sep '17

On 31 August 2017 at 14:00, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > - if (transform) > - { > - if (!d2d_matrix_invert(&g_i, transform)) > - return D2DERR_UNSUPPORTED_OPERATION; > - d2d_point_transform(&point, &g_i, point.x, point.y); > - } > + if (FAILED(hr = geometry_inv_transform_point(transform, &point))) > + return hr; That's fine in principle, but please do it in a separate commit, and please follow the existing conventions. E.g.: if (transform && FAILED(hr = d2d_point_transform_inverse(&point, transform, point.x, point.y))) return hr; > + stroke_width /= 2.0f; > + > + rect = geometry->u.rectangle.rect; > + rect.left -= stroke_width; > + rect.right += stroke_width; > + rect.top -= stroke_width; > + rect.bottom += stroke_width; > + > + dx = max(fabsf((rect.right + rect.left) / 2.0f - point.x) - (rect.right - rect.left) / 2.0f, 0.0f); > + dy = max(fabsf((rect.bottom + rect.top) / 2.0f - point.y) - (rect.bottom - rect.top) / 2.0f, 0.0f); > + > + if ((*contains = tolerance * tolerance > (dx * dx + dy * dy))) > + { > + rect = geometry->u.rectangle.rect; > + stroke_width += tolerance; > + rect.left += stroke_width; > + rect.right -= stroke_width; > + rect.top += stroke_width; > + rect.bottom -= stroke_width; > + > + *contains = rect.left >= point.x || rect.right <= point.x || rect.top >= point.y || rect.bottom <= point.y; > + } I think that looks a little more complicated than it needs to be. How about: D2D1_POINT_2F d, s; ... s.x = rect.right - rect.left; s.y = rect.bottom - rect.top; d.x = fabsf((rect.right + rect.left) * 0.5f - point.x); d.y = fabsf((rect.bottom + rect.top) * 0.5f - point.y); if (d.x < (s.x - stroke_width) * 0.5f - tolerance && d.y < (s.y - stroke_width) * 0.5f - tolerance) { *contains = FALSE; return S_OK; } d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f); d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f); *contains = (d.x * d.x + d.y * d.y) < tolerance * tolerance; I notice you don't have any tests with transformations, is that intentional? I think you'd need to transform the stroke width with the transformation matrix.

2 1

Re: [PATCH 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Matteo Bruni 31 Aug '17

31 Aug '17

2017-08-31 19:40 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > static HRESULT WINAPI ID3DXEffectImpl_ValidateTechnique(ID3DXEffect* iface, D3DXHANDLE technique) > { > - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface); > + struct d3dx9_base_effect *base = &effect->base_effect; > + struct d3dx_technique *tech = get_valid_technique(base, technique); > + HRESULT ret = D3D_OK; > + unsigned int i, j; > > - FIXME("(%p)->(%p): stub\n", This, technique); > + FIXME("iface %p, technique %p semi-stub.\n", iface, technique); Do you know what else is missing for ValidateTechnique()? Replying to myself: the return values mentioned in https://msdn.microsoft.com/en-us/library/windows/desktop/bb205835(v=vs.85).… give some ideas (although E_FAIL is not in the list, figures...)

2 1

Re: [PATCH 1/3] d3dx9/tests: Add test for effect with unsupported shader.
by Matteo Bruni 31 Aug '17

31 Aug '17

2017-08-31 19:40 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 185 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 185 insertions(+) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 9658d2dc82..3e8df11d24 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -6981,6 +6981,190 @@ static void test_effect_skip_constants(IDirect3DDevice9 *device) > effect->lpVtbl->Release(effect); > } > > +#if 0 > +vertexshader vs_arr[2] = > +{ > + asm > + { > + vs_3_0 > + def c0, 1, 1, 1, 1 > + dcl_position o0 > + mov o0, c0 > + }, > + > + asm > + { > + vs_3_sw > + def c256, 1, 1, 1, 1 > + dcl_position o0 > + mov o0, c256 > + }, > +}; > + > +int i; > + > +technique tech0 > +{ > + pass p0 > + { > + VertexShader = vs_arr[1]; > + } > +} > +technique tech1 > +{ > + pass p0 > + { > + VertexShader = vs_arr[i]; > + } > +} > +#endif > +static const DWORD test_effect_unsupported_shader_blob[] = > +{ > + 0xfeff0901, 0x000000ac, 0x00000000, 0x00000010, 0x00000004, 0x00000020, 0x00000000, 0x00000002, > + 0x00000001, 0x00000002, 0x00000007, 0x615f7376, 0x00007272, 0x00000002, 0x00000000, 0x0000004c, > + 0x00000000, 0x00000000, 0x00000001, 0x00000001, 0x00000000, 0x00000002, 0x00000069, 0x00000003, > + 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x00000003, 0x00003070, 0x00000006, > + 0x68636574, 0x00000030, 0x00000004, 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, > + 0x00000003, 0x00003070, 0x00000006, 0x68636574, 0x00000031, 0x00000002, 0x00000002, 0x00000006, > + 0x00000005, 0x00000004, 0x00000018, 0x00000000, 0x00000000, 0x0000002c, 0x00000048, 0x00000000, > + 0x00000000, 0x00000074, 0x00000000, 0x00000001, 0x0000006c, 0x00000000, 0x00000001, 0x00000092, > + 0x00000000, 0x00000058, 0x00000054, 0x000000a0, 0x00000000, 0x00000001, 0x00000098, 0x00000000, > + 0x00000001, 0x00000092, 0x00000000, 0x00000084, 0x00000080, 0x00000002, 0x00000002, 0x00000001, > + 0x00000038, 0xfffe0300, 0x05000051, 0xa00f0000, 0x3f800000, 0x3f800000, 0x3f800000, 0x3f800000, > + 0x0200001f, 0x80000000, 0xe00f0000, 0x02000001, 0xe00f0000, 0xa0e40000, 0x0000ffff, 0x00000002, > + 0x00000038, 0xfffe0300, 0x05000051, 0xa00f0100, 0x3f800000, 0x3f800000, 0x3f800000, 0x3f800000, > + 0x0200001f, 0x80000000, 0xe00f0000, 0x02000001, 0xe00f0000, 0xa0e40100, 0x0000ffff, 0x00000001, > + 0x00000000, 0xffffffff, 0x00000000, 0x00000002, 0x000000e4, 0x00000008, 0x615f7376, 0x00007272, > + 0x46580200, 0x0023fffe, 0x42415443, 0x0000001c, 0x00000057, 0x46580200, 0x00000001, 0x0000001c, > + 0x00000100, 0x00000054, 0x00000030, 0x00000002, 0x00000001, 0x00000034, 0x00000044, 0xabab0069, > + 0x00020000, 0x00010001, 0x00000001, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > + 0x4d007874, 0x6f726369, 0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461, 0x6d6f4320, > + 0x656c6970, 0x2e392072, 0x392e3932, 0x332e3235, 0x00313131, 0x0002fffe, 0x54494c43, 0x00000000, > + 0x000cfffe, 0x434c5846, 0x00000001, 0x10000001, 0x00000001, 0x00000000, 0x00000002, 0x00000000, > + 0x00000000, 0x00000004, 0x00000000, 0xf0f0f0f0, 0x0f0f0f0f, 0x0000ffff, 0x00000000, 0x00000000, > + 0xffffffff, 0x00000000, 0x00000001, 0x0000000a, 0x615f7376, 0x315b7272, 0x0000005d, > +}; > + > +#define TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS 81 > +#define TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_LEN 14 > + > +static void test_effect_unsupported_shader(void) > +{ > + D3DPRESENT_PARAMETERS present_parameters = {0}; > + IDirect3DVertexShader9 *vshader; > + unsigned int passes_count; > + IDirect3DDevice9 *device; > + UINT byte_code_size; > + ID3DXEffect *effect; > + IDirect3D9 *d3d; > + void *byte_code; > + ULONG refcount; > + HWND window; > + HRESULT hr; > + > + if (!(window = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0, > + 640, 480, NULL, NULL, NULL, NULL))) > + { > + skip("Couldn't create application window\n"); > + return; > + } > + if (!(d3d = Direct3DCreate9(D3D_SDK_VERSION))) > + { > + skip("Couldn't create IDirect3D9 object\n"); > + DestroyWindow(window); > + return; > + } > + present_parameters.Windowed = TRUE; > + present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD; > + hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, > + D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device); > + if (FAILED(hr)) { > + skip("Failed to create IDirect3DDevice9 object, hr %#x\n", hr); > + IDirect3D9_Release(d3d); > + DestroyWindow(window); > + return; > + } > + > + hr = D3DXCreateEffectEx(device, test_effect_unsupported_shader_blob, sizeof(test_effect_unsupported_shader_blob), > + NULL, NULL, NULL, 0, NULL, &effect, NULL); > + todo_wine > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + if (FAILED(hr)) > + { > + skip("Failed to create effect, skipping test.\n"); > + goto cleanup; > + } > + > + hr = effect->lpVtbl->ValidateTechnique(effect, "missing_technique"); > + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech0"); > + ok(hr == E_FAIL, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + effect->lpVtbl->SetInt(effect, "i", 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == E_FAIL, "Got result %#x.\n", hr); > + effect->lpVtbl->SetInt(effect, "i", 0); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetTechnique(effect, "tech0"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->Begin(effect, &passes_count, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(!vshader, "Got non NULL vshader.\n"); > + > + hr = effect->lpVtbl->EndPass(effect); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->End(effect); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->Begin(effect, &passes_count, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(!!vshader, "Got NULL vshader.\n"); > + 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 == TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_LEN * sizeof(DWORD), > + "Got unexpected byte code size %u.\n", byte_code_size); > + ok(!memcmp(byte_code, > + &test_effect_unsupported_shader_blob[TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS], > + byte_code_size), "Incorrect shader selected.\n"); I was confused by this at first, I thought somehow the unsupported vs_3_sw shader was set. That's not the case, TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS is the position of the vs_3_0 shader, but the naming throw me off. Can you rename those defines to "BYTECODE_VS_3_0_POS" or something that makes it clear that it refers to the hardware shader in vs_arr[0]?

2 2

Re: [PATCH v2 10/11] progman: Use proper size for combobox controls
by Alexandre Julliard 31 Aug '17

31 Aug '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/progman/progman.rc | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/programs/progman/progman.rc b/programs/progman/progman.rc > index 4804330aed..9f451a736b 100644 > --- a/programs/progman/progman.rc > +++ b/programs/progman/progman.rc > @@ -119,7 +119,7 @@ LTEXT "", PM_PROGRAM, 95, 5, 90, 15 > LTEXT "From group:", -1, 5, 13, 90, 15 > LTEXT "", PM_FROM_GROUP, 95, 13, 90, 15 > LTEXT "&To group:", PM_TO_GROUP_TXT, 5, 28, 140, 15 > -COMBOBOX PM_TO_GROUP, 5, 38, 140, 50, WS_TABSTOP | CBS_DROPDOWNLIST > +COMBOBOX PM_TO_GROUP, 5, 38, 140, 60, WS_TABSTOP | CBS_DROPDOWNLIST That kind of change doesn't seem necessary. We don't need all comboboxes to have the exact same height, only something reasonable. -- Alexandre Julliard julliard(a)winehq.org

3 2

Wine fails to compile in kernel32
by Fabian Maurer 30 Aug '17

30 Aug '17

Hello wine-devel, I'm running ArchLinux with gcc 7.2.0. Since commit e99811aee2dff88c41d6e2875401810a85325839 wine fails to build with > ../../../wine-git/dlls/kernel32/module.c: In function ‘load_library’: > ../../../wine-git/dlls/kernel32/module.c:987:44: error: initializer element is not constant > static const DWORD unsupported_flags = load_library_search_flags | Minimal test-program to show the error, just use something like "gcc main.c" to compile. ---------- static const int load_library_search_flags = 0; int main() { static const int unsupported_flags = load_library_search_flags; return 0; } ---------- Looks like an compiler error, but I'm not sure. Mind looking into that? Regards, Fabian Maurer

1 1

Re: [PATCH] winecfg: Use proper size for combobox controls
by Nikolay Sivov 30 Aug '17

30 Aug '17

On 29.08.2017 20:06, Fabian Maurer wrote: > Fixes Bug 43632. > winecfg used the broken behavior that was fixed > in f7f7b89e2e9117811c91269643868c6d063db5e9, > so we need to adjust it. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/winecfg/winecfg.rc | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > Interesting thing to test I think is to verify if current cross-built winecfg shows the same issue on Windows, before changing them all.

2 1

Re: [PATCH] winecfg: Use proper size for combobox controls
by Alexandre Julliard 30 Aug '17

30 Aug '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > Fixes Bug 43632. > winecfg used the broken behavior that was fixed > in f7f7b89e2e9117811c91269643868c6d063db5e9, > so we need to adjust it. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/winecfg/winecfg.rc | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) There are resources in other modules that look like they would have the same problem, a grep over the codebase would probably be useful. -- Alexandre Julliard julliard(a)winehq.org

2 3

WINED3DFMT_RESZ in wined3d.
by lorenzo Skotos 30 Aug '17

30 Aug '17

Hi. I was investigating a regression that cause the CheckDeviceFormat function to return WINED3DERR_NOTAVAILABLE when quering for RESZ surfaces. (WINED3DFMT_RESZ). This is causing OblivionReloaded (that worked properly until wine 2.5) to not work properly (it detect an incompatible hardware in wine or NVIDIA in wine-staging). I tacked the regression to this commit : https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979… [https://camo.githubusercontent.com/db3db2bf09d082a1f4244859890e83954d8410ab…]<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…> wined3d: Allow all formats with "glInternal" set in CheckSurfaceCapab… · wine-mirror/wine@8c98be4<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…> github.com …ility(). Rendertarget, depth/stencil and texturing restrictions are applied later in wined3d_check_device_format(), so to the extent this make a difference it mostly affects off-screen plain surf... I tried to fix this issue. The last patch I made in my local branch is adding a wined3d_format_texture_info in the format_texture_info array: + {WINED3DFMT_RESZ, GL_DEPTH24_STENCIL8, GL_DEPTH24_STENCIL8, 0, + GL_DEPTH_STENCIL, GL_UNSIGNED_INT, 0, + WINED3DFMT_FLAG_TEXTURE | WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL | WINED3DFMT_FLAG_RENDERTARGET | WINED3DFMT_FLAG_FBO_ATTACHABLE, + ARB_FRAMEBUFFER_OBJECT, NULL}, However I know almost nothing about the internal work of wined3d and opengl and don't know if this kind of addition is correct. OblivionReloaded works and can properly use the RESZ depth texture. I based this patch on the INTZ and RAWZ surfaces that are somewhat reported to be similar in scope in some docs. Also I don't know if the flags are correct or even meaningful in this scope. I could not find any documentation about the internal meaning of these flags. As the good Jon Snow I know nothing. Note I never did a patch in wine before (except adding some debug message). So please tell me if I'm in a completly wrong direction. Also note that I didn't run the test suite. Using make test is failing hard on an advapi32 test even without this patch applied. I attached the full patch file.

1 0

Re: [PATCH] user32: In GetMinMaxInfo, use the default X pos even if the Y pos changes
by Alexandre Julliard 30 Aug '17

30 Aug '17

Andrew Eikum <aeikum(a)codeweavers.com> writes: > This fixes maximizing to the wrong monitor on a Mac with multiple > monitors. Maximizing with multiple monitors continues to work correctly > on Linux for me. > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > dlls/user32/winpos.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) I'm not sure it makes sense to use only one coordinate of the monitor rectangle. What are the coordinate values that trigger this? -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/2] d3dx9: Get rid of constants modification bitmasks.
by Matteo Bruni 29 Aug '17

29 Aug '17

2017-08-10 12:02 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> The patch is fine overall, I only have a few questions / nitpicks. I'll skip the part about the delay with the review because it's old news by now... :/ > --- > dlls/d3dx9_36/d3dx9_private.h | 3 +- > dlls/d3dx9_36/preshader.c | 312 ++++++++++++++++++------------------------ > 2 files changed, 135 insertions(+), 180 deletions(-) > > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index 2727463a18..5ac5e63aeb 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -230,7 +230,7 @@ enum pres_reg_tables > struct d3dx_const_param_eval_output > { > struct d3dx_parameter *param; > - unsigned int table; > + enum pres_reg_tables table; The change is good but (unrelated to this patch) the enum type should really use the singular form. > static void dump_bytecode(void *data, unsigned int size) > @@ -674,6 +614,40 @@ static HRESULT append_const_set(struct d3dx_const_tab *const_tab, struct d3dx_co > return D3D_OK; > } > > +static void append_pres_const_set(struct d3dx_const_tab *const_tab, struct d3dx_preshader *pres) Nitpick: this function appends all the struct d3dx_const_param_eval_output from the preshader so calling it "append_pres_const_sets" (or even something more explicit to that effect, e.g. clarifying that you're appending entries about the preshader outputs, which are shader inputs) would be preferable. > +{ > + unsigned int i; > + struct d3dx_const_param_eval_output const_set; > + > + memset(&const_set, 0, sizeof(const_set)); You could initialize const_set in the definition instead, like: struct d3dx_const_param_eval_output const_set = {NULL}; which looks slightly better to me. It doesn't matter much of course. > @@ -971,10 +946,30 @@ static HRESULT get_constants_desc(unsigned int *byte_code, struct d3dx_const_tab > if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index]))) > goto cleanup; > } > + if (pres) > + append_pres_const_set(out, pres); > if (out->const_set_count) > { > struct d3dx_const_param_eval_output *new_alloc; > > + qsort(out->const_set, out->const_set_count, sizeof(*out->const_set), compare_const_set); > + for (i = 0; i < out->const_set_count - 1; ++i) > + { > + if (out->const_set[i].constant_class == D3DXPC_FORCE_DWORD > + && out->const_set[i + 1].constant_class == D3DXPC_FORCE_DWORD > + && out->const_set[i].table == out->const_set[i + 1].table > + && out->const_set[i].register_index + out->const_set[i].register_count > + >= out->const_set[i + 1].register_index) > + { > + out->const_set[i].register_count = out->const_set[i + 1].register_index > + + out->const_set[i + 1].register_count - out->const_set[i].register_index; I think this might lose account of some registers. E.g. consider the case where const_set[i] has register_index == 0 and register_count == 4 while const_set[i + 1] has register_index == 1 and register_count == 1. This would seem to lose track of the last two components. I know that preshaders found in the wild seem not to overwrite already written output registers but I wouldn't feel safe with (silently) depending on it. > + memmove(&out->const_set[i + 1], &out->const_set[i + 2], sizeof(out->const_set[i]) > + * (out->const_set_count - i - 2)); > + --out->const_set_count; > + --i; Mostly matter of personal preference but rather than decrementing the for counter in the if when coalescing I'd write this as a while and increment "i" only in the (currently absent) else case. > @@ -1529,22 +1534,56 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > data += param->rows * param->columns; > } > start_offset = get_offset_reg(table, const_set->register_index); > - if (table_info[table].type == param_type) > - regstore_set_modified(rs, table, start_offset, > - get_offset_reg(table, const_set->register_count) * const_set->element_count); > - else > + if (table_info[table].type != param_type) > regstore_set_data(rs, table, start_offset, (unsigned int *)rs->tables[table] + start_offset, > get_offset_reg(table, const_set->register_count) * const_set->element_count, param_type); > } > + > + > const_tab->update_version = new_update_version; One blank line too many. > -} > + if (!update_device) > + return D3D_OK; > > + for (const_idx = 0; const_idx < const_tab->const_set_count; ++const_idx) > + { > + struct d3dx_const_param_eval_output *const_set = &const_tab->const_set[const_idx]; > + > + if (device_update_all || (const_set->param > + ? is_param_dirty(const_set->param, update_version) : pres_dirty)) > + { > + enum pres_reg_tables table = const_set->table; > + > + if (table == device_table && device_start + device_count == const_set->register_index) Again nitpicking: are there better names for those variables? Maybe use the "current_" prefix instead? It's pretty clear what's going on either way, so up to you.

2 6

Re: [PATCH] comctl32/taskdialog: Add callback and tests
by Nikolay Sivov 28 Aug '17

28 Aug '17

On 28.07.2017 19:53, Fabian Maurer wrote: > > +static HRESULT callback(struct taskdialog_info *dialog_info, UINT uNotification, WPARAM wParam, LPARAM lParam) > +{ > + if(dialog_info->callback) > + return dialog_info->callback(dialog_info->hwnd, uNotification, wParam, lParam, dialog_info->callback_data); > + return S_OK; > +} Please use consistent naming and formatting, like space after 'if' for example, and uNotification -> notification. > + > static INT_PTR CALLBACK taskdialog_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) > { > + static const WCHAR taskdialog_info_propnameW[] = {'T','a','s','k','D','i','a','l','o','g','I','n','f','o',0}; > + struct taskdialog_info *dialog_info; > + > TRACE("hwnd=%p msg=0x%04x wparam=%lx lparam=%lx\n", hwnd, msg, wParam, lParam); > > + if(msg != WM_INITDIALOG && msg != WM_NCDESTROY) > + dialog_info = GetPropW(hwnd, taskdialog_info_propnameW); > + Why WM_NCDESTROY is a special case? > switch (msg) > { > + case WM_INITDIALOG: > + dialog_info = (struct taskdialog_info *)lParam; > + dialog_info->hwnd = hwnd; > + SetPropW(hwnd, taskdialog_info_propnameW, dialog_info); > + > + callback(dialog_info, TDN_DIALOG_CONSTRUCTED, 0, 0); > + callback(dialog_info, TDN_CREATED, 0, 0); > + break; This looks suspicious, documentation is not strict on this, but it seems TDN_CREATED corresponds for to WM_SHOWWINDOW stage. > case WM_COMMAND: > if (HIWORD(wParam) == BN_CLICKED) > { > WORD command_id = LOWORD(wParam); > - EndDialog(hwnd, command_id); > + click_button(dialog_info, command_id); > return TRUE; > } > break; > + case WM_DESTROY: > + callback(dialog_info, TDN_DESTROYED, 0, 0); > + RemovePropW(hwnd, taskdialog_info_propnameW); > + break; > + > + /* Custom messages*/ What do you mean by "custom"? > + > + case TDM_CLICK_BUTTON: > + click_button(dialog_info, wParam); > + break; > } > return FALSE; > } > @@ -507,17 +556,24 @@ HRESULT WINAPI TaskDialogIndirect(const TASKDIALOGCONFIG *taskconfig, int *butto > int *radio_button, BOOL *verification_flag_checked) > { > DLGTEMPLATE *template; > + struct taskdialog_info dialog_info; > INT ret; > > TRACE("%p, %p, %p, %p\n", taskconfig, button, radio_button, verification_flag_checked); > > + if(!taskconfig || taskconfig->cbSize != sizeof(TASKDIALOGCONFIG)) > + return E_INVALIDARG; Tests you added only check cbSize == 0 case. > + > + dialog_info.callback = taskconfig->pfCallback; > + dialog_info.callback_data = taskconfig->lpCallbackData; > + > template = create_taskdialog_template(taskconfig); > - ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, 0); > + ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, (LPARAM)&dialog_info); > Free(template); > > if (button) *button = ret; > if (radio_button) *radio_button = taskconfig->nDefaultButton; > - if (verification_flag_checked) *verification_flag_checked = TRUE; > + if (verification_flag_checked) *verification_flag_checked = FALSE; Why do you need this flag value change? > > return S_OK; > } > diff --git a/dlls/comctl32/tests/taskdialog.c b/dlls/comctl32/tests/taskdialog.c > index c937127897..867361617c 100644 > --- a/dlls/comctl32/tests/taskdialog.c > +++ b/dlls/comctl32/tests/taskdialog.c > @@ -24,13 +24,250 @@ > #include "winuser.h" > #include "commctrl.h" > > +#include "wine/list.h" > #include "wine/test.h" > #include "v6util.h" > +#include "msg.h" > + > +#define WM_TD_CALLBACK (WM_APP) /* Custom dummy message to wrap callback notifications */ > + > +#define NUM_MSG_SEQUENCES 1 > +#define TASKDIALOG_SEQ_INDEX 0 > + > +#define TEST_NUM_BUTTONS 20 /* Number of custom buttons to test with */ > + > +#define ID_START 20 /* Lower IDs might be used by the system */ > +#define ID_START_BUTTON (ID_START + 0) > > static HRESULT (WINAPI *pTaskDialogIndirect)(const TASKDIALOGCONFIG *, int *, int *, BOOL *); > static HRESULT (WINAPI *pTaskDialog)(HWND, HINSTANCE, const WCHAR *, const WCHAR *, const WCHAR *, > TASKDIALOG_COMMON_BUTTON_FLAGS, const WCHAR *, int *); > > +static struct msg_sequence *sequences[NUM_MSG_SEQUENCES]; > + > +/* Message lists to test against */ > + > +struct message_send_info > +{ > + UINT send_message; > + WPARAM send_wparam; > + LPARAM send_lparam; > + > + BOOL post; /* post instead of send */ > + const CHAR *title_target; /* control text, 0 means it's send to the dialog form instead */ > +}; > + > +struct message_info > +{ > + UINT recv_message; /* Message the callback receives */ > + WPARAM recv_wparam; > + LPARAM recv_lparam; > + > + HRESULT ret; /* Value the callback should return */ > + > + struct message_send_info send[9]; /* Message to send to trigger the next callback message */ > +}; > + > +static const struct message_info *current_message_info; > + > +static const struct message_info mes_return_press_ok[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { WM_KEYDOWN, VK_RETURN, 0, TRUE }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; > + > +static const struct message_info mes_cancel_button_press[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, S_FALSE, {{ 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, 0xFF, {{ 0 }}}, /* Random return value tested here */ > + { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; > + > +static const struct message_info mes_send_button_directly[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { WM_LBUTTONDOWN, MK_LBUTTON, 0, TRUE, "02" }, > + { WM_LBUTTONUP, 0, 0, TRUE, "02" }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, ID_START + 2, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; This looks too complicated, I'd just check notifications callback received, using 'struct message' data, same way WM_NOTIFY tests work. > + > +/* Create a message to test against */ > +static struct message create_test_message(UINT message, WPARAM wParam, LPARAM lParam) > +{ > + struct message mes; > + > + mes.message = WM_TD_CALLBACK; > + mes.id = message; > + mes.flags = sent|wparam|lparam|id; > + mes.wParam = wParam; > + mes.lParam = lParam; > + > + return mes; > +} > + > +/* Our only way to get a button handle, since GetDlgItem and FindWindowEx don't work for the official taskdialog */ > + > +static HWND taskdialog_child; > +BOOL CALLBACK enum_taskdialog_children_proc(HWND hwnd, LPARAM lParam) > +{ > + CHAR text[100]; > + const CHAR *title = (const CHAR *)lParam; > + > + GetWindowTextA(hwnd, text, sizeof(text)); > + > + if(lstrcmpA(text, title) == 0) > + { > + taskdialog_child = hwnd; > + return FALSE; > + } > + > + return TRUE; > +} > + > +static HWND get_child_from_title(HWND hwnd_parent, const CHAR *title) > +{ > + taskdialog_child = NULL; > + EnumChildWindows(hwnd_parent, enum_taskdialog_children_proc, (LPARAM)title); > + return taskdialog_child; > +} I don't think we need to get that deep. > > + init_msg_sequences(sequences, NUM_MSG_SEQUENCES); > + test_TaskDialogIndirect(); This name is too generic, in a test function like that you can test invalid parameter handling for example. Then have another function for callback notifications, and another one for TDM_* messages. I think for first iteration test should simply check creation/destruction notifications and callback user data value.

2 1

Re: [PATCH] ws2_32: Also check the FQDN when injecting the local IP.
by Alexandre Julliard 28 Aug '17

28 Aug '17

Stefan Dösinger <stefan(a)codeweavers.com> writes: > @@ -6576,6 +6592,8 @@ int WINAPI WS_getaddrinfo(LPCSTR nodename, LPCSTR servname, const struct WS_addr > > hostname = get_hostname(); > if (!hostname) return WSA_NOT_ENOUGH_MEMORY; > + fqdn = get_fqdn(); > + if (!fqdn) return WSA_NOT_ENOUGH_MEMORY; It seems to me that you shouldn't need to retrieve both separately, the FQDN contains the hostname already. -- Alexandre Julliard julliard(a)winehq.org

1 0

Issue with suspended process
by Jon Doron 25 Aug '17

25 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

3 3

Re: [PATCH resend] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.
by Nikolay Sivov 25 Aug '17

25 Aug '17

On 03.08.2017 18:17, Alex Henrie wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=42468 > > To be clear, I am not trying to fix all whitespace processing issues. I > am only trying to prevent "" from being erroneously inserted into > the output. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/msxml3/tests/domdoc.c | 106 +++++++++++++++++++++++++++++++++++++-------- > dlls/msxml3/text.c | 42 +++++++++++++++--- > 2 files changed, 122 insertions(+), 26 deletions(-) > > diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c I think it needs more tests, for xml:space and put_preserveWhiteSpace(). > diff --git a/dlls/msxml3/text.c b/dlls/msxml3/text.c > index 3a12d4bf62..6814c1f817 100644 > --- a/dlls/msxml3/text.c > +++ b/dlls/msxml3/text.c > @@ -519,10 +519,15 @@ static HRESULT WINAPI domtext_get_xml( > BSTR* p) > { > domtext *This = impl_from_IXMLDOMText( iface ); > + HRESULT hr; > > TRACE("(%p)->(%p)\n", This, p); > > - return node_get_xml(&This->node, FALSE, p); > + hr = node_get_xml(&This->node, FALSE, p); > + if (hr == S_OK) > + *p = EnsureCorrectEOL(*p); > + > + return hr; Will node_get_xml(..., TRUE, ...) work? > } > > static HRESULT WINAPI domtext_transformNode( > @@ -616,15 +621,38 @@ static HRESULT WINAPI domtext_put_data( > BSTR data) > { > domtext *This = impl_from_IXMLDOMText( iface ); > - static const WCHAR rnW[] = {'\r','\n',0}; > + BSTR normalized_data = NULL; > + HRESULT hr; > + int i, j; > > TRACE("(%p)->(%s)\n", This, debugstr_w(data)); > > - if (data && !strcmpW(rnW, data)) > - This->node.node->name = xmlStringTextNoenc; > - else > - domtext_reset_noenc(This); > - return node_set_content(&This->node, data); > + if (data) > + { > + /* normalize line endings */ > + normalized_data = SysAllocStringLen(NULL, SysStringLen(data)); > + if (!normalized_data) return E_OUTOFMEMORY; > + for (i = 0, j = 0; data[i]; i++) > + { > + if (data[i] == '\r') > + { > + if (data[i + 1] == '\n') > + continue; /* change \r\n to just \n */ > + else > + normalized_data[j] = '\n'; /* change \r by itself to \n */ > + } > + else > + normalized_data[j] = data[i]; > + j++; > + } > + normalized_data[j] = 0; The only purpose for that is to inhibit libxml2 escaping, right? Could this be done the way it is now, but with something more elaborate than comparing to "\r\n"? > + } > + > + domtext_reset_noenc(This); > + hr = node_set_content(&This->node, normalized_data); > + > + SysFreeString(normalized_data); > + return hr; > } > > static HRESULT WINAPI domtext_get_length( >

2 1

Re: [v2] d3d9/tests: Add test for SW shader in MVP mode.
by Marvin 25 Aug '17

25 Aug '17

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=32837 Your paranoid android. === w8 (32 bit visual) === visual.c:7952: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080

2 1

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel