On Wed, Sep 2, 2015 at 2:20 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> In various versions of OpenGL and GLSL, it's possible to declare
> multiple VS input variables with aliasing attribute locations.
>
> So, when computing the storage requirements for vertex attributes,
> we can't simply add up the sizes. Instead, we need to look at the
> enabled slots.
>
> This patch begins tracking which attributes are double types that
> are larger than 128-bits (i.e. take up two vec4 slots). We then
> count normal attributes once, and count the double-size attributes
> a second time.
>
> Fixes deQP functional.attribute_location.bind_aliasing.max_cond_* tests
> on i965, which regressed with commit ad208d975a6d3aebe14f7c2c16039ee20.
This is all subtle stuff. I'd feel a whole lot better if there were
some piglit coverage for this.
>
> Cc: "10.6 11.0" <mesa-sta...@lists.freedesktop.org>
> Cc: Matt Turner <matts...@gmail.com.
> Cc: Ilia Mirkin <imir...@alum.mit.edu>
> Cc: Dave Airlie <airl...@gmail.com>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/glsl/linker.cpp | 64
> ++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 47f7d25..934062f 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -2339,6 +2339,7 @@ assign_attribute_or_color_locations(gl_shader_program
> *prog,
> */
> unsigned used_locations = (max_index >= 32)
> ? ~0 : ~((1 << max_index) - 1);
> + unsigned double_storage_locations = 0;
>
> assert((target_index == MESA_SHADER_VERTEX)
> || (target_index == MESA_SHADER_FRAGMENT));
> @@ -2452,34 +2453,6 @@ assign_attribute_or_color_locations(gl_shader_program
> *prog,
>
> const unsigned slots = var->type->count_attribute_slots();
>
> - /* From GL4.5 core spec, section 11.1.1 (Vertex Attributes):
> - *
> - * "A program with more than the value of MAX_VERTEX_ATTRIBS active
> - * attribute variables may fail to link, unless device-dependent
> - * optimizations are able to make the program fit within available
> - * hardware resources. For the purposes of this test, attribute
> variables
> - * of the type dvec3, dvec4, dmat2x3, dmat2x4, dmat3, dmat3x4, dmat4x3,
> - * and dmat4 may count as consuming twice as many attributes as
> equivalent
> - * single-precision types. While these types use the same number of
> - * generic attributes as their single-precision equivalents,
> - * implementations are permitted to consume two single-precision
> vectors
> - * of internal storage for each three- or four-component
> double-precision
> - * vector."
> - * Until someone has a good reason in Mesa, enforce that now.
> - */
> - if (target_index == MESA_SHADER_VERTEX) {
> - total_attribs_size += slots;
> - if (var->type->without_array() == glsl_type::dvec3_type ||
> - var->type->without_array() == glsl_type::dvec4_type ||
> - var->type->without_array() == glsl_type::dmat2x3_type ||
> - var->type->without_array() == glsl_type::dmat2x4_type ||
> - var->type->without_array() == glsl_type::dmat3_type ||
> - var->type->without_array() == glsl_type::dmat3x4_type ||
> - var->type->without_array() == glsl_type::dmat4x3_type ||
> - var->type->without_array() == glsl_type::dmat4_type)
> - total_attribs_size += slots;
> - }
> -
> /* If the variable is not a built-in and has a location statically
> * assigned in the shader (presumably via a layout qualifier), make
> sure
> * that it doesn't collide with other assigned locations. Otherwise,
> @@ -2594,6 +2567,38 @@ assign_attribute_or_color_locations(gl_shader_program
> *prog,
> }
>
> used_locations |= (use_mask << attr);
> +
> + /* From the GL 4.5 core spec, section 11.1.1 (Vertex Attributes):
> + *
> + * "A program with more than the value of MAX_VERTEX_ATTRIBS
> + * active attribute variables may fail to link, unless
> + * device-dependent optimizations are able to make the program
> + * fit within available hardware resources. For the purposes
> + * of this test, attribute variables of the type dvec3, dvec4,
> + * dmat2x3, dmat2x4, dmat3, dmat3x4, dmat4x3, and dmat4 may
> + * count as consuming twice as many attributes as equivalent
> + * single-precision types. While these types use the same number
> + * of generic attributes as their single-precision equivalents,
> + * implementations are permitted to consume two single-precision
> + * vectors of internal storage for each three- or four-component
> + * double-precision vector."
> + *
> + * Mark this attribute slot as taking up twice as much space
> + * so we can count it properly against limits. According to
> + * issue (3) of the GL_ARB_vertex_attrib_64bit behavior, this
> + * is optional behavior, but it seems preferable.
> + */
> + const glsl_type *type = var->type->without_array();
> + if (type == glsl_type::dvec3_type ||
> + type == glsl_type::dvec4_type ||
> + type == glsl_type::dmat2x3_type ||
> + type == glsl_type::dmat2x4_type ||
> + type == glsl_type::dmat3_type ||
> + type == glsl_type::dmat3x4_type ||
> + type == glsl_type::dmat4x3_type ||
> + type == glsl_type::dmat4_type) {
> + double_storage_locations |= (use_mask << attr);
> + }
> }
>
> continue;
> @@ -2605,6 +2610,9 @@ assign_attribute_or_color_locations(gl_shader_program
> *prog,
> }
>
> if (target_index == MESA_SHADER_VERTEX) {
> + unsigned total_attribs_size =
> + _mesa_bitcount(used_locations & ((1 << max_index) - 1)) +
> + _mesa_bitcount(double_storage_locations);
Does this also need the & ((1 << max_index) - 1) treatment? [Well, not
_need_ but seems like it'd avoid needlessly counting things?]
> if (total_attribs_size > max_index) {
> linker_error(prog,
> "attempt to use %d vertex attribute slots only %d
> available ",
> --
> 2.5.0
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
