Re: [Mesa-dev] [PATCH 1/8] gallium: Add a cap for offset_units_unscaled

2016-06-15 Thread Axel Davy 

On 15/06/2016 03:04, Roland Scheidegger wrote:

Am 15.06.2016 um 01:08 schrieb Axel Davy:

On 15/06/2016 00:21, Roland Scheidegger wrote:

Am 14.06.2016 um 23:33 schrieb Axel Davy:

diff --git a/src/gallium/include/pipe/p_state.h
b/src/gallium/include/pipe/p_state.h
index 396f563..7dce80a 100644
--- a/src/gallium/include/pipe/p_state.h
+++ b/src/gallium/include/pipe/p_state.h
@@ -139,6 +139,13 @@ struct pipe_rasterizer_state
  unsigned clip_halfz:1;
/**
+* When true do not scale offset_units and use same rules for
unorm and
+* float depth buffers (D3D9). When false use GL/D3D1X behaviour.
+* This depends on PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED.
+*/
+   unsigned offset_units_unscaled;
+
+   /**
   * Enable bits for clipping half-spaces.
   * This applies to both user clip planes and shader clip distances.
   * Note that if the bound shader exports any clip distances, these


I don't like this. Generally, for unorm formats, you can easily enough
translate this from d3d9 to gl (or d3d10) rules (but yes, obviously it's
going to be format dependent). (With one big caveat, in general not all
gl drivers think the minimum resolvable difference is the same, that
might range from 2^-22 to 2^-24 for 24bit unorm depth for instance, and
I don't think it's quite consistent with gallium drivers neither).

You are right though for float depth the formula is different, and you
can't translate it. But do you really need float depth buffer support?
AFAIK no d3d9 app really depends on it, everything can fall back to d24.

Roland


Hi,


That's true float depth buffer do not seem to be widely used in d3d9.

The two float depth buffers available in d3d9, as far as I know, are
D32F_LOCKABLE and D24FS8.

We can see the support for those and other depth buffers here (note that
these are mainly old cards):

http://zp.amsnet.pl/cdragan/query.php?dxversion=9=formats=selected=all[]=45[]=44[]=41[]=42[]=43[]=40[]=39[]=46=SURFACE=DEPTHSTENCIL=horizontal


It is likely not a requirement for any game to support these formats.


We could ignore these formats, and add to gallium a way to get the
minimum resolvable difference per depth buffer format from drivers. We
considered this option.


That said, the driver is the best location to know about the minimum
resolvable difference, and we made the choice to let the driver do the
scaling instead of doing it based on some driver query in the state
tracker.

As for floating point depth buffers behaviour, I understand for some
drivers it may be harder than for others to implement.

That doesn't seem however a reason to drop floating depth buffer support
in Gallium Nine. D32F_LOCKABLE is particularly useful for debugging,
being lockable, it can be used to show depth buffer content after some
draw calls for d3d on windows, and compare with nine. And some apps may
use it for some particular effects.

I'd be ok if we make the float depth buffer part of
offset_units_unscaled optional given how rare the combination float
depth buffers + depth bias must be used. However if hw can do it, I see
no reason why we wouldn't support the capability?

On second look, it doesn't really look too bad (and fwiw we actually
could probably put it to use here if we'd support it in llvmpipe).
Albeit,
unsigned offset_units_unscaled;
needs to be
unsigned offset_units_unscaled:1;

Good catch, this was the reason I had put it in this place of the structure,
but somehow forgot the :1 ...


I'm just very sceptical when it comes to capabilities solely to the
benefit of fringe state trackers (and everything not st/mesa counts
here). It usually means driver authors aren't going to bother. And you
probably can't implement it in all drivers yourselves even if the hw
could do it.

That said, I'm ok with this if there's no objections from others.

Roland



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

Re: [Mesa-dev] [PATCH 1/8] gallium: Add a cap for offset_units_unscaled

2016-06-14 Thread Roland Scheidegger 
Am 15.06.2016 um 01:08 schrieb Axel Davy:
> On 15/06/2016 00:21, Roland Scheidegger wrote:
>> Am 14.06.2016 um 23:33 schrieb Axel Davy:
>>> diff --git a/src/gallium/include/pipe/p_state.h
>>> b/src/gallium/include/pipe/p_state.h
>>> index 396f563..7dce80a 100644
>>> --- a/src/gallium/include/pipe/p_state.h
>>> +++ b/src/gallium/include/pipe/p_state.h
>>> @@ -139,6 +139,13 @@ struct pipe_rasterizer_state
>>>  unsigned clip_halfz:1;
>>>/**
>>> +* When true do not scale offset_units and use same rules for
>>> unorm and
>>> +* float depth buffers (D3D9). When false use GL/D3D1X behaviour.
>>> +* This depends on PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED.
>>> +*/
>>> +   unsigned offset_units_unscaled;
>>> +
>>> +   /**
>>>   * Enable bits for clipping half-spaces.
>>>   * This applies to both user clip planes and shader clip distances.
>>>   * Note that if the bound shader exports any clip distances, these
>>>
>> I don't like this. Generally, for unorm formats, you can easily enough
>> translate this from d3d9 to gl (or d3d10) rules (but yes, obviously it's
>> going to be format dependent). (With one big caveat, in general not all
>> gl drivers think the minimum resolvable difference is the same, that
>> might range from 2^-22 to 2^-24 for 24bit unorm depth for instance, and
>> I don't think it's quite consistent with gallium drivers neither).
>>
>> You are right though for float depth the formula is different, and you
>> can't translate it. But do you really need float depth buffer support?
>> AFAIK no d3d9 app really depends on it, everything can fall back to d24.
>>
>> Roland
>>
> Hi,
> 
> 
> That's true float depth buffer do not seem to be widely used in d3d9.
> 
> The two float depth buffers available in d3d9, as far as I know, are
> D32F_LOCKABLE and D24FS8.
> 
> We can see the support for those and other depth buffers here (note that
> these are mainly old cards):
> 
> http://zp.amsnet.pl/cdragan/query.php?dxversion=9=formats=selected=all[]=45[]=44[]=41[]=42[]=43[]=40[]=39[]=46=SURFACE=DEPTHSTENCIL=horizontal
> 
> 
> It is likely not a requirement for any game to support these formats.
> 
> 
> We could ignore these formats, and add to gallium a way to get the
> minimum resolvable difference per depth buffer format from drivers. We
> considered this option.
> 
> 
> That said, the driver is the best location to know about the minimum
> resolvable difference, and we made the choice to let the driver do the
> scaling instead of doing it based on some driver query in the state
> tracker.
> 
> As for floating point depth buffers behaviour, I understand for some
> drivers it may be harder than for others to implement.
> 
> That doesn't seem however a reason to drop floating depth buffer support
> in Gallium Nine. D32F_LOCKABLE is particularly useful for debugging,
> being lockable, it can be used to show depth buffer content after some
> draw calls for d3d on windows, and compare with nine. And some apps may
> use it for some particular effects.
> 
> I'd be ok if we make the float depth buffer part of
> offset_units_unscaled optional given how rare the combination float
> depth buffers + depth bias must be used. However if hw can do it, I see
> no reason why we wouldn't support the capability?

On second look, it doesn't really look too bad (and fwiw we actually
could probably put it to use here if we'd support it in llvmpipe).
Albeit,
unsigned offset_units_unscaled;
needs to be
unsigned offset_units_unscaled:1;

I'm just very sceptical when it comes to capabilities solely to the
benefit of fringe state trackers (and everything not st/mesa counts
here). It usually means driver authors aren't going to bother. And you
probably can't implement it in all drivers yourselves even if the hw
could do it.

That said, I'm ok with this if there's no objections from others.

Roland

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

Re: [Mesa-dev] [PATCH 1/8] gallium: Add a cap for offset_units_unscaled

2016-06-14 Thread Axel Davy 

On 15/06/2016 00:21, Roland Scheidegger wrote:

Am 14.06.2016 um 23:33 schrieb Axel Davy:

diff --git a/src/gallium/include/pipe/p_state.h 
b/src/gallium/include/pipe/p_state.h
index 396f563..7dce80a 100644
--- a/src/gallium/include/pipe/p_state.h
+++ b/src/gallium/include/pipe/p_state.h
@@ -139,6 +139,13 @@ struct pipe_rasterizer_state
 unsigned clip_halfz:1;
  
 /**

+* When true do not scale offset_units and use same rules for unorm and
+* float depth buffers (D3D9). When false use GL/D3D1X behaviour.
+* This depends on PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED.
+*/
+   unsigned offset_units_unscaled;
+
+   /**
  * Enable bits for clipping half-spaces.
  * This applies to both user clip planes and shader clip distances.
  * Note that if the bound shader exports any clip distances, these


I don't like this. Generally, for unorm formats, you can easily enough
translate this from d3d9 to gl (or d3d10) rules (but yes, obviously it's
going to be format dependent). (With one big caveat, in general not all
gl drivers think the minimum resolvable difference is the same, that
might range from 2^-22 to 2^-24 for 24bit unorm depth for instance, and
I don't think it's quite consistent with gallium drivers neither).

You are right though for float depth the formula is different, and you
can't translate it. But do you really need float depth buffer support?
AFAIK no d3d9 app really depends on it, everything can fall back to d24.

Roland


Hi,


That's true float depth buffer do not seem to be widely used in d3d9.

The two float depth buffers available in d3d9, as far as I know, are 
D32F_LOCKABLE and D24FS8.


We can see the support for those and other depth buffers here (note that 
these are mainly old cards):


http://zp.amsnet.pl/cdragan/query.php?dxversion=9=formats=selected=all%5B%5D=45%5B%5D=44%5B%5D=41%5B%5D=42%5B%5D=43%5B%5D=40%5B%5D=39%5B%5D=46=SURFACE=DEPTHSTENCIL=horizontal


It is likely not a requirement for any game to support these formats.


We could ignore these formats, and add to gallium a way to get the 
minimum resolvable difference per depth buffer format from drivers. We 
considered this option.



That said, the driver is the best location to know about the minimum 
resolvable difference, and we made the choice to let the driver do the 
scaling instead of doing it based on some driver query in the state tracker.


As for floating point depth buffers behaviour, I understand for some 
drivers it may be harder than for others to implement.


That doesn't seem however a reason to drop floating depth buffer support 
in Gallium Nine. D32F_LOCKABLE is particularly useful for debugging, 
being lockable, it can be used to show depth buffer content after some 
draw calls for d3d on windows, and compare with nine. And some apps may 
use it for some particular effects.


I'd be ok if we make the float depth buffer part of 
offset_units_unscaled optional given how rare the combination float 
depth buffers + depth bias must be used. However if hw can do it, I see 
no reason why we wouldn't support the capability?



Yours,


Axel Davy

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

Re: [Mesa-dev] [PATCH 1/8] gallium: Add a cap for offset_units_unscaled

2016-06-14 Thread Roland Scheidegger 
Am 14.06.2016 um 23:33 schrieb Axel Davy:
> D3D9 has a different behaviour for depth bias.
> 
> For OGL/D3D1X, the depth bias unit is the
> minimal resolvable value for the depth buffer,
> which depends on the format (and has different
> behaviour for float depth buffers).
> 
> For D3D9, the depth bias unit is 1.0f.
> 
> Signed-off-by: Axel Davy 
> ---
>  src/gallium/docs/source/cso/rasterizer.rst   | 6 ++
>  src/gallium/docs/source/screen.rst   | 2 ++
>  src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
>  src/gallium/drivers/i915/i915_screen.c   | 1 +
>  src/gallium/drivers/ilo/ilo_screen.c | 1 +
>  src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
>  src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 1 +
>  src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 1 +
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 1 +
>  src/gallium/drivers/r300/r300_screen.c   | 1 +
>  src/gallium/drivers/r600/r600_pipe.c | 1 +
>  src/gallium/drivers/radeonsi/si_pipe.c   | 1 +
>  src/gallium/drivers/softpipe/sp_screen.c | 1 +
>  src/gallium/drivers/svga/svga_screen.c   | 1 +
>  src/gallium/drivers/swr/swr_screen.cpp   | 1 +
>  src/gallium/drivers/vc4/vc4_screen.c | 1 +
>  src/gallium/drivers/virgl/virgl_screen.c | 1 +
>  src/gallium/include/pipe/p_defines.h | 1 +
>  src/gallium/include/pipe/p_state.h   | 7 +++
>  19 files changed, 31 insertions(+)
> 
> diff --git a/src/gallium/docs/source/cso/rasterizer.rst 
> b/src/gallium/docs/source/cso/rasterizer.rst
> index 8d473b8..616e451 100644
> --- a/src/gallium/docs/source/cso/rasterizer.rst
> +++ b/src/gallium/docs/source/cso/rasterizer.rst
> @@ -127,6 +127,12 @@ offset_tri
>  
>  offset_units
>  Specifies the polygon offset bias
> +offset_units_unscaled
> +Specifies the unit of the polygon offset bias. If false, use the
> +GL/D3D1X behaviour. If true, offset_units is a floating point offset
> +which isn't scaled (D3D9). Note that GL/D3D1X behaviour has different
> +formula whether the depth buffer is unorm or float, which is not
> +the case for D3D9.
>  offset_scale
>  Specifies the polygon offset scale
>  offset_clamp
> diff --git a/src/gallium/docs/source/screen.rst 
> b/src/gallium/docs/source/screen.rst
> index 920da42..9c26604 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -340,6 +340,8 @@ The integer capabilities:
>extension and thus implements proper support for culling planes.
>  * ``PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES``: Whether primitive restart is
>supported for patch primitives.
> +* ``PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED``: If true, the driver implements 
> support
> +  for ``pipe_rasterizer_state::offset_units_unscaled``.
>  
>  
>  .. _pipe_capf:
> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
> b/src/gallium/drivers/freedreno/freedreno_screen.c
> index ad15aab..ed61456 100644
> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
> @@ -262,6 +262,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
> pipe_cap param)
>   case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
>   case PIPE_CAP_CULL_DISTANCE:
>   case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
> + case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
>   return 0;
>  
>   case PIPE_CAP_MAX_VIEWPORTS:
> diff --git a/src/gallium/drivers/i915/i915_screen.c 
> b/src/gallium/drivers/i915/i915_screen.c
> index c0e06e5..ea451e6 100644
> --- a/src/gallium/drivers/i915/i915_screen.c
> +++ b/src/gallium/drivers/i915/i915_screen.c
> @@ -273,6 +273,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap 
> cap)
> case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
> case PIPE_CAP_CULL_DISTANCE:
> case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
> +   case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
>return 0;
>  
> case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS:
> diff --git a/src/gallium/drivers/ilo/ilo_screen.c 
> b/src/gallium/drivers/ilo/ilo_screen.c
> index c847a90..c9b8d81 100644
> --- a/src/gallium/drivers/ilo/ilo_screen.c
> +++ b/src/gallium/drivers/ilo/ilo_screen.c
> @@ -502,6 +502,7 @@ ilo_get_param(struct pipe_screen *screen, enum pipe_cap 
> param)
> case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
> case PIPE_CAP_CULL_DISTANCE:
> case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
> +   case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
>return 0;
>  
> case PIPE_CAP_VENDOR_ID:
> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c 
> b/src/gallium/drivers/llvmpipe/lp_screen.c
> index 5fc4427..f9217d6 100644
> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
> @@ -329,6 +329,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum 
> pipe_cap param)
> case

[Mesa-dev] [PATCH 1/8] gallium: Add a cap for offset_units_unscaled

2016-06-14 Thread Axel Davy 
D3D9 has a different behaviour for depth bias.

For OGL/D3D1X, the depth bias unit is the
minimal resolvable value for the depth buffer,
which depends on the format (and has different
behaviour for float depth buffers).

For D3D9, the depth bias unit is 1.0f.

Signed-off-by: Axel Davy 
---
 src/gallium/docs/source/cso/rasterizer.rst   | 6 ++
 src/gallium/docs/source/screen.rst   | 2 ++
 src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
 src/gallium/drivers/i915/i915_screen.c   | 1 +
 src/gallium/drivers/ilo/ilo_screen.c | 1 +
 src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
 src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 1 +
 src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 1 +
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 1 +
 src/gallium/drivers/r300/r300_screen.c   | 1 +
 src/gallium/drivers/r600/r600_pipe.c | 1 +
 src/gallium/drivers/radeonsi/si_pipe.c   | 1 +
 src/gallium/drivers/softpipe/sp_screen.c | 1 +
 src/gallium/drivers/svga/svga_screen.c   | 1 +
 src/gallium/drivers/swr/swr_screen.cpp   | 1 +
 src/gallium/drivers/vc4/vc4_screen.c | 1 +
 src/gallium/drivers/virgl/virgl_screen.c | 1 +
 src/gallium/include/pipe/p_defines.h | 1 +
 src/gallium/include/pipe/p_state.h   | 7 +++
 19 files changed, 31 insertions(+)

diff --git a/src/gallium/docs/source/cso/rasterizer.rst 
b/src/gallium/docs/source/cso/rasterizer.rst
index 8d473b8..616e451 100644
--- a/src/gallium/docs/source/cso/rasterizer.rst
+++ b/src/gallium/docs/source/cso/rasterizer.rst
@@ -127,6 +127,12 @@ offset_tri
 
 offset_units
 Specifies the polygon offset bias
+offset_units_unscaled
+Specifies the unit of the polygon offset bias. If false, use the
+GL/D3D1X behaviour. If true, offset_units is a floating point offset
+which isn't scaled (D3D9). Note that GL/D3D1X behaviour has different
+formula whether the depth buffer is unorm or float, which is not
+the case for D3D9.
 offset_scale
 Specifies the polygon offset scale
 offset_clamp
diff --git a/src/gallium/docs/source/screen.rst 
b/src/gallium/docs/source/screen.rst
index 920da42..9c26604 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
@@ -340,6 +340,8 @@ The integer capabilities:
   extension and thus implements proper support for culling planes.
 * ``PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES``: Whether primitive restart is
   supported for patch primitives.
+* ``PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED``: If true, the driver implements 
support
+  for ``pipe_rasterizer_state::offset_units_unscaled``.
 
 
 .. _pipe_capf:
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
b/src/gallium/drivers/freedreno/freedreno_screen.c
index ad15aab..ed61456 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -262,6 +262,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
case PIPE_CAP_CULL_DISTANCE:
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
+   case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
return 0;
 
case PIPE_CAP_MAX_VIEWPORTS:
diff --git a/src/gallium/drivers/i915/i915_screen.c 
b/src/gallium/drivers/i915/i915_screen.c
index c0e06e5..ea451e6 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -273,6 +273,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap 
cap)
case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
case PIPE_CAP_CULL_DISTANCE:
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
+   case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
   return 0;
 
case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS:
diff --git a/src/gallium/drivers/ilo/ilo_screen.c 
b/src/gallium/drivers/ilo/ilo_screen.c
index c847a90..c9b8d81 100644
--- a/src/gallium/drivers/ilo/ilo_screen.c
+++ b/src/gallium/drivers/ilo/ilo_screen.c
@@ -502,6 +502,7 @@ ilo_get_param(struct pipe_screen *screen, enum pipe_cap 
param)
case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
case PIPE_CAP_CULL_DISTANCE:
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
+   case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
   return 0;
 
case PIPE_CAP_VENDOR_ID:
diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c 
b/src/gallium/drivers/llvmpipe/lp_screen.c
index 5fc4427..f9217d6 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -329,6 +329,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum 
pipe_cap param)
case PIPE_CAP_FRAMEBUFFER_NO_ATTACHMENT:
case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
+   case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
   return 0;
}
/* should only get here on unhandled cases */
diff --git