Re: [v4 3/6] d3dx9: implement fxlc constants (expressions) in effect.

[Matteo Bruni]

2016-03-17 12:59 GMT+01:00 Paul Gofman :
> @@ -2616,7 +2630,7 @@ static void d3dx9_set_material_parameter(enum MATERIAL_TYPE op, D3DMATERIAL9 *ma
>              D3DCOLORVALUE c = *(D3DCOLORVALUE *)value;
>          TRACE("%s, value (%f %f %f %f).\n", material_tbl[op].name, c.r, c.g, c.b, c.a);
> -
>        *(D3DCOLORVALUE *)((char *)material + material_tbl[op].offset) = c;
> -
>        *(D3DCOLORVALUE *)((BYTE *)material + material_tbl[op].offset) = c;
>        break;
>    }
>    default:

While you're at it, you could change the format strings to use %.8e
for the float values.
> +static void regstore_reset_table(struct d3dx_regstore *rs, unsigned int table)
> +{
> - unsigned int sz;
> -
> - sz = rs->table_sizes[table] * table_info[table].reg_component_count * table_info[table].component_size;
> -
> - memset(rs->tables[table], 0, sz);
> - memset(rs->table_value_set[table], 0,
> -
>        sizeof(*rs->table_value_set[table]) *
> -
>        ((rs->table_sizes[table] + PRES_VS_BITS_PER_WORD - 1) / PRES_VS_BITS_PER_WORD));
>
> +}

This function is unused and the compiler should throw a warning about it.
> - rs->table_value_set[table][reg_idx / PRES_VS_BITS_PER_WORD] |=
> -
>            1 << (reg_idx % PRES_VS_BITS_PER_WORD);

That should be "1u", otherwise here shifting by 31 is technically
undefined behavior and there are some compilers that complain about
it.
The line continuation is also misindented.
> - while (param->member_count && param->element_count)
> - {
> -
>    if (param->element_count > 1)
> -
>    {
> -
>        FIXME("Unexpected param having both elements and members.\n");
> -
>        return D3DERR_INVALIDCALL;
> -
>    }
> -
>    param = &param->members[0];
> - }

I don't quite understand what you're trying to do here but it doesn't
look right.
> - TRACE("%s rows %u, par columns %u, par class %u, par flags %u, par bytes %u, c rows %u," \
> -
>        "c columns %u, c class %u, c bytes %u, transpose %u.\n",
> -
>        desc.Name, param->rows, param->columns, param->class, param->flags, param->bytes,
> -
>        desc.Rows, desc.Columns, desc.Class, desc.Bytes, transpose);

I would make two separate TRACEs for parameter and constant info. Also
the string comes from the application so you should use debugstr_a().
Something like:

TRACE("Constant %s rows %u, columns %u, class %u, bytes %u.\n",
        debugstr_a(desc.Name), desc.Rows, desc.Columns, desc.Class, desc.Bytes);
TRACE("Parameter rows %u, columns %u, class %u, flags %#x, bytes %u,
transpose %#x.\n",
        param->rows, param->columns, param->class, param->flags,
param->bytes, transpose);

Gmail will probably mangle the code but you get the point.