[Mesa-dev] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate

[Sagar Ghuge]

On 08/13/2018 03:52 PM, Ian Romanick wrote:
> On 08/09/2018 01:09 PM, Marek Olšák wrote:
>> On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge <sagar.ghuge at intel.com> wrote:
>>> enable _mesa_PushAttrib() and _mesa_PopAttrib()
>>> to handle GL_DEPTH_CLAMP_NEAR_AMD and
>>> GL_DEPTH_CLAMP_FAR_AMD tokens.
>>>
>>> Signed-off-by: Sagar Ghuge <sagar.ghuge at intel.com>
>>> ---
>>>  src/mesa/main/attrib.c | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>>> index cbe93ab6fa..d9f165b428 100644
>>> --- a/src/mesa/main/attrib.c
>>> +++ b/src/mesa/main/attrib.c
>>> @@ -73,6 +73,8 @@ struct gl_enable_attrib
>>>     GLboolean ColorMaterial;
>>>     GLboolean CullFace;
>>>     GLboolean DepthClamp;
>>> +   GLboolean DepthClampNear;
>>> +   GLboolean DepthClampFar;
>>
>> The first patch uses this. Also, DepthClamp can be removed, because
>> DepthClampNear+Far replace it, right?
> 
> Based on your comment on patch 4 and my comments on patch 0, maybe we
> should:
> 
> - Remove DepthClamp.  Add _DepthClamp, DepthClampNear, and DepthClampFar.

I might be missing some pieces. But DepthClampNear + far can replaces 
DepthClamp. so why do we need _DepthClamp ? (Adding _DepthClamp means 
it will be derived from DepthClampNear+far, correct ? removing DepthClamp
here means, need to completely get rid of every reference of 
DepthClamp in source code? ) 

> 
> - If GL_DEPTH_CLAMP is set, set all three.  If GL_DEPTH_CLAMP is
> cleared, clear all three.
> 
> - If either of GL_DEPTH_CLAMP_FAR_AMD or GL_DEPTH_CLAMP_NEAR_AMD
> changes, change _DepthClamp to DepthClampNear || DepthClampFar.
> 

We only need to handle this case - "Querying DEPTH_CLAMP will return TRUE if DEPTH_CLAMP_NEAR_AMD _or_
DEPTH_CLAMP_FAR_AMD is enabled." 
I think we don't have to keep changing _DepthClamp, because if we do it
then it will enable depth clamping for both the planes and will get different behavior.
Please correct me if I am wrong or missing anything. 

> - Drivers that enable AMD_depth_clamp_separate will only ever look at
> DepthClampNear and DepthClampFar.
> 
> I think that gets all the cases correct with the minimum fuss.  Marek,
> what do you think?
> 
>> Marek
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev