On Thu, Aug 2, 2018 at 2:44 PM, Ian Romanick <i...@freedesktop.org> wrote: > On 08/02/2018 11:30 AM, Ian Romanick wrote: >> On 08/01/2018 08:31 PM, Sagar Ghuge wrote: >>> Add some basic types and storage for the >>> AMD_depth_clamp_separate extension. > > I mentioned this on patch 5, but you should word wrap the commit message > to 70 or 72 columns. > > More substantive comments are below... > >>> Signed-off-by: Sagar Ghuge <sagar.gh...@intel.com> >>> --- >>> include/GL/glcorearb.h | 2 ++ >>> src/mesa/main/extensions_table.h | 1 + >>> src/mesa/main/mtypes.h | 9 +++++++++ >>> 3 files changed, 12 insertions(+) >>> >>> diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h >>> index a78bbb6e18..d73ca5a8df 100644 >>> --- a/include/GL/glcorearb.h >>> +++ b/include/GL/glcorearb.h >>> @@ -1558,6 +1558,8 @@ typedef int64_t GLint64; >>> #define GL_MAX_FRAGMENT_INPUT_COMPONENTS 0x9125 >>> #define GL_CONTEXT_PROFILE_MASK 0x9126 >>> #define GL_DEPTH_CLAMP 0x864F >>> +#define GL_DEPTH_CLAMP_NEAR_AMD 0x901E >>> +#define GL_DEPTH_CLAMP_FAR_AMD 0x901F >>> #define GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION 0x8E4C >>> #define GL_FIRST_VERTEX_CONVENTION 0x8E4D >>> #define GL_LAST_VERTEX_CONVENTION 0x8E4E >> >> We should just import the updated versions of the Khronos headers. I >> think Marek sent out a patch to do this. Does that work? >> >>> diff --git a/src/mesa/main/extensions_table.h >>> b/src/mesa/main/extensions_table.h >>> index 3f01896cae..8dc668e087 100644 >>> --- a/src/mesa/main/extensions_table.h >>> +++ b/src/mesa/main/extensions_table.h >>> @@ -9,6 +9,7 @@ >>> EXT(3DFX_texture_compression_FXT1 , >>> TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) >>> >>> EXT(AMD_conservative_depth , ARB_conservative_depth >>> , GLL, GLC, x , x , 2009) >>> +EXT(AMD_depth_clamp_separate , AMD_depth_clamp_separate >>> , x , GLC, x , x , 2009) >>> EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend >>> , GLL, GLC, x , x , 2009) >>> EXT(AMD_performance_monitor , AMD_performance_monitor >>> , GLL, GLC, x , ES2, 2007) >>> EXT(AMD_pinned_memory , AMD_pinned_memory >>> , GLL, GLC, x , x , 2013) >>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h >>> index d71872835d..406746a84c 100644 >>> --- a/src/mesa/main/mtypes.h >>> +++ b/src/mesa/main/mtypes.h >>> @@ -1280,6 +1280,8 @@ struct gl_transform_attrib >>> GLboolean RescaleNormals; /**< >>> GL_EXT_rescale_normal */ >>> GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip >>> */ >>> GLboolean DepthClamp; /**< GL_ARB_depth_clamp */ >>> + GLboolean DepthClampNear; /**< >>> GL_AMD_depth_clamp_separate */ >>> + GLboolean DepthClampFar; /**< >>> GL_AMD_depth_clamp_separate */ > > I think we actually need two more flags here: _DepthClampNear and > _DepthClampFar. The spec is a little unclear, so you may need to test > on some AMD closed-source drivers. Specifically, the spec says > > "In addition to DEPTH_CLAMP_NEAR_AMD and DEPTH_CLAMP_FAR_AMD, the > token DEPTH_CLAMP may be used to simultaneously enable or disable > depth clamping at both the near and far planes." > > Based on that, I'm not sure what you're supposed to get if you do: > > glDisable(GL_DEPTH_CLAMP_NEAR_AMD); > glEnable(GL_DEPTH_CLAMP); > glGetIntegerv(GL_DEPTH_CLAMP_NEAR_AMD, &v); > > Should v contain GL_TRUE or GL_FALSE? It seems clear that rendering > should have the near plane clamped. > > Depending on the results of testing on AMD drivers, we either need > enable / disable of GL_DEPTH_CLAMP to set / reset > gl_transform_attrib::DepthClampNear and > gl_transform_attrib::DepthClampFar or we need to add _DepthClampNear and > _DepthClampFar that get modified. In the latter case, the driver would > only ever look at _DepthClampNear and _DepthClampFar. > >>> /** GL_ARB_clip_control */ >>> GLenum16 ClipOrigin; /**< GL_LOWER_LEFT or GL_UPPER_LEFT */ >>> GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */ >>> @@ -4235,6 +4237,7 @@ struct gl_extensions >>> GLboolean OES_texture_view; >>> GLboolean OES_viewport_array; >>> /* vendor extensions */ >>> + GLboolean AMD_depth_clamp_separate; >>> GLboolean AMD_performance_monitor; >>> GLboolean AMD_pinned_memory; >>> GLboolean AMD_seamless_cubemap_per_texture; >>> @@ -4577,6 +4580,12 @@ struct gl_driver_flags >>> /** gl_context::Transform::DepthClamp */ >>> uint64_t NewDepthClamp; >>> >>> + /** gl_context::Transform::DepthClampNear */ >>> + uint64_t NewDepthClampNear; >>> + >>> + /** gl_context::Transform::DepthClampFar */ >>> + uint64_t NewDepthClampFar; >>> + > > I don't think we need separate flags for near and far. I think we can > just use the NewDepthClamp flag for all three.
I agree. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
