[PATCH 1/3] d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromClipper (try 4).

Stefan Dösinger stefandoesinger at gmail.com
Tue Jun 23 15:22:07 CDT 2015


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Looks pretty good, there are 2 minor issues left (see below) and 3
code style things:

Am 2015-06-23 um 20:19 schrieb Aaryaman Vasishta:
> +BOOL primary_surface_found = FALSE;
Please make it static. Also no need to initialize global variables,
they're set to zero by the compiler. (Same as static variables inside
a function).

> +    if ((desc->ddsCaps.dwCaps & DDSCAPS_PRIMARYSURFACE) == DDSCAPS_PRIMARYSURFACE)
desc->ddsCaps.dwCaps & DDSCAPS_PRIMARYSURFACE is enough in this case
as there's only one flag.

> +                clipper = context;
> +                ok(clipper == d3drm_clipper, "Expected clipper returned == %p, got %p.\n", clipper, d3drm_clipper);
> +                IDirectDrawClipper_Release(d3drm_clipper);
ok(clipper == context, "..."); should work, no need to have an extra
variable for this.

> +static void test_create_device_from_clipper(void)
> +{
This function tests IDirect3DRM2 / IDirect3DRMDevice2. Think about how
you want to add version 1 and 3 tests. Either add them in the same
function or create 3 test functions. I prefer the latter case, but
then you may want to name them test_create_device_from_clipper{1,2,3}
or similar.

> +    hr = IDirect3DRM2_CreateDeviceFromClipper(d3drm2, clipper, &driver, 300, 200, &device2);
> +    ok(hr == D3DRM_OK, "Cannot create IDirect3DRMDevice2 interface (hr = %x).\n", hr);
> +    ref2 = get_refcount((IUnknown *)d3drm1);
> +    cref2 = get_refcount((IUnknown *)clipper);
> +    todo_wine ok(ref2 > ref1, "expected ref2 > ref1, got ref1 = %u , ref2 = %u.\n", ref1, ref2);
> +    todo_wine ok(cref2 > cref1, "expected cref2 > cref1, got cref1 = %u , cref2 = %u.\n", cref1, cref2);
Please also show that the d3drm2 refcount is untouched. This also
applies to patches 2 and 3.

> +    hr = IDirectDraw_SetDisplayMode(ddraw, 640, 480, 16);
> +    ok(hr == DD_OK, "Cannot set display mode to 16bpp (hr = %x).\n", hr);
Please use the current desktop width and height here (i.e., what was
returned from GetDisplayMode earlier). Setting a lower resolution has
the tendency to screw up window and desktop icon positioning.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVib/vAAoJEN0/YqbEcdMw8UoP+gLjxDhe3SjrslH3I0h3cM0M
W9KbODhgTmn+C2j3a2rOXvbllAXkjAI9oSnbKp3msE4uJw7GT5QOx+y1828xVF54
RhxoQsYJuXAptAzzlwOS9Dpj5f8sebfj8AG3KENEeMBaKPJFnuER2AHD7o0Bl9Td
LVXbSWjvkhsGIRSW2JWKVFQIRdCKaeiRciA0ScH/yQxxL0h/1BOdoLqzc5+Nv6wW
/drwTUVR0MQrqzNt3qKPnhNNe2CXS7DQvc4KA1s0WsNxTbrl1orKXDclls1UhL5L
iIIJtVF0H7jPE5kF5yVsW0uyjojNI7EnEbVtQQHecFV1UfLQQhykIbw3Vc/tg8Up
lpkaEtQvSUYsJMIWKyG9nTAsoz1ZAeb8k97YH8SS/HtG3dZzOThLkPjfbpOPRxeK
07d2P+I4FkyXdiDNZ/dzXIuJUGoBriXM/rPJFqnHWrOm3Lor6GqhKXE+SxOpPeOA
rUcVYfd/XEJPT+7PIOdeu/C9ldUPjVmV3BpWze0oXaj9noLJUWojk4E0emsazaUB
bXXcJfj0r65SnFUUbxmwjZK50OdF36iku3Oohgl7kpmatWF3IN1g9BmgoDnXuHi8
8xVAi+6OQm7+oU04kZi/4KHPAg41FcMBnxtuxYBASn0NZvaY002ZMWpfCAbdV6pX
LouyQXU2ct2Y5h78u9FG
=F7Cf
-----END PGP SIGNATURE-----



More information about the wine-devel mailing list