[PATCH 2/2] d3dx9: Get rid of constants modification bitmasks.

Matteo Bruni matteo.mystral at gmail.com
Mon Aug 28 17:51:12 CDT 2017


2017-08-28 23:32 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 08/28/2017 09:00 PM, Matteo Bruni wrote:
>>
>>
>>> @@ -971,10 +946,30 @@ static HRESULT get_constants_desc(unsigned int
>>> *byte_code, struct d3dx_const_tab
>>>           if (FAILED(hr = init_set_constants_param(out, ctab, hc,
>>> inputs_param[index])))
>>>               goto cleanup;
>>>       }
>>> +    if (pres)
>>> +        append_pres_const_set(out, pres);
>>>       if (out->const_set_count)
>>>       {
>>>           struct d3dx_const_param_eval_output *new_alloc;
>>>
>>> +        qsort(out->const_set, out->const_set_count,
>>> sizeof(*out->const_set), compare_const_set);
>>> +        for (i = 0; i < out->const_set_count - 1; ++i)
>>> +        {
>>> +            if (out->const_set[i].constant_class == D3DXPC_FORCE_DWORD
>>> +                    && out->const_set[i + 1].constant_class ==
>>> D3DXPC_FORCE_DWORD
>>> +                    && out->const_set[i].table == out->const_set[i +
>>> 1].table
>>> +                    && out->const_set[i].register_index +
>>> out->const_set[i].register_count
>>> +                    >= out->const_set[i + 1].register_index)
>>> +            {
>>> +                out->const_set[i].register_count = out->const_set[i +
>>> 1].register_index
>>> +                        + out->const_set[i + 1].register_count -
>>> out->const_set[i].register_index;
>>
>> I think this might lose account of some registers. E.g. consider the
>> case where const_set[i] has register_index == 0 and register_count ==
>> 4 while const_set[i + 1] has register_index == 1 and register_count ==
>> 1. This would seem to lose track of the last two components.
>>
>> I know that preshaders found in the wild seem not to overwrite already
>> written output registers but I wouldn't feel safe with (silently)
>> depending on it.
>
> This 'if' selects just preshader output constants. The entries for const_set
> handled (merged) in this 'if' are those provided preshader instructions
> which output anything to shader constants. They can potentially overlap
> (that's why '>=' in the last 'if' check).  I am not sure what can be wrong
> with that? We track preshader output as a single "commit", the same does
> native implementation according to our tests.

I don't have any issues with the if condition. It's the
const_set[i].register_count assignment which looks troublesome since
it doesn't consider the old count. In the (artificial, even forced if
you want) example I gave const_set[i].register_count would go from 4
to 2 which doesn't seem right.



More information about the wine-devel mailing list