From: Conor McCarthy <conor.mccarthy.444@gmail.com>
Subject: Re: [PATCH vkd3d 00/11] vkd3d-shader: Fix patch constant handling in hull shaders.
Message-Id: <CAOua6--vaBg86fyFBbDGtXCp_ZeVS4Og_5Zm+WzM50w1Ex1VfA@mail.gmail.com>
Date: Fri, 18 Oct 2019 23:59:21 +1000
In-Reply-To: <20191016195544.23720-1-philip.rebohle@tu-dortmund.de>
References: <20191016195544.23720-1-philip.rebohle@tu-dortmund.de>

Hi Philip,

Is the descriptor update issue expected to crash the game? Using these
patches and tessellation I get a hard crash requiring reboot at the start
of benchmark scene 2. Mesa 19.2.1 and AMD RX 580.

On Thu, Oct 17, 2019 at 5:57 AM Philip Rebohle <
philip.rebohle@tu-dortmund.de> wrote:

>
> This series mainly changes the way patch constant outputs are generated
> in hull shaders to always use a private array variable. This is done to
> fix several issues in the current imlementation, and to avoid excessive
> complexity in dealing with patch constants which stem from DXBC's design:
>
> 1) Tessellation factors and regular patch constants share the same
>    register space in DXBC and can both be dynamically or statically
>    indexed. The current implementation only works correctly in case
>    tessellation factors do not share registers with regular outputs,
>    and does not support all indexing patterns, leading to invalid
>    code being generated in some cases.
>
> 2) Join phases can read patch constants written by a fork phase,
>    also with dynamic indexing. However, the index ranges used by
>    the join phase are not known at the time the fork phase is
>    compiled.
>
> With these changes, tessellation shaders work correctly in Shadow of
> the Tomb Raider, which previously contained invalid SPIR-V and would
> crash the game if enabled.
>
> Note that enabling the tessellation option in the game triggers a
> separate issue related to descriptor updates, which will have to
> be addressed separately.
>
>
>

<div dir="ltr"><div>Hi Philip,</div><div><br></div><div>Is the descriptor update issue expected to crash the game? Using these patches and tessellation I get a hard crash requiring reboot at the start of benchmark scene 2. Mesa 19.2.1 and AMD RX 580.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 17, 2019 at 5:57 AM Philip Rebohle &lt;<a href="mailto:philip.rebohle@tu-dortmund.de">philip.rebohle@tu-dortmund.de</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
This series mainly changes the way patch constant outputs are generated<br>
in hull shaders to always use a private array variable. This is done to<br>
fix several issues in the current imlementation, and to avoid excessive<br>
complexity in dealing with patch constants which stem from DXBC&#39;s design:<br>
<br>
1) Tessellation factors and regular patch constants share the same<br>
   register space in DXBC and can both be dynamically or statically<br>
   indexed. The current implementation only works correctly in case<br>
   tessellation factors do not share registers with regular outputs,<br>
   and does not support all indexing patterns, leading to invalid<br>
   code being generated in some cases.<br>
<br>
2) Join phases can read patch constants written by a fork phase,<br>
   also with dynamic indexing. However, the index ranges used by<br>
   the join phase are not known at the time the fork phase is<br>
   compiled.<br>
<br>
With these changes, tessellation shaders work correctly in Shadow of<br>
the Tomb Raider, which previously contained invalid SPIR-V and would<br>
crash the game if enabled.<br>
<br>
Note that enabling the tessellation option in the game triggers a<br>
separate issue related to descriptor updates, which will have to<br>
be addressed separately.<br>
<br>
<br>
</blockquote></div>