Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/28 Matteo Bruni <matteo.mystral(a)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().
2009/12/29 Henri Verbeet <hverbeet(a)gmail.com>:
2009/12/28 Matteo Bruni <matteo.mystral(a)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_xxxx_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.
2009/12/29 Matteo Bruni <matteo.mystral(a)gmail.com>:
I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create_xxxx_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.
2009/12/29 Henri Verbeet <hverbeet(a)gmail.com>:
2009/12/29 Matteo Bruni <matteo.mystral(a)gmail.com>:
I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create_xxxx_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...
participants (2)
-
Henri Verbeet -
Matteo Bruni