[Mesa-dev] [RFC][PATCH 0/5] mesa: Add types for AMD_depth_clamp_separate.
Marek Olšák
maraeo at gmail.com
Mon Aug 20 23:21:47 UTC 2018
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.ghuge at 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 <idr at 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.ghuge at 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
> >
More information about the mesa-dev
mailing list