[v2 2/6] d3dx9: Apply changed states only in CommitChanges.

[Matteo Bruni]

2017-03-16 17:15 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:
>
>>>>> -    set_constants(rs, &pres->inputs);
>>>>> -    if (FAILED(hr = execute_preshader(pres)))
>>>>> -        return hr;
>>>>> +    if (update_all || is_const_tab_input_dirty(&pres->inputs))
>>>>> +    {
>>>>> +        set_constants(rs, &pres->inputs, TRUE);
>>>>> +        if (FAILED(hr = execute_preshader(pres)))
>>>>> +            return hr;
>>>>> +        regstore_reset_table(rs, PRES_REGTAB_CONST);
>>>>> +    }
>>>>
>>>> This is technically a separate bugfix which would be better as a
>>>> separate patch, before this one.
>>>>
>>> This chunk actually avoids redundant preshader recompute. I don't quite
>>> understand how can I do it as a separate previous patch, as this patch
>>> introduce "update_all" parameter, is_const_tab_input_dirty() function and
>>> all the other logic around it. Or can it be that you were thinking of
>>> changes on light and material state update, and copy pasted different
>>> chunk?
>>
>> I just meant the regstore_reset_table() part.
>>
> I am sorry, I still don't understand how it is a separate bugfix. I don't
> see the absence of it as a bug in previous code, reset of constants is not
> required as they are updated anyway.

I guess you can argue that, sure. It's mostly that
set_shader_constants_device() calls regstore_reset_table() and, for
symmetry if not in practice, it seems better to clear here too.
Not a big deal either way you choose.

> Actually regstore_reset_table() call
> along with ultimate TRUE for update_all parameter in set_constants() call
> here is redundant. I added it just to make things a bit more robuts and more
> robust checking of unitialized input in preshader in case of some bugs in
> their update logic, which got more complicated with selective constants
> update. It should actually work (without updating constants from unchanged
> parameters) if change set_constants(rs, &pres->inputs, TRUE); to
> set_constants(rs, &pres->inputs, update_all); and remove
> regstore_reset_table(). Maybe I should do it like this?

Right, that's a good idea I think. If that breaks the tests it means
there is some bug somewhere in the update logic.
Do you need to remove regstore_reset_table() though?