[Mesa-dev] [PATCH v3] glsl: add support for GL_OES_geometry_shader
Ian Romanick
idr at freedesktop.org
Thu Dec 3 17:06:38 PST 2015
On 12/02/2015 01:33 AM, Lofstedt, Marta wrote:
>> -----Original Message-----
>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
>> Behalf Of Ian Romanick
>> Sent: Tuesday, December 1, 2015 7:52 PM
>> To: Marta Lofstedt; mesa-dev at lists.freedesktop.org
>> Cc: Emil Velikov
>> Subject: Re: [Mesa-dev] [PATCH v3] glsl: add support for
>> GL_OES_geometry_shader
>>
>> On 12/01/2015 06:49 AM, Marta Lofstedt wrote:
>>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>>
>>> This adds glsl support of GL_OES_geometry_shader for OpenGL ES 3.1.
>>>
>>> Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
>>> ---
>>> src/glsl/builtin_variables.cpp | 25 +++++++++++++------------
>>> src/glsl/glsl_parser.yy | 4 ++--
>>> src/glsl/glsl_parser_extras.cpp | 1 +
>>> src/glsl/glsl_parser_extras.h | 7 +++++++
>>> 4 files changed, 23 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/src/glsl/builtin_variables.cpp
>>> b/src/glsl/builtin_variables.cpp index e8eab80..6f87600 100644
>>> --- a/src/glsl/builtin_variables.cpp
>>> +++ b/src/glsl/builtin_variables.cpp
>>> @@ -667,7 +667,7 @@ builtin_variable_generator::generate_constants()
>>> add_const("gl_MaxVaryingComponents", state->ctx-
>>> Const.MaxVarying * 4);
>>> }
>>>
>>> - if (state->is_version(150, 0)) {
>>> + if (state->has_geometry_shader()) {
>>> add_const("gl_MaxVertexOutputComponents",
>>> state->Const.MaxVertexOutputComponents);
>>> add_const("gl_MaxGeometryInputComponents",
>>> @@ -730,12 +730,11 @@ builtin_variable_generator::generate_constants()
>>> add_const("gl_MaxAtomicCounterBindings",
>>> state->Const.MaxAtomicBufferBindings);
>>>
>>> - /* When Mesa adds support for GL_OES_geometry_shader and
>>> - * GL_OES_tessellation_shader, this will need to change.
>>> - */
>>> - if (!state->es_shader) {
>>> + if (state->has_geometry_shader()) {
>>> add_const("gl_MaxGeometryAtomicCounters",
>>> state->Const.MaxGeometryAtomicCounters);
>>> + }
>>> + if (!state->es_shader) {
>>> add_const("gl_MaxTessControlAtomicCounters",
>>> state->Const.MaxTessControlAtomicCounters);
>>
>> I think you got a little over excited here. I don't see MaxTess*Atomic
>> anywhere in any GLES extension... not even in GL_EXT_tessellation_shader,
>> which seems like a bug in that extension spec.
>>
>> https://www.khronos.org/bugzilla/show_bug.cgi?id=1427
>>
> Ian, I am not sure I understand your comment. My patch enables gl_MaxGeometryAtomicCounters and gl_MaxGeometryAtomicCounterBuffers if you have geometry shader. These are part of the GL_OES_geometry_shader extension, but was previously not exposed for ES shaders.
>
> The "tess" stuff stays the same as it was before my patch, i.e. they are not exposed to ES shaders.
Yeah... I'm not sure I understand my comment either. I think I read the
patch "backwards"... as removing the second "if (!state->es_shader)"
instead of adding it. Re-reading it with a clearer head, this patch is
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
But I still think GL_EXT_tessellation_shader should add the
gl_MaxTess*Atomic built-in variables. :)
>>> add_const("gl_MaxTessEvaluationAtomicCounters",
>>> @@ -753,12 +752,11 @@ builtin_variable_generator::generate_constants()
>>> add_const("gl_MaxAtomicCounterBufferSize",
>>> state->Const.MaxAtomicCounterBufferSize);
>>>
>>> - /* When Mesa adds support for GL_OES_geometry_shader and
>>> - * GL_OES_tessellation_shader, this will need to change.
>>> - */
>>> - if (!state->es_shader) {
>>> + if (state->has_geometry_shader()) {
>>> add_const("gl_MaxGeometryAtomicCounterBuffers",
>>> state->Const.MaxGeometryAtomicCounterBuffers);
>>> + }
>>> + if(!state->es_shader) {
>>
>> Here too.
>>
>> With these two things reverted, this patch is
>>
>> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>>
>>> add_const("gl_MaxTessControlAtomicCounterBuffers",
>>> state->Const.MaxTessControlAtomicCounterBuffers);
>>> add_const("gl_MaxTessEvaluationAtomicCounterBuffers",
>>> @@ -814,13 +812,16 @@ builtin_variable_generator::generate_constants()
>>> add_const("gl_MaxCombinedImageUniforms",
>>> state->Const.MaxCombinedImageUniforms);
>>>
>>> + if (state->has_geometry_shader()) {
>>> + add_const("gl_MaxGeometryImageUniforms",
>>> + state->Const.MaxGeometryImageUniforms);
>>> + }
>>> +
>>> if (!state->es_shader) {
>>> add_const("gl_MaxCombinedImageUnitsAndFragmentOutputs",
>>> state->Const.MaxCombinedShaderOutputResources);
>>> add_const("gl_MaxImageSamples",
>>> state->Const.MaxImageSamples);
>>> - add_const("gl_MaxGeometryImageUniforms",
>>> - state->Const.MaxGeometryImageUniforms);
>>> }
>>>
>>> if (state->is_version(450, 310)) { @@ -1057,7 +1058,7 @@
>>> builtin_variable_generator::generate_fs_special_vars()
>>> if (state->is_version(120, 100))
>>> add_input(VARYING_SLOT_PNTC, vec2_t, "gl_PointCoord");
>>>
>>> - if (state->is_version(150, 0)) {
>>> + if (state->has_geometry_shader()) {
>>> var = add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, "gl_PrimitiveID");
>>> var->data.interpolation = INTERP_QUALIFIER_FLAT;
>>> }
>>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy index
>>> 5a8f980..7a875fa 100644
>>> --- a/src/glsl/glsl_parser.yy
>>> +++ b/src/glsl/glsl_parser.yy
>>> @@ -1262,7 +1262,7 @@ layout_qualifier_id:
>>> }
>>> }
>>>
>>> - if ($$.flags.i && !state->is_version(150, 0)) {
>>> + if ($$.flags.i && !state->has_geometry_shader()) {
>>> _mesa_glsl_error(& @1, state, "#version 150 layout "
>>> "qualifier `%s' used", $1);
>>> }
>>> @@ -1499,7 +1499,7 @@ layout_qualifier_id:
>>> if (match_layout_qualifier("max_vertices", $1, state) == 0) {
>>> $$.flags.q.max_vertices = 1;
>>> $$.max_vertices = new(ctx) ast_layout_expression(@1, $3);
>>> - if (!state->is_version(150, 0)) {
>>> + if (!state->has_geometry_shader()) {
>>> _mesa_glsl_error(& @3, state,
>>> "#version 150 max_vertices qualifier "
>>> "specified", $3); diff --git
>>> a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
>>> index 29cf0c6..cd0714e 100644
>>> --- a/src/glsl/glsl_parser_extras.cpp
>>> +++ b/src/glsl/glsl_parser_extras.cpp
>>> @@ -635,6 +635,7 @@ static const _mesa_glsl_extension
>> _mesa_glsl_supported_extensions[] = {
>>> /* OES extensions go here, sorted alphabetically.
>>> */
>>> EXT(OES_EGL_image_external, false, true,
>> OES_EGL_image_external),
>>> + EXT(OES_geometry_shader, false, true,
>> OES_geometry_shader),
>>> EXT(OES_standard_derivatives, false, true,
>> OES_standard_derivatives),
>>> EXT(OES_texture_3D, false, true, dummy_true),
>>> EXT(OES_texture_storage_multisample_2d_array, false, true,
>>> ARB_texture_multisample), diff --git a/src/glsl/glsl_parser_extras.h
>>> b/src/glsl/glsl_parser_extras.h index 17ff0b5..b47c540 100644
>>> --- a/src/glsl/glsl_parser_extras.h
>>> +++ b/src/glsl/glsl_parser_extras.h
>>> @@ -260,6 +260,11 @@ struct _mesa_glsl_parse_state {
>>> return ARB_compute_shader_enable || is_version(430, 310);
>>> }
>>>
>>> + bool has_geometry_shader() const
>>> + {
>>> + return OES_geometry_shader_enable || is_version(150, 320);
>>> + }
>>> +
>>> void process_version_directive(YYLTYPE *locp, int version,
>>> const char *ident);
>>>
>>> @@ -579,6 +584,8 @@ struct _mesa_glsl_parse_state {
>>> */
>>> bool OES_EGL_image_external_enable;
>>> bool OES_EGL_image_external_warn;
>>> + bool OES_geometry_shader_enable;
>>> + bool OES_geometry_shader_warn;
>>> bool OES_standard_derivatives_enable;
>>> bool OES_standard_derivatives_warn;
>>> bool OES_texture_3D_enable;
>>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list