On 12/02/2015 01:33 AM, Lofstedt, Marta wrote: >> -----Original Message----- >> From: mesa-dev [mailto:[email protected]] On >> Behalf Of Ian Romanick >> Sent: Tuesday, December 1, 2015 7:52 PM >> To: Marta Lofstedt; [email protected] >> 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 <[email protected]> >>> >>> This adds glsl support of GL_OES_geometry_shader for OpenGL ES 3.1. >>> >>> Signed-off-by: Marta Lofstedt <[email protected]> >>> --- >>> 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 <[email protected]> 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 <[email protected]> >> >>> 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 >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
