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

[Marek Olšák]

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.

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