2017-03-16 10:52 GMT+01:00 Paul Gofman gofmanp@gmail.com:
On 03/16/2017 01:39 AM, Matteo Bruni wrote:
The reason I left ERR in d3dx9_get_param_value_ptr is now our implementation for out of bounds selectors errors handling is different from native implementation for certain cases. Should I still avoid ERR or FIXME, or maybe change TRACE("d3dx9_get_param_value_ptr error %#x.\n", hr); to ERR("d3dx9_get_param_value_ptr error %#x, returning D3D_OK.\n", hr); when returning OK status only?
I think WARN is what you want to use in d3dx9_get_param_value_ptr(). A FIXME for the broken case only would be okay but at that point you could just fix it properly as well so I don't think you need anything else. The todo_wine in the tests are enough. You can keep the TRACE when returning D3D_OK (definitely not ERR) but I think it's better to only override the return on out-of-bounds, so the thing with using a different HRESULT for that case.
+BOOL is_param_eval_input_dirty(struct d3dx_param_eval *peval) +{
- return is_const_tab_input_dirty(&peval->pres.inputs)
|| is_const_tab_input_dirty(&peval->shader_inputs);+}
If I'm not mistaken this only needs to check pres.inputs, given that it's only reasonably going to be called on FXLC and ARRAY_SELECTOR preshaders (not on actual shaders' preshaders). It doesn't matter much though.
If it is FXLC or ARRAY_SELECTOR peval->shader_inputs is an empty table with zero count, and I thought that doing a check in is_const_tab_input_dirty() with zero count in table is not much different from explicitly checking the type of param_eval, while it is shorter/simpler and more generic. Maybe I could leave it this way?
Yeah, it's fine.
- 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.