[PATCH 2/3] d3dx9: Support relative addressing in preshader.

[Paul Gofman]

On 03/24/2017 03:31 AM, Matteo Bruni wrote:
> 2017-03-24 0:39 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:
>> On 03/24/2017 01:57 AM, Matteo Bruni wrote:
>>> 2017-03-23 16:06 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:
>>>> Signed-off-by: Paul Gofman <gofmanp at gmail.com>
>>>> ---
>>>>    dlls/d3dx9_36/preshader.c    | 103
>>>> ++++++++++++++++++++++++++++++++++++-------
>>>>    dlls/d3dx9_36/tests/effect.c |  38 ++++++++--------
>>>>    2 files changed, 106 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
>>>> index 9aedc99..2974ffd 100644
>>>> --- a/dlls/d3dx9_36/preshader.c
>>>> +++ b/dlls/d3dx9_36/preshader.c
>>>> @@ -191,6 +191,8 @@ struct d3dx_pres_operand
>>>>        /* offset is component index, not register index, e. g.
>>>>           offset for component c3.y is 13 (3 * 4 + 1) */
>>>>        unsigned int offset;
>>>> +    enum pres_reg_tables base_reg_table;
>>>> +    unsigned int base_reg_offset;
>>>>    };
>>> It would be nicer to create a struct for storing together table and
>>> offset and to pass it around to the various functions taking both
>>> arguments, like parse_pres_reg().
>>> It should probably be a separate patch before this one.
>> Actually I can't guess the case when base_reg_table would be not a floating
>> constant preshader input. Maybe I just remove this variable and check with
>> FIXME during parsing?
> Sure, that's fine. Still the struct seems useful for parameter
> passing, if not right there in struct d3dx_pres_operand.
>
>>>> +    size = rs->table_sizes[table] *
>>>> table_info[table].reg_component_count;
>>>> +    if (offset >= size)
>>>> +    {
>>>> +        if ((offset & 255) < size)
>>>> +            offset &= 255;
>>>> +        else
>>>> +            offset = offset % size;
>>>> +    }
>>> Is this just to try and avoid the modulo operation in some cases or is
>>> there a deeper reason?
>>> If the former, you could extend that to all the power-of-two sizes by
>>> first checking if that's the case (!(size & size - 1)) and, if so,
>>> and-ing offset with the mask (size - 1). That's assuming the compiler
>>> doesn't do the same optimization already.
>>>
>>> Let me know if there is some other reason instead.
>> Just doing (offset % size) will lead to different results and will break the
>> tests. The logic in this out of range index checking is a bit mind breaking.
> I guess there was a deeper reason indeed ;)
>
> Is that visible in the tests included in this series? Either way,
> maybe add a comment so that it's obvious why that thing is there.
>
Oh, I was sure that yes, but just checked once again and it worked 
without that first check. It could happen that either (most likely) I 
need it on some other testing I made in process, or potentially could 
happen that it was "helping" before I figured out the correct way for 
floating constant table indexing. I will check that and either remove 
this check or add another test (probably as a separate patch, as it 
would likely require effect blob update).