[PATCH 2/4] d3dx9: Pass the "flags" argument to D3DXFXCreateEffectEx() as compiler flags, not effect flags.

[Matteo Bruni]

On Fri, Feb 11, 2022 at 5:00 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
>
> On 2/9/22 05:54, Matteo Bruni wrote:
> > On Tue, Feb 8, 2022 at 12:42 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
> >>
> >> And strip D3DXFX_* creation flags from them.
> >>
> >> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> >> ---
> >>   dlls/d3dx9_36/effect.c | 11 ++++++-----
> >>   1 file changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> >> index e623c10035a..b8c957245a6 100644
> >> --- a/dlls/d3dx9_36/effect.c
> >> +++ b/dlls/d3dx9_36/effect.c
> >> @@ -6393,7 +6393,7 @@ static const char **parse_skip_constants_string(char *skip_constants_string, uns
> >>
> >>   static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device,
> >>           const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include,
> >> -        UINT eflags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string)
> >> +        UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string)
> >>   {
> >>   #if D3DX_SDK_VERSION <= 36
> >>       UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY;
> >> @@ -6409,9 +6409,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
> >>       unsigned int i, j;
> >>       HRESULT hr;
> >>
> >> -    TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, eflags %#x, errors %p, "
> >> +    TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, "
> >>               "pool %p, skip_constants %s.\n",
> >> -            effect, device, data, data_size, defines, include, eflags, errors, pool,
> >> +            effect, device, data, data_size, defines, include, flags, errors, pool,
> >>               debugstr_a(skip_constants_string));
> >>
> >>       effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl;
> >> @@ -6426,7 +6426,7 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
> >>       IDirect3DDevice9_AddRef(device);
> >>       effect->device = device;
> >>
> >> -    effect->flags = eflags;
> >> +    effect->flags = flags;
> >>
> >>       list_init(&effect->parameter_block_list);
> >>
> >> @@ -6436,8 +6436,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
> >>       if (tag != d3dx9_effect_version(9, 1))
> >>       {
> >>           TRACE("HLSL ASCII effect, trying to compile it.\n");
> >> +        compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE);
> >
> > The D3DXFX_DONOTSAVE[*]STATE flags might also need to be filtered out.
> > They conflict with D3DXSHADER_DEBUG, D3DXSHADER_SKIPVALIDATION and
> > D3DXSHADER_SKIPOPTIMIZATION (and their D3DCOMPILE_ counterparts). It
> > might be possible to verify the expected behavior for those, or at
> > least for the validation flag.
>
> Do you know offhand how to test the validation flag? I don't know
> immediately any way to write an HLSL shader that will compile if and
> only if that flag is set.

I thought that defining too many constants would be a quick and easy
way to do that but it doesn't seem to be the case, so no, not
really...

> FWIW, the documentation does state that those flags are passed to
> ID3DXEffect::Begin(), not to creation functions [1]. Not that the
> documentation is always trustworthy, of course, but it is the reasonable
> behaviour.
>
> [1] https://docs.microsoft.com/en-us/windows/win32/direct3d9/d3dxfx

Oh, right, of course. That means there shouldn't be any actual
conflict with the D3DXSHADER_ / D3DXCOMPILE_ options. Given that and
the above, I guess checking that passing those flags to
D3DXCreateEffect() doesn't cause any immediate error might be good
enough.