[PATCH 2/2] d3d10/effect: Handle vector arguments in expression instructions.

[Nikolay Sivov]

On 11/19/21 4:41 PM, Matteo Bruni wrote:
> On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov <nsivov at codeweavers.com> wrote:
>> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
>> ---
>>  dlls/d3d10/effect.c | 39 +++++++++++++++++++--------------------
>>  1 file changed, 19 insertions(+), 20 deletions(-)
>>
>> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
>> index 9c2012768dd..74ee4bf468c 100644
>> --- a/dlls/d3d10/effect.c
>> +++ b/dlls/d3d10/effect.c
>> @@ -191,17 +191,21 @@ struct preshader_instr
>>      unsigned int scalar     :  1;
>>  };
>>
>> -typedef float (*pres_op_func)(float **args, unsigned int n);
>> +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
>>
>> -static float pres_ftou(float **args, unsigned int n)
>> +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr)
>>  {
>>      unsigned int u = *args[0];
>> -    return *(float *)&u;
>> +    *args[1] = *(float *)&u;
>>  }
> Is ftou always scalar? Not that I'm quite sure how to test it i.e.
> what would be a case where a vector ftou could be in effect?
>
> What I'm getting at, I think it would be safer to handle vector ftou,
> just in case that's a thing.
Yes, I guess it makes sense. I can't think of a case where this could
happen in effects, I've only seen ftou() as a conversion to index
integer, and since index is scalar, it always generated ftou() on a
single component.
>
>> -static float pres_add(float **args, unsigned int n)
>> +static void pres_add(float **args, unsigned int n, const struct preshader_instr *instr)
>>  {
>> -    return *args[0] + *args[1];
>> +    float *retval = args[2];
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < instr->comp_count; ++i)
>> +        retval[i] = args[0][instr->scalar ? 0 : i] + args[1][i];
>>  }
>>
>>  struct preshader_op_info
>> @@ -333,10 +337,10 @@ static float * d3d10_effect_preshader_get_reg_ptr(const struct d3d10_effect_pres
>>
>>  static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p)
>>  {
>> -    unsigned int i, j, regt, offset, instr_count, input_count;
>> +    unsigned int i, j, regt, offset, instr_count, arg_count;
>>      const DWORD *ip = ID3D10Blob_GetBufferPointer(p->code);
>> -    float *dst, *args[4], *retval;
>>      struct preshader_instr ins;
>> +    float *dst, *args[4];
>>
>>      dst = d3d10_effect_preshader_get_reg_ptr(p, D3D10_REG_TABLE_RESULT, 0);
>>      memset(dst, 0, sizeof(float) * p->reg_tables[D3D10_REG_TABLE_RESULT].count);
>> @@ -355,16 +359,15 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p)
>>      for (i = 0; i < instr_count; ++i)
>>      {
>>          *(DWORD *)&ins = *ip++;
>> -        input_count = *ip++;
>> +        arg_count = 1 + *ip++;
>>
>> -        if (input_count > ARRAY_SIZE(args))
>> +        if (arg_count > ARRAY_SIZE(args))
>>          {
>> -            FIXME("Unexpected argument count %u.\n", input_count);
>> +            FIXME("Unexpected argument count %u.\n", arg_count);
>>              return E_FAIL;
>>          }
>>
>> -        /* Arguments */
>> -        for (j = 0; j < input_count; ++j)
>> +        for (j = 0; j < arg_count; ++j)
>>          {
>>              ip++; /* TODO: argument register flags are currently ignored */
>>              regt = *ip++;
>> @@ -373,12 +376,7 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p)
>>              args[j] = d3d10_effect_preshader_get_reg_ptr(p, regt, offset);
>>          }
>>
>> -        ip++; /* TODO: result register flags are currently ignored */
>> -        regt = *ip++;
>> -        offset = *ip++;
>> -        retval = d3d10_effect_preshader_get_reg_ptr(p, regt, offset);
>> -
>> -        *retval = d3d10_effect_get_op_info(ins.opcode)->func(args, input_count);
>> +        d3d10_effect_get_op_info(ins.opcode)->func(args, arg_count, &ins);
> I guess a small comment mentioning that the last argument is
> (usually?) the return register could be useful.
Ok, I think it's always present.
>
>>      }
>>
>>      return S_OK;
>> @@ -2100,7 +2098,7 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context *
>>          unsigned int *offset, const char **ptr, unsigned int data_size)
>>  {
>>      const struct preshader_op_info *op_info;
>> -    unsigned int i, param_count;
>> +    unsigned int i, param_count, size;
>>      struct preshader_instr ins;
>>      uint32_t input_count;
>>
>> @@ -2152,7 +2150,8 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context *
>>              case D3D10_REG_TABLE_CB:
>>              case D3D10_REG_TABLE_RESULT:
>>              case D3D10_REG_TABLE_TEMP:
>> -                context->table_sizes[regt] = max(context->table_sizes[regt], param_offset + 1);
>> +                size = param_offset + (ins.scalar && i == 0 ? 1 : ins.comp_count);
>> +                context->table_sizes[regt] = max(context->table_sizes[regt], size);
>>                  break;
>>              default:
>>                  FIXME("Unexpected register table index %u.\n", regt);