Re: [PATCH 6/6] d3dx9: Merge constant setting for child parameters when possible.

[Matteo Bruni]

2017-05-24 11:46 GMT+02:00 Paul Gofman :
> -
>        if (!(const_set->table == current_table && current_start_offset == start_offset
> -
>                && const_set->direct_copy == first_const->direct_copy
> -
>                && current_data == const_set->param->data
> -
>                && (const_set->direct_copy || (first_const->param->type == const_set->param->type
> -
>                && first_const->param->class == const_set->param->class
> -
>                && first_const->param->columns == const_set->param->columns
> -
>                && first_const->param->rows == const_set->param->rows
> -
>                && first_const->register_count == const_set->register_count
> -
>                && (i == const_tab->const_set_count - 1
> -
>                || first_const->param->element_count == const_set->param->element_count)))))
> -
>        {
> -
>            TRACE("direct_copy %u, i %u, index %u, param %s.%s, current_data %p, const_set->param->data %p, "\
> -
>                    "current_start_offset %u, start_offset %u, const_set->table %u, current_table %u.\n",
> -
>                    const_set->direct_copy, i, index, debugstr_a(param->name),
> -
>                    debugstr_a(const_set->param->name), current_data, const_set->param->data,
> -
>                    current_start_offset, start_offset, const_set->table, current_table);
> -
>            break;
> -
>        }

This looks like a debug trace.
> -
>    TRACE("Merging %u child parameters for %s, not merging %u, direct_copy %u.\n", i - index,
> -
>            debugstr_a(param->name), const_tab->const_set_count - i, first_const->direct_copy);

I guess it's intentional that you're printing this even when not
merging anything, in that case the TRACE sounds a bit awkward though.
Maybe have a separate TRACE for that case?

BTW, please format the BOOL with %#x.
> -
>    if (i > index + 1)
> -
>    {
> -
>        first_const->element_count = element_count;
> -
>        if (first_const->direct_copy)
> -
>        {
> -
>            first_const->element_count = 1;
> -
>            if (index == start_index
> -
>                    && !(param->type == D3DXPT_VOID && param->class == D3DXPC_STRUCT))
> -
>            {
> -
>                if (param_type_to_table_type(param->type) == PRES_VT_MAX)
> -
>                    return D3DERR_INVALIDCALL;
> -
>                first_const->param = param;
> -
>            }
> -
>            first_const->register_count = get_reg_offset(current_table, current_start_offset)
> -
>                    - first_const->register_index;
> -
>        }
> -
>        memmove(&const_tab->const_set[index + 1], &const_tab->const_set[i],
> -
>                sizeof(*const_tab->const_set) * (const_tab->const_set_count - i));
> -
>        const_tab->const_set_count -= i - index - 1;
> -
>    }

Just for my own curiosity, do you think it's significant to merge
non-direct_copy entries? Not that it hurts anything to have it.