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.gh...@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.gh...@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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev