[Mesa-dev] [PATCH 07/12] glsl: Add arb_cull_distance support
Dave Airlie
airlied at gmail.com
Mon Apr 4 22:06:47 UTC 2016
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.
More information about the mesa-dev
mailing list