On 4 April 2016 at 23:07, Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de> wrote: > > > On 04.04.2016 04:48, Timothy Arceri wrote: >> >> On Mon, 2016-04-04 at 12:15 +1000, Dave Airlie wrote: >>> >>> From: Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de> >>> >>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de >>> --- >>> src/compiler/glsl/ast_to_hir.cpp | 14 +++ >>> src/compiler/glsl/builtin_variables.cpp | 11 ++- >>> src/compiler/glsl/glcpp/glcpp-parse.y | 3 + >>> src/compiler/glsl/glsl_parser_extras.cpp | 1 + >>> src/compiler/glsl/glsl_parser_extras.h | 2 + >>> src/compiler/glsl/link_varyings.cpp | 10 +++ >>> src/compiler/glsl/link_varyings.h | 1 + >>> src/compiler/glsl/linker.cpp | 122 >>> +++++++++++++++++++++------ >>> src/compiler/glsl/standalone_scaffolding.cpp | 1 + >>> src/compiler/glsl/tests/varyings_test.cpp | 27 ++++++ >>> src/compiler/shader_enums.h | 4 + >>> 11 files changed, 169 insertions(+), 27 deletions(-) >>> >>> diff --git a/src/compiler/glsl/ast_to_hir.cpp >>> b/src/compiler/glsl/ast_to_hir.cpp >>> index 7c9be81..47db841 100644 >>> --- a/src/compiler/glsl/ast_to_hir.cpp >>> +++ b/src/compiler/glsl/ast_to_hir.cpp >>> @@ -1210,6 +1210,20 @@ check_builtin_array_max_size(const char *name, >>> unsigned size, >>> _mesa_glsl_error(&loc, state, "`gl_ClipDistance' array size >>> cannot " >>> "be larger than gl_MaxClipDistances (%u)", >>> state->Const.MaxClipPlanes); >>> + } else if (strcmp("gl_CullDistance", name) == 0 >>> + && size > state->Const.MaxClipPlanes) { >>> + /* From the ARB_cull_distance spec: >>> + * >>> + * "The gl_CullDistance array is predeclared as unsized and >>> + * must be sized by the shader either redeclaring it with >>> + * a size or indexing it only with integral constant >>> + * expressions. The size determines the number and set of >>> + * enabled cull distances and can be at most >>> + * gl_MaxCullDistances." >>> + */ >>> + _mesa_glsl_error(&loc, state, "`gl_CullDistance' array size >>> cannot " >>> + "be larger than gl_MaxCullDistances (%u)", >>> + state->Const.MaxClipPlanes); >>> } >>> } >>> diff --git a/src/compiler/glsl/builtin_variables.cpp >>> b/src/compiler/glsl/builtin_variables.cpp >>> index f31f9f6..8d049c8 100644 >>> --- a/src/compiler/glsl/builtin_variables.cpp >>> +++ b/src/compiler/glsl/builtin_variables.cpp >>> @@ -302,7 +302,7 @@ public: >>> const glsl_type *construct_interface_instance() const; >>> private: >>> - glsl_struct_field fields[10]; >>> + glsl_struct_field fields[11]; >>> unsigned num_fields; >>> }; >>> @@ -675,6 +675,11 @@ builtin_variable_generator::generate_constants() >>> add_const("gl_MaxClipDistances", state->Const.MaxClipPlanes); >>> add_const("gl_MaxVaryingComponents", state->ctx- >>>> >>>> Const.MaxVarying * 4); >>> >>> } >>> + if (state->is_version(450, 0) || state->ARB_cull_distance_enable) >>> { >>> + add_const("gl_MaxCullDistances", state->Const.MaxClipPlanes); >>> + add_const("gl_MaxCombinedClipAndCullDistances", >>> + state->Const.MaxClipPlanes); >>> + } >>> if (state->has_geometry_shader()) { >>> add_const("gl_MaxVertexOutputComponents", >>> @@ -1246,6 +1251,10 @@ >>> builtin_variable_generator::generate_varyings() >>> add_varying(VARYING_SLOT_CLIP_DIST0, array(float_t, 0), >>> "gl_ClipDistance"); >>> } >>> + if (state->is_version(450, 0) || state->ARB_cull_distance_enable) >>> { >>> + add_varying(VARYING_SLOT_CULL_DIST0, array(float_t, 0), >>> + "gl_CullDistance"); >>> + } >>> if (compatibility) { >>> add_varying(VARYING_SLOT_TEX0, array(vec4_t, 0), >>> "gl_TexCoord"); >>> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y >>> b/src/compiler/glsl/glcpp/glcpp-parse.y >>> index a48266c..e44f074 100644 >>> --- a/src/compiler/glsl/glcpp/glcpp-parse.y >>> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y >>> @@ -2457,6 +2457,9 @@ >>> _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, >>> intmax_t versio >>> if (extensions->ARB_shader_draw_parameters) >>> add_builtin_define(parser, >>> "GL_ARB_shader_draw_parameters", 1); >>> + >>> + if (extensions->ARB_cull_distance) >>> + add_builtin_define(parser, "GL_ARB_cull_distance", 1); >>> } >>> } >>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp >>> b/src/compiler/glsl/glsl_parser_extras.cpp >>> index 76321aa..9b1d53f 100644 >>> --- a/src/compiler/glsl/glsl_parser_extras.cpp >>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp >>> @@ -569,6 +569,7 @@ static const _mesa_glsl_extension >>> _mesa_glsl_supported_extensions[] = { >>> EXT(ARB_arrays_of_arrays, true, false, ARB_array >>> s_of_arrays), >>> EXT(ARB_compute_shader, true, false, ARB_compu >>> te_shader), >>> EXT(ARB_conservative_depth, true, false, ARB_conse >>> rvative_depth), >>> + EXT(ARB_cull_distance, true, false, ARB_cull_ >>> distance), >>> EXT(ARB_derivative_control, true, false, ARB_deriv >>> ative_control), >>> EXT(ARB_draw_buffers, true, false, dummy_tru >>> e), >>> EXT(ARB_draw_instanced, true, false, ARB_draw_ >>> instanced), >>> diff --git a/src/compiler/glsl/glsl_parser_extras.h >>> b/src/compiler/glsl/glsl_parser_extras.h >>> index c774fbe..85a8ebf 100644 >>> --- a/src/compiler/glsl/glsl_parser_extras.h >>> +++ b/src/compiler/glsl/glsl_parser_extras.h >>> @@ -518,6 +518,8 @@ struct _mesa_glsl_parse_state { >>> bool ARB_compute_shader_warn; >>> bool ARB_conservative_depth_enable; >>> bool ARB_conservative_depth_warn; >>> + bool ARB_cull_distance_enable; >>> + bool ARB_cull_distance_warn; >>> bool ARB_derivative_control_enable; >>> bool ARB_derivative_control_warn; >>> bool ARB_draw_buffers_enable; >>> diff --git a/src/compiler/glsl/link_varyings.cpp >>> b/src/compiler/glsl/link_varyings.cpp >>> index 8e74981..d4cc68f 100644 >>> --- a/src/compiler/glsl/link_varyings.cpp >>> +++ b/src/compiler/glsl/link_varyings.cpp >>> @@ -573,6 +573,10 @@ tfeedback_decl::init(struct gl_context *ctx, >>> const void *mem_ctx, >>> strcmp(this->var_name, "gl_ClipDistance") == 0) { >>> this->lowered_builtin_array_variable = clip_distance; >>> } >>> + if (ctx- >>>> >>>> Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].LowerCombinedClipCul >>> >>> lDistance && >>> + strcmp(this->var_name, "gl_CullDistance") == 0) { >>> + this->lowered_builtin_array_variable = cull_distance; >>> + } >>> if (ctx->Const.LowerTessLevel && >>> (strcmp(this->var_name, "gl_TessLevelOuter") == 0)) >>> @@ -633,6 +637,9 @@ tfeedback_decl::assign_location(struct gl_context >>> *ctx, >>> case clip_distance: >>> actual_array_size = prog->LastClipDistanceArraySize; >>> break; >>> + case cull_distance: >>> + actual_array_size = prog->LastCullDistanceArraySize; >>> + break; >>> case tess_level_outer: >>> actual_array_size = 4; >>> break; >>> @@ -853,6 +860,9 @@ tfeedback_decl::find_candidate(gl_shader_program >>> *prog, >>> case clip_distance: >>> name = "gl_ClipDistanceMESA"; >>> break; >>> + case cull_distance: >>> + name = "gl_CullDistanceMESA"; >>> + break; >>> case tess_level_outer: >>> name = "gl_TessLevelOuterMESA"; >>> break; >>> diff --git a/src/compiler/glsl/link_varyings.h >>> b/src/compiler/glsl/link_varyings.h >>> index 543b80f..0ad4f74 100644 >>> --- a/src/compiler/glsl/link_varyings.h >>> +++ b/src/compiler/glsl/link_varyings.h >>> @@ -210,6 +210,7 @@ private: >>> enum { >>> none, >>> clip_distance, >>> + cull_distance, >>> tess_level_outer, >>> tess_level_inner, >>> } lowered_builtin_array_variable; >>> diff --git a/src/compiler/glsl/linker.cpp >>> b/src/compiler/glsl/linker.cpp >>> index 9acf253..77142aa 100644 >>> --- a/src/compiler/glsl/linker.cpp >>> +++ b/src/compiler/glsl/linker.cpp >>> @@ -641,19 +641,25 @@ link_invalidate_variable_locations(exec_list >>> *ir) >>> /** >>> - * Set clip_distance_array_size based on the given shader. >>> + * Set clip_distance_array_size based and cull_distance_array_size >>> on the given >>> + * shader. >>> * >>> * Also check for errors based on incorrect usage of gl_ClipVertex >>> and >>> - * gl_ClipDistance. >>> + * gl_ClipDistance and gl_CullDistance. >>> + * Additionally test whether the arrays gl_ClipDistance and >>> gl_CullDistance >>> + * exceed the maximum size defined by >>> gl_MaxCombinedClipAndCullDistances. >>> * >>> * Return false if an error was reported. >>> */ >>> static void >>> -analyze_clip_usage(struct gl_shader_program *prog, >>> - struct gl_shader *shader, >>> - GLuint *clip_distance_array_size) >>> +analyze_clip_cull_usage(struct gl_shader_program *prog, >>> + struct gl_shader *shader, >>> + struct gl_context *ctx, >>> + GLuint *clip_distance_array_size, >>> + GLuint *cull_distance_array_size) >>> { >>> *clip_distance_array_size = 0; >>> + *cull_distance_array_size = 0; >>> if (!prog->IsES && prog->Version >= 130) { >>> /* From section 7.1 (Vertex Shader Special Variables) of the >>> @@ -667,22 +673,71 @@ analyze_clip_usage(struct gl_shader_program >>> *prog, >>> */ >>> find_assignment_visitor clip_vertex("gl_ClipVertex"); >>> find_assignment_visitor clip_distance("gl_ClipDistance"); >>> + find_assignment_visitor cull_distance("gl_CullDistance"); >>> clip_vertex.run(shader->ir); >>> clip_distance.run(shader->ir); >>> + cull_distance.run(shader->ir); >>> + >>> + /* From the ARB_cull_distance spec: >>> + * >>> + * It is a compile-time or link-time error for the set of >>> shaders forming >>> + * a program to statically read or write both gl_ClipVertex >>> and either >>> + * gl_ClipDistance or gl_CullDistance. >>> + * >>> + * This does not apply to GLSL ES shaders, since GLSL ES >>> defines neither >>> + * gl_ClipVertex, gl_ClipDistance or gl_CullDistance. >>> + */ >>> if (clip_vertex.variable_found() && >>> clip_distance.variable_found()) { >>> linker_error(prog, "%s shader writes to both >>> `gl_ClipVertex' " >>> "and `gl_ClipDistance'\n", >>> _mesa_shader_stage_to_string(shader->Stage)); >>> return; >>> } >>> + if (clip_vertex.variable_found() && >>> cull_distance.variable_found()) { >>> + linker_error(prog, "%s shader writes to both >>> `gl_ClipVertex' " >>> + "and `gl_CullDistance'\n", >>> + _mesa_shader_stage_to_string(shader->Stage)); >>> + return; >>> + } >>> if (clip_distance.variable_found()) { >>> ir_variable *clip_distance_var = >>> - shader->symbols->get_variable("gl_ClipDistance"); >>> - >>> + shader->symbols->get_variable("gl_ClipDistance"); >>> assert(clip_distance_var); >>> - *clip_distance_array_size = clip_distance_var->type- >>>> >>>> length; >>> >>> + if (clip_distance_var->type->was_unsized) >>> + *clip_distance_array_size = >>> clip_distance.variable_found() ? >>> + clip_distance_var->type- >>>> >>>> length : >>> >>> + clip_distance_var->type- >>>> >>>> length -1; >>> >>> + else >>> + *clip_distance_array_size = clip_distance_var->type- >>>> >>>> length; >>> >>> + } >>> + if (cull_distance.variable_found()) { >>> + ir_variable *cull_distance_var = >>> + shader->symbols->get_variable("gl_CullDistance"); >>> + assert(cull_distance_var); >>> + if (cull_distance_var->type->was_unsized) >>> + *cull_distance_array_size = >>> cull_distance.variable_found() ? >>> + cull_distance_var->type- >>>> >>>> length : >>> >>> + cull_distance_var->type- >>>> >>>> length -1; >>> >>> + else >>> + *cull_distance_array_size = cull_distance_var->type- >>>> >>>> length; >> >> I think we need a few piglit test cases for >> the gl_MaxCombinedClipAndCullDistance test below. The above code makes >> no sense. >> >> We are inside an if cull_distance.variable_found() >> >> Then we check for cull_distance_var->type->was_unsized >> >> and if cull_distance.variable_found() we do: >> >> *cull_distance_array_size = cull_distance_var->type->length; >> >> So in otherwords we always just do >> >> *cull_distance_array_size = cull_distance_var->type->length; >> >> The was_unsized makes no difference same goes for the clip_distance >> changes above. I don't think this is intended. >> > > It was intended back then to fix an off-by-one for the combined size, but i > can't remember the actual test i fixed with that (there are 5 additional > piglit tests never got up-streamed for cull-distance, maybe it was one of > those i'll check when i got some time)
They got pushed yesterday. Dave. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev