Re: [PATCH 04/11] wined3d: Use color source for output colors in process_vertices_strided().

[Paul Gofman]

On 5/16/19 21:01, Henri Verbeet wrote:
> On Wed, 15 May 2019 at 19:39, Paul Gofman  wrote:
> > +static void color_from_mcs(struct wined3d_color *color, enum wined3d_material_color_source mcs,
> > -
> >    const struct wined3d_color *material_color, unsigned int index,
> > -
> >    const struct wined3d_stream_info *stream_info)
> >
> > +{
> > - const struct wined3d_stream_info_element *element = NULL;
> > -
> > - switch (mcs)
> > - {
> > -
> >    case WINED3D_MCS_MATERIAL:
> > -
> >        *color = *material_color;
> > -
> >        return;
> > -
> >    case WINED3D_MCS_COLOR1:
> > -
> >        if (!(stream_info->use_map & (1u << WINED3D_FFP_DIFFUSE)))
> > -
> >        {
> > -
> >            color->r = color->g = color->b = color->a = 1.0f;
> > -
> >            return;
> > -
> >        }
> > -
> >        element = &stream_info->elements[WINED3D_FFP_DIFFUSE];
> > -
> >        break;
> > -
> >    case WINED3D_MCS_COLOR2:
> > -
> >        if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> > -
> >        {
> > -
> >            color->r = color->g = color->b = 0.0f;
> > -
> >            color->a = 1.0f;
> > -
> >            return;
> > -
> >        }
> > -
> >        element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> > -
> >        break;
> > -
> >    default:
> > -
> >        ERR("Invalid material color source %#x.\n", mcs);
> > -
> >        break;
> > - }
> > - wined3d_color_from_d3dcolor(color, element ? *(const DWORD *)(element->data.addr + index * element->stride) : 0);
> >
> > +}
>
> The naming perhaps doesn't make it obvious, but
> wined3d_color_from_d3dcolor() assumes the source format is
> WINED3DFMT_B8G8R8A8_UNORM. While that should always be true for ddraw,
> it isn't necessarily for d3d9.

Oh yeah, I should add a generic conversion function based on declaration 
type. Still, should I maybe add just FIXME now for if decltype is not 
_D3DCOLOR for now, and imlement a generic conversion later when I will 
be adding some tests for process vertices in higher Directx versions?
> >       if (dst_fvf & WINED3DFVF_SPECULAR)
> >       {
> > -
> >        /* What's the color value in the feedback buffer? */
> > -
> >        const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> > -
> >        const DWORD *color_s = (const DWORD *)(element->data.addr + i * element->stride);
> > -
> >        if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> > -
> >        {
> > -
> >            static BOOL warned = FALSE;
> > -
> >        struct wined3d_color material_specular;
> >
> > -
> >            if(!warned) {
> > -
> >                ERR("No specular color in source, but destination has one\n");
> > -
> >                warned = TRUE;
> > -
> >            }
> > -
> >        memset(&material_specular, 0, sizeof(material_specular));
> > -
> >        color_from_mcs(&material_specular, specular_source, state->render_states[WINED3D_RS_SPECULARENABLE]
> > -
> >                ? &state->material.specular : &material_specular, i, stream_info);
>
> That memset() is redundant, right?

It's purpose is to provide zeroes for input material color if 
_RS_SPECULARENABLE is not set. How it is currently written, 
material_specular will end up uninitialized if I remove the memset and 
specular lihgting is not enabled.