Re: [PATCH 1/3] d3drm: Store reference to render target in IDirect3DRM*::CreateDeviceFromD3D.

[Henri Verbeet]

On 9 August 2016 at 03:16, Aaryaman Vasishta <jem456.vasishta(a)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?

[PATCH 1/3] d3drm: Store reference to render target in IDirect3DRM*::CreateDeviceFromD3D.

[Aaryaman Vasishta]

On Tue, Aug 9, 2016 at 9:23 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
> On 9 August 2016 at 03:16, Aaryaman Vasishta <jem456.vasishta(a)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