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

[Stefan Dösinger]

Hi,

> +    hr = DirectDrawCreateClipper(0, &clipper, NULL);
> +    ref = get_refcount((IUnknown *)clipper);
You're not reading ref anywhere as far as I can see. Did you forget to re-check the refcount after creating the device?

If the refcount is incremented by 2 or more: See if the clipper is assigned to the render target and / or depth stencil. This wouldn't really do anything, but it's Microsoft we're talking about here.

> +    hr = IDirect3DRM_QueryInterface(d3drm1, &IID_IDirect3DRM2, (void **)&d3drm2);
> +    ok(hr == D3DRM_OK, "Cannot get IDirect3DRM3 interface (hr = %x).\n", hr);
Minor mistake: IDirect3DRM2 vs IDirect3DRM3.

> +    memcpy(&driver, &IID_IDirect3DRGBDevice, sizeof(GUID));
I think an assignment should do the job.

> +    hr = IDirect3DRM2_CreateDeviceFromClipper(d3drm2, clipper, &driver, rc.right, rc.bottom, &device2);
> +    ok(hr == D3DRM_OK, "Cannot get IDirect3DRMDevice2 interface (hr = %x).\n", hr);
I'd say "Cannot create IDirect3DRMDevice2 (hr = ...)" here. You're not retrieving an interface here.

> +    /* Fetch immediate mode device in order to access render target */
> +    hr = IDirect3DRMDevice2_GetDirect3DDevice2(device2, &d3ddevice2);
> +    todo_wine ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice2 interface (hr = %x).\n", hr);
> +    if (FAILED(hr))
> +    {
> +        IDirect3DRMDevice2_Release(device2);
> +        IDirect3DRM2_Release(d3drm2);
> +        IDirect3DRM_Release(d3drm1);
> +        IDirectDrawClipper_Release(clipper);
> +        return;
> +    }
You can use the cleanup label here too if you initialize the interface pointers to NULL and add checks for NULL before calling release.

> +    /* Check properties of primary and depth surfaces */
> +    desc.dwSize = sizeof(desc);
> +    hr = IDirectDrawSurface_GetSurfaceDesc(surface, &desc);
> +    ok(hr == DD_OK, "Cannot get surface desc structure (hr = %x).\n", hr);
> +    if (FAILED(hr))
> +        goto cleanup;
Minor nitpick: A "primary" surface represents the screen content. The correct term in in this case is "render target".  Note that a primary surface can be a render target in fullscreen mode.

> +    ok((desc.dwWidth == rc.right) && (desc.dwHeight == rc.bottom), "Expected surface dimentions = %d, %d, got %d, %d.\n",
> +        rc.right, rc.bottom, desc.dwWidth, desc.dwHeight);
> +    expected_caps = DDSCAPS_OFFSCREENPLAIN | DDSCAPS_3DDEVICE | DDSCAPS_SYSTEMMEMORY;
> +    todo_wine ok(desc.ddsCaps.dwCaps == expected_caps, "Expected %x for caps, got %x.\n", expected_caps, desc.ddsCaps.dwCaps);
As discussed on IRC ignore DDSCAPS_SYSTEMMEMORY here. I think it happens because you use a RGB device, and I don't think it's something we should care about unless an application depends on this.

Check dwRGBBitCount of the render target. It should match the bit depth of the screen mode.

> +    if (FAILED(hr = IDirectDraw_SetDisplayMode(ddraw, desc.dwWidth, desc.dwHeight, 8)))
> +    {
> +        win_skip("Cannot set display mode to 8bpp (hr = %x), skipping tests.\n", hr);
> +        IDirectDrawSurface7_Release(surface7);
> +        IDirectDrawSurface_Release(ds);
> +        IDirectDrawSurface_Release(surface);
> +        IUnknown_Release(unknown);
> +        IDirectDraw_Release(ddraw);
> +        goto cleanup;
> +    }
There's no need to try to set the display mode to 8. Also testing if changing the mode works is outside the scope of this test. What's interesting is that the pixel format of the primary surface of a new device after changing the mode.

Please also check the Z bit depth, and if the surface has stencil bits.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20150611/1585401e/attachment.sig>