-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
Here's the feedback for the patch you posted on IRC:
> + GUID driver = IID_IDirect3DRGBDevice;
Do you need this? Can't you just pass &IID_IDirect3DRGBDevice to CreateDeviceFromClipper? It may generate a const warning though.
> + hr = DirectDrawCreateClipper(0, &clipper, NULL);
> + ok(hr == DD_OK, "Cannot get IDirectDrawClipper interface (hr = %x).\n", hr);
> + hr = IDirectDrawClipper_SetHWnd(clipper, 0, window);
> + ok(hr == DD_OK, "Cannot set HWnd to Clipper (hr = %x).\n", hr);
Can you pass a NULL clipper? If this crashes, just put a comment in the test.
What if the clipper has no HWND and no clip list?
Call GetClipper on the render target and see if the clipper is assigned.
> + hr = IDirect3DRM2_CreateDeviceFromClipper(d3drm2, clipper, &driver, 300, 200, &device2);
For the sake of completeness pass 0 width and height. I expect one of the two results: Either it returns an error, or d3drm starts to use the window dimensions for the render target now.
> + todo_wine ok(ref2 > ref1, "expected ref2 > ref1, got ref1 = %d , ref2 = %d\n", ref1, ref2);
Minor detail: Use %u for unsigned.
> + ok((surface_desc.dwWidth == 300) && (surface_desc.dwHeight == 200), "Expected surface dimentions = 300, 200, got %d, %d.\n",
> + surface_desc.dwWidth, surface_desc.dwHeight);
> + ok((surface_desc.ddsCaps.dwCaps & (DDSCAPS_OFFSCREENPLAIN | DDSCAPS_3DDEVICE)) == (DDSCAPS_OFFSCREENPLAIN | DDSCAPS_3DDEVICE),
> + "Expected caps containing %x, got %x.\n", DDSCAPS_OFFSCREENPLAIN | DDSCAPS_3DDEVICE, surface_desc.ddsCaps.dwCaps);
> + expected_flags = DDSD_PIXELFORMAT | DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT | DDSD_PITCH;
> + ok(surface_desc.dwFlags == expected_flags, "Expected %x for flags, got %x.\n", expected_flags, surface_desc.dwFlags);
Test the pixel format here. First query the current bpp with EnumDisplaySettings and check that the pixel format matches the screen properties.
> + /* Check if ddraw supports change of display mode */
The comment is somewhat outdated. Either remove it or say something like "Test if the render target format follows the screen format".
> + ok(surface_desc.ddpfPixelFormat.dwRGBBitCount == 16, "Expected 16bpp, got %dbpp.\n",
> + surface_desc.ddpfPixelFormat.dwRGBBitCount);
Similar to the %u -> %u above
Once these are fixed the patch can go to wine-patches as far as I'm concerned.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJVgt8QAAoJEN0/YqbEcdMwOTYP/jXwDY+gUs/zmAyqB5hJp8by
FO0YSZsi0Z3QMMzfqenCy9+/PmqjMHyRPQvhM8CFpcdWsUi8bH6PzTiooAYOcvzf
GXEnj46T1O0wiJfLK+RnzfvYNzITLWf2XK6ek1OtP1z6Y3JhADXwoLW/tSM+kKp7
Crih31t9ewiSMG5iebcSdviI59WrFtUHmS9Mdp5UnWVQB8smZII8c+VAOf/SmWZ1
cGKosFncJVNuXpF0fq5gWiy3m1VcJ1xoWsG1rXbrOzVFvpw+20twa1FgZm9Z6I3c
Pxj2prH2Lz9u7tDZUr75dg3ucRPcUe2QP2bTwwBDDq4oaUZYKgXBLrz94AVZ/Y+j
datcVFY7KwY2byGmxmO+0I+iWyj26CIg13jo20gqfRfjt92DFlfi5/Mm/e97rCa1
0V38s0kqN5o4rvU+QbUwHfvHIgr4kAFWZPjDmZd3YececQuoeC7FszND/43UD0tK
MTd1Na3hFa5m6zO7oe4fPfGYX55tVhEzphiRkH4K0R0ZTxRsZGGqQoSFAHeAJDH6
0hPd3XjVHxSpHQNd8/8aFeOMJ8pBR+4MRUMcFBDn0XlDUYw0kUdvRkttKoNVCSGw
H0BS00wj/fs/G4D/pCLEFEX7wF9qDM5O6xKQRoyXG3+i9oN+zdzCNMs6SV1J2Bci
Jedz7Aw7+yAByiH/GePR
=aa6D
-----END PGP SIGNATURE-----