-----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.