[10/11] ddraw/tests: Add assorted D3D3 DrawPrimitive/visual tests.

Octavian Voicu octavian.voicu at gmail.com
Tue Nov 8 04:39:50 CST 2011

[resent to include wine-devel]

On Tue, Nov 8, 2011 at 1:23 AM, Stefan Dösinger <stefandoesinger at gmx.at> wrote:
> Design-wise I think the 8 triangles and readback grid make the test fairly
> difficult to read. You could set up a structure that contains the d3d parameters
> you're changing between the tests and the expected color and then loop over
> the table. Something like this:
> const struct
> {
>        BOOL d3drs_lighting;
>        DWORD dp_flags; <- for things like D3DP_DONOTLIGHT
>        DWORD fvf;
>        void *vertices;
>        DWORD expected_color;
> }
> test_data[] =
> {
>        ...
> };
> You can also think about sharing this control table between d3d2, d3d3 and
> d3d7 by having different color fields for that, but I don't know how well this
> will work.

Henry wasn't very happy about this either. Your fix sounds
interesting, I'm going to give it a try.

The idea behind using triangles was to fit everything in one screen so
I could debug it visually very easily. I'm thinking I can still have
that if I draw full-screen quads in a loop, like you suggest, then
blit one small rectangle to the primary surface, in different
positions for each test.

> On Monday 07 November 2011 23:27:13 Octavian Voicu wrote:
> BLUE };
> Please make data const where you can.
Point taken.

>> +        /* Note: IDirect3DDevice3::DrawPrimitive calls with D3DVT_ vertex
>> types should fail. */ +
>> +        /* Triangle 0 -- D3DFVF_VERTEX (vertices with normals, but without
>> colors). */ +        hr = IDirect3DDevice3_DrawPrimitive(Direct3DDevice3,
>> +                                            vertices, 3, D3DDP_WAIT);
>> +        ok(hr != DDERR_INVALIDPARAMS, "DrawPrimitive returned: %08x\n",
>> hr);
> I don't think there's a point in testing this, both D3DVT_* and D3DFVF_* are
> defines / enums that map to integers, and it is obvious that you get wrong
> behavior if you mix those.
> The ok condition looks strange. If you're trying to avoid a todo_wine, don't
> do that, just mark tests that show a difference between Wine and Windows as
> todo_wine.
> (I guess Wine currently returns DD_OK, and Windows returns

DirectX SDK 6.1 docs says that "If you attempt to use one of the
members of D3DVERTEXTYPE, the method fails, returning
DDERR_INVALIDPARAMS", so I wanted to test exactly that. I accidentally
wrote "!=" instead of "==" and forgot about it since Windows did not
complain (SDK is probably inaccurate), nor did Wine. Will look into it
and probably write some separate tests, outside the test loop. I think
we should cover special error cases if there are any (that is, if
D3DERR_INVALIDVERTEXTYPE is not returned for D3DVT_ constants).

>> +    /* Make sure getPixelColor works correctly. */
>> +    color = getPixelColor(device, 320, 240);
>> +    ok(color == RED, "getPixelColor returned: %08x\n", color);
> We have some sanity checks for that in the main function

I know that, but I figured it didn't hurt to have an extra check
there, since each D3D2, D3D3, and D3D7 test uses a different version
of the getPixelColor function.

> Most of this applies to the d3d2 and d3d3 tests as well. I haven't looked over
> the cleanup patches yet.

Thanks for the quick and thorough feedback, it was really helpful!

About the ddraw/visual tests: I think we need one structure for each
of d3d, d3d2, d3d3, d3d7, where to have most of the things we store
globally or in local vars now, and make the create/release functions
work with that. With my cleanup patches I think it will be easier to
refactor code to use such a structure. However, my patch stack was
getting uncomfortably large and I wanted to commit it, so I can get to
the good part -- fixing the game I'm debugging. I might look into such
a refactoring later.


More information about the wine-devel mailing list