[PATCH 4/5] ddraw: Accept all 3 versions of D3DFINDDEVICERESULT

[Andrew D'Addesio]

On 10/05/2015 02:21 AM, Stefan Dösinger wrote:
> Is there a nicer way? I can't think of one to be honest, but this code
> feels somewhat unsatisfying.

Well it makes pretty logically clear statements. If it makes you happier I could
change

> desc_ptr += desc_size;

to

> desc_ptr = ((BYTE*)&fdr->ddHwDesc) + desc_size;

so that the two blocks can be swapped freely.

On 10/05/2015 02:21 AM, Stefan Dösinger wrote:
> Also I guess we're lucky that all versions of D3DDEVICEDESC have a
> DWORD-aligned size, so we don't have to worry about padding between them.
>
> Native ddraw doesn't support d3d in 64 bit. Ours does though, mostly
> out of laziness. Did you run the test in a 64 bit build? It might fail
> due to different struct alignment.

I defined D3D*_DESC_SIZE like such:

> /* Size of D3DDEVICEDESC in Direct3D 1-3 */
> enum {
>     D3D1_DESC_SIZE = FIELD_OFFSET(D3DDEVICEDESC, dwMinTextureWidth), /* 172 */
>     D3D2_DESC_SIZE = FIELD_OFFSET(D3DDEVICEDESC, dwMaxTextureRepeat), /* 204 */
>     D3D3_DESC_SIZE = sizeof(D3DDEVICEDESC) /* 252 */
> };

where dwMinTextureWidth is the first field not in D3D1 and dwMaxTextureRepeat is
the first field not in D3D2. This should work regardless of struct padding.

The primary concern is whether the comments (172, 204, 252) are still correct in
64-bit. Here is a test which shows that they're still correct:

> #include <windows.h>
> #include <d3d.h>
> #include <stdio.h>
> enum {
>     D3D1_DESC_SIZE = FIELD_OFFSET(D3DDEVICEDESC, dwMinTextureWidth),
>     D3D2_DESC_SIZE = FIELD_OFFSET(D3DDEVICEDESC, dwMaxTextureRepeat),
>     D3D3_DESC_SIZE = sizeof(D3DDEVICEDESC)
> };
> int main() {
>     printf("D3D1_DESC_SIZE: %u\n", D3D1_DESC_SIZE);
>     printf("D3D2_DESC_SIZE: %u\n", D3D2_DESC_SIZE);
>     printf("D3D3_DESC_SIZE: %u\n", D3D3_DESC_SIZE);
>     return 0;
> }

My results are:

> C:\sizetest>"C:\mingw32\bin\gcc.exe" -m32 -Wall -Wextra -ansi -pedantic -o
sizetest-32.exe sizetest.c
>
> C:\sizetest>sizetest-32.exe
> D3D1_DESC_SIZE: 172
> D3D2_DESC_SIZE: 204
> D3D3_DESC_SIZE: 252
>
> C:\sizetest>"C:\mingw64\bin\gcc.exe" -m64 -Wall -Wextra -ansi -pedantic -o
sizetest-64.exe sizetest.c
>
> C:\sizetest>sizetest-64.exe
> D3D1_DESC_SIZE: 172
> D3D2_DESC_SIZE: 204
> D3D3_DESC_SIZE: 252
>
> C:\sizetest>

On 10/05/2015 02:21 AM, Stefan Dösinger wrote:
> What happens when you use something between D3D1_DESC_SIZE and
> D3D2_DESC_SIZE, or e.g. the largest size + 4?

I can add those two cases if you want. I can also do what test_get_caps1 (in
tests/d3d.c) currently does and test all sizes from 0 to 1023, like such:

>     /* Test all matching sizes (0, 0), (1, 1), ..., (1023, 1023). */
>     for (i = 0; i < 1024; i++)
>     {
>         search.dwSize = sizeof(search);
>         search.dwFlags = 0;
>         clear_device_result(&result, i, i);
>         hr = IDirect3D_FindDevice(Direct3D1, &search, &result);
>
>         if (i == D3D1_DESC_SIZE || i == D3D2_DESC_SIZE || i == D3D3_DESC_SIZE)
>         {
>             ok(hr == D3D_OK, "FindDevice with size %u returned hr 0x%08X,
expected D3D_OK.\n", i, hr);
>         }
>         else
>         {
>             ok(hr == DDERR_INVALIDPARAMS,
>                "FindDevice with size %u returned hr 0x%08X, expected
DDERR_INVALIDPARAMS.\n", i, hr);
>         }
>     }

In this loop, I wouldn't really test all size-pairs (0, 0), (0, 1), ..., (0,
1023), (1, 0), (1, 1), ... (1023, 1023); I would only test the matching pairs
(0, 0), (1, 1), ..., (1023, 1023).

> I recommend to put the valid and invalid tests in a table together
> with an expect HRESULT. That way you don't have to duplicate the
> structure assignment code + FindDevice call and can just iterate over
> the table.

With the for loop we won't need a table. But if we don't use the for-loop, then
sure, I can add a table.

Regards,
Andrew