[Mesa-dev] [RFC][PATCH 0/5] mesa: Add types for AMD_depth_clamp_separate.
Sagar Ghuge
sagar.ghuge at intel.com
Tue Aug 21 00:11:18 UTC 2018
Thank you for volunteering to test my branch. But before I point you to the branch,
I will rework patches according to your comment on patch 3.
Again thanks a lot for your and Ian's input.
- Sagar
On 08/20/2018 04:21 PM, Marek Olšák wrote:
> 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