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

[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&feature=formats&featuregroup=selected&adaptergroup=all&featureselected[]=45&featureselected[]=44&featureselected[]=41&featureselected[]=42&featureselected[]=43&featureselected[]=40&featureselected[]=39&featureselected[]=46&resource=SURFACE&usage=DEPTHSTENCIL&orientation=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