Timothy Arceri <[email protected]> writes: > > glsl_get_sampler_dim() contains the following: > > assert(glsl_type_is_sampler(type) || glsl_type_is_image(type)); > > Which leads me to believe the code above should just be: > > const struct glsl_type *glsl_type = glsl_without_array(var->type); > > If you agree please squash this patch into the previous patch where you > can keep my r-b.
I agree, that sounds sensible. It looks like var->interface_type was being overloaded as a place to store the type for the image or sampler without the array type. The comment above interface_type says: “For variables that are in an interface block or are an instance of an interface block, this is the GLSL_TYPE_INTERFACE type for that block.” So it seems a little strange to use it for this purpose. It presumably works because opaque types can’t be in interfaces so the interface_type is otherwise unused. I guess with the previous patch interface_type will always be NULL so you’re right that the check for whether it is NULL in seems pointless. Maybe we should however add a comment to the commit message for the previous commit along these lines: “The previous code appeared to be using var->interface_type as a place to store the type of the variable without the enclosing array for images and samplers. I guess this worked because opaque types can not appear in interfaces so the interface_type is sort of unused. This patch removes the overloading of var->interface_type and any places that needed the type without the array can now just deduce it from var->type.” Thanks for looking at the patch. Regards, - Neil
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
