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

Re: [PATCH 1/2] include: Fix include arguments.
by Matteo Bruni July 13, 2016

July 13, 2016

This probably makes sense (although, WRT the patch title, this is probably more of a consistency improvement rather than a "fix"). Maybe split the d3dtypes.h change to a separate patch?

1 0

Re: include: Add d3dx10math.inl.
by Matteo Bruni July 13, 2016

July 13, 2016

2016-07-04 19:26 GMT+02:00 Andrey Gusev <andrey.goosev(a)gmail.com>: +#ifndef __D3DX10MATH_INL__ +#define __D3DX10MATH_INL__ My copy of the file (from the latest DX SDK) uses __D3DXMATH_INL__ as header guard. I think we should do the same for compatibility. +/* constructors & operators */ Nitpicking but this comment doesn't seem particularly useful. +inline D3DXVECTOR2::D3DXVECTOR2(const FLOAT *pf) Just use plain float. + if(!pf) return; Space before '(', return on a separate line. +inline D3DXVECTOR2::operator FLOAT* () +{ + return (FLOAT*)&x; Please put a space between "float" and '*' in the cast. Probably do the same in the operator definition, although I don't really care in that regard. +inline D3DXMATRIX D3DXMATRIX::operator - () const +{ + return D3DXMATRIX(-_11, -_12, -_13, -_14, + -_21, -_22, -_23, -_24, + -_31, -_32, -_33, -_34, + -_41, -_42, -_43, -_44); +} This is almost identical to the MS's header. Please use 8 space indentation for line continuations. Probably drop the space between the "operator" keyword and the operator and between the operator and the opening parenthesis, in the operator declaration. +inline D3DXCOLOR::operator UINT () const +{ + UINT _r = r >= 1.0f ? 0xff : r <= 0.0f ? 0x00 : (UINT)(r * 255.0f + 0.5f); + UINT _g = g >= 1.0f ? 0xff : g <= 0.0f ? 0x00 : (UINT)(g * 255.0f + 0.5f); + UINT _b = b >= 1.0f ? 0xff : b <= 0.0f ? 0x00 : (UINT)(b * 255.0f + 0.5f); + UINT _a = a >= 1.0f ? 0xff : a <= 0.0f ? 0x00 : (UINT)(a * 255.0f + 0.5f); + + return (_a << 24) | (_r << 16) | (_g << 8) | (_b << 0); +} This one is also virtually identical. There seems to be room for writing a slightly different but equivalent implementation. FWIW identifiers starting with an underscore are supposed to be reserved, to some degree. It's generally better to avoid them. +inline D3DXFLOAT16::D3DXFLOAT16() +{ +} These default constructors are in (native) d3dx10math.h. I know that with d3dx9* headers we did put those in the .inl file but I think that was a bad idea. +inline BOOL D3DXFLOAT16::operator == (const D3DXFLOAT16 &f) const +{ This one is also almost identical, implementation-wise. Now maybe there isn't that much room for variations here but still, at the very least replace the "== 0" with a '!'. If you can find other reasonable ways to differentiate, that's even better. +inline D3DXCOLOR::D3DXCOLOR(UINT col) +{ + const FLOAT f = 1.0f / 255.0f; + r = f * (FLOAT)(unsigned char)(col >> 16); + g = f * (FLOAT)(unsigned char)(col >> 8); + b = f * (FLOAT)(unsigned char)(col >> 0); + a = f * (FLOAT)(unsigned char)(col >> 24); +} Another one essentially identical. I would write it as something like: inline D3DXCOLOR::D3DXCOLOR(UINT col) { float inv = 1.0f / 255.0f; a = ((col >> 24) & 0xff) * inv; r = ((col >> 16) & 0xff) * inv; g = ((col >> 8) & 0xff) * inv; b = (col & 0xff) * inv; } +static inline D3DXCOLOR* D3DXColorLerp(D3DXCOLOR *pout, const D3DXCOLOR *pc1, const D3DXCOLOR *pc2, FLOAT s) Don't use those prefixed argument names: out, c1 and c2 work just fine. Also you want a space between D3DXCOLOR and '*' and no space between '*' and the function name. +{ + if ( !pout || !pc1 || !pc2 ) return NULL; Please no spaces after the '(' and before the ')'. Return on a separate line. + pout->r = (1-s) * (pc1->r) + s *(pc2->r); You want spaces around the '-' operator and a space after the multiplication operator. No need for the parentheses around pc1->r and pc2->r. Regardless, MS's header uses the "optimized" linear interpolation formula, with just one multiplication. Your current implementation will generally give slightly different results (because of the limited precision of floats) so you should stick to the same formula instead. This isn't an exhaustive list of issues. Many of the above comments apply to multiple places over the patch.

1 0

Re: [PATCH 4/5] d3d11/tests: Add test for writing special floating-point values to render target.
by Henri Verbeet July 13, 2016

July 13, 2016

On 13 July 2016 at 10:52, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +#ifndef NAN > +static inline float __port_nan(void) > +{ > + static const unsigned __nan_bytes = 0x7fc00000; > + return *(const float *)&__nan_bytes; > +} > +#define NAN __port_nan() > +#endif This looks unused. > + const struct vec4 *value = get_readback_vec4(&rb, x, y); > + ok(isnan(value->x) && value->y == INFINITY && value->z == -INFINITY && value->w == 1.0f, > + "Got unexpected value {%.8e, %.8e, %.8e, %.8e} at (%u, %u).\n", > + value->x, value->y, value->z, value->w, x, y); It would slightly change the meaning of the test, but you could also just read these back as uvec4's and compare the bit patterns. > @@ -9312,4 +9400,5 @@ START_TEST(d3d11) > test_check_feature_support(); > test_create_unordered_access_view(); > test_immediate_constant_buffer(); > + test_special_float_values(); For what it's worth, those are usually called "floating-point specials". (And hence fp_special_test() in the d3d9 tests.)

1 0

Re: ntdll/tests: Use inline implementation of NtCurrentTeb for exception tests.
by Marvin July 13, 2016

July 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=24168 Your paranoid android. === wvistau64 (32 bit exception) === exception.c:489: Test failed: 42: Unexpected eip 0x3d0000/0x3d0001 exception: unhandled exception 80000004 at 003D0000 exception.c:489: Test failed: 42: Unexpected eip 0x77baed97/0x3d0001 === w7u (32 bit exception) === exception.c:489: Test failed: 42: Unexpected eip 0x280000/0x280001 exception: unhandled exception 80000004 at 00280000 exception.c:489: Test failed: 42: Unexpected eip 0x77cde34f/0x280001 === w1064 (32 bit exception) === An error occurred while waiting for the test to complete: network read timed out (wait2/connect:AgentVersion.h:0/9) The test VM has crashed, rebooted or lost connectivity (or the TestAgent server died) === w1064 (64 bit exception) === An error occurred while waiting for the test to complete: network read timed out (wait2/connect:AgentVersion.h:0/9) The test VM has crashed, rebooted or lost connectivity (or the TestAgent server died)

3 2

Re: [PATCH] wined3d: Fix the size parameter of glVertexAttrib*Pointer().
by Józef Kucia July 13, 2016

July 13, 2016

On Wed, Jul 13, 2016 at 2:48 AM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > The mistake is obvious when looking at the GL 3.0 spec: > p29: > void VertexAttribPointer(uint index, int size, enum type, boolean normalized, sizei stride, const void *pointer); > void VertexAttribIPointer(uint index, int size, enum type, sizei stride, const void *pointer); > > p30: > Command Sizes > VertexAttribPointer 1, 2, 3, 4 > VertexAttribIPointer 1, 2, 3, 4 > > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/state.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) This looks wrong to me. The "gl_vtx_format" field is equal to the "component_count" field. There's one exception though, the "gl_vtx_format" is set to GL_BGRA for WINED3DFMT_B8G8R8A8_UNORM. GL_BGRA was added as an accepted value by GL_ARB_vertex_array_bgra (core since OpenGL 3.2). What are you trying to fix?

2 2

Re: ncrypt: add some ncrypt stubs
by Alex Henrie July 12, 2016

July 12, 2016

+ return NTE_INVALID_PARAMETER; I think NTE_NOT_SUPPORTED makes more sense for stub functions in this DLL. Other than that, the patch looks good. -Alex

2 1

Ubuntu considering dropping 32-bit libraries
by Josh DuBois July 12, 2016

July 12, 2016

A Slashdot post yesterday pointed to a worrying thread on the Ubuntu mailing list archive about dropping 32-bit support from Ubuntu. The Ubuntu thread is here: https://lists.ubuntu.com/archives/ubuntu-devel/2016-June/039420.html It is not 100% clear to me what they are considering - they mention dropping 32-bit .isos and also ceasing to support users who want to run 32-bit apps (at least unless the app is packaged in a container alongside all of the necessary libraries). Their thought is to do this in a couple of years, with Ubuntu 18.10: 18.10+: * Stop providing i386 port * Run legacy i386 only application in snaps / containers / virtual machines Disturbingly, no one on the Ubuntu thread ever mentions Wine. Oddly there is discussion about IoT applications which might need 32-bit libraries and a proposal to continue building select libs for those things, but maybe minus the graphics stack :-(. In the Slashdot post someone at least mentions Wine, but the little discussion that prompted was seemingly in error until Francois came along to correct them. I think Wine should be aware of the issue and make some noise about it before distros grow attached to the idea of ceasing to build 32-bit libraries. I will probably post to the Ubuntu thread to at least mention Wine, unless someone here is involved in that community and thinks they can do a better job. The more distros hear from the Wine community about the value of 32-bit libraries, the better. It might be that they will conclude on their own that users need them to continue building 32-bit libraries, but I don't know that we can trust distros not to break things if we don't speak up.

1 1

Re: [PATCH 3/3] wined3d/resource: Performance improvements, try 2
by Henri Verbeet July 12, 2016

July 12, 2016

This could do with a more descriptive subject line. On 11 July 2016 at 18:13, Patrick Rudolph <siro(a)das-labor.org> wrote: > Tests showed that D3DLOCK_NOOVERWRITE is prefered over D3DLOCK_DISCARD > and that D3DLOCK_NOOVERWRITE doesn't depend on dynamic buffers. "preferred", but you'll probably want to say "takes precedence over" instead. > Unigine Heavon maps a lot of buffers with both flags, but run on about > 85% GPU-load, as WINE removes both hints and the game has to wait for > the buffer to be mapped. "Unigine Heaven", "Wine" > OpenGL core profile version string: 3.3 (Core Profile) Mesa 12.1.0-devel (git-60c68c5) It doesn't make much of a difference here, but note that unless you set the "MaxVersionGL" registry key, wined3d doesn't use core profiles. > --- > dlls/d3d9/tests/visual.c | 10 +++++----- > dlls/wined3d/resource.c | 33 ++++++++++++++++----------------- > 2 files changed, 21 insertions(+), 22 deletions(-) What about the d3d8 tests? > @@ -359,29 +359,28 @@ DWORD wined3d_resource_sanitize_map_flags(const struct wined3d_resource *resourc > { > /* Not all flags make sense together, but Windows never returns an error. > * Catch the cases that could cause issues. */ > - if (flags & WINED3D_MAP_READONLY) > + if ((flags & (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) == > + (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) > { > - if (flags & WINED3D_MAP_DISCARD) > - { > - WARN("WINED3D_MAP_READONLY combined with WINED3D_MAP_DISCARD, ignoring flags.\n"); > - return 0; > - } > - if (flags & WINED3D_MAP_NOOVERWRITE) > - { > - WARN("WINED3D_MAP_READONLY combined with WINED3D_MAP_NOOVERWRITE, ignoring flags.\n"); > - return 0; > - } > + WARN("WINED3D_MAP_DISCARD and WINED3D_MAP_NOOVERWRITE used together, using WINED3D_MAP_NOOVERWRITE.\n"); > + flags &= ~WINED3D_MAP_DISCARD; > + } > + if ((flags & (WINED3D_MAP_READONLY | WINED3D_MAP_NOOVERWRITE)) == > + (WINED3D_MAP_READONLY | WINED3D_MAP_NOOVERWRITE)) > + { > + WARN("WINED3D_MAP_READONLY and WINED3D_MAP_NOOVERWRITE used together, clearing WINED3D_MAP_READONLY.\n"); > + flags &= ~WINED3D_MAP_READONLY; > } > - else if ((flags & (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) > - == (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) > + > + if ((flags & (WINED3D_MAP_READONLY | WINED3D_MAP_DISCARD)) == > + (WINED3D_MAP_READONLY | WINED3D_MAP_DISCARD)) > { > - WARN("WINED3D_MAP_DISCARD and WINED3D_MAP_NOOVERWRITE used together, ignoring.\n"); > + WARN("WINED3D_MAP_READONLY combined with WINED3D_MAP_DISCARD, ignoring flags.\n"); > return 0; > } > - else if (flags & (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE) > - && !(resource->usage & WINED3DUSAGE_DYNAMIC)) > + else if ((flags & WINED3D_MAP_DISCARD) && !(resource->usage & WINED3DUSAGE_DYNAMIC)) > { > - WARN("DISCARD or NOOVERWRITE map on non-dynamic buffer, ignoring.\n"); > + WARN("DISCARD on non dynamic buffer, ignoring.\n"); > return 0; > } This changes several things at once. One (logical) change per commit, please.

1 0

Re: [PATCH 1/3] d3d9/tests/visual: Add Vertexbuffer locking tests, try 2
by Henri Verbeet July 12, 2016

July 12, 2016

On 11 July 2016 at 18:13, Patrick Rudolph <siro(a)das-labor.org> wrote: > +static void test_vb_lock_flags(void) > +{ > + IDirect3DVertexBuffer9 *buffer; > + IDirect3DDevice9 *device; > + IDirect3D9 *d3d9; > + DWORD i, j, tri, quads; > + HWND window; > + HRESULT hr; > + ULONG size; > + unsigned refcount; "ULONG refcount;", but that's minor. > + { > + "0", > + 0, > + 0, > + FALSE, > + FALSE > + }, This may have been inspired by other tests, but as long as it fits I'd prefer the more compact option of keeping this all on a single line. You may have to shorten the "flags_s" values a bit to do that. > + static struct > + { > + struct > + { > + struct vec3 position; > + DWORD diffuse; > + } strip[4]; > + } > + *quad_strip_array, quad_strip1 = > + { > + { > + {{-1.0f, -1.0f, 0.0f}, 0xffff0000}, > + {{-1.0f, 1.0f, 0.0f}, 0xff00ff00}, > + {{ 1.0f, -1.0f, 0.0f}, 0xff0000ff}, > + {{ 1.0f, 1.0f, 0.0f}, 0xffffffff}, > + } > + }, > + quad_strip2 = > + { > + { > + {{-1.0f, -1.0f, 0.0f}, 0xffffff00}, > + {{-1.0f, 1.0f, 0.0f}, 0xffffff00}, > + {{ 1.0f, -1.0f, 0.0f}, 0xffffff00}, > + {{ 1.0f, 1.0f, 0.0f}, 0xffffff00}, > + } > + }; These are triangle strips or quads, but I don't think "quad strips" makes sense here. "quad_strip1" and "quad_strip2" can be const, and "quad_strip_array" doesn't need to be static, right? > + hr = IDirect3DDevice9_CreateVertexBuffer(device, size, tests[i].usage, 0, D3DPOOL_DEFAULT, &buffer, NULL); > + ok(SUCCEEDED(hr), "Failed to create vertex buffer, hr %#x.\n", hr); > + if (!SUCCEEDED(hr)) > + continue; The continue is pointless, the test would have already failed at that point. > + if (tests[i].unsyncronized == unsyncronized) > + break; This means the test will only try the smallest buffer size for cases that you expect to be synchronised, right? That seems problematic, since the reason you try the larger buffer sizes is that you can't necessarily reliably detect the unsynchronised cases with the smaller buffer sizes. > + if (tests[i].todo) > + { > + todo_wine ok(tests[i].unsyncronized == unsyncronized, "Expected buffer mapped %s. Flags = %s. Usage = %d.\n", > + tests[i].unsyncronized ? "unsyncronized" : "syncronized", tests[i].flags_s, tests[i].usage); > + } > + else > + { > + ok(tests[i].unsyncronized == unsyncronized, "Expected buffer mapped %s. Flags = %s. Usage = %d.\n", > + tests[i].unsyncronized ? "unsyncronized" : "syncronized", tests[i].flags_s, tests[i].usage); > + } I think "todo_wine_if" would be appropriate here. Please use %#x to print the usage. Note that ddraw has the mostly equivalent flags DDLOCK_NOOVERWRITE and DDLOCK_DISCARDCONTENTS as well. Direct3D 10 has D3D10_MAP_WRITE_DISCARD and D3D10_MAP_WRITE_NO_OVERWRITE, but you can't use them at the same time because they're not flags. Direct3D 11 is similar to Direct3D 10. You'll need to account for the testbot failures in some way. They may be specific to WARP, but it's suspicious that the "D3DLOCK_NOOVERWRITE" tests do seem to pass. Also, please follow the usual conventions for the subject line. (I.e., "d3d9/tests:", "d3d8/tests:", "wined3d:".)

1 0

Re: [PATCH] dxgi: Implement IDXGISwapChain GetFullscreenState
by Henri Verbeet July 12, 2016

July 12, 2016

On 11 July 2016 at 12:08, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +static HRESULT STDMETHODCALLTYPE dxgi_swapchain_GetFullscreenState(IDXGISwapChain *iface, > + BOOL *fullscreen, IDXGIOutput **target) > +{ > + HRESULT hr = S_OK; > + > + TRACE("iface %p, fullscreen %p, target %p semi-stub.\n", iface, fullscreen, target); > + > + if (fullscreen) > + *fullscreen = TRUE; > + > + if (target) > + hr = dxgi_swapchain_GetContainingOutput(iface, target); > + > + return hr; > +} That's not a real implementation, of course. > @@ -496,6 +498,22 @@ static void test_create_swapchain(void) > refcount = IUnknown_Release(parent); > todo_wine ok(refcount == 4, "Got unexpected refcount %u.\n", refcount); > > + hr = IDXGISwapChain_GetFullscreenState(swapchain, NULL, NULL); > + ok(hr == S_OK, "GetFullscreenState failed, hr %#x.\n", hr); > + > + hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, NULL); > + ok(hr == S_OK, "GetFullscreenState failed, hr %#x.\n", hr); > + > + hr = IDXGISwapChain_GetFullscreenState(swapchain, NULL, &target); > + ok(hr == S_OK, "GetFullscreenState failed, hr %#x.\n", hr); > + if(target) > + IDXGIOutput_Release(target); > + > + hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, &target); > + ok(hr == S_OK, "GetFullscreenState failed, hr %#x.\n", hr); > + if(target) > + IDXGIOutput_Release(target); > + And this doesn't test much beyond the function returning S_OK.

1 0

Re: [PATCH v2] kernel32: Use setprogname(3) to set the program name if available.
by Theodore Dubois July 11, 2016

July 11, 2016

This patch causes a segfault in the event that __wine_main_argv is not the argv that got passed to main. This happened in a program I wrote that acts as a custom wine loader. It creates an argv by using malloc, then passes that to wine_init. Why wouldn’t setprogname(argv[1]) be adequate? ~Theodore > On Feb 19, 2016, at 2:33 PM, Charles Davis <cdavis5x(a)gmail.com> wrote: > > Signed-off-by: Charles Davis <cdavis5x(a)gmail.com> > --- > Try 2: > * Clean up the #ifdefs a bit. (I'm not happy about the duplicated code, though.) > * Don't copy the progname into a static array. There's no need; the argument vector lasts as long as the process itself. (I suppose I was being overly defensive with that.) > > configure.ac | 1 + > dlls/kernel32/process.c | 17 ++++++++++++++--- > 2 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/configure.ac b/configure.ac > index c9445e7..2ebbb10 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -2026,6 +2026,7 @@ AC_CHECK_FUNCS(\ > sched_yield \ > select \ > setproctitle \ > + setprogname \ > setrlimit \ > settimeofday \ > sigaltstack \ > diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c > index 86333be..4771108 100644 > --- a/dlls/kernel32/process.c > +++ b/dlls/kernel32/process.c > @@ -1118,9 +1118,20 @@ static void set_process_name( int argc, char *argv[] ) > { > #ifdef HAVE_SETPROCTITLE > setproctitle("-%s", argv[1]); > -#endif > + /* remove argv[0] */ > + memmove( argv, argv + 1, argc * sizeof(argv[0]) ); > +#elif defined(HAVE_SETPROGNAME) > + int i, offset; > + char *end = argv[argc-1] + strlen(argv[argc-1]) + 1; > > -#ifdef HAVE_PRCTL > + offset = argv[1] - argv[0]; > + memmove( argv[1] - offset, argv[1], end - argv[1] ); > + memset( end - offset, 0, offset ); > + for (i = 1; i < argc; i++) argv[i-1] = argv[i] - offset; > + argv[i-1] = NULL; > + > + setprogname( argv[0] ); > +#elif defined(HAVE_PRCTL) > int i, offset; > char *p, *prctl_name = argv[1]; > char *end = argv[argc-1] + strlen(argv[argc-1]) + 1; > @@ -1141,11 +1152,11 @@ static void set_process_name( int argc, char *argv[] ) > argv[i-1] = NULL; > } > else > -#endif /* HAVE_PRCTL */ > { > /* remove argv[0] */ > memmove( argv, argv + 1, argc * sizeof(argv[0]) ); > } > +#endif /* HAVE_PRCTL */ > } > > > -- > 2.7.1 > > >

4 12

Re: ntoskrnl.exe: add IoAttachDevice stub
by Sebastian Lackner July 11, 2016

July 11, 2016

On 12.07.2016 05:07, Austin English wrote: > + > +/*********************************************************************** > + * IoAttachDevice (NTOSKRNL.EXE.@) > + */ > +NTSTATUS IoAttachDevice(DEVICE_OBJECT *source, UNICODE_STRING *target, DEVICE_OBJECT *attached) You forgot to specify the calling convention (WINAPI) - this is probably also the reason why your stub is not sufficient yet. Otherwise the patch looks fine to me. > +{ > + FIXME("(%p, %s, %p): stub\n", source, debugstr_us(target), attached); > + return STATUS_NOT_IMPLEMENTED; > +}

1 0

Re: [PATCH] kernel32/tests/fiber: Add tests for fiber-local storage.
by Sebastian Lackner July 11, 2016

July 11, 2016

On 08.07.2016 00:16, John Sheu wrote: > Signed-off-by: John Sheu <sheu(a)google.com> > --- > dlls/kernel32/tests/fiber.c | 291 +++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 258 insertions(+), 33 deletions(-) > > diff --git a/dlls/kernel32/tests/fiber.c b/dlls/kernel32/tests/fiber.c > index 1e4ca66..4f61f80 100644 > --- a/dlls/kernel32/tests/fiber.c > +++ b/dlls/kernel32/tests/fiber.c > @@ -33,9 +33,13 @@ static BOOL (WINAPI *pFlsFree)(DWORD); > static PVOID (WINAPI *pFlsGetValue)(DWORD); > static BOOL (WINAPI *pFlsSetValue)(DWORD,PVOID); > > -static LPVOID fibers[2]; > +static LPVOID fibers[3]; > static BYTE testparam = 185; > -static WORD cbCount; > +static DWORD fls_index_to_set = FLS_OUT_OF_INDEXES; > +static PVOID fls_value_to_set = NULL; > + > +static int fiberCount = 0; > +static int cbCount = 0; > > static VOID init_funcs(void) > { > @@ -59,15 +63,44 @@ static VOID init_funcs(void) > > static VOID WINAPI FiberLocalStorageProc(PVOID lpFlsData) > { > + ok(lpFlsData == fls_value_to_set, > + "FlsData expected not to be changed, value is %p, expected %p\n", > + lpFlsData, fls_value_to_set); > cbCount++; > - ok(lpFlsData == (PVOID) 1587, "FlsData expected not to be changed\n"); > } > > static VOID WINAPI FiberMainProc(LPVOID lpFiberParameter) > { > BYTE *tparam = (BYTE *)lpFiberParameter; > - cbCount++; > + fiberCount++; > ok(*tparam == 185, "Parameterdata expected not to be changed\n"); > + if (fls_index_to_set != FLS_OUT_OF_INDEXES) > + { > + if (!pFlsSetValue || !pFlsGetValue) > + { > + win_skip( "Fiber Local Storage not supported\n" ); You can get rid of this check, fls_index_to_set should not be set when Fiber Local Storage is not supported. > + } > + else > + { > + PVOID pret; Usually "void *" types and variable names without "p" prefix are preferred. > + BOOL bret; > + > + pret = pFlsGetValue(fls_index_to_set); > + ok(pret == NULL, "FlsGetValue returned %p, expected NULL\n", pret); > + > + /* Set the FLS value */ > + SetLastError( 0xdeadbeef ); You can remove this line when you are not testing the error value. > + bret = pFlsSetValue(fls_index_to_set, fls_value_to_set); > + ok(bret, "FlsSetValue failed with error %u\n", GetLastError()); > + > + /* Verify that FlsGetValue retrieves the value set by FlsSetValue */ > + SetLastError( 0xdeadbeef ); > + pret = pFlsGetValue(fls_index_to_set); > + ok(pret == fls_value_to_set, > + "FlsGetValue returned %p, expected %p\n", pret, fls_value_to_set); > + ok(GetLastError() == ERROR_SUCCESS, "FlsGetValue error %u\n", GetLastError()); > + } > + } > pSwitchToFiber(fibers[0]); > } > > @@ -110,9 +143,23 @@ static void test_ConvertFiberToThread(void) > } > } > > +static void test_IsThreadAFiber(BOOL is_fiber) > +{ > + if (pIsThreadAFiber) > + { > + BOOL ret = pIsThreadAFiber(); > + ok(ret == is_fiber, "IsThreadAFiber reported %d, expected %d\n", ret, is_fiber); > + } > + else > + { > + win_skip( "IsThreadAFiber not present\n" ); > + return; return at the end of a function is unnecessary. Also, I'm not sure if it makes much sense to have this as a separate function. You could probably just inline it in this case. > + } > +} > + > static void test_FiberHandling(void) > { > - cbCount = 0; > + fiberCount = 0; > fibers[0] = pCreateFiber(0,FiberMainProc,&testparam); > ok(fibers[0] != 0, "CreateFiber failed with error %d\n", GetLastError()); > pDeleteFiber(fibers[0]); > @@ -124,12 +171,11 @@ static void test_FiberHandling(void) > else > test_ConvertThreadToFiber(); > > - > fibers[1] = pCreateFiber(0,FiberMainProc,&testparam); > ok(fibers[1] != 0, "CreateFiber failed with error %d\n", GetLastError()); > > pSwitchToFiber(fibers[1]); > - ok(cbCount == 1, "Wrong callback count: %d\n", cbCount); > + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); > pDeleteFiber(fibers[1]); > > if (!pCreateFiberEx) > @@ -143,46 +189,72 @@ static void test_FiberHandling(void) > ok(fibers[1] != 0, "CreateFiberEx failed with error %d\n", GetLastError()); > > pSwitchToFiber(fibers[1]); > - ok(cbCount == 2, "Wrong callback count: %d\n", cbCount); > + ok(fiberCount == 2, "Wrong fiber count: %d\n", fiberCount); > pDeleteFiber(fibers[1]); > > - if (!pIsThreadAFiber) > - { > - win_skip( "IsThreadAFiber not present\n" ); > - return; > - } > - > - ok(pIsThreadAFiber(), "IsThreadAFiber reported FALSE\n"); > + test_IsThreadAFiber(TRUE); > test_ConvertFiberToThread(); > - ok(!pIsThreadAFiber(), "IsThreadAFiber reported TRUE\n"); > + test_IsThreadAFiber(FALSE); > } > > -static void test_FiberLocalStorage(PFLS_CALLBACK_FUNCTION cbfunc) > +static void test_FiberLocalStorage(void) > { > - DWORD fls; > + DWORD fls, fls_2; > BOOL ret; > - PVOID val = (PVOID) 1587; > + PVOID val; "void *" would be preferred here. > > - if (!pFlsAlloc) > + if (!pFlsAlloc || !pFlsSetValue || !pFlsGetValue || !pFlsFree) > { > win_skip( "Fiber Local Storage not supported\n" ); > return; > } > - cbCount = 0; > > - fls = pFlsAlloc(cbfunc); > - ok(fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %d\n", GetLastError()); > + /* Test an unallocated index > + * FlsFree should fail > + * FlsGetValue and FlsSetValue should succeed > + */ > + SetLastError( 0xdeadbeef ); > + ret = pFlsFree( 127 ); > + ok( !ret, "freeing fls index 127 (unallocated) succeeded\n" ); > + ok( GetLastError() == ERROR_INVALID_PARAMETER, > + "freeing fls index 127 (unallocated) wrong error %u\n", GetLastError() ); > > - ret = pFlsSetValue(fls, val); > - ok(ret, "FlsSetValue failed\n"); > - ok(val == pFlsGetValue(fls), "FlsGetValue failed\n"); > + SetLastError( 0xdeadbeef ); This is unnecessary if you don't test the error. > + val = pFlsGetValue( 127 ); > + ok( val == NULL, > + "getting fls index 127 (unallocated) failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); Same here. > + ret = pFlsSetValue( 127, (PVOID) 0x217 ); "(void *)0x217" would be preferred here (and at a couple of places below). > + ok( ret, "setting fls index 127 (unallocated) failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); > + val = pFlsGetValue( 127 ); > + ok( val == (PVOID) 0x217, "fls index 127 (unallocated) wrong value %p\n", val ); > + ok( GetLastError() == ERROR_SUCCESS, > + "getting fls index 127 (unallocated) failed with error %u\n", GetLastError() ); > + > + /* FlsFree, FlsGetValue, and FlsSetValue out of bounds should return > + * ERROR_INVALID_PARAMETER > + */ > + SetLastError( 0xdeadbeef ); > + ret = pFlsFree( 128 ); > + ok( !ret, "freeing fls index 128 (out of bounds) succeeded\n" ); > + ok( GetLastError() == ERROR_INVALID_PARAMETER, > + "freeing fls index 128 (out of bounds) wrong error %u\n", GetLastError() ); > > - ret = pFlsFree(fls); > - ok(ret, "FlsFree failed\n"); > - if (cbfunc) > - todo_wine ok(cbCount == 1, "Wrong callback count: %d\n", cbCount); > + SetLastError( 0xdeadbeef ); > + ret = pFlsSetValue( 128, (PVOID) 0x217 ); > + ok( !ret, "setting fls index 128 (out of bounds) succeeded\n" ); > + ok( GetLastError() == ERROR_INVALID_PARAMETER, > + "setting fls index 128 (out of bounds) wrong error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); > + val = pFlsGetValue( 128 ); It would make sense to test "val" aswell. > + ok( GetLastError() == ERROR_INVALID_PARAMETER, > + "setting fls index 128 (out of bounds) wrong error %u\n", GetLastError() ); The function above is no set function. > > - /* test index 0 */ > + /* Test index 0 */ > SetLastError( 0xdeadbeef ); > val = pFlsGetValue( 0 ); > ok( !val, "fls index 0 set to %p\n", val ); > @@ -196,6 +268,7 @@ static void test_FiberLocalStorage(PFLS_CALLBACK_FUNCTION cbfunc) > ok( !val, "fls index 0 wrong value %p\n", val ); > ok( GetLastError() == ERROR_INVALID_PARAMETER, "setting fls index wrong error %u\n", GetLastError() ); > > + /* Test creating an FLS index */ > fls = pFlsAlloc( NULL ); > ok( fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed\n" ); > ok( fls != 0, "fls index 0 allocated\n" ); > @@ -205,11 +278,162 @@ static void test_FiberLocalStorage(PFLS_CALLBACK_FUNCTION cbfunc) > ok( ret, "setting fls index %u failed\n", fls ); > val = pFlsGetValue( fls ); > ok( val == (void *)0xdeadbeef, "fls index %u wrong value %p\n", fls, val ); > + ok( GetLastError() == ERROR_SUCCESS, > + "getting fls index %u failed with error %u\n", fls, GetLastError() ); > pFlsFree( fls ); > + > + /* Undefined behavior: verify the value is NULL after it the slot is freed */ > + val = pFlsGetValue( fls ); > + ok( val == NULL, "fls index %u wrong value %p\n", fls, val ); > + ok( GetLastError() == ERROR_SUCCESS, > + "getting fls index %u failed with error %u\n", fls, GetLastError() ); > + > + /* Undefined behavior: verify the value is settable after the slot is freed */ > ret = pFlsSetValue( fls, (void *)0xdeadbabe ); > ok( ret, "setting fls index %u failed\n", fls ); > val = pFlsGetValue( fls ); > ok( val == (void *)0xdeadbabe, "fls index %u wrong value %p\n", fls, val ); > + > + /* Try to create the same FLS index again, and verify that is initialized to NULL */ > + fls_2 = pFlsAlloc( NULL ); > + ok( fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed\n" ); > + /* If this fails it is not an API error, but the test will be inconclusive */ > + ok( fls_2 == fls, "different FLS index allocated, was %u, now %u\n", fls, fls_2 ); > + val = pFlsGetValue( fls_2 ); > + ok( val == NULL, "fls index %u wrong value %p\n", fls, val ); > + ok( GetLastError() == ERROR_SUCCESS, > + "getting fls index %u failed with error %u\n", fls_2, GetLastError() ); A SetLastError() call is missing here, otherwise this test is not really convincing. > + pFlsFree( fls_2 ); > +} > + > +static void test_FiberLocalStorageCallback(PFLS_CALLBACK_FUNCTION cbfunc) { All the other functions have the "{" on the next line. > + DWORD fls; > + BOOL ret; > + PVOID val, val2; > + > + if (!pFlsAlloc || !pFlsSetValue || !pFlsGetValue || !pFlsFree) > + { > + win_skip( "Fiber Local Storage not supported\n" ); > + return; > + } > + > + /* Test that the callback is executed */ > + SetLastError( 0xdeadbeef ); This is unnecessary. > + cbCount = 0; > + fls = pFlsAlloc( cbfunc ); > + ok( fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); This also. > + val = (PVOID) 0x1587; > + fls_value_to_set = val; > + ret = pFlsSetValue( fls, val ); > + ok(ret, "FlsSetValue failed with error %u\n", GetLastError() ); > + > + val2 = pFlsGetValue( fls ); > + ok(val == val2, "FlsGetValue returned %p, expected %p\n", val2, val); > + > + SetLastError( 0xdeadbeef ); Same here. > + ret = pFlsFree( fls ); > + ok(ret, "FlsFree failed with error %u\n", GetLastError() ); > + todo_wine > + { > + ok( cbCount == 1, "Wrong callback count: %d\n", cbCount ); > + } > + > + /* Test that callback is not executed if value is NULL */ > + SetLastError( 0xdeadbeef ); Same here. > + cbCount = 0; > + fls = pFlsAlloc( cbfunc ); > + ok( fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); Same here. > + ret = pFlsSetValue( fls, NULL ); > + ok( ret, "FlsSetValue failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); Same here. > + pFlsFree( fls ); > + ok( ret, "FlsFree failed with error %u\n", GetLastError() ); > + ok( cbCount == 0, "Wrong callback count: %d\n", cbCount ); > +} > + > +static void test_FiberLocalStorageWithFibers(PFLS_CALLBACK_FUNCTION cbfunc) { > + PVOID val1 = (PVOID) 0x314; > + PVOID val2 = (PVOID) 0x152; > + BOOL ret; > + > + if (!pConvertThreadToFiber || !pSwitchToFiber || !pDeleteFiber || !pConvertFiberToThread) > + { > + win_skip( "Fibers not supported\n" ); > + return; > + } > + if (!pFlsAlloc || !pFlsFree || !pFlsSetValue || !pFlsGetValue) > + { > + win_skip( "Fiber Local Storage not supported\n" ); > + return; > + } > + > + > + fls_index_to_set = pFlsAlloc(cbfunc); > + ok(fls_index_to_set != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %d\n", GetLastError()); Not really critical, but in the code above you used %u for GetLastError values. > + > + test_ConvertThreadToFiber(); > + > + fiberCount = 0; > + cbCount = 0; > + fibers[1] = pCreateFiber(0,FiberMainProc,&testparam); > + fibers[2] = pCreateFiber(0,FiberMainProc,&testparam); > + ok(fibers[1] != 0, "CreateFiber failed with error %d\n", GetLastError()); > + ok(fibers[2] != 0, "CreateFiber failed with error %d\n", GetLastError()); CreateFiber returns a pointer, so it would be better to compare against NULL. > + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val1; > + pSwitchToFiber(fibers[1]); > + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val2; > + pSwitchToFiber(fibers[2]); > + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + fls_value_to_set = val2; > + ret = pFlsSetValue(fls_index_to_set, fls_value_to_set); > + ok(ret, "FlsSetValue failed\n"); > + ok(val2 == pFlsGetValue(fls_index_to_set), "FlsGetValue failed\n"); > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val1; > + pDeleteFiber(fibers[1]); > + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); > + todo_wine > + { > + ok(cbCount == 1, "Wrong callback count: %d\n", cbCount); > + } > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val2; > + pFlsFree(fls_index_to_set); > + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); > + todo_wine > + { > + ok(cbCount == 2, "Wrong callback count: %d\n", cbCount); > + } > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val1; > + pDeleteFiber(fibers[2]); > + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + test_ConvertFiberToThread(); > } > > START_TEST(fiber) > @@ -223,6 +447,7 @@ START_TEST(fiber) > } > > test_FiberHandling(); > - test_FiberLocalStorage(NULL); > - test_FiberLocalStorage(FiberLocalStorageProc); > + test_FiberLocalStorage(); > + test_FiberLocalStorageCallback(FiberLocalStorageProc); > + test_FiberLocalStorageWithFibers(FiberLocalStorageProc); > } >

2 2

Re: [PATCH 3/3] wined3d/resource: Performance improvements, try 2
by Marvin July 11, 2016

July 11, 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=24152 Your paranoid android. === w8 (32 bit visual) === visual.c:8177: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 visual.c:8345: Test failed: Got unexpected color 0x00ff0000 for no color attribute test. visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w864 (32 bit visual) === visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w1064 (32 bit visual) === visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w864 (64 bit visual) === visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w1064 (64 bit visual) === visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4529: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512.

1 0

Re: [PATCH 1/3] d3d9/tests/visual: Add Vertexbuffer locking tests, try 2
by Marvin July 11, 2016

July 11, 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=24150 Your paranoid android. === w8 (32 bit visual) === visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w864 (32 bit visual) === visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w1064 (32 bit visual) === visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w864 (64 bit visual) === visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512. === w1064 (64 bit visual) === visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 0. visual.c:4524: Test failed: Expected buffer mapped unsyncronized. Flags = D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD. Usage = 512.

1 0

Re: [v3 1/2] winejoystick.drv: Print location ID in debugstr_device on the Mac
by Ken Thomases July 11, 2016

July 11, 2016

On Jun 30, 2016, at 7:02 PM, David Lawrie <david.dljunk(a)gmail.com> wrote: > > Tested on OS X 10.10.5. > > Signed-off-by: David Lawrie <david.dljunk(a)gmail.com> > --- > dlls/winejoystick.drv/joystick_osx.c | 30 ++++++++++++++++++++++++++++-- > 1 file changed, 28 insertions(+), 2 deletions(-) This has similar issues as the dinput version, the main one being the pointless "result" variable in get_device_property_long(). -Ken

2 3

Re: [v3 2/3] dinput: Sort devices by location ID on the Mac
by DavidL July 11, 2016

July 11, 2016

This patch will be superseded in a later version. Signed-off-by: David Lawrie <david.dljunk(a)gmail.com> On Sun, Jul 10, 2016 at 3:22 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > > > >

1 0

Re: [v3 3/3] dinput: Alter function name and code style
by Ken Thomases July 11, 2016

July 11, 2016

On Jun 30, 2016, at 7:02 PM, David Lawrie <david.dljunk(a)gmail.com> wrote: > @@ -490,10 +490,10 @@ static int find_osx_devices(void) > if (devset) > { > CFIndex num_devices, num_main_elements, idx; > - CFMutableArrayRef devices = CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks); > - CFSetApplyFunction(devset, CFSetApplierFunctionCopyToCFArray, devices); > - CFRelease( devset); > - num_devices = CFArrayGetCount(devices); > + num_devices = CFSetGetCount(devset); > + CFMutableArrayRef devices = CFArrayCreateMutable(kCFAllocatorDefault, num_devices, &kCFTypeArrayCallBacks); You've now interleaved statements and declarations. Wine's code standard requires that declarations all come before statements. So, you could change the assignment of num_devices to an initialization or split the declaration and initialization of devices to separate declaration and assignment. Wine should be using -Wdeclaration-after-statement for its compile commands. Therefore, this change would have added a new warning. I know that some of the Mac files have a lot of warnings, which makes it hard to see new ones, but you should strive to avoid adding more. -Ken

2 1

Re: usp10/tests: Use SetRect() instead of open coding it.
by Marvin July 11, 2016

July 11, 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=24145 Your paranoid android. === w1064 (32 bit usp10) === usp10.c:1467: Test failed: 0: uJustification incorrect (0) usp10.c:1467: Test failed: 1: uJustification incorrect (0) usp10.c:1467: Test failed: 2: uJustification incorrect (0) usp10.c:1467: Test failed: 3: uJustification incorrect (0) usp10.c:1467: Test failed: 4: uJustification incorrect (0) usp10.c:1467: Test failed: 5: uJustification incorrect (0) usp10.c:1467: Test failed: 6: uJustification incorrect (0) usp10.c:1467: Test failed: 7: uJustification incorrect (0) usp10.c:1467: Test failed: 8: uJustification incorrect (0) usp10.c:1467: Test failed: 9: uJustification incorrect (0) usp10.c:1483: Test failed: 3: uJustification incorrect (0) usp10.c:1483: Test failed: 5: fDiacritic incorrect (1) usp10.c:1483: Test failed: 5: fZeroWidth incorrect (1) usp10.c:1483: Test failed: 15: fDiacritic incorrect (1) usp10.c:1483: Test failed: 15: fZeroWidth incorrect (1) === w1064 (64 bit usp10) === usp10.c:1467: Test failed: 0: uJustification incorrect (0) usp10.c:1467: Test failed: 1: uJustification incorrect (0) usp10.c:1467: Test failed: 2: uJustification incorrect (0) usp10.c:1467: Test failed: 3: uJustification incorrect (0) usp10.c:1467: Test failed: 4: uJustification incorrect (0) usp10.c:1467: Test failed: 5: uJustification incorrect (0) usp10.c:1467: Test failed: 6: uJustification incorrect (0) usp10.c:1467: Test failed: 7: uJustification incorrect (0) usp10.c:1467: Test failed: 8: uJustification incorrect (0) usp10.c:1467: Test failed: 9: uJustification incorrect (0) usp10.c:1483: Test failed: 3: uJustification incorrect (0) usp10.c:1483: Test failed: 5: fDiacritic incorrect (1) usp10.c:1483: Test failed: 5: fZeroWidth incorrect (1) usp10.c:1483: Test failed: 15: fDiacritic incorrect (1) usp10.c:1483: Test failed: 15: fZeroWidth incorrect (1)

2 1

Re: [PATCH 4/4] d3drm: Implement IDirect3DRMDevice*::Add/DeleteDestroyCallback. (v2)
by Stefan Dösinger July 11, 2016

July 11, 2016

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-07-10 um 21:49 schrieb Aaryaman Vasishta: > I believe the "obj == ctxt->obj" tests in destroy_callback and > destroy_callback1 should cover this. See test_destroy_callback. If I understand it correctly not quite, because you're only ever calling AddDestroyCallback via IDirect3DRMObject, never via IDirect3DRMSomethingElse. (Re-CCed wine-devel) -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXgsLBAAoJEN0/YqbEcdMwlsUP/2AIUfNl//T23Ik/gnL5FHVd 4axt1f/F5l1Hb5C3049Eh/se5SfF8jM6YYJte62al60f8nf0ERCW8S9JCzRueV2+ Q4ETvXbxhffKh9Ceb8W83ijxkwGf3GplypnXK7OQnqFUg88AR4YTGIL4I5pTPZVq Bw/MKgOfnDAHn+QsRDnzaQTtZcJPDNx+mrf26S/x15Wdi+PQjdVWb3uPbtgfkHN4 TWVwAgTQwPcnJQDaQ2wmWlEG+uxvxqpCF5BFCf1JwRV6oMdYYCQeHH+nvBrnPjEN /7Upv092RGbazucxMr6OWc9GJaCy4K9pfer8U2Ahdb47eAsF9UTO4AoW718GnIfU CJPvh8kMxukMw81U3U5Dr61v9cxd2xywz8ZMWMfKScrN4b6W7/hiE19B3+0w/XHi JHDf0NlPG8RDS+LR3N67X7dkaVqtEkRNCyVfAKdOozlz2EIlzs6lHmYNSxd17GO1 foIPVad9Ovo7nNQhB6FK+0zMZCKAz7be6ai9orlCk4ThhBhVwlaQXX16LBxgrYBi cU/3emOonGzto6sqzDrXyTNEqQTzvFMXnqitH/ITgvKuey0GBLHVSeOAHdECbrN3 bdGYdYPuXnxdksTey/zGORo4u0plUxS3gXiAGM0Ch0oCb6E9uzWMqKJpH6rIoR22 yAuKU7HOboWb3rlcAs0i =s5bk -----END PGP SIGNATURE-----

2 3

Re: [PATCH] ddraw/tests: Skip A4R4G4B4 ckey tests on Nvidia (v3)
by Marvin July 10, 2016

July 10, 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=24144 Your paranoid android. === w1064 (32 bit ddraw4) === ddraw4.c:2540: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2540: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw7) === ddraw7.c:2270: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw7) === ddraw7.c:2270: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw1) === ddraw1.c:2204: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2204: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw2) === ddraw2.c:2355: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2355: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [v3 1/3] dinput: Print location ID in debugstr_device on the Mac
by Ken Thomases July 10, 2016

July 10, 2016

Hi, Sorry for the late review. See in-line comments: On Jun 30, 2016, at 7:02 PM, David Lawrie <david.dljunk(a)gmail.com> wrote: > > +static void get_device_property_long(IOHIDDeviceRef in_device, CFStringRef in_key, long * out_value) As Dmitry suggested, you could make this simply return the property value rather than provide it through the indirect output parameter. > +{ > + Boolean result = FALSE; This variable isn't really used. It's assigned, but never read. > + CFTypeRef type; This variable isn't holding a type, it's holding a value or, if you prefer, an object reference. (The fact that its type is "CFTypeRef" doesn't imply it holds a type. Frankly, "CFTypeRef" is also a lousy name, but that's on Apple.) > + > + if (in_device) > + { > + assert(IOHIDDeviceGetTypeID() == CFGetTypeID(in_device)); > + > + type = IOHIDDeviceGetProperty(in_device, in_key); > + > + if (type) > + { > + if (CFNumberGetTypeID() == CFGetTypeID(type)) You could combine these two "if" statements using &&. -Ken

1 0

Re: [v2 01/10] webservices: Add support for writing double values.
by Sebastian Lackner July 10, 2016

July 10, 2016

On 08.07.2016 10:20, Hans Leidekker wrote: > v2: Fix build failure. > > Signed-off-by: Hans Leidekker <hans(a)codeweavers.com> > --- > dlls/webservices/reader.c | 8 +-- > dlls/webservices/tests/writer.c | 113 +++++++++++++++++++++++++++++++++ > dlls/webservices/webservices_private.h | 2 + > dlls/webservices/writer.c | 100 +++++++++++++++++++++++++++++ > include/webservices.h | 5 ++ > 5 files changed, 224 insertions(+), 4 deletions(-) > Unfortunately your patch does not work as expected here (gcc 5.3): ../../../tools/runtest -q -P wine -T ../../.. -M webservices.dll -p webservices_test.exe.so writer && touch writer.ok writer.c:358: Test failed: 1941: got 11 expected 10 writer.c:360: Test failed: 1941: got <t>:E-3</t>>000000004</t> expected <t>1E-2</t> writer.c:358: Test failed: 1941: got 12 expected 11 writer.c:360: Test failed: 1941: got <t>-:E-3</t>000000004</t> expected <t>-1E-2</t> Makefile:346: die Regel für Ziel „writer.ok“ scheiterte If you need any details to investigate this issue please let me know. Regards, Sebastian

2 2

Re: [PATCH 4/4] d3drm: Implement IDirect3DRMDevice*::Add/DeleteDestroyCallback. (v2)
by Stefan Dösinger July 10, 2016

July 10, 2016

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-07-10 um 17:41 schrieb Aaryaman Vasishta: > static HRESULT WINAPI d3drm_device1_AddDestroyCallback(IDirect3DRMDevice *iface, > D3DRMOBJECTCALLBACK cb, void *ctx) > { > - FIXME("iface %p, cb %p, ctx %p stub!\n", iface, cb, ctx); > + struct d3drm_device *device = impl_from_IDirect3DRMDevice(iface); > > - return E_NOTIMPL; > + TRACE("iface %p, cb %p, ctx %p.\n", iface, cb, ctx); > + > + return d3drm_device3_AddDestroyCallback(&device->IDirect3DRMDevice3_iface, cb, ctx); > } It doesn't fit in this patch, but at some point we should test which interface is passed to the destroy callback: Is it the interface on which AddDestroyCallback was called, or is it the interface returned by iface->QueryInterface(&IID_IDirect3DRMObject)? Please correct me if we already have tests for that and I missed them :- ) . -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXgpstAAoJEN0/YqbEcdMwWkcP/j25Gk+jAQlDrb0l0YV5Tz0k V0D/PZW7lHMcGCeAW4Y8GyzzKAFVD+d8YL9RRHcxyWbUnl5f6nR/yjAe3da6FFpD AF+vDvKGlV1POG1QhC8d/34SxVYXsngKNq2tvxYVQOhldvKh9d9Ti255dLFicqG3 aBBzPF1OvY9ZaxQJQAyJdU9944YtaH3CUd6cxjOH95jR1hAiz55akKWDRQHm7Uc+ S77/XRaaUmGTX5gpma/EeY7Rb/eG3cEDF8UBoCI6rfu+kjIEa+KTJVZKzeSA5+RU xPETE28jUX94XZnRz8lqLJbSRboBXSa2MWDdb2KvBUqSTLPS1cTm55cLYRngWDAD czF8Wfo+UOfGeG9oGhorUE2GkOleDd/WtXQtAfJeAvxfBxd4aA5v66Z7IwhcVeEV sSEhsqxFfc31VBB/f4Ii3F/5nFyMN+LfcDUIHI4tmOMjwj+VMlIES//71BvYQpqm iJNo9DPSVUyqApIgPSNsYL7vORUl2AKsklJKk4fxka1j1010TLJdpFIyv4leLfL8 mBUY13S+wzaEObhztQJiMJ5usWu9BoClm38x4Oj5JzMZ2Pg2If0WAcVRLOLkb9Br GIyfN5K89SUmiJp6GoMtKG99QXypMsg1fanYe6imBTL8Cj5tGuVAoJlPDMpc8i07 JxITcNBrsu5/WwP8tGeq =Zf/v -----END PGP SIGNATURE-----

1 0

Re: [PATCH] kernel32/tests/fiber: Add tests for fiber-local storage.
by André Hentschel July 10, 2016

July 10, 2016

Hi, comments inline Am 08.07.2016 um 00:16 schrieb John Sheu: > +static void test_FiberLocalStorageCallback(PFLS_CALLBACK_FUNCTION cbfunc) { > + DWORD fls; > + BOOL ret; > + PVOID val, val2; > + > + if (!pFlsAlloc || !pFlsSetValue || !pFlsGetValue || !pFlsFree) > + { > + win_skip( "Fiber Local Storage not supported\n" ); > + return; > + } > + > + /* Test that the callback is executed */ > + SetLastError( 0xdeadbeef ); > + cbCount = 0; > + fls = pFlsAlloc( cbfunc ); > + ok( fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); > + val = (PVOID) 0x1587; > + fls_value_to_set = val; > + ret = pFlsSetValue( fls, val ); > + ok(ret, "FlsSetValue failed with error %u\n", GetLastError() ); > + > + val2 = pFlsGetValue( fls ); > + ok(val == val2, "FlsGetValue returned %p, expected %p\n", val2, val); > + > + SetLastError( 0xdeadbeef ); > + ret = pFlsFree( fls ); > + ok(ret, "FlsFree failed with error %u\n", GetLastError() ); > + todo_wine > + { > + ok( cbCount == 1, "Wrong callback count: %d\n", cbCount ); > + } Please make those one-liners, like todo_wine ok(...); and the patch should be fine then > + > + /* Test that callback is not executed if value is NULL */ > + SetLastError( 0xdeadbeef ); > + cbCount = 0; > + fls = pFlsAlloc( cbfunc ); > + ok( fls != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); > + ret = pFlsSetValue( fls, NULL ); > + ok( ret, "FlsSetValue failed with error %u\n", GetLastError() ); > + > + SetLastError( 0xdeadbeef ); > + pFlsFree( fls ); > + ok( ret, "FlsFree failed with error %u\n", GetLastError() ); > + ok( cbCount == 0, "Wrong callback count: %d\n", cbCount ); > +}

3 3