[v9 3/6] d3dx9: Implement fxlc constants (expressions) in effect.

[Matteo Bruni]

2016-04-07 10:37 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 04/06/2016 04:42 PM, Matteo Bruni wrote:
>>
>>> +static void regstore_set_double(struct d3dx_regstore *rs, unsigned int table, unsigned int offset, double v)
>>> +{
>>> +    BYTE *p;
>>> +    unsigned int reg_idx;
>>> +
>>> +    p = (BYTE *)rs->tables[table] + table_info[table].component_size * offset;
>>> +    switch (table_info[table].type)
>>> +    {
>>> +        case PRES_VT_FLOAT : *(float *)p = v; break;
>>> +        case PRES_VT_DOUBLE: *(double *)p = v; break;
>>> +        case PRES_VT_INT   : *(int *)p = roundf(v); break;
>> Same here. Also it's not clear that rounding away from 0 is correct,
>> this would need tests (although IIRC preshaders generated by the MS
>> compiler explicitly compute integral values for integer constants,
>> that would make this hard to test and mostly irrelevant in practice).
>> So please try to test for rounding behavior with 0.5 / -0.5 / 1.5, if
>> nothing else to confirm that my memory serves me right. BTW unless
>> testing proves otherwise lrint() is probably the best option here
>> since it generates much nicer code (at least when building with gcc
>> for 32 bits).
> In fact the main test now does not have integer constants. Preshader
> code I've seen generated by MS compiler so far indeed does not use any
> rounding functions, doing some math with fract, neg and min/max instead.
> I already have some further patches involving tests of the newly added
> operations which do not overwrite the blob but modify the opcode in
> place. I used the same mini framework to test the other edge case which
> is currently not in the main test: 0 * INF (BTW the results are funny,
> preshaders exhibit IEEE FP behaviour, like in my implementation, while
> d3d9 shaders have 0 * INF == 0). I suppose it will be easier to send and
> less code changes to review if we consider the whole updated test blob
> with all the planned changes at once afterwards, can we go this way?

That sounds about what I expected and using lrint() here seems ideal.
Yes, the additional tests can come after this set of patches.