Hi,
I'm writing because I found a bug in asm shader preprocessor in Wine and I'm not sure what's the best way to fix it.
The D3DPreprocess function is an universal shader preprocess for HLSL and asm shaders. In fact, it's C-like language preprocessor and it doesn't understand assembler style comments (comments from ';' to the end of line). The fact that it doesn't understand these comments is a root of whole problem. Valid asm shaders can contain single or double quotation characters in assembler style comments, and these quotation characters will be treated as a beginning of quoted string constants while preprocessing.
The Wine's implementation of D3DPreprocess behaves differently than Windows implementation in respect to strings passing the end of line. Generally, strings passing the end of line are an error in C, but they can result in a correct asm shader when contained in a assembler style comment. D3DPreprocess on Windows works fine for shaders which contains single or double quotation marks. It only issues an error in case of unmatched double quotation mark, but returns D3D_OK anyway. In Wine situation is different, wpp tries to find a matching quotation mark to the end of file, while shader preprocessor on Windows gives up on parsing a string when it gets a first newline character. The result of wpp's behavior is a truncated output and an error returned from D3DPreprocess.
I wonder if it's fine to change wpp behavior for strings passing the end of line to mimic the behavior observed on Windows. Perhaps, strings passing the end of line could be treated as an error by default. Whereas D3DCompiler would activate an option to treat such strings as a warning.
Thanks, Józef Kucia
Am Montag, 12. März 2012, 22:13:45 schrieb Józef Kucia:
I wonder if it's fine to change wpp behavior for strings passing the end of line to mimic the behavior observed on Windows. Perhaps, strings passing the end of line could be treated as an error by default. Whereas D3DCompiler would activate an option to treat such strings as a warning.
My first guess would be to see how other users of wpp are behaving, or are supposed to behave. grep claims that the other parts of Wine that use wpp are toold/widl and tools/wrc. Beyond those it's probably a good idea to keep HLSL in mind, even if we don't have a HLSL compiler yet.
What I don't know is if/how you can write proper tests for widl and wrt. They have counterparts on Windows(midl and windres afaik), but they live in tools/ and not programs/ in the Wine source.
On Mon, Mar 12, 2012 at 11:31 PM, Stefan Dösinger wrote:
My first guess would be to see how other users of wpp are behaving, or are supposed to behave. grep claims that the other parts of Wine that use wpp are toold/widl and tools/wrc. Beyond those it's probably a good idea to keep HLSL in mind, even if we don't have a HLSL compiler yet.
I'll make some tests to check how wrc and widl should behave. However I'm not sure if submitting tests for wrc and widl would be welcome. We don't have any tests for these tools yet and I can imagine not everyone, who's running the tests on Windows, has Visual Studio or SDK installed. Perhaps such tests could rely on presence of these tools in PATH, and would be skipped in case of their absence. Does it make any sense?
Am Dienstag, 13. März 2012, 00:52:58 schrieb Józef Kucia:
I'll make some tests to check how wrc and widl should behave. However I'm not sure if submitting tests for wrc and widl would be welcome. We don't have any tests for these tools yet and I can imagine not everyone, who's running the tests on Windows, has Visual Studio or SDK installed. Perhaps such tests could rely on presence of these tools in PATH, and would be skipped in case of their absence. Does it make any sense?
Yes, skipping tests if the tested application/library isn't there is the way to go. The test framework already takes care of this for libraries, not sure about programs. What I'm more worried about is the build system, and that widl and wrc have a different name than their Windows counterparts. Maybe someone else knows if and how those can be tested. Maybe we just don't care about 1:1 compatibility for widl and wrc and it's ok to adjust wpp to make d3dx9 happy.
Il 12 marzo 2012 22:13, Józef Kucia joseph.kucia@gmail.com ha scritto:
Hi,
I'm writing because I found a bug in asm shader preprocessor in Wine and I'm not sure what's the best way to fix it.
The D3DPreprocess function is an universal shader preprocess for HLSL and asm shaders. In fact, it's C-like language preprocessor and it doesn't understand assembler style comments (comments from ';' to the end of line). The fact that it doesn't understand these comments is a root of whole problem. Valid asm shaders can contain single or double quotation characters in assembler style comments, and these quotation characters will be treated as a beginning of quoted string constants while preprocessing.
The Wine's implementation of D3DPreprocess behaves differently than Windows implementation in respect to strings passing the end of line. Generally, strings passing the end of line are an error in C, but they can result in a correct asm shader when contained in a assembler style comment. D3DPreprocess on Windows works fine for shaders which contains single or double quotation marks. It only issues an error in case of unmatched double quotation mark, but returns D3D_OK anyway. In Wine situation is different, wpp tries to find a matching quotation mark to the end of file, while shader preprocessor on Windows gives up on parsing a string when it gets a first newline character. The result of wpp's behavior is a truncated output and an error returned from D3DPreprocess.
I wonder if it's fine to change wpp behavior for strings passing the end of line to mimic the behavior observed on Windows. Perhaps, strings passing the end of line could be treated as an error by default. Whereas D3DCompiler would activate an option to treat such strings as a warning.
Thanks, Józef Kucia
That is bug http://bugs.winehq.org/show_bug.cgi?id=24614. My shot at it was http://www.winehq.org/pipermail/wine-patches/2011-February/098609.html. You can see that my approach was essentially what you are suggesting. Still, that's not a very nice thing to do probably (and indeed my patch wasn't accepted at the time ;). Rereading it now, I notice that the patch also contains some wpp cleanup which should have been in separate patches, but I don't think that it was the reason of the reject... Dan tried a slightly different solution a while later, but still no dice.
I'm unsure how a "correct" solution would be. The tests included in my patch essentially show that this asm-style comments thing is a PITA even for native d3dcompiler, so probably there can't be a really clean and nice solution. Also, some ad-hoc changes to wpp are needed anyway and have to "kick in" only when wpp is used by d3dcompiler, without affecting the other users.
I'll think about it again as soon as I'll get some time. If you get any idea, I'd like to hear it (or, writing a good fix would be even better ...)
On Tue, Mar 13, 2012 at 1:22 PM, Matteo Bruni wrote:
That is bug http://bugs.winehq.org/show_bug.cgi?id=24614. My shot at it was http://www.winehq.org/pipermail/wine-patches/2011-February/098609.html. You can see that my approach was essentially what you are suggesting. Still, that's not a very nice thing to do probably (and indeed my patch wasn't accepted at the time ;). Rereading it now, I notice that the patch also contains some wpp cleanup which should have been in separate patches, but I don't think that it was the reason of the reject... Dan tried a slightly different solution a while later, but still no dice.
I'll do a better search before posting to the mailing list next time. Actually I found this bug in irrEdit CooperCube.
I'll think about it again as soon as I'll get some time. If you get any idea, I'd like to hear it (or, writing a good fix would be even better ...)
In the attachment is my fix. It won't harm widl nor wrc. However, I found out that Wine's wrc isn't compatible with Microsoft Windows Resource Compiler. In the second attachment there is an example of a file, which RC.exe compiles without even issuing warning, and wrc fails to compile it with "Unterminated string" error message. Anyway wpp will preprocess this file correctly, which can be checked by running wrc with -E option. If it's an issue, it should be fixed in wrc parser.
-
Józef Kucia -
Matteo Bruni -
Stefan Dösinger