There are two distinctly different uses of this struct. The first
is to store GL shader objects. The second is to store information
about a shader stage thats been linked.
The two uses actually share few fields and there is clearly confusion
about their use. For example the linked shaders map one to one with
a program so can simply be destroyed along with the program. However
previously we were calling reference counting on the linked shaders.
We were also creating linked shaders with a name even though it
is always 0 and called the driver version of the _mesa_new_shader()
function unnecessarily for GL shader objects.
Acked-by: Iago Toral Quiroga <itoral@igalia.com>
The GL4.5 spec quote seems clear on this:
"The value -1 will be returned by either command if an error occurs,
if name does not identify an active variable on programInterface,
or if name identifies an active variable that does not have a valid
location assigned, as described above."
This fixes:
GL45-CTS.program_interface_query.output-built-in
[airlied: use _mesa_program_resource_location_index as
suggested by Eduardo]
Reviewed-by: Eduardo Lima Mitev <elima@igalia.com>
Cc: "12.0" <mesa-stable@lists.freedesktop.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The string "[0]\0" is the same as "[0]" as far as the C string datatype
is concerned. That string has length 3. strncmp(s, length_3_string, 4)
is the same as strcmp(s, length_3_string), so make it be strcmp.
v2: Not the same as strncmp(..., 3). Noticed by Ilia.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Cc: "12.0" <mesa-stable@lists.freedesktop.org>
The spec says gl_NextBuffer and gl_SkipComponents need to be
returned to userspace in the program interface queries.
We currently throw those away, this requires a complete piglit
run to make sure no drivers fallover due to the extra varyings.
This fixes:
GL45-CTS.program_interface_query.transform-feedback-built-in
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Fixes the following dEQP tests on SKL:
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_vertex_smooth_fragment_flat
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_implicit_explicit_location_1
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_array_element_type
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_vertex_flat_fragment_none
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_struct_member_order
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_struct_member_type
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_vertex_centroid_fragment_flat
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_array_length
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_type
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_struct_member_precision
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_explicit_location_type
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_vertex_flat_fragment_centroid
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_explicit_location
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_vertex_flat_fragment_smooth
dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_struct_member_name
It regresses one test:
dEQP-GLES31.functional.separate_shader.validation.varying.match_different_struct_names
Hoever, this test is based on language in the OpenGL ES 3.1 spec that I
believe is incorrect. I have already submitted a spec bug:
https://www.khronos.org/bugzilla/show_bug.cgi?id=1500
v2: Move spec quote about built-in variables to the first place where
it's relevant. Suggested by Alejandro.
v3: Move patch earlier in series, fix rebase issues.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com> [v2]
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com> [v2]
From Section 7.3.1.1 (Naming Active Resources) of the OpenGL 4.5 spec:
"For the property LOCATION_COMPONENT, a single integer indicating the first
component of the location assigned to an active input or output variable is
written to params. For input and output variables with a component specified
by a layout qualifier, the specified component is written. For all other
input and output variables, the value zero is written."
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
With this change we create the UBO and SSBO arrays separately from the
beginning rather than putting them into a combined array and splitting
it apart later.
A bug is with UBO and SSBO stage reference querying is also fixed as
we now use the block index to lookup the references in the separate arrays
not the combined buffer block array.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
This allows us to simplify the code and drop InterfaceBlockStageIndex
which is a per stage array of integers the size of all blocks in the
program combined including duplicates across stages. Adding a stage
ref per block will use less memory.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
With SSO, the GL_PROGRAM_INPUT and GL_PROGRAM_OUTPUT interfaces refer to
the first and last shader stage linked into a program. This may not be
the vertex and fragment shader stages.
So, subtracting VERT_ATTRIB_GENERIC0 and FRAG_RESULT_DATA0 is bogus.
We need to subtract VERT_ATTRIB_GENERIC0 for VS inputs,
FRAG_RESULT_DATA0 for FS outputs, and VARYING_SLOT_VAR0 for other cases.
Note that built-in variables get a location of -1.
Fixes 4 dEQP-GLES31.functional.program_interface_query tests:
- program_input.location.separable_fragment.var_explicit_location
- program_input.location.separable_fragment.var_array_explicit_location
- program_output.location.separable_vertex.var_array_explicit_location
- program_output.location.separable_vertex.var_array_explicit_location
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
We were recording locations for all variables, even ones without an
explicit location set. Implement the rules from the spec, and record
-1 in the resource list accordngly. Make program_resource_location
stop doing math on negative values. Remove hacks that are no longer
necessary now that we've stopped doing that.
Fixes 4 dEQP-GLES31.functional.program_interface_query tests:
- program_input.location.separable_fragment.var
- program_input.location.separable_fragment.var_array
- program_output.location.separable_vertex.var_array
- program_output.location.separable_vertex.var_array
v2: Delete more code
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
A program will either have gl_VertexID or gl_VertexIDMESA (the lowered
zero-based version), not both. Just spoof it in the resource list so
the hacks are done in a single place.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
System values are just built-in input variables that we've opted to
special-case out of convenience. We need to consider all inputs,
regardless of how we've classified them.
Unfortunately, there's one exception: we shouldn't add gl_BaseVertex
unless ARB_shader_draw_parameters is enabled, because it doesn't
actually exist in the language, and shouldn't be counted in the
GL_ACTIVE_RESOURCES query.
Fixes dEQP-GLES31.functional.program_interface_query.program_input.
resource_list.compute.empty, which expects gl_NumWorkGroups to appear
in the resource list.
v2: Delete more code
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Patch changes linker to allocate gl_shader_variable instead of using
ir_variable. This makes it possible to get rid of ir_variables and ir
in memory after linking.
v2: check that we do not create duplicate entries with
packed varyings
v3: document 'patch' bit (Ilia Mirkin)
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
This fixes some piglit subtests for ARB_program_interface_query.
V3: remove some of the unnecessary parentheses
V2: fix alignment
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Patch makes following changes for interface matching:
- do not try to match builtin variables
- handle swizzle in input name, as example 'a.z' should
match with 'a'
- add matching by location
- check that amount of inputs and outputs matches
These changes make interface matching tests to work in:
ES31-CTS.sepshaderobjs.StateInteraction
The test still does not pass completely due to errors in rendering
output. IMO this is unrelated to interface matching.
Note that type matching is not done due to varying packing which
changes type of variable, this can be added later on. Preferably
when we have quicker way to iterate resources and have a complete
list of all existed varyings (before packing) available.
v2: add spec reference, return true on desktop since we do not
have failing cases for it, inputs and outputs amount do not
need to match on desktop.
v3: add some more spec reference, remove desktop specifics since
not used for now on desktop, add match by location qualifier,
rename input_stage and output_stage as producer and consumer
as suggested by Timothy.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
If we have a dmat2[4], then dmat2[0] is at 17, dmat2[1] at 19,
dmat2[2] at 21 etc. The old code was returning 17,18,19.
I think this code is also wrong for float matricies as well.
There is now a piglit for the float case.
This partly fixes:
GL41-CTS.vertex_attrib_64bit.limits_test
[airlied: update with Tapani suggestion to clean it up].
Cc: "11.0 11.1" <mesa-stable@lists.freedesktop.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This function is unfinished there is a bunch more validation rules
that need to be applied here. We will still want to call it for desktop
GL we just don't want to validate precision so move the ES check to
reflect this.
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
ARB_explicit_uniform_location allows the index for subroutine functions
to be explicitly set in the shader.
This patch reduces the restriction on the index qualifier in
validate_layout_qualifiers() to allow it to be applied to subroutines
and adds the new subroutine qualifier validation to ast_function::hir().
ast_fully_specified_type::has_qualifiers() is updated to allow the
index qualifier on subroutine functions when explicit uniform locations
is available.
A new check is added to ast_type_qualifier::merge_qualifier() to stop
multiple function qualifiers from being defied, before this patch this
would cause a segfault.
Finally a new variable is added to ir_function_signature to store the
index. This value is validated and the non explicit values assigned in
link_assign_subroutine_types().
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Precision qualifier should be ignored on desktop OpenGL.
v2: include spec quote (Samuel)
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
gl_active_atomic_buffer contains index to UniformStorage, we need to
calculate resource index for that gl_uniform_storage.
Fixes following CTS tests:
ES31-CTS.program_interface_query.atomic-counters
ES31-CTS.program_interface_query.atomic-counters-one-buffer
No Piglit regressions.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Marta Lofstedt <marta.lofstedt@intel.com>
Commit 4565b6f did not update the basename match's check for
the case that string would exactly match the name of the
variable if the suffix "[0]" were appended to it.
Fixes two dEQP-GLES31 tests:
dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array
dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element
v2:
- Change the position of rname_has_array_index_zero to avoid an out-of-bounds
read. Reported by Tapani Pälli.
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
From ARB_program_query_interface spec:
"uint GetProgramResourceIndex(uint program, enum programInterface,
const char *name);
[...]
If <name> exactly matches the name string of one of the active resources
for <programInterface>, the index of the matched resource is returned.
Additionally, if <name> would exactly match the name string of an active
resource if "[0]" were appended to <name>, the index of the matched
resource is returned. [...]"
"A string provided to GetProgramResourceLocation or
GetProgramResourceLocationIndex is considered to match an active variable
if:
[...]
* if the string identifies the base name of an active array, where the
string would exactly match the name of the variable if the suffix
"[0]" were appended to the string;
[...]
"
Fixes the following two dEQP-GLES31 tests:
dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array
dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element
v2:
- Add AoA support (Timothy)
- Apply it too for GetUniformLocation(), GetUniformName() and others
because ARB_program_interface_query says that they are equivalent
to GetProgramResourceLocation() and GetProgramResourceName() (Tapani)
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Patch adds additional check to make sure we don't return locations for
structures or arrays of structures.
From page 79 of the OpenGL 4.2 spec:
"A valid name cannot be a structure, an array of structures, or any
portion of a single vector or a matrix."
v2: use without-array() to simplify code (Timothy)
No Piglit or CTS regressions observed.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Patch moves existing calculation code from shader_query.cpp to happen
during program resource list creation.
No Piglit or CTS regressions were observed during testing.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Currently, these arrays in gl_shader and gl_shader_program hold both
UBOs and SSBOs, so this looks like a better name. We were already
using NumBufferInterfaceBlocks in gl_shader_program, so this makes
things more consistent as well.
In a later patch we will add {Num}UniformBlocks and
{Num}ShaderStorageBlocks which will contain only references to
UBOs and SSBOs respectively that will provide backends with
a separate index space for both types of objects.
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
From ARB_program_query_interface:
For the property ARRAY_SIZE, a single integer identifying the number of
active array elements of an active variable is written to <params>. The
array size returned is in units of the type associated with the property
TYPE. For active variables not corresponding to an array of basic types,
the value one is written to <params>. If the variable is a shader
storage block member in an array with no declared size, the value zero
is written to <params>.
v2:
- Unsized arrays of arrays have an array size different than zero
v3:
- Arrays and unsized arrays will have an array_stride > 0. Use it
instead of is_unsized_array flag (Timothy).
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Timothy Arceri <t_arceri@yahoo.com.au>
From ARB_shader_storage_buffer_object:
"When using the ARB_program_interface_query extension to enumerate the
set of active buffer variables, only the first element of arrays (sized
or unsized) will be enumerated"
_mesa_program_resource_array_size() is used when getting the name (and
name length) of the active variables. When it is an unsized array,
we want to indicate it has one active element so the returned name
would have "[0]" at the end.
v2:
- Use array_stride > 0 and array_elements == 0 to detect unsized
arrays. Because of that, we don't need is_unsized_array flag
(Timothy)
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Timothy Arceri <t_arceri@yahoo.com.au>
When the active variable is an array which is already a top-level
shader storage block member, don't return its array size and stride
when querying TOP_LEVEL_ARRAY_SIZE and TOP_LEVEL_ARRAY_STRIDE
respectively.
Fixes the following 12 dEQP-GLES31 tests:
dEQP-GLES31.functional.ssbo.layout.single_basic_array.shared.mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.shared.row_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.shared.column_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.packed.mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.packed.row_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.packed.column_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.std140.mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.std140.row_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.std140.column_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.std430.mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.std430.row_major_mat3x4
dEQP-GLES31.functional.ssbo.layout.single_basic_array.std430.column_major_mat3x4
v2:
- Fix check when the shader storage block is instanced
- Write auxiliary function to do the check.
v3:
- Check if full_instanced_name is NULL just after allocation (Ilia)
- Remove () from one strcmp() in the if statement (Ilia)
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Tested-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Since we store both in UniformBlocks, we can't just compute the index by
subtracting the array address start, we need to count the number of
buffers of the approriate type.
v2:
- Just fall back to calc_resource_index (Tapani)
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Patch also refactors name length queries which were using array size
in computation, this has to be done in same time to avoid regression in
arb_program_interface_query-resource-query Piglit test.
Fixes rest of the failures with
ES31-CTS.program_interface_query.no-locations
v2: make additional check only for GS inputs
v3: create helper function for resource name length
so that it gets calculated only in one place
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
v2:
- Add strndup.h to Makefile.sources (Emil)
- Use calloc instead of malloc (Emil).
- Check if allocation fails (Emil, Jose)
- Add '#pragma once' and include stdlib.h to strndup.h (Jose)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92124
Reviewed-by: Jose Fonseca <jfonseca@vmware.com>
