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

[Ian Romanick]

On 08/20/2018 04:02 PM, Marek Olšák wrote:
> I wouldn't add _DepthClamp. Having just DepthClampNear and
> DepthClampFar should be enough. Drivers not supporting the extension
> can use either variable, because they will be equal.
> 
> The glGet query can be handled as LOC_CUSTOM.

Yeah, that works for me.

> Marek
> 
> On Sun, Aug 19, 2018 at 6:43 PM Sagar Ghuge <sagar.ghuge at intel.com> wrote:
>>
>>
>>
>> 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