Re: [PATCH 1/2] d3drm/tests: Add tests for IDirect3DRM*::CreateObject.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

Some curiosities:

Am 2016-04-03 um 20:04 schrieb Aaryaman Vasishta:
> - for (i = 0; i < sizeof(tests) / sizeof(*tests); ++i)
> - {
> -
>    hr = IDirect3DRM_CreateObject(d3drm1, tests[i].clsid, NULL, NULL, NULL);
> -
>    todo_wine ok(hr == D3DRMERR_BADVALUE, "Test %u: expected hr == D3DRMERR_BADVALUE, got %#x.\n", i, hr);

What does clsid != NULL, refiid == NULL, output object != NULL do?

I guess that clsid is the object you create, and iid is the interface to that object that is returned. So I guess in theory you can pass clsid = IDirect3DRMTexture3, iid = IDirect3DRMTexture and you'd get the same result as creating a d3drmtexture3 object and then calling QI(IDirect3DRMTexture). It might be worth adding explicit tests for this in future patches.

(And if you use aggregation you'll want to pass iid == IUnknown, otherwise there's no way you get the inner IUnknown and will never be able to release the object)
> -
>        ref2 = get_refcount((IUnknown *)d3drm1);
> -
>        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2);
> -
>        IUnknown_Release(unknown);
> -
>        ref2 = get_refcount((IUnknown *)d3drm1);
> -
>        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2);

So what will increment the refcount? InitFrom*?
> -
>        hr = IDirect3DRM2_CreateObject(d3drm2, tests[i].clsid, NULL, tests[i].iid, (void **)&unknown);
> -
>        ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK, got %#x.\n", i, hr);
> -
>        ref2 = get_refcount((IUnknown *)d3drm1);
> -
>        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2);
> -
>        ref3 = get_refcount((IUnknown *)d3drm2);
> -
>        ok(ref3 == ref1, "Test %u: expected ref3 == ref1, got ref1 = %u, ref3 = %u.\n", i, ref1, ref3);
>
> ...
> -
> -
>        hr = IDirect3DRM3_CreateObject(d3drm3, tests[i].clsid, NULL, tests[i].iid, (void **)&unknown);
> -
>        ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK, got %#x.\n", i, hr);
> -
>        ref2 = get_refcount((IUnknown *)d3drm1);
> -
>        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2);
> -
>        ref3 = get_refcount((IUnknown *)d3drm3);
> -
>        ok(ref3 == ref1, "Test %u: expected ref3 == ref1, got ref1 = %u, ref3 = %u.\n", i, ref1, ref3);

Since you're testing the refcounts of 1,2 and 1,3 you might as well test the refcounts of all interfaces in all 3 versions.

Cheers,
Stefan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXAXWIAAoJEN0/YqbEcdMwbkIQAI9ItseZE8bef+z24Cisul9g
UYxXhve3Ll9Pz5vVB+jG0H0F5iPk4vdmoHIwsJgd018xBnwXc1C0+QkTknsTHcfI
LOb1l4PbjherhDNDn+4t95Hy3fdHKrutFkMhYp73m1+0V5GwY7MANCmIxu4b/z+b
nW545EvcpPGluZXPcPyWiwGi9qB33VkGRvDkUpii8qQtwC+vsrDuerWcT2V7fbXj
2R0O9QSuvlbd83p16i3nVx2R8ZQRweYQoSKKdLBca07cIt3/xW6lBJOz34yUmupE
5PdKK0wOPWMKXVCuAplWuDGrvgCOtyMcIB1FmsUfxPfLjonfX3UeXoWfTwMmrMBo
XkzgZj8S6JTPK2qOegzg8nGz8FZXvsvWkUZWzCtypuuAQ3rGn5p940JwIu3pW+Cq
4Z3K+r7Ka9lYxSJfiliNyhqPIlrGytHbrpN0aTmWJGn79Jqs7Ffd01+ujOQ2NlOf
POWffmglWRHoYDZjSOS2JaL597hhna7ktM0xlhTWHT7HFgRB4oCV2DJAHmSeZUME
egudzSfSbRgB9u8bQQpfrYr/kV6MJ0OE2d3VoTn8RRJ3RJL5UFp08N7m+YXkQW9T
LAzfiyaAjNILLTCj4tpqjEikH2ftGE/hBH+tInXTKlwnf1NyB6iz5/1+dAm+7JYk
MKdVO/nQCpGffRqmYNtG
=H6sj
-----END PGP SIGNATURE-----

[PATCH 1/2] d3drm/tests: Add tests for IDirect3DRM*::CreateObject.

[Aaryaman Vasishta]

On Mon, Apr 4, 2016 at 1:26 AM, Stefan Dösinger 
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi,
>
> Some curiosities:
>
> Am 2016-04-03 um 20:04 schrieb Aaryaman Vasishta:
> > - for (i = 0; i < sizeof(tests) / sizeof(*tests); ++i)
> > - {
> > -
> >    hr = IDirect3DRM_CreateObject(d3drm1, tests[i].clsid, NULL,
>
> NULL, NULL);
> > -
> >    todo_wine ok(hr == D3DRMERR_BADVALUE, "Test %u: expected hr ==
>
> D3DRMERR_BADVALUE, got %#x.\n", i, hr);
> What does clsid != NULL, refiid == NULL, output object != NULL do?
>
> I guess that clsid is the object you create, and iid is the interface to
> that object that is returned. So I guess in theory you can pass clsid =
> IDirect3DRMTexture3, iid = IDirect3DRMTexture and you'd get the same result
> as creating a d3drmtexture3 object and then calling QI(IDirect3DRMTexture).
> It might be worth adding explicit tests for this in future patches.
>
> The CLSID's defined in the headers are only for version 1 ones it seems.

So there's only one way to get a version 2/3 object i.e. from a version 1
CLSID.
> > -
> >        ref2 = get_refcount((IUnknown *)d3drm1);
> > -
> >        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1
>
> = %u, ref2 = %u.\n", i, ref1, ref2);
> > -
> >        IUnknown_Release(unknown);
> > -
> >        ref2 = get_refcount((IUnknown *)d3drm1);
> > -
> >        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1
>
> = %u, ref2 = %u.\n", i, ref1, ref2);
> So what will increment the refcount? InitFrom*?

Right, InitFrom* increments the refcount. One more thing I've noticed (and
you can see that in my LoadTexture patch I sent a while ago) is that the
refcount is incremented even if the function fails.
> > -
> >        hr = IDirect3DRM2_CreateObject(d3drm2, tests[i].clsid,
>
> NULL, tests[i].iid, (void **)&unknown);
> > -
> >        ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK, got
>
> %#x.\n", i, hr);
> > -
> >        ref2 = get_refcount((IUnknown *)d3drm1);
> > -
> >        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1
>
> = %u, ref2 = %u.\n", i, ref1, ref2);
> > -
> >        ref3 = get_refcount((IUnknown *)d3drm2);
> > -
> >        ok(ref3 == ref1, "Test %u: expected ref3 == ref1, got ref1
>
> = %u, ref3 = %u.\n", i, ref1, ref3);
> > ...
> > -
> > -
> >        hr = IDirect3DRM3_CreateObject(d3drm3, tests[i].clsid,
>
> NULL, tests[i].iid, (void **)&unknown);
> > -
> >        ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK, got
>
> %#x.\n", i, hr);
> > -
> >        ref2 = get_refcount((IUnknown *)d3drm1);
> > -
> >        ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1
>
> = %u, ref2 = %u.\n", i, ref1, ref2);
> > -
> >        ref3 = get_refcount((IUnknown *)d3drm3);
> > -
> >        ok(ref3 == ref1, "Test %u: expected ref3 == ref1, got ref1
>
> = %u, ref3 = %u.\n", i, ref1, ref3);
> Since you're testing the refcounts of 1,2 and 1,3 you might as well test
> the refcounts of all interfaces in all 3 versions.

Cheers,
> Stefan

Right, I'll add the new tests and submit a new patch.

Cheers,
Aaryaman