Re: [PATCH vkd3d v3 09/11] vkd3d-shader/hlsl: Support all complex initalizers.

[Giovanni Mascellani]

Hi,

I am not completely in love with the additional runtime complexity of 
this solution, but it's true that it's quite clean.

Il 22/04/22 12:25, Giovanni Mascellani ha scritto:
> +/* Returns the register offset of a given component within a type, given its index.
> -
> - *comp_type, will be set to the type of the component, unless it is NULL. */
>
> +unsigned int hlsl_compute_component_reg_offset(struct hlsl_ctx *ctx, struct hlsl_type *type,
> -
>    unsigned int idx, struct hlsl_type **comp_type, const struct vkd3d_shader_location *loc)

Is the "loc" parameter actually used for something, except being passed 
around?

Also, personally I'd just write "offset" rather than "reg_offset". It 
seems to be clear enough. This applies also to the variables below.
> +{
> - switch (type->type)
> - {
> -
>    case HLSL_CLASS_SCALAR:
> -
>    case HLSL_CLASS_VECTOR:
> -
>    {
> -
>        assert(idx < type->dimx * type->dimy);
> -
> -
>        if (comp_type)
> -
>            *comp_type = hlsl_get_scalar_type(ctx, type->base_type);

I think you can assume that comp_type will always be valid and skip this 
check.
> -
>        return idx;
> -
>    }
> -
>    case HLSL_CLASS_MATRIX:
> -
>    {
> -
>        unsigned int minor, major, x = idx % type->dimx, y = idx / type->dimx;
> -
> -
>        assert(idx < type->dimx * type->dimy);
> -
> -
>        if (hlsl_type_is_row_major(type))
> -
>        {
> -
>            minor = x;
> -
>            major = y;
> -
>        }
> -
>        else
> -
>        {
> -
>            minor = y;
> -
>            major = x;
> -
>        }
> -
> -
>        if (comp_type)
> -
>            *comp_type = hlsl_get_scalar_type(ctx, type->base_type);
> -
>        return 4 * major + minor;
> -
>    }
> -
> -
>    case HLSL_CLASS_ARRAY:
> -
>    {
> -
>        unsigned int elem_comp_count = hlsl_type_component_count(type->e.array.type);
> -
>        unsigned int array_idx = idx / elem_comp_count;
> -
>        unsigned int idx_in_elem = idx - array_idx * elem_comp_count;

BTW, here you are just computing a modulus, you could directlyuse "%". 
Addressing arrays is not fundamentally different than addressing 
matrices, except you don't have to decide which one is major and which 
one is minor.
> -
>        assert(array_idx < type->e.array.elements_count);
> -
> -
>        return array_idx * hlsl_type_get_array_element_reg_size(type->e.array.type) +
> -
>                hlsl_compute_component_reg_offset(ctx, type->e.array.type, idx_in_elem, comp_type, loc);
> -
>    }
> -
> -
>    case HLSL_CLASS_STRUCT:
> -
>    {
> -
>        struct hlsl_struct_field *field;
> -
> -
>        LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
> -
>        {
> -
>            unsigned int elem_comp_count = hlsl_type_component_count(field->type);
> -
> -
>            if (idx < elem_comp_count)
> -
>            {
> -
>                return field->reg_offset +
> -
>                        hlsl_compute_component_reg_offset(ctx, field->type, idx, comp_type, loc);
> -
>            }
> -
>            idx -= elem_comp_count;
> -
>        }
> -
> -
>        assert(0);
> -
>        return 0;
> -
>    }
> -
> -
>    case HLSL_CLASS_OBJECT:
> -
>    {
> -
>        assert(idx == 0);
> -
>        if (comp_type)
> -
>            *comp_type = type;
> -
>        return 0;
> -
>    }
> - }
> -
> - assert(0);
> - return 0;
>
> +}
> -

Thanks, Giovanni.