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

[Andrew D'Addesio]

On 10/05/2015 03:57 AM, Henri Verbeet wrote:
> On 5 October 2015 at 09:21, Stefan Dösinger <stefandoesinger at gmail.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA256
>>
>> Am 2015-10-04 um 19:52 schrieb Andrew D'Addesio:
>>> +    /* Write ddHwDesc */
>>> +    desc_ptr = (BYTE*)&fdr->ddHwDesc;
>>> +    memcpy(desc_ptr, &desc1, desc_size);
>>> +    ((D3DDEVICEDESC*)desc_ptr)->dwSize = desc_size;
>>> +
>>> +    /* Write ddSwDesc */
>>> +    desc_ptr += desc_size;
>>> +    memcpy(desc_ptr, &desc1, desc_size);
>>> +    ((D3DDEVICEDESC*)desc_ptr)->dwSize = desc_size;
>> Is there a nicer way? I can't think of one to be honest, but this code
>> feels somewhat unsatisfying.
>>
> One way would be to just declare different versions of the structure
> for internal use (i.e., D3DFINDDEVICERESULTv1, etc.), and then pick
> the right one based on fdr->dwSize. That would probably simplify a
> couple of other things in the patch as well.

In that case, we could do:

>     if (ver == 1)
>     {
>         D3DFINDDEVICERESULTv1 *fdrv1 = (D3DFINDDEVICERESULTv1 *)fdr;
>         D3DDEVICEDESCv1 *desc1v1 = (D3DDEVICEDESCv1 *)&desc1;
>         fdrv1->ddHwDesc = *desc1v1;
>         fdrv1->ddSwDesc = *desc1v1;
>     }
>     else if (ver == 2)
>     {
>         /* ... */

On 10/05/2015 03:57 AM, Henri Verbeet wrote:
> There are also a couple
> of style issues in these patches. Please follow the style of the
> surrounding code, or that of wined3d when in doubt.

Sorry, I was so busy checking for code correctness that I forgot about
indentation. I think that's mainly what you're referring to. For example, this:

>            hel_desc.dwFlags = D3DDD_COLORMODEL | D3DDD_DEVCAPS | D3DDD_TRANSFORMCAPS
>                | D3DDD_LIGHTINGCAPS | D3DDD_BCLIPPING;

should become:

>            hel_desc.dwFlags = D3DDD_COLORMODEL | D3DDD_DEVCAPS | D3DDD_TRANSFORMCAPS
>                    | D3DDD_LIGHTINGCAPS | D3DDD_BCLIPPING;

and similarly:

>        ok(device > ctx->prev_device, "D3D%d %s enumerated after %s\n", ver,
>            expected_name, device_list[ctx->prev_device].device_name);

should become:

>        ok(device > ctx->prev_device, "D3D%d %s enumerated after %s\n", ver,
>           expected_name, device_list[ctx->prev_device].device_name);

And I think the other big problem was that I did "enum {" instead of placing
the { on its own line.

Is there anything else you had in mind that I should know about?

Thanks,
Andrew