On Tue, Aug 9, 2016 at 9:23 PM, Henri Verbeet <hverbeet@gmail.com> wrote:
On 9 August 2016 at 03:16, Aaryaman Vasishta <jem456.vasishta@gmail.com> wrote:
> diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c
> index 1abaed5..2c82ecb 100644
> --- a/dlls/d3drm/tests/d3drm.c
> +++ b/dlls/d3drm/tests/d3drm.c
> @@ -4572,7 +4572,7 @@ static void test_create_device_from_d3d1(void)
>      ref3 = get_refcount((IUnknown *)d3drm1);
>      ok(ref3 > ref2, "expected ref3 > ref1, got ref1 = %u , ref3 = %u.\n", ref1, ref3);
>      ref3 = get_refcount((IUnknown *)temp_d3ddevice1);
> -    ok(ref3 == device_ref2, "Expected ref3 == device_ref2, got ref3 = %u, device_ref2 = %u.\n", ref3, device_ref2);
> +    todo_wine ok(ref3 == device_ref2, "Expected ref3 == device_ref2, got ref3 = %u, device_ref2 = %u.\n", ref3, device_ref2);
So what does this patch fix?
 
Since the render target is created by the application in the case of CreateDeviceFromD3D, I AddRef'd it (just like I did for CreateDeviceFromSurface, see d3drm_device_init). Though the todo_wine does show that it's not consistent with what windows does.

This patch ensures that the device object contains a reference to the render target in the case of CreateDeviceFromD3D as well. In the past it was only CreateDeviceFromSurface/Clipper which did this. I needed this in order to fetch the depth surface from the render target in IDirect3DRMViewport*::Clear, to check whether it's present or not. In the tests, I created a device without depth surface attached and passed it to CreateDeviceFromD3D.

I could have QI'd the render target within Clear from the d3d device, but I thought this patch would make things consistent i.e. fetching the render target would be the same regardless of how the device was created. I'll probably have to do something similar for the primary surface as well, though CreateDeviceFromSurface/D3D doesn't seem to create one internally.

Is there a better approach? I kept a separate reference to the render target in struct d3drm_device as the refcounting suggests so.

Sorry for the long email.. Thanks for the review!

Cheers,
Aaryaman