Re: [PATCH 1/7] ddraw: D3DLIGHT lights default to active.

[Henri Verbeet]

On 30 March 2015 at 20:18, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
> @@ -184,7 +184,7 @@ static HRESULT WINAPI d3d_light_SetLight(IDirect3DLight *iface, D3DLIGHT *data)
> -    if (data->dwSize >= sizeof(D3DLIGHT2) && (((D3DLIGHT2 *)data)->dwFlags & D3DLIGHT_NO_SPECULAR))
> +    if (!(flags & D3DLIGHT_NO_SPECULAR))
>          light7->dcvSpecular = data->dcvColor;
>      else
>          light7->dcvSpecular = *(const D3DCOLORVALUE *)zero_value;
> I'm not holding this against the patch too much because it's ddraw,
> but I think "if (flags & D3DLIGHT_NO_SPECULAR)" and then flipping the
> branches reads a bit nicer. If the declaration of "zero_value" is
> fixed you can avoid the cast as well.
> @@ -200,9 +200,10 @@ static HRESULT WINAPI d3d_light_SetLight(IDirect3DLight *iface, D3DLIGHT *data)
> +    memcpy(&light->light, data, sizeof(D3DLIGHT));
> There's something to be said for "sizeof(D3DLIGHT)" here, although I
> don't think "sizeof(*data)" would have been worse.

[PATCH 1/7] ddraw: D3DLIGHT lights default to active.

[Matteo Bruni]

2015-03-31 14:36 GMT+02:00 Henri Verbeet <hverbeet(a)gmail.com>:
> On 30 March 2015 at 20:18, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
> > @@ -184,7 +184,7 @@ static HRESULT WINAPI d3d_light_SetLight(IDirect3DLight *iface, D3DLIGHT *data)
> > -    if (data->dwSize >= sizeof(D3DLIGHT2) && (((D3DLIGHT2 *)data)->dwFlags & D3DLIGHT_NO_SPECULAR))
> > +    if (!(flags & D3DLIGHT_NO_SPECULAR))
> >          light7->dcvSpecular = data->dcvColor;
> >      else
> >          light7->dcvSpecular = *(const D3DCOLORVALUE *)zero_value;
> > I'm not holding this against the patch too much because it's ddraw,
> > but I think "if (flags & D3DLIGHT_NO_SPECULAR)" and then flipping the
> > branches reads a bit nicer. If the declaration of "zero_value" is
> > fixed you can avoid the cast as well.
> > @@ -200,9 +200,10 @@ static HRESULT WINAPI d3d_light_SetLight(IDirect3DLight *iface, D3DLIGHT *data)
> > +    memcpy(&light->light, data, sizeof(D3DLIGHT));
> > There's something to be said for "sizeof(D3DLIGHT)" here, although I
> > don't think "sizeof(*data)" would have been worse.

Fair points, I'll write a patch to cleanup those.