Am Wednesday 12 August 2009 21:57:36 schrieb Matteo Bruni: > > I think it doesn't need two include files, especially since both of them > > are d3dx9-private anyway. The question is, is it better to just merge > > them into d3dx9_36_private.h, or have assembler things in a separate > > file? I tend towards putting everything into d3dx9_36_private.h, but I > > don't have a strong opinion about that. > > I merged everything into d3dx9_36_private.h Ok.
> >> +%option prefix="asmshader_" > >> + buffer = asmshader__scan_string(text, asm_ctx.yyscanner); > > > > It seems that flex adds an underscore to the prefix anyway, so I guess > > its fine to remove the one from the prefix option. > > Removing the underscore from the flex prefix produces a (better) > asmshader_scan_string function but also an ugly asmshaderlex_init. I > decided to keep the underscore. Fair enough > >> + WINED3DSIO_NOP = 0, > >> + WINED3DSIO_MOV = 1, > >> + WINED3DSIO_ADD = 2, > >> and others > > > > Can you give them a different name prefix, to make it clear that they > > don't have any relationship with wined3d any more? Hmm. Maybe D3DXSIO_*, or WINED3DXSIO_*, but I'll think about a better one. Also this applies to many other enums. > I named them BWRITERxxxx (the same for wine_shader, now it is named > bwriter_shader, enforcing the idea that that represents the bytecode > writer interface). Is it ok? Aren't they used by the parser as well? Ie, to generate the wineshader structure? > > A swich-case statement or a lookup table would probably be nicer. (A > > lookup table has the disadvantage that you still have to check min and > > max separately though) > > Looking better at this, I noticed that I wasn't checking the register > number, while native has this check. So I reworked this part, and now > the code is as the attached file. Looks ok to me > Regarding Henri's review, I followed everything, except keeping > my_alloc/realloc/free (now asm_alloc and c.) functions, as it seems > Stefan likes them very much :) I added them after I got fed up with writing HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size); all the time, and I considered them ugly (A while ago there was some talk about a wine-wide HeapAlloc wrapper macro too) > Supposing my changes are ok, what should I do now? Wait for reviews of > the rest? Or maybe send the updated code? In this case, should I send > it again in wine-devel or have a try with wine-patches? I'd say give wine-patches a try :-) Make sure Alexandre can see how these patches will work together with the wpp ones and the actual implementation of D3DXAssembleShader
