On 03/19/2016 12:55 AM, Matteo Bruni wrote:
2016-03-17 12:59 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
+enum PRES_REG_TABLES +{ + PRES_REGTAB_NONE, + PRES_REGTAB_IMMED, /* immediate double constants from CLIT */ + PRES_REGTAB_CONST, + PRES_REGTAB_VERTEX, /* not used */ + PRES_REGTAB_OCONST, + PRES_REGTAB_OBCONST, + PRES_REGTAB_OICONST, + PRES_REGTAB_REG, + PRES_REGTAB_MAX, + PRES_REGTAB_FIRST = PRES_REGTAB_IMMED, + PRES_REGTAB_LAST = PRES_REGTAB_REG, +}; You can drop PRES_REGTAB_NONE unless it's going to be used. These constants currently match values in preshader bytecode. PRES_REGTAB_NONE is used as a placeholder for value 0. I can remove it, but then have PRES_REGTAB_IMMED = 1 explicitly, or introduce a separate mapping table between original constants and these enum.
+#define PRES_VS_BITS_PER_WORD (sizeof(unsigned int) * 8) What does the _VS_ part of the name mean? I'm also a bit thrown off by the _WORD part, mostly because in Win32 WORD is a 16 bit while DWORD is 32 bit and unsigned int is also 32 bit. Maybe it's just me, I don't know. VS means "value set" :) Yes, I realize WORD is confusing (what MS calls DWORD is actually a halfword nowadays). I suggest to change unsigned int to uintptr_t (which is functionally more appropriate), and call it PRES_BITMASK_BLOCK_SIZE. The traces of the disassembled preshader can also potentially go to a separate patch (leaving only dump_shader_bytecode() in the initial implementation - btw, I think it should be called dump_preshader_bytecode() given the renaming of the other stuff). It actually used to dump the whole binary blob, which can contains shader also (or just shader sometimes). I suggest I better use just dump_bytecode then.