[PATCH 4/5] d3dcompiler: Always use vkd3d-shader's preprocessor.

[Zebediah Figura]

On 3/28/22 10:11, Matteo Bruni wrote:
> On Mon, Mar 28, 2022 at 5:09 PM Matteo Bruni <matteo.mystral at gmail.com> wrote:
>>
>> On Mon, Mar 28, 2022 at 4:59 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
>>>
>>> On 3/28/22 07:12, Matteo Bruni wrote:
>>>> diff --git a/dlls/d3dx9_36/tests/asm.c b/dlls/d3dx9_36/tests/asm.c
>>>> index 9b4e82f218a..bfc84b2f9c2 100644
>>>> --- a/dlls/d3dx9_36/tests/asm.c
>>>> +++ b/dlls/d3dx9_36/tests/asm.c
>>>> @@ -259,7 +259,7 @@ static void assembleshader_test(void)
>>>>        messages = NULL;
>>>>        hr = D3DXAssembleShader(testshader2, strlen(testshader2), NULL, &include.ID3DXInclude_iface,
>>>>                                D3DXSHADER_SKIPVALIDATION, &shader, &messages);
>>>> -    ok(hr == D3D_OK, "D3DXAssembleShader test failed with error 0x%x - %d\n", hr, hr & 0x0000FFFF);
>>>> +    todo_wine ok(hr == D3D_OK, "D3DXAssembleShader test failed with error 0x%x - %d\n", hr, hr & 0x0000FFFF);
>>>>        if(messages) {
>>>>            trace("recursive D3DXAssembleShader messages:\n%s", (char *)ID3DXBuffer_GetBufferPointer(messages));
>>>>            ID3DXBuffer_Release(messages);
>>>
>>> What's the cause of this failure?
>>
>> There is no define for "REGISTER" there so it ends up unchanged in the
>> preprocessed source. Somehow native doesn't flinch and returns S_OK
>> anyway (IIRC the output shader has no actual instructions, just the
>> version token and the end token)...
> 
> Native D3DXAssembleShader(), that is.

I wasn't quite satisfied with this explanation, so I looked deeper into it.

It turns out that including the null terminator in the included file 
(i.e. using sizeof() instead of strlen()) causes it to be emitted in the 
output. The preprocessor will continue to parse directives after that, 
but the assembler won't. So the last statement just gets ignored. It 
also means that a lot of the other tests in this file are a lot less 
interesting than they look.

vkd3d-shader behaves incorrectly in this respect. That's probably not 
worth blocking the patch, though...