[PATCH vkd3d] vkd3d: Do not report a root signature version higher than requested.

[Rémi Bernon]

This fixes Shadow of the Tomb Raider crashing because of NULL root
signatures being passed since c002aee119b638d30eeb7cdc91099449ccafeafc.

Signed-off-by: Rémi Bernon 
---
 libs/vkd3d/device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
index d30c447c..30cee112 100644
--- a/libs/vkd3d/device.c
+++ b/libs/vkd3d/device.c
@@ -2512,7 +2512,8 @@ static HRESULT STDMETHODCALLTYPE d3d12_device_CheckFeatureSupport(ID3D12Device *
                 return E_INVALIDARG;
             }

-            data->HighestVersion = D3D_ROOT_SIGNATURE_VERSION_1_1;
+            TRACE("Root signature requested %#x.\n", data->HighestVersion);
+            data->HighestVersion = min(data->HighestVersion, D3D_ROOT_SIGNATURE_VERSION_1_1);

TRACE("Root signature version %#x.\n", data->HighestVersion);
             return S_OK;
--
2.24.0.rc0

[Sveinar Søpler]

This patch, aswell as the patch from 
https://www.winehq.org/pipermail/wine-devel/2019-October/152984.html 
gives this error with World of Warcraft:

trace:d3d12_device_CheckFeatureSupport: Root signature requested 0x2.
trace:d3d12_device_CheckFeatureSupport: Root signature version 0x2.
trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 0x7, 
feature_data 0x22f608, feature_data_size 4.
trace:d3d12_device_CheckFeatureSupport: Request shader model 0x64.
trace:d3d12_device_CheckFeatureSupport: Shader model 0x51.
trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 0x15, 
feature_data 0x22f4a0, feature_data_size 20.
fixme:d3d12_device_CheckFeatureSupport: No support for any 
D3D12_OPTIONS3 optional features.

I guess WoW perhaps requests root signature 1.1, but it is not really 
supported?

Reverting c002aee119b638d30eeb7cdc91099449ccafeafc works, and i guess 
forces 1.0 no matter what is requested. Perhaps something else is 
missing with proper root sig 1.1 support?

Sveinar

On 25.10.2019 14:55, Rémi Bernon wrote:
> This fixes Shadow of the Tomb Raider crashing because of NULL root
> signatures being passed since c002aee119b638d30eeb7cdc91099449ccafeafc.
> Signed-off-by: Rémi Bernon
>
> libs/vkd3d/device.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
> index d30c447c..30cee112 100644
> --- a/libs/vkd3d/device.c
> +++ b/libs/vkd3d/device.c
> @@ -2512,7 +2512,8 @@ static HRESULT STDMETHODCALLTYPE d3d12_device_CheckFeatureSupport(ID3D12Device *
>                   return E_INVALIDARG;
>               }
> -
>        data->HighestVersion = D3D_ROOT_SIGNATURE_VERSION_1_1;
> -
>        TRACE("Root signature requested %#x.\n", data->HighestVersion);
> -
>        data->HighestVersion = min(data->HighestVersion, D3D_ROOT_SIGNATURE_VERSION_1_1);
>
>         TRACE("Root signature version %#x.\n", data->HighestVersion);
>         return S_OK;
>
> --
> 2.24.0.rc0

[Hans-Kristian Arntzen]

It does indeed sound like something is off with the RS 1.1 support, 
however, it's not clear to me what is going wrong or what the app is 
doing differently if it detects 1.1. As far as I know, the only 
difference with RS 1.1 is the addition of descriptor/data volatility 
information.

I didn't hit any issues with RS 1.1 on SOTTR when I tried it, fwiw.

Cheers,
Hans-Kristian

On 10/25/19 4:26 PM, Sveinar Søpler wrote:
> This patch, aswell as the patch from 
> https://www.winehq.org/pipermail/wine-devel/2019-October/152984.html 
> gives this error with World of Warcraft:
>
> trace:d3d12_device_CheckFeatureSupport: Root signature requested 0x2.
> trace:d3d12_device_CheckFeatureSupport: Root signature version 0x2.
> trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 0x7, 
> feature_data 0x22f608, feature_data_size 4.
> trace:d3d12_device_CheckFeatureSupport: Request shader model 0x64.
> trace:d3d12_device_CheckFeatureSupport: Shader model 0x51.
> trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 
> 0x15, feature_data 0x22f4a0, feature_data_size 20.
> fixme:d3d12_device_CheckFeatureSupport: No support for any 
> D3D12_OPTIONS3 optional features.
>
> I guess WoW perhaps requests root signature 1.1, but it is not really 
> supported?
>
> Reverting c002aee119b638d30eeb7cdc91099449ccafeafc works, and i guess 
> forces 1.0 no matter what is requested. Perhaps something else is 
> missing with proper root sig 1.1 support?
>
> Sveinar
>
> On 25.10.2019 14:55, Rémi Bernon wrote:
> > This fixes Shadow of the Tomb Raider crashing because of NULL root
> > signatures being passed since c002aee119b638d30eeb7cdc91099449ccafeafc.
> > Signed-off-by: Rémi Bernon
> >
> > libs/vkd3d/device.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
> > index d30c447c..30cee112 100644
> > --- a/libs/vkd3d/device.c
> > +++ b/libs/vkd3d/device.c
> > @@ -2512,7 +2512,8 @@ static HRESULT STDMETHODCALLTYPE 
> > d3d12_device_CheckFeatureSupport(ID3D12Device *
> >                   return E_INVALIDARG;
> >               }
> >
> > -            data->HighestVersion = D3D_ROOT_SIGNATURE_VERSION_1_1;
> > +            TRACE("Root signature requested %#x.\n", 
> > data->HighestVersion);
> > +            data->HighestVersion = min(data->HighestVersion, 
> > D3D_ROOT_SIGNATURE_VERSION_1_1);
> >
> > TRACE("Root signature version %#x.\n", 
> > data->HighestVersion);
> >               return S_OK;
> > -- 
> > 2.24.0.rc0

[Rémi Bernon]

On 10/25/19 5:14 PM, Hans-Kristian Arntzen wrote:
> It does indeed sound like something is off with the RS 1.1 support, 
> however, it's not clear to me what is going wrong or what the app is 
> doing differently if it detects 1.1. As far as I know, the only 
> difference with RS 1.1 is the addition of descriptor/data volatility 
> information.
>
> I didn't hit any issues with RS 1.1 on SOTTR when I tried it, fwiw.
>
> Cheers,
> Hans-Kristian

I clearly have a crash on SOTTR with NVIDIA GPU. The game requests root 
signature 1.0 and we reply with 1.1, it then skips a bunch of 
CreateRootSignature calls that I can see when we return 1.0 only.

Then I have a couple of messages like:

warn:d3d12_pipeline_state_init_graphics:Root signature is NULL.

And quickly followed by a crash because of the assert in:

libs/vkd3d/command.c:3973

which tries to access root_signature->parameters with a NULL pointer.

MSDN says that this feature request should not reply with a higher value 
than requested. I didn't feel like it required a test, but I can add one.

[Hans-Kristian Arntzen]

On 10/25/19 5:22 PM, Rémi Bernon wrote:
> On 10/25/19 5:14 PM, Hans-Kristian Arntzen wrote:
> > It does indeed sound like something is off with the RS 1.1 support, 
> > however, it's not clear to me what is going wrong or what the app is 
> > doing differently if it detects 1.1. As far as I know, the only 
> > difference with RS 1.1 is the addition of descriptor/data volatility 
> > information.
> >
> > I didn't hit any issues with RS 1.1 on SOTTR when I tried it, fwiw.
> >
> > Cheers,
> > Hans-Kristian
>
> I clearly have a crash on SOTTR with NVIDIA GPU. The game requests 
> root signature 1.0 and we reply with 1.1, it then skips a bunch of 
> CreateRootSignature calls that I can see when we return 1.0 only.
>
> Then I have a couple of messages like:
>
> warn:d3d12_pipeline_state_init_graphics:Root signature is NULL.
>
> And quickly followed by a crash because of the assert in:
>
> libs/vkd3d/command.c:3973
>
> which tries to access root_signature->parameters with a NULL pointer.
>
> MSDN says that this feature request should not reply with a higher 
> value than requested. I didn't feel like it required a test, but I can 
> add one.

Right, CheckFeatureSupport does specify it returns minimum version based 
on input. I missed that.

https://docs.microsoft.com/en-us/windows/win32/api/d3d12/ns-d3d12-d3d12_feat...

Cheers,
Hans-Kristian

[Henri Verbeet]

On Fri, 25 Oct 2019 at 18:52, Rémi Bernon  wrote:
> MSDN says that this feature request should not reply with a higher value
> than requested. I didn't feel like it required a test, but I can add one.

I'd be surprised if a test showed anything other than that this is
clearly the right thing to do, but it should also be easy enough to
add that it's worth it.

[Rémi Bernon]

On 10/25/19 4:26 PM, Sveinar Søpler wrote:
> This patch, aswell as the patch from 
> https://www.winehq.org/pipermail/wine-devel/2019-October/152984.html 
> gives this error with World of Warcraft:
>
> trace:d3d12_device_CheckFeatureSupport: Root signature requested 0x2.
> trace:d3d12_device_CheckFeatureSupport: Root signature version 0x2.
> trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 0x7, 
> feature_data 0x22f608, feature_data_size 4.
> trace:d3d12_device_CheckFeatureSupport: Request shader model 0x64.
> trace:d3d12_device_CheckFeatureSupport: Shader model 0x51.
> trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 0x15, 
> feature_data 0x22f4a0, feature_data_size 20.
> fixme:d3d12_device_CheckFeatureSupport: No support for any 
> D3D12_OPTIONS3 optional features.
>
> I guess WoW perhaps requests root signature 1.1, but it is not really 
> supported?
>
> Reverting c002aee119b638d30eeb7cdc91099449ccafeafc works, and i guess 
> forces 1.0 no matter what is requested. Perhaps something else is 
> missing with proper root sig 1.1 support?
>
> Sveinar

After testing a bit it looks like you're just going to need this wine 
patch as well:

https://source.winehq.org/patches/data/171377

With it I was able to run WoW on wine-staging and the latest vkd3d (and 
eventually my patch but it doesn't change anything).

[Sveinar Søpler]

Yep, that seems to fix things for World of Warcraft

--

trace:d3d12_device_CheckFeatureSupport: Root signature requested 0x2.
trace:d3d12_device_CheckFeatureSupport: Root signature version 0x2.
trace:vkd3d_serialize_versioned_root_signature: desc 0x22f6b0, blob 
0x22f9d8, error_blob 0x22f660.
trace:d3d12_device_CreateRootSignature: iface 0x7cf17580, node_mask 
0x00000000, bytecode 0x7d457db0, bytecode_length 672, riid 
{c54a6b66-72df-4ee8-8be5-a946a1429214}, root_signature 0x412f238.
trace:d3d12_root_signature_create: Created root signature 0x7cee93f0.
trace:vkd3d_serialize_versioned_root_signature: desc 0x22f6b0, blob 
0x22f9d8, error_blob 0x22f660.
trace:d3d12_device_CreateRootSignature: iface 0x7cf17580, node_mask 
0x00000000, bytecode 0x7d298af0, bytecode_length 272, riid 
{c54a6b66-72df-4ee8-8be5-a946a1429214}, root_signature 0x412f240.
trace:d3d12_root_signature_create: Created root signature 0x7cc419d0.
trace:d3d12_device_CreateCommandSignature: iface 0x7cf17580, desc 
0x22f648, root_signature (nil), iid 
{c36a797c-ec80-4f0a-8985-a7b2475082d1}, command_signature 0x412f248.
trace:d3d12_command_signature_create: Created command signature 0x7d447e20.
trace:d3d12_device_CreateCommandSignature: iface 0x7cf17580, desc 
0x22f648, root_signature (nil), iid 
{c36a797c-ec80-4f0a-8985-a7b2475082d1}, command_signature 0x412f250.
trace:d3d12_command_signature_create: Created command signature 0x7d065150.
--

I used both these for wine-staging-4.18:

https://source.winehq.org/patches/data/171377

https://source.winehq.org/patches/data/171378

Aswell as the proposed patch in this thread, and the 
0001-vkd3d-Support-D3D12_FEATURE_D3D12_OPTIONS3.patch for vkd3d posted 
https://www.winehq.org/pipermail/wine-devel/attachments/20191023/7f53c2b0/at...

Sveinar

On 25.10.2019 20:12, Rémi Bernon wrote:
> On 10/25/19 4:26 PM, Sveinar Søpler wrote:
> > This patch, aswell as the patch from 
> > https://www.winehq.org/pipermail/wine-devel/2019-October/152984.html 
> > gives this error with World of Warcraft:
> >
> > trace:d3d12_device_CheckFeatureSupport: Root signature requested 0x2.
> > trace:d3d12_device_CheckFeatureSupport: Root signature version 0x2.
> > trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 
> > 0x7, feature_data 0x22f608, feature_data_size 4.
> > trace:d3d12_device_CheckFeatureSupport: Request shader model 0x64.
> > trace:d3d12_device_CheckFeatureSupport: Shader model 0x51.
> > trace:d3d12_device_CheckFeatureSupport: iface 0x7dc9eea0, feature 
> > 0x15, feature_data 0x22f4a0, feature_data_size 20.
> > fixme:d3d12_device_CheckFeatureSupport: No support for any 
> > D3D12_OPTIONS3 optional features.
> >
> > I guess WoW perhaps requests root signature 1.1, but it is not really 
> > supported?
> >
> > Reverting c002aee119b638d30eeb7cdc91099449ccafeafc works, and i guess 
> > forces 1.0 no matter what is requested. Perhaps something else is 
> > missing with proper root sig 1.1 support?
> >
> > Sveinar
>
> After testing a bit it looks like you're just going to need this wine 
> patch as well:
>
> https://source.winehq.org/patches/data/171377
>
> With it I was able to run WoW on wine-staging and the latest vkd3d 
> (and eventually my patch but it doesn't change anything).