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

[Henri Verbeet]

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.
> +static void clamp_vec(float *dst, const float *src, unsigned int count,
> -
>    float min_value, float max_value)
>
> +{
> - unsigned int i;
> -
> - for (i = 0; i < count; ++i)
> -
>    dst[i] = clamp(src[i], min_value, max_value);
>
> +}

The way this is used, that should probably be called "wined3d_color_clamp()".
>      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?
> +#define D3DCOLOR_R_B(dw) (((BYTE)(dw)) << 16)
> +#define D3DCOLOR_G_B(dw) (((BYTE)(dw)) << 8)
> +#define D3DCOLOR_A_B(dw) (((BYTE)(dw)) << 24)
> -
>
> +static inline DWORD d3dcolor_from_wined3d_color(const struct wined3d_color *wined3d_color)
> +{
> - return D3DCOLOR_R_B(lrint(wined3d_color->r * 255.0f)) | D3DCOLOR_G_B(lrint(wined3d_color->g * 255.0f))
> -
>        | D3DCOLOR_B_B(lrint(wined3d_color->b * 255.0f)) | D3DCOLOR_A_B(lrint(wined3d_color->a * 255.0f));
>
> +}
> -

You're probably looking for wined3d_format_convert_from_float().
That's likely to be slightly slower than what you have here, but if
performance were the main concern there would probably be other
optimisations we could do first.