Rob Clark <robdcl...@gmail.com> writes: > diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp > b/src/mesa/state_tracker/st_glsl_to_nir.cpp > new file mode 100644 > index 0000000..c15c537 > --- /dev/null > +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +static void
> +st_nir_assign_uniform_locations(struct gl_program *prog,
> + struct gl_shader_program *shader_program,
> + struct exec_list *uniform_list, unsigned
> *size)
> +{
> + int max = 0;
> + int shaderidx = 0;
> +
> + nir_foreach_variable(uniform, uniform_list) {
> + int loc;
> +
> + if (uniform->type->is_sampler()) {
> + unsigned val;
> + bool found = shader_program->UniformHash->get(val, uniform->name);
> + loc = shaderidx++;
> + assert(found);
> + /* this ensure that nir_lower_samplers looks at the correct
> + * shader_program->UniformStorage[location]:
> + */
> + uniform->data.location = val;
> + } else if (strncmp(uniform->name, "gl_", 3) == 0) {
> + const gl_state_index *const stateTokens = (gl_state_index
> *)uniform->state_slots[0].tokens;
> + /* This state reference has already been setup by ir_to_mesa, but
> we'll
> + * get the same index back here.
> + */
> + loc = _mesa_add_state_reference(prog->Parameters, stateTokens);
> + } else {
> + loc = _mesa_lookup_parameter_index(prog->Parameters, uniform->name);
> + }
> +
> + /* is there a better way to do this? If we have something like:
> + *
> + * struct S {
> + * float f;
> + * vec4 v;
> + * };
> + * uniform S color;
> + *
> + * Then what we get in prog->Parameters looks like:
> + *
> + * 0: Name=color.f, Type=6, DataType=1406, Size=1
> + * 1: Name=color.v, Type=6, DataType=8b52, Size=4
> + *
> + * So the name doesn't match up and _mesa_lookup_parameter_index()
> + * fails. In this case just find the first matching "color.*"..
> + *
> + * Note for arrays you could end up w/ color[n].f, for example.
> + */
> + if (loc < 0) {
> + int namelen = strlen(uniform->name);
> + for (unsigned i = 0; i < prog->Parameters->NumParameters; i++) {
> + struct gl_program_parameter *p =
> &prog->Parameters->Parameters[i];
> + if ((strncmp(p->Name, uniform->name, namelen) == 0) &&
> + ((p->Name[namelen] == '.') || (p->Name[namelen] == '['))) {
> + loc = i;
> + break;
> + }
> + }
> + }
Yep, this looks like what you're stuck with. You could make a helper
with a name related to _mesa_lookup_parameter_index(), or just always
use the block instead of _mesa_lookup_parameter_index(), I think, but
meh.
> + uniform->data.driver_location = loc;
> +
> + /*
> + * UBO's have their own address spaces, so don't count them towards the
> + * number of global uniforms
> + */
> + if ((uniform->data.mode == nir_var_uniform || uniform->data.mode ==
> nir_var_shader_storage) &&
> + uniform->interface_type != NULL)
> + continue;
Why are we setting the driver_location according to a Parameters[]
lookup for UBOs? I would think would skip that step entirely. i965
also seemed to skip the driver_location setup for them.
With this UBO question answered somehow, I think I'll just give this
patch an acked-by. I've read through it for quite some time, but it's a
huge change and most of what I care about for review in it is "does a
driver converting to this new path not regress?"
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
