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

[Matteo Bruni]

On Mon, Mar 28, 2022 at 8:16 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
>
> On 3/28/22 13:12, Matteo Bruni wrote:
> > On Mon, Mar 28, 2022 at 7:20 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
> >>
> >> 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...
> >
> > Thanks for investigating deeper, in hindsight (always 20/20) it makes
> > a lot of sense. I'll write some followup patches, unless you want to
> > take care of it.
>
> I did already start testing the exact behaviour regarding null
> terminators in vkd3d, but I hadn't written anything for Wine or d3dx9 yet.

Okay, I guess I'll write something at least for the d3dx9 tests (maybe
just adding a couple of comments pointing out what's actually being
tested).