[Mesa-dev] [PATCH 07/12] glsl: Add arb_cull_distance support
Tobias Klausmann
tobias.johannes.klausmann at mni.thm.de
Mon Apr 4 22:27:15 UTC 2016
On 05.04.2016 00:06, Dave Airlie wrote:
> On 4 April 2016 at 23:07, Tobias Klausmann
> <tobias.johannes.klausmann at 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.klausmann at mni.thm.de>
>>>>
>>>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at 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.
Oh, i was not aware of that, apologies!
Tobias
More information about the mesa-dev
mailing list