Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Andres Gomez]

On Sun, 2017-07-09 at 14:21 +0200, Marek Olšák wrote:
> On Sat, Jul 8, 2017 at 3:56 PM, Andres Gomez  wrote:
> > Marek, is it worth the effort of trying to bring this series into
> > -stable?
> 
> Probably not. The incorrect rendering in Rocket League isn't
> noticeable if you don't know what to look for.

OK, thanks! ☺

-- 
Br,

Andres
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Marek Olšák]

On Sat, Jul 8, 2017 at 3:56 PM, Andres Gomez  wrote:
> Marek, is it worth the effort of trying to bring this series into
> -stable?

Probably not. The incorrect rendering in Rocket League isn't
noticeable if you don't know what to look for.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Andres Gomez]

Marek, is it worth the effort of trying to bring this series into
-stable?

On Wed, 2017-06-21 at 00:54 +0200, Marek Olšák wrote:
> Hi,
> 
> This series updates pipe loaders so that flags such as drirc options
> can be passed to create_screen(). I have compile-tested everything
> except clover.
> 
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag forces
> radeonsi to use the DX behavior for that game.
> 
> Please review.
> 
> Thanks,
> Marek
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-- 
Br,

Andres
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Ilia Mirkin]

On Thu, Jun 22, 2017 at 2:53 PM, Roland Scheidegger  wrote:
> Am 22.06.2017 um 19:35 schrieb Ilia Mirkin:
>> On Wed, Jun 21, 2017 at 9:58 AM, Ilia Mirkin  wrote:
>>> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
 The first pipe_screen flag is a drirc option to fix incorrect grass
 rendering in Rocket League for radeonsi. Rocket League expects DirectX
 behavior for partial derivative computations after discard/kill, but
 radeonsi implements the more efficient but stricter OpenGL behavior
 and that will remain our default behavior. The new screen flag forces
 radeonsi to use the DX behavior for that game.
>>>
>>> For those of us following along at home... can you provide a brief
>>> reminder of what the DX behavior is, and how does it differ from GL
>>> behavior? In case one might want to fix this for nouveau (if a fix is
>>> needed at all)...
>>
>> From what I gather, this is the difference between discard exiting
>> immediately in that thread and keeping going.
>>
>> Does this make sense to be surfaced in gallium via a tgsi program
>> property? That way, e.g. a GL extension could be written to expose
>> that program property.
>
> I suppose it would make sense, that way this could be exposed cleanly.
> But of course the apps would have to use the extension, otherwise the
> override is just in a different place (and in theory not just glsl,
> vulkan works the same too).

Right... I was thinking Wine. Or some porting company.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Roland Scheidegger]

Am 22.06.2017 um 19:35 schrieb Ilia Mirkin:
> On Wed, Jun 21, 2017 at 9:58 AM, Ilia Mirkin  wrote:
>> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
>>> The first pipe_screen flag is a drirc option to fix incorrect grass
>>> rendering in Rocket League for radeonsi. Rocket League expects DirectX
>>> behavior for partial derivative computations after discard/kill, but
>>> radeonsi implements the more efficient but stricter OpenGL behavior
>>> and that will remain our default behavior. The new screen flag forces
>>> radeonsi to use the DX behavior for that game.
>>
>> For those of us following along at home... can you provide a brief
>> reminder of what the DX behavior is, and how does it differ from GL
>> behavior? In case one might want to fix this for nouveau (if a fix is
>> needed at all)...
> 
> From what I gather, this is the difference between discard exiting
> immediately in that thread and keeping going.
> 
> Does this make sense to be surfaced in gallium via a tgsi program
> property? That way, e.g. a GL extension could be written to expose
> that program property.

I suppose it would make sense, that way this could be exposed cleanly.
But of course the apps would have to use the extension, otherwise the
override is just in a different place (and in theory not just glsl,
vulkan works the same too).

Roland



>   -ilia
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=DtcJeXTTZaceLSUoEtaDPZx10-oO7zRC9aAB6JCRK4s=ziGPeqqnGOWrPY08hnA0n14mdaI0AGGGThU2PsrBR7I=
>  
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Ilia Mirkin]

On Wed, Jun 21, 2017 at 9:58 AM, Ilia Mirkin  wrote:
> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
>> The first pipe_screen flag is a drirc option to fix incorrect grass
>> rendering in Rocket League for radeonsi. Rocket League expects DirectX
>> behavior for partial derivative computations after discard/kill, but
>> radeonsi implements the more efficient but stricter OpenGL behavior
>> and that will remain our default behavior. The new screen flag forces
>> radeonsi to use the DX behavior for that game.
>
> For those of us following along at home... can you provide a brief
> reminder of what the DX behavior is, and how does it differ from GL
> behavior? In case one might want to fix this for nouveau (if a fix is
> needed at all)...

From what I gather, this is the difference between discard exiting
immediately in that thread and keeping going.

Does this make sense to be surfaced in gallium via a tgsi program
property? That way, e.g. a GL extension could be written to expose
that program property.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Roland Scheidegger]

Am 22.06.2017 um 18:22 schrieb Marek Olšák:
> On Thu, Jun 22, 2017 at 6:13 PM, Alex Smith  
> wrote:
>> On 22 June 2017 at 15:52, Roland Scheidegger  wrote:
>>> Am 22.06.2017 um 13:09 schrieb Nicolai Hähnle:
 On 22.06.2017 10:14, Michel Dänzer wrote:
> On 22/06/17 04:34 PM, Nicolai Hähnle wrote:
>> On 22.06.2017 03:38, Rob Clark wrote:
>>> On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:
 On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark 
 wrote:
> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák 
> wrote:
>> Hi,
>>
>> This series updates pipe loaders so that flags such as drirc options
>> can be passed to create_screen(). I have compile-tested everything
>> except clover.
>>
>> The first pipe_screen flag is a drirc option to fix incorrect grass
>> rendering in Rocket League for radeonsi. Rocket League expects
>> DirectX
>> behavior for partial derivative computations after discard/kill, but
>> radeonsi implements the more efficient but stricter OpenGL behavior
>> and that will remain our default behavior. The new screen flag
>> forces
>> radeonsi to use the DX behavior for that game.
>>
>
> do we really want this to be a *global* option for the screen?

 Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
 compiler option also has to be global. We can't look at the context
 during the TGSI->LLVM translation.
>>>
>>> well, I didn't really mean per-screen vs per-context, as much as
>>> per-screen vs per-shader (or maybe more per-screen vs
>>> per-instruction?)
>>
>> I honestly don't think it's worth the trouble. Applications that are
>> properly coded against GLSL can benefit from the relaxed semantics, and
>> applications that get it wrong in one shader are rather likely to get it
>> wrong everywhere.
>>
>> Since GLSL simply says derivatives are undefined after non-uniform
>> discard, and this option makes it defined instead, setting this flag can
>> never break the behavior of a correctly written shader.
>
> BTW, how expensive is the radeonsi workaround when it isn't needed?
>
> I'm starting to wonder if we shouldn't just make it always safe and call
> it a day, saving the trouble of identifying broken apps and plumbing the
> info through the API layers...

 As-is, the workaround can be *very* expensive in the worst case. A large
 number of pixels could be disabled by a discard early in the shader, and
 we're now moving the discard down, which means a lot of unnecessary
 texture fetches may be happening.

 Also, I think I spoke too soon about this flag not having negative
 effects: if a shader has an image/buffer write after a discard, that
 write is now no longer disabled.

 A more efficient workaround can be done at the LLVM level by doing the
 discard early, but then re-enabling WQM "relative to" the new set of
 active pixels. It's a bit involved, especially when the discard itself
 happens in a branch, and still a little more expensive, but it's an option.

>>>
>>> I'm wondering what your driver for the other OS does (afaik dx10 is
>>> really the odd man out, all of glsl, spir-v, even metal have undefined
>>> derivatives after non-uniform discards). Thinking surely there must be
>>> something clever you could do...
>>
>> I'm wondering the same.
>>
>> This is an issue we come across from time to time, where a game's
>> shaders are expecting the D3D behaviour of derivatives remaining
>> defined post-discard. For this we usually do essentially what this
>> workaround is doing, just postpone the discard until the very end of
>> the shader.
>>
>> However it seems like doing this is less performant than the original
>> shaders running on D3D. One case I've seen had a big performance loss
>> against D3D when doing a delayed discard (which was being used early
>> in a complex shader to cull a lot of unneeded pixels), on both AMD and
>> NVIDIA.
>>
>> Given that, I've wondered whether there's something clever that the
>> D3D drivers are doing to optimise this. Maybe, for example, discarding
>> immediately if all pixels in a quad used for derivative calculations
>> get discarded? Is something like that possible on AMD hardware?
> 
> Yes, it's possible but not implemented in LLVM yet.
> 

Albeit if you'd wanted to do it correctly in the app, I'm not sure how
you could achieve that...

Roland

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Marek Olšák]

On Thu, Jun 22, 2017 at 6:13 PM, Alex Smith  wrote:
> On 22 June 2017 at 15:52, Roland Scheidegger  wrote:
>> Am 22.06.2017 um 13:09 schrieb Nicolai Hähnle:
>>> On 22.06.2017 10:14, Michel Dänzer wrote:
 On 22/06/17 04:34 PM, Nicolai Hähnle wrote:
> On 22.06.2017 03:38, Rob Clark wrote:
>> On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:
>>> On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark 
>>> wrote:
 On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák 
 wrote:
> Hi,
>
> This series updates pipe loaders so that flags such as drirc options
> can be passed to create_screen(). I have compile-tested everything
> except clover.
>
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects
> DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag
> forces
> radeonsi to use the DX behavior for that game.
>

 do we really want this to be a *global* option for the screen?
>>>
>>> Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
>>> compiler option also has to be global. We can't look at the context
>>> during the TGSI->LLVM translation.
>>
>> well, I didn't really mean per-screen vs per-context, as much as
>> per-screen vs per-shader (or maybe more per-screen vs
>> per-instruction?)
>
> I honestly don't think it's worth the trouble. Applications that are
> properly coded against GLSL can benefit from the relaxed semantics, and
> applications that get it wrong in one shader are rather likely to get it
> wrong everywhere.
>
> Since GLSL simply says derivatives are undefined after non-uniform
> discard, and this option makes it defined instead, setting this flag can
> never break the behavior of a correctly written shader.

 BTW, how expensive is the radeonsi workaround when it isn't needed?

 I'm starting to wonder if we shouldn't just make it always safe and call
 it a day, saving the trouble of identifying broken apps and plumbing the
 info through the API layers...
>>>
>>> As-is, the workaround can be *very* expensive in the worst case. A large
>>> number of pixels could be disabled by a discard early in the shader, and
>>> we're now moving the discard down, which means a lot of unnecessary
>>> texture fetches may be happening.
>>>
>>> Also, I think I spoke too soon about this flag not having negative
>>> effects: if a shader has an image/buffer write after a discard, that
>>> write is now no longer disabled.
>>>
>>> A more efficient workaround can be done at the LLVM level by doing the
>>> discard early, but then re-enabling WQM "relative to" the new set of
>>> active pixels. It's a bit involved, especially when the discard itself
>>> happens in a branch, and still a little more expensive, but it's an option.
>>>
>>
>> I'm wondering what your driver for the other OS does (afaik dx10 is
>> really the odd man out, all of glsl, spir-v, even metal have undefined
>> derivatives after non-uniform discards). Thinking surely there must be
>> something clever you could do...
>
> I'm wondering the same.
>
> This is an issue we come across from time to time, where a game's
> shaders are expecting the D3D behaviour of derivatives remaining
> defined post-discard. For this we usually do essentially what this
> workaround is doing, just postpone the discard until the very end of
> the shader.
>
> However it seems like doing this is less performant than the original
> shaders running on D3D. One case I've seen had a big performance loss
> against D3D when doing a delayed discard (which was being used early
> in a complex shader to cull a lot of unneeded pixels), on both AMD and
> NVIDIA.
>
> Given that, I've wondered whether there's something clever that the
> D3D drivers are doing to optimise this. Maybe, for example, discarding
> immediately if all pixels in a quad used for derivative calculations
> get discarded? Is something like that possible on AMD hardware?

Yes, it's possible but not implemented in LLVM yet.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Alex Smith]

On 22 June 2017 at 15:52, Roland Scheidegger  wrote:
> Am 22.06.2017 um 13:09 schrieb Nicolai Hähnle:
>> On 22.06.2017 10:14, Michel Dänzer wrote:
>>> On 22/06/17 04:34 PM, Nicolai Hähnle wrote:
 On 22.06.2017 03:38, Rob Clark wrote:
> On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:
>> On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark 
>> wrote:
>>> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák 
>>> wrote:
 Hi,

 This series updates pipe loaders so that flags such as drirc options
 can be passed to create_screen(). I have compile-tested everything
 except clover.

 The first pipe_screen flag is a drirc option to fix incorrect grass
 rendering in Rocket League for radeonsi. Rocket League expects
 DirectX
 behavior for partial derivative computations after discard/kill, but
 radeonsi implements the more efficient but stricter OpenGL behavior
 and that will remain our default behavior. The new screen flag
 forces
 radeonsi to use the DX behavior for that game.

>>>
>>> do we really want this to be a *global* option for the screen?
>>
>> Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
>> compiler option also has to be global. We can't look at the context
>> during the TGSI->LLVM translation.
>
> well, I didn't really mean per-screen vs per-context, as much as
> per-screen vs per-shader (or maybe more per-screen vs
> per-instruction?)

 I honestly don't think it's worth the trouble. Applications that are
 properly coded against GLSL can benefit from the relaxed semantics, and
 applications that get it wrong in one shader are rather likely to get it
 wrong everywhere.

 Since GLSL simply says derivatives are undefined after non-uniform
 discard, and this option makes it defined instead, setting this flag can
 never break the behavior of a correctly written shader.
>>>
>>> BTW, how expensive is the radeonsi workaround when it isn't needed?
>>>
>>> I'm starting to wonder if we shouldn't just make it always safe and call
>>> it a day, saving the trouble of identifying broken apps and plumbing the
>>> info through the API layers...
>>
>> As-is, the workaround can be *very* expensive in the worst case. A large
>> number of pixels could be disabled by a discard early in the shader, and
>> we're now moving the discard down, which means a lot of unnecessary
>> texture fetches may be happening.
>>
>> Also, I think I spoke too soon about this flag not having negative
>> effects: if a shader has an image/buffer write after a discard, that
>> write is now no longer disabled.
>>
>> A more efficient workaround can be done at the LLVM level by doing the
>> discard early, but then re-enabling WQM "relative to" the new set of
>> active pixels. It's a bit involved, especially when the discard itself
>> happens in a branch, and still a little more expensive, but it's an option.
>>
>
> I'm wondering what your driver for the other OS does (afaik dx10 is
> really the odd man out, all of glsl, spir-v, even metal have undefined
> derivatives after non-uniform discards). Thinking surely there must be
> something clever you could do...

I'm wondering the same.

This is an issue we come across from time to time, where a game's
shaders are expecting the D3D behaviour of derivatives remaining
defined post-discard. For this we usually do essentially what this
workaround is doing, just postpone the discard until the very end of
the shader.

However it seems like doing this is less performant than the original
shaders running on D3D. One case I've seen had a big performance loss
against D3D when doing a delayed discard (which was being used early
in a complex shader to cull a lot of unneeded pixels), on both AMD and
NVIDIA.

Given that, I've wondered whether there's something clever that the
D3D drivers are doing to optimise this. Maybe, for example, discarding
immediately if all pixels in a quad used for derivative calculations
get discarded? Is something like that possible on AMD hardware?

Alex

>
> Roland
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Roland Scheidegger]

Am 22.06.2017 um 13:09 schrieb Nicolai Hähnle:
> On 22.06.2017 10:14, Michel Dänzer wrote:
>> On 22/06/17 04:34 PM, Nicolai Hähnle wrote:
>>> On 22.06.2017 03:38, Rob Clark wrote:
 On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:
> On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark 
> wrote:
>> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák 
>> wrote:
>>> Hi,
>>>
>>> This series updates pipe loaders so that flags such as drirc options
>>> can be passed to create_screen(). I have compile-tested everything
>>> except clover.
>>>
>>> The first pipe_screen flag is a drirc option to fix incorrect grass
>>> rendering in Rocket League for radeonsi. Rocket League expects
>>> DirectX
>>> behavior for partial derivative computations after discard/kill, but
>>> radeonsi implements the more efficient but stricter OpenGL behavior
>>> and that will remain our default behavior. The new screen flag
>>> forces
>>> radeonsi to use the DX behavior for that game.
>>>
>>
>> do we really want this to be a *global* option for the screen?
>
> Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
> compiler option also has to be global. We can't look at the context
> during the TGSI->LLVM translation.

 well, I didn't really mean per-screen vs per-context, as much as
 per-screen vs per-shader (or maybe more per-screen vs
 per-instruction?)
>>>
>>> I honestly don't think it's worth the trouble. Applications that are
>>> properly coded against GLSL can benefit from the relaxed semantics, and
>>> applications that get it wrong in one shader are rather likely to get it
>>> wrong everywhere.
>>>
>>> Since GLSL simply says derivatives are undefined after non-uniform
>>> discard, and this option makes it defined instead, setting this flag can
>>> never break the behavior of a correctly written shader.
>>
>> BTW, how expensive is the radeonsi workaround when it isn't needed?
>>
>> I'm starting to wonder if we shouldn't just make it always safe and call
>> it a day, saving the trouble of identifying broken apps and plumbing the
>> info through the API layers...
> 
> As-is, the workaround can be *very* expensive in the worst case. A large
> number of pixels could be disabled by a discard early in the shader, and
> we're now moving the discard down, which means a lot of unnecessary
> texture fetches may be happening.
> 
> Also, I think I spoke too soon about this flag not having negative
> effects: if a shader has an image/buffer write after a discard, that
> write is now no longer disabled.
> 
> A more efficient workaround can be done at the LLVM level by doing the
> discard early, but then re-enabling WQM "relative to" the new set of
> active pixels. It's a bit involved, especially when the discard itself
> happens in a branch, and still a little more expensive, but it's an option.
> 

I'm wondering what your driver for the other OS does (afaik dx10 is
really the odd man out, all of glsl, spir-v, even metal have undefined
derivatives after non-uniform discards). Thinking surely there must be
something clever you could do...

Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Nicolai Hähnle]

On 22.06.2017 10:14, Michel Dänzer wrote:

On 22/06/17 04:34 PM, Nicolai Hähnle wrote:

On 22.06.2017 03:38, Rob Clark wrote:

On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:

On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark  wrote:

On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:

Hi,

This series updates pipe loaders so that flags such as drirc options
can be passed to create_screen(). I have compile-tested everything
except clover.

The first pipe_screen flag is a drirc option to fix incorrect grass
rendering in Rocket League for radeonsi. Rocket League expects DirectX
behavior for partial derivative computations after discard/kill, but
radeonsi implements the more efficient but stricter OpenGL behavior
and that will remain our default behavior. The new screen flag forces
radeonsi to use the DX behavior for that game.



do we really want this to be a *global* option for the screen?


Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
compiler option also has to be global. We can't look at the context
during the TGSI->LLVM translation.


well, I didn't really mean per-screen vs per-context, as much as
per-screen vs per-shader (or maybe more per-screen vs
per-instruction?)


I honestly don't think it's worth the trouble. Applications that are
properly coded against GLSL can benefit from the relaxed semantics, and
applications that get it wrong in one shader are rather likely to get it
wrong everywhere.

Since GLSL simply says derivatives are undefined after non-uniform
discard, and this option makes it defined instead, setting this flag can
never break the behavior of a correctly written shader.


BTW, how expensive is the radeonsi workaround when it isn't needed?

I'm starting to wonder if we shouldn't just make it always safe and call
it a day, saving the trouble of identifying broken apps and plumbing the
info through the API layers...


As-is, the workaround can be *very* expensive in the worst case. A large 
number of pixels could be disabled by a discard early in the shader, and 
we're now moving the discard down, which means a lot of unnecessary 
texture fetches may be happening.


Also, I think I spoke too soon about this flag not having negative 
effects: if a shader has an image/buffer write after a discard, that 
write is now no longer disabled.


A more efficient workaround can be done at the LLVM level by doing the 
discard early, but then re-enabling WQM "relative to" the new set of 
active pixels. It's a bit involved, especially when the discard itself 
happens in a branch, and still a little more expensive, but it's an option.


Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Samuel Pitoiset]

On 06/21/2017 03:34 PM, Timothee Besset wrote:



On Wed, Jun 21, 2017 at 4:50 AM, Samuel Pitoiset 
> wrote:




On 06/21/2017 11:48 AM, Marek Olšák wrote:



On Jun 21, 2017 8:46 AM, "Samuel Pitoiset"

>> wrote:

 Can't this be fixed at game level?


We should ask TTimo about this, it would be much better to fix the
game instead.


The shaders are not handcrafted, the behavior that Marek's patch enables 
is likely the default assumption of UE3's shader authoring tools. For 
better or worse this is also how the NVidia drivers behave.


I think Marek's fix is the best solution. This is likely to show up in 
other UE3 products so the flag makes sense.


Okay, thanks for your answer.

This solution looks good to me then.



TTimo




No without rewriting some of its shaders.

Marek



 On 06/21/2017 12:54 AM, Marek Olšák wrote:

 Hi,

 This series updates pipe loaders so that flags such as
drirc options
 can be passed to create_screen(). I have compile-tested
everything
 except clover.

 The first pipe_screen flag is a drirc option to fix
incorrect grass
 rendering in Rocket League for radeonsi. Rocket League
expects
 DirectX
 behavior for partial derivative computations after
discard/kill, but
 radeonsi implements the more efficient but stricter
OpenGL behavior
 and that will remain our default behavior. The new
screen flag
 forces
 radeonsi to use the DX behavior for that game.

 Please review.

 Thanks,
 Marek
 ___
 mesa-dev mailing list
mesa-dev@lists.freedesktop.org

 >
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


>


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Michel Dänzer]

On 22/06/17 04:34 PM, Nicolai Hähnle wrote:
> On 22.06.2017 03:38, Rob Clark wrote:
>> On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:
>>> On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark  wrote:
 On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
> Hi,
>
> This series updates pipe loaders so that flags such as drirc options
> can be passed to create_screen(). I have compile-tested everything
> except clover.
>
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag forces
> radeonsi to use the DX behavior for that game.
>

 do we really want this to be a *global* option for the screen?
>>>
>>> Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
>>> compiler option also has to be global. We can't look at the context
>>> during the TGSI->LLVM translation.
>>
>> well, I didn't really mean per-screen vs per-context, as much as
>> per-screen vs per-shader (or maybe more per-screen vs
>> per-instruction?)
> 
> I honestly don't think it's worth the trouble. Applications that are
> properly coded against GLSL can benefit from the relaxed semantics, and
> applications that get it wrong in one shader are rather likely to get it
> wrong everywhere.
> 
> Since GLSL simply says derivatives are undefined after non-uniform
> discard, and this option makes it defined instead, setting this flag can
> never break the behavior of a correctly written shader.

BTW, how expensive is the radeonsi workaround when it isn't needed?

I'm starting to wonder if we shouldn't just make it always safe and call
it a day, saving the trouble of identifying broken apps and plumbing the
info through the API layers...


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Nicolai Hähnle]

On 21.06.2017 00:54, Marek Olšák wrote:

Hi,

This series updates pipe loaders so that flags such as drirc options
can be passed to create_screen(). I have compile-tested everything
except clover.

The first pipe_screen flag is a drirc option to fix incorrect grass
rendering in Rocket League for radeonsi. Rocket League expects DirectX
behavior for partial derivative computations after discard/kill, but
radeonsi implements the more efficient but stricter OpenGL behavior
and that will remain our default behavior. The new screen flag forces
radeonsi to use the DX behavior for that game.

Please review.


Makes sense.

The one thing I don't particularly like about this series is that it 
forces drirc through flags that are defined at the Gallium API level. 
There may be an opportunity for drirc options that have numeric values, 
for example to change hardware-specific texture sampling quality 
settings. But maybe that's a bridge to be crossed when we get there.


Series is:

Reviewed-by: Nicolai Hähnle 




Thanks,
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Nicolai Hähnle]

On 22.06.2017 03:38, Rob Clark wrote:

On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:

On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark  wrote:

On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:

Hi,

This series updates pipe loaders so that flags such as drirc options
can be passed to create_screen(). I have compile-tested everything
except clover.

The first pipe_screen flag is a drirc option to fix incorrect grass
rendering in Rocket League for radeonsi. Rocket League expects DirectX
behavior for partial derivative computations after discard/kill, but
radeonsi implements the more efficient but stricter OpenGL behavior
and that will remain our default behavior. The new screen flag forces
radeonsi to use the DX behavior for that game.



do we really want this to be a *global* option for the screen?


Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
compiler option also has to be global. We can't look at the context
during the TGSI->LLVM translation.


well, I didn't really mean per-screen vs per-context, as much as
per-screen vs per-shader (or maybe more per-screen vs
per-instruction?)


I honestly don't think it's worth the trouble. Applications that are 
properly coded against GLSL can benefit from the relaxed semantics, and 
applications that get it wrong in one shader are rather likely to get it 
wrong everywhere.


Since GLSL simply says derivatives are undefined after non-uniform 
discard, and this option makes it defined instead, setting this flag can 
never break the behavior of a correctly written shader.


Cheers,
Nicolai



I'm just thinking, some drivers use lowering passes that internally
generate kill's.  I *guess* it would only matter if they also had
ddx/ddy instructions, but not sure.

not really sure if this would actually be a problem or not..


Whether or not this affects you depends on how your hardware
implements kill/discard. Not just ddx/ddy, texture instructions
computing derivatives internally are affected by kill/discard too.



I guess most of the lowering passes that introduce kill's are more
like legacy gl things (clip-planes, glBitmap(), maybe some others but
pretty sure it is all legacy type stuff), so maybe it isn't likely to
matter.  I'm not totally sure about the expected interactions between
kill and other instructions, so not totally sure about whether I
should care.. but figured I should point it out.. and if it doesn't
matter, it doesn't matter.

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Rob Clark]

On Wed, Jun 21, 2017 at 8:15 PM, Marek Olšák  wrote:
> On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark  wrote:
>> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
>>> Hi,
>>>
>>> This series updates pipe loaders so that flags such as drirc options
>>> can be passed to create_screen(). I have compile-tested everything
>>> except clover.
>>>
>>> The first pipe_screen flag is a drirc option to fix incorrect grass
>>> rendering in Rocket League for radeonsi. Rocket League expects DirectX
>>> behavior for partial derivative computations after discard/kill, but
>>> radeonsi implements the more efficient but stricter OpenGL behavior
>>> and that will remain our default behavior. The new screen flag forces
>>> radeonsi to use the DX behavior for that game.
>>>
>>
>> do we really want this to be a *global* option for the screen?
>
> Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
> compiler option also has to be global. We can't look at the context
> during the TGSI->LLVM translation.

well, I didn't really mean per-screen vs per-context, as much as
per-screen vs per-shader (or maybe more per-screen vs
per-instruction?)

>>
>> I'm just thinking, some drivers use lowering passes that internally
>> generate kill's.  I *guess* it would only matter if they also had
>> ddx/ddy instructions, but not sure.
>>
>> not really sure if this would actually be a problem or not..
>
> Whether or not this affects you depends on how your hardware
> implements kill/discard. Not just ddx/ddy, texture instructions
> computing derivatives internally are affected by kill/discard too.
>

I guess most of the lowering passes that introduce kill's are more
like legacy gl things (clip-planes, glBitmap(), maybe some others but
pretty sure it is all legacy type stuff), so maybe it isn't likely to
matter.  I'm not totally sure about the expected interactions between
kill and other instructions, so not totally sure about whether I
should care.. but figured I should point it out.. and if it doesn't
matter, it doesn't matter.

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Marek Olšák]

On Wed, Jun 21, 2017 at 10:37 PM, Rob Clark  wrote:
> On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
>> Hi,
>>
>> This series updates pipe loaders so that flags such as drirc options
>> can be passed to create_screen(). I have compile-tested everything
>> except clover.
>>
>> The first pipe_screen flag is a drirc option to fix incorrect grass
>> rendering in Rocket League for radeonsi. Rocket League expects DirectX
>> behavior for partial derivative computations after discard/kill, but
>> radeonsi implements the more efficient but stricter OpenGL behavior
>> and that will remain our default behavior. The new screen flag forces
>> radeonsi to use the DX behavior for that game.
>>
>
> do we really want this to be a *global* option for the screen?

Yes. Shaders are pipe_screen (global) objects in radeonsi, so a
compiler option also has to be global. We can't look at the context
during the TGSI->LLVM translation.

>
> I'm just thinking, some drivers use lowering passes that internally
> generate kill's.  I *guess* it would only matter if they also had
> ddx/ddy instructions, but not sure.
>
> not really sure if this would actually be a problem or not..

Whether or not this affects you depends on how your hardware
implements kill/discard. Not just ddx/ddy, texture instructions
computing derivatives internally are affected by kill/discard too.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Rob Clark]

On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
> Hi,
>
> This series updates pipe loaders so that flags such as drirc options
> can be passed to create_screen(). I have compile-tested everything
> except clover.
>
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag forces
> radeonsi to use the DX behavior for that game.
>

do we really want this to be a *global* option for the screen?

I'm just thinking, some drivers use lowering passes that internally
generate kill's.  I *guess* it would only matter if they also had
ddx/ddy instructions, but not sure.

not really sure if this would actually be a problem or not..

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Edmondo Tommasina]

Hi Marek

In patch 5 you say the words KILL and WQM and I automatically
think at Witcher 2.

This series with a the drirc option set for Witcher 2 fixes the
longstanding black transition bug.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98238

It's quite probable that VP will not update TW2, so a drirc driven fix like
this would be great to have.

I'll will send the drirc patch on your series for TW2 later after some
more testing.

Thanks
edmondo



On Wed, Jun 21, 2017 at 12:54 AM, Marek Olšák  wrote:
> Hi,
>
> This series updates pipe loaders so that flags such as drirc options
> can be passed to create_screen(). I have compile-tested everything
> except clover.
>
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag forces
> radeonsi to use the DX behavior for that game.
>
> Please review.
>
> Thanks,
> Marek
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Ilia Mirkin]

On Tue, Jun 20, 2017 at 6:54 PM, Marek Olšák  wrote:
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag forces
> radeonsi to use the DX behavior for that game.

For those of us following along at home... can you provide a brief
reminder of what the DX behavior is, and how does it differ from GL
behavior? In case one might want to fix this for nouveau (if a fix is
needed at all)...

Thanks,

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Timothee Besset]

On Wed, Jun 21, 2017 at 4:50 AM, Samuel Pitoiset 
wrote:

>
>
> On 06/21/2017 11:48 AM, Marek Olšák wrote:
>
>>
>>
>> On Jun 21, 2017 8:46 AM, "Samuel Pitoiset" > > wrote:
>>
>> Can't this be fixed at game level?
>>
>
> We should ask TTimo about this, it would be much better to fix the game
> instead.
>

The shaders are not handcrafted, the behavior that Marek's patch enables is
likely the default assumption of UE3's shader authoring tools. For better
or worse this is also how the NVidia drivers behave.

I think Marek's fix is the best solution. This is likely to show up in
other UE3 products so the flag makes sense.

TTimo


>
>
>>
>> No without rewriting some of its shaders.
>>
>> Marek
>>
>>
>>
>> On 06/21/2017 12:54 AM, Marek Olšák wrote:
>>
>> Hi,
>>
>> This series updates pipe loaders so that flags such as drirc
>> options
>> can be passed to create_screen(). I have compile-tested everything
>> except clover.
>>
>> The first pipe_screen flag is a drirc option to fix incorrect
>> grass
>> rendering in Rocket League for radeonsi. Rocket League expects
>> DirectX
>> behavior for partial derivative computations after discard/kill,
>> but
>> radeonsi implements the more efficient but stricter OpenGL
>> behavior
>> and that will remain our default behavior. The new screen flag
>> forces
>> radeonsi to use the DX behavior for that game.
>>
>> Please review.
>>
>> Thanks,
>> Marek
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> 
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> 
>>
>>
>> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Samuel Pitoiset]

On 06/21/2017 11:48 AM, Marek Olšák wrote:



On Jun 21, 2017 8:46 AM, "Samuel Pitoiset" > wrote:


Can't this be fixed at game level?


We should ask TTimo about this, it would be much better to fix the game 
instead.





No without rewriting some of its shaders.

Marek



On 06/21/2017 12:54 AM, Marek Olšák wrote:

Hi,

This series updates pipe loaders so that flags such as drirc options
can be passed to create_screen(). I have compile-tested everything
except clover.

The first pipe_screen flag is a drirc option to fix incorrect grass
rendering in Rocket League for radeonsi. Rocket League expects
DirectX
behavior for partial derivative computations after discard/kill, but
radeonsi implements the more efficient but stricter OpenGL behavior
and that will remain our default behavior. The new screen flag
forces
radeonsi to use the DX behavior for that game.

Please review.

Thanks,
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Marek Olšák]

On Jun 21, 2017 8:46 AM, "Samuel Pitoiset" 
wrote:

Can't this be fixed at game level?


No without rewriting some of its shaders.

Marek



On 06/21/2017 12:54 AM, Marek Olšák wrote:

> Hi,
>
> This series updates pipe loaders so that flags such as drirc options
> can be passed to create_screen(). I have compile-tested everything
> except clover.
>
> The first pipe_screen flag is a drirc option to fix incorrect grass
> rendering in Rocket League for radeonsi. Rocket League expects DirectX
> behavior for partial derivative computations after discard/kill, but
> radeonsi implements the more efficient but stricter OpenGL behavior
> and that will remain our default behavior. The new screen flag forces
> radeonsi to use the DX behavior for that game.
>
> Please review.
>
> Thanks,
> Marek
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/5] Gallium: Passing drirc options to create_screen() and fixing Rocket League

[Samuel Pitoiset]

Can't this be fixed at game level?

On 06/21/2017 12:54 AM, Marek Olšák wrote:

Hi,

This series updates pipe loaders so that flags such as drirc options
can be passed to create_screen(). I have compile-tested everything
except clover.

The first pipe_screen flag is a drirc option to fix incorrect grass
rendering in Rocket League for radeonsi. Rocket League expects DirectX
behavior for partial derivative computations after discard/kill, but
radeonsi implements the more efficient but stricter OpenGL behavior
and that will remain our default behavior. The new screen flag forces
radeonsi to use the DX behavior for that game.

Please review.

Thanks,
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev