2017-03-24 2:16 GMT+01:00 Paul Gofman gofmanp@gmail.com:
On 03/24/2017 03:31 AM, Matteo Bruni wrote:
2017-03-24 0:39 GMT+01:00 Paul Gofman gofmanp@gmail.com:
On 03/24/2017 01:57 AM, Matteo Bruni wrote:
2017-03-23 16:06 GMT+01:00 Paul Gofman gofmanp@gmail.com:
Signed-off-by: Paul Gofman gofmanp@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.
Sorry, I messed up things actually, base register actually can come from temporary (register) table also, which happens in the test. So I will introduce extra structure for registers.
So I had a look at my stuff. As you know it has got very little and specific testing and is most likely missing some cases but I see that: - for relative addressing I accept any register type as base but expect only temporary registers as index (i.e. I look for "flags" == 1 and then for the "base" fields only when parsing TEMP registers) - for CONST registers the "flags" != 0 field has a different meaning. In that case it is an offset in the IMMED table and there is no additional "base" register type + index in the effect bytecode, so it's indexed like imm["flags" + cidx].
Unfortunately I don't think I have any effect with the latter feature around. Supporting that should probably be a separate patch but try to see if you can replicate that.
static double exec_get_arg(struct d3dx_regstore *rs, const struct d3dx_pres_operand *opr, unsigned int comp) {
- if (!regstore_is_val_set_reg(rs, opr->table, (opr->offset + comp)
/ table_info[opr->table].reg_component_count))
WARN("Using uninitialized input, table %u, offset %u.\n",opr->table, opr->offset + comp);
- unsigned int base_offset, index;
- if (opr->base_reg_table == PRES_REGTAB_COUNT)
index = 0;- else
index = (int)exec_get_reg_value(rs, opr->base_reg_table,opr->base_reg_offset);
I think it's better with lrint() in place of the cast.
I just want to note that whenever the input is from floating values FX compiler generates preshader instructions which do the rounding, and skips that if the base register comes from integers. Do you think we need to do extra rounding in this case?
Ah, you're right, I somehow overlooked that. Actually I think doublechecking (e.g. with a FIXME) that the source is really integer would be useful. If it isn't too much of a mess it would be nice to avoid the roundtrip to doubles for the index entirely.
Source of the base register value seems not so straightforward to check.The input table always has float format. To do such check, we need to find which parameter the register is initialized from, we don't have such a link now.
Right, that's hard to do in the current architecture. Also I confused this with the IMMED relative addressing I mentioned above, for those my code expects the input CONST register to be mapped to an INT parameter. That's only relevant in my architecture since I don't fill all the input registers before executing the preshader but go fetch them "on-the-fly" for each instruction. That is also clearly worse than your solution, of course. Long story short, just ignore this.
Besides, using float register as index seems totally ok in effect, compiler generates rounding code for this case. Moreover, base register used in instruction may come from temporary ("r") registers and be a result of some operations, like rounding or multiplication when indexing anything with stride other than 4xfloat.
Right as well. I don't know, lrintf() seems like a good idea still.
Or could it be you had in mind not checking the source of registervalue, but rather that the value is actually an integer (fractional part is zero)?
That might be a bit fragile (because of potential numerical accuracy troubles), although as you noticed the compiler seems to go to great lengths to make sure the value is exactly an integer. Maybe doing that kind of check only in a if (WARN_ON(d3dx)) might make sense as it would be somewhat surprising and it might reveal something else going on but it doesn't seem worth it to me in general.