I can try to test the extension with the radeonsi driver. Do you have a Mesa branch with the final patches?
Marek On Mon, Aug 13, 2018 at 5:35 PM Sagar Ghuge <sagar.gh...@intel.com> wrote: > > Hi everyone, > > I am kind of stuck on this part actually. I don't have > latest AMD graphics card to test following behavior which > Ian and Marek suggested me. > > I have written a piglit test : > https://gitlab.freedesktop.org/sagarghuge/piglit/blob/320b91ffb131b380f1d27d9c05ab141e0cd9e557/tests/spec/amd_depth_clamp_separate/depth_clamp_get_test.c > > It would be great if someone can help me or test it in their > spare time on latest AMD graphics card and provide some input > on the extension behavior on AMD's closed source driver. > > > On 08/09/2018 01:11 PM, Marek Olšák wrote: > > 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
