Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/28 Matteo Bruni matteo.myst...@gmail.com:
Why do you need the fake parser? Can't you just not support those
shader versions yet? There's also (in general) not much of a point in
adding structure fields that aren't used yet.
+/* This file needs the original d3d9 definitions. The bwriter ones
+ * aren't useable because they are wine-internal things. We're writing
+ * d3d8/9 shaders here, so we need the d3d9 definitions (which are
+ * equal to the d3d8 ones)
+ */
This doesn't seem to match what the code actually does.
+/* Debug utility routines. Some are not reentrant, check asmutils.c */
Same as above.
+const char *debug_print_dstreg(const struct shader_reg *reg, shader_type st)
{
+return wine_dbg_sprintf(%s, get_regname(reg, st));
+}
+
+const char *debug_print_srcreg(const struct shader_reg *reg, shader_type st)
{
+switch(reg-srcmod) {
+case BWRITERSPSM_NONE:
+return wine_dbg_sprintf(%s, get_regname(reg, st));
+}
+return Unknown modifier;
+}
return get_regname(reg, st); should work at least as well.
+/* Mutex used to guarantee a single invocation
+ of the D3DXAssembleShader function (or its variants) at a time.
+ This is needed as wpp isn't thread-safe */
+extern CRITICAL_SECTION wpp_mutex;
It's probably easier to just statically initialize the critical
section in shader.c.
As for splitting things up, I think it's ok to e.g. add the
pre-processor first, and just return E_NOTIMPL from assemble_shader().
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
Matteo Bruni matteo.myst...@gmail.com writes: + +%option reentrant bison-bridge These won't work on old flex versions, and will get you in trouble with the flex police (aka Michael Stefaniuc ;-) -- Alexandre Julliard julli...@winehq.org
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Henri Verbeet hverb...@gmail.com: 2009/12/28 Matteo Bruni matteo.myst...@gmail.com: Why do you need the fake parser? Can't you just not support those shader versions yet? There's also (in general) not much of a point in adding structure fields that aren't used yet. I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create__parser functions (and I prefer to use parser_fake for the unimplemented shader versions instead of parser_vs_3) and I want to avoid having some tests for the other shader versions pass by chance inside the todo_wine (this is accomplished by the asmparser_end_fake function... probably this can be seen as a hack). Agreed on the structure fields. +/* This file needs the original d3d9 definitions. The bwriter ones + * aren't useable because they are wine-internal things. We're writing + * d3d8/9 shaders here, so we need the d3d9 definitions (which are + * equal to the d3d8 ones) + */ This doesn't seem to match what the code actually does. That #include is only used from the next patch. I'll move the include and the comment in the right place (maybe rephrasing it somewhat). +/* Debug utility routines. Some are not reentrant, check asmutils.c */ Same as above. Yep, that's not true anymore, courtesy of wine_dbg_sprintf. As for splitting things up, I think it's ok to e.g. add the pre-processor first, and just return E_NOTIMPL from assemble_shader(). You mean a patch which adds only the first half of the D3DXAssembleShader implementation (returning just after the preprocessing)? That seems reasonable. Btw, ok for your other points also.
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Alexandre Julliard julli...@winehq.org: Matteo Bruni matteo.myst...@gmail.com writes: + +%option reentrant bison-bridge These won't work on old flex versions, and will get you in trouble with the flex police (aka Michael Stefaniuc ;-) Alexandre, now you have another person on your side against RHEL 5 and old flex versions :) Joking aside, that's not a big issue as wpp is not reentrant anyway and I'm using the mutex to allow a single call at a time to D3DXAssembleShader. Just a question: as now I have to keep the parser context in a global structure defined in asmshader.y, how should I give access to it from asmshader.l? Do you prefer to give visibility to the global struct from asmshader.l or to create a getter (like a struct asm_parser *asm_get_context(void) function)?
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Matteo Bruni matteo.myst...@gmail.com: I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create__parser functions (and I prefer to use parser_fake for the unimplemented shader versions instead of parser_vs_3) and I want to avoid having some tests for the other shader versions pass by chance inside the todo_wine (this is accomplished by the asmparser_end_fake function... probably this can be seen as a hack). Agreed on the structure fields. Can't you just let the parser fail when it encounters an unsupported shader version? That's more or less what happens anyway, but I don't think you have to wait until after parsing all the instructions for that.
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
On 12/29/2009 04:54 PM, Matteo Bruni wrote: 2009/12/29 Alexandre Julliardjulli...@winehq.org: Matteo Brunimatteo.myst...@gmail.com writes: + +%option reentrant bison-bridge These won't work on old flex versions, and will get you in trouble with the flex police (aka Michael Stefaniuc ;-) All that because the Wine maintainer puts a lot of emphasis on portability ... Alexandre, now you have another person on your side against RHEL 5 and old flex versions :) I have just submitted a patch for configure.ac to require a newer flex version to build Wine. People on RHEL5 can just rebuild a SRPM from Fedora (those from F7 to F10 are just a rpmbuild --rebuild). Now the Wine maintainer needs to just accept that patch ;) bye michael
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Henri Verbeet hverb...@gmail.com: 2009/12/29 Matteo Bruni matteo.myst...@gmail.com: I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create__parser functions (and I prefer to use parser_fake for the unimplemented shader versions instead of parser_vs_3) and I want to avoid having some tests for the other shader versions pass by chance inside the todo_wine (this is accomplished by the asmparser_end_fake function... probably this can be seen as a hack). Agreed on the structure fields. Can't you just let the parser fail when it encounters an unsupported shader version? That's more or less what happens anyway, but I don't think you have to wait until after parsing all the instructions for that. Yep, you are right. I believe previously I had found no way to halt the parser early, so I simply let it go until the end and I needed the fake backend to cope with that. Seems like the YYABORT macro does just what I need...
