Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
On 08/28/2014 01:50 AM, Micael wrote: I am getting another error regarding explicit locations, which I believe has a one-line fix. If I only have a single uniform in my shader, and that uniform is explicitely given location 1 (or anything above zero), my program will crash if I attempt to give the unnassigned locs a value with glProgramUniform1i(). A quick view into the code appears to reveal reserve_explicit_locations() as the culprit by initializing the remap table entries to NULL instead of INACTIVE_UNIFORM_EXPLICIT_LOCATION, which will allow validate_uniform_parameters() to pass the test if (shProg-UniformRemapTable[location] ==INACTIVE_UNIFORM_EXPLICIT_LOCATION) later. Should I open another bug report? Yes please, I'll write a Piglit test for this. Thanks; On Wed, Aug 27, 2014 at 11:15 AM, Tapani Pälli tapani.pa...@intel.com mailto:tapani.pa...@intel.com wrote: On 08/26/2014 10:37 PM, Ian Romanick wrote: On 08/23/2014 07:51 AM, Micael wrote: On second thought, the glsl_type::uniform_locations() method comment in the header file says: /** * Calculate the number of unique values from glGetUniformLocation for the * elements of the type. */ Which makes me believe that maybe we should return at least 1 for every case because this is only called for explicit locations and therefore will not make shaders failing to link unless they've explicitely used more locations than they should. Maybe glsl_type::uniform_locations() should be renamed to something that makes it clear it's only used for explicit locations too, or make it clear that it reflects the size for the API, not GPU memory (since it's only used to fill prog-UniformRemapTable). I spent some time this morning looking at this code a bit more closely, and I think you're right. The problem is that GL overloads locations both to mean the number of handles a uniform gets and to mean amount of storage space the uniform occupies. For some things this is the same (simple variables, arrays, structures), but for other things it is not (matrices, samplers, other opaque types). I think it's weird for uniform_locations to call component_slots at all. Yep, at the moment of writing this I have gotten confused of the locations and actual storage space (resulting in vectors using too many locs). As this is my mess, I've sent a patch that implements the count as you describe below + adds a bit more documentation to the header. Looking at the OpenGL 4.2 spec, it seems that everything except atomic counters and subroutines (not yet supported by Mesa) should have at least one location per array element. Here's what I think would be better: have uniform_locations mirror the structure of component_slots. Eliminate the is_matrix() check at the top, and just have all the basic types (e.g., GLSL_TYPE_FLOAT) return 1. Keeping functions separate that count separate kinds of things seems like a good idea. On Sat, Aug 23, 2014 at 2:07 PM, Micael kam1k...@gmail.com mailto:kam1k...@gmail.com mailto:kam1k...@gmail.com mailto:kam1k...@gmail.com wrote: On Fri, Aug 22, 2014 at 9:23 PM, Ian Romanick i...@freedesktop.org mailto:i...@freedesktop.org mailto:i...@freedesktop.org mailto:i...@freedesktop.org wrote: I'm not sure this is correct, and I think we need a more complex fix. As Dave points out, bindless will make it even more complex. In a non-bindless, static-sampler-array-indexing world, applications assume that samplers will use zero uniform locations. The sampler information is baked into the instructions, and it isn't represented anywhere else in GPU memory. As a result, I would not be surprised to see previously working applications fail to link (due to using too many uniforms) with this change. It seems that the only time a sampler needs non-zero space is when it is part of any array samplers (or array of structures containing samplers, etc.) or is bindless. In the array cases, it is probably only necessary when the index is dynamic. I think the array splitting and structure splitting passes may make that moot. Did you miss the case of assigning an explicit location to a sampler, or did you ommit on purpose? I'm not very familiar with mesa source, but as far as I could analyze it,
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
I am getting another error regarding explicit locations, which I believe has a one-line fix. If I only have a single uniform in my shader, and that uniform is explicitely given location 1 (or anything above zero), my program will crash if I attempt to give the unnassigned locs a value with glProgramUniform1i(). A quick view into the code appears to reveal reserve_explicit_locations() as the culprit by initializing the remap table entries to NULL instead of INACTIVE_UNIFORM_EXPLICIT_LOCATION, which will allow validate_uniform_parameters() to pass the test if (shProg-UniformRemapTable[location] ==INACTIVE_UNIFORM_EXPLICIT_LOCATION) later. Should I open another bug report? On Wed, Aug 27, 2014 at 11:15 AM, Tapani Pälli tapani.pa...@intel.com wrote: On 08/26/2014 10:37 PM, Ian Romanick wrote: On 08/23/2014 07:51 AM, Micael wrote: On second thought, the glsl_type::uniform_locations() method comment in the header file says: /** * Calculate the number of unique values from glGetUniformLocation for the * elements of the type. */ Which makes me believe that maybe we should return at least 1 for every case because this is only called for explicit locations and therefore will not make shaders failing to link unless they've explicitely used more locations than they should. Maybe glsl_type::uniform_locations() should be renamed to something that makes it clear it's only used for explicit locations too, or make it clear that it reflects the size for the API, not GPU memory (since it's only used to fill prog-UniformRemapTable). I spent some time this morning looking at this code a bit more closely, and I think you're right. The problem is that GL overloads locations both to mean the number of handles a uniform gets and to mean amount of storage space the uniform occupies. For some things this is the same (simple variables, arrays, structures), but for other things it is not (matrices, samplers, other opaque types). I think it's weird for uniform_locations to call component_slots at all. Yep, at the moment of writing this I have gotten confused of the locations and actual storage space (resulting in vectors using too many locs). As this is my mess, I've sent a patch that implements the count as you describe below + adds a bit more documentation to the header. Looking at the OpenGL 4.2 spec, it seems that everything except atomic counters and subroutines (not yet supported by Mesa) should have at least one location per array element. Here's what I think would be better: have uniform_locations mirror the structure of component_slots. Eliminate the is_matrix() check at the top, and just have all the basic types (e.g., GLSL_TYPE_FLOAT) return 1. Keeping functions separate that count separate kinds of things seems like a good idea. On Sat, Aug 23, 2014 at 2:07 PM, Micael kam1k...@gmail.com mailto:kam1k...@gmail.com wrote: On Fri, Aug 22, 2014 at 9:23 PM, Ian Romanick i...@freedesktop.org mailto:i...@freedesktop.org wrote: I'm not sure this is correct, and I think we need a more complex fix. As Dave points out, bindless will make it even more complex. In a non-bindless, static-sampler-array-indexing world, applications assume that samplers will use zero uniform locations. The sampler information is baked into the instructions, and it isn't represented anywhere else in GPU memory. As a result, I would not be surprised to see previously working applications fail to link (due to using too many uniforms) with this change. It seems that the only time a sampler needs non-zero space is when it is part of any array samplers (or array of structures containing samplers, etc.) or is bindless. In the array cases, it is probably only necessary when the index is dynamic. I think the array splitting and structure splitting passes may make that moot. Did you miss the case of assigning an explicit location to a sampler, or did you ommit on purpose? I'm not very familiar with mesa source, but as far as I could analyze it, only reserve_explicit_locations() and validate_explicit_location() call glsl_type::uniform_locations(), everything else uses glsl_type::component_slots(). Now I agree with you that simply returning 1 from glsl_type::uniform_locations() for samplers can be misleading. How about if glsl_type::uniform_locations() takes a bool isExplicitLocation and return at least 1 for every type? I am aware that this won't solve the bindless textures issue, but it would fix this bug (and the integer underflows) for now, I think. The 82921 bug seems to be an orthognal issue. There is a difference between the API visible
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
On 08/23/2014 07:51 AM, Micael wrote: On second thought, the glsl_type::uniform_locations() method comment in the header file says: /** * Calculate the number of unique values from glGetUniformLocation for the * elements of the type. */ Which makes me believe that maybe we should return at least 1 for every case because this is only called for explicit locations and therefore will not make shaders failing to link unless they've explicitely used more locations than they should. Maybe glsl_type::uniform_locations() should be renamed to something that makes it clear it's only used for explicit locations too, or make it clear that it reflects the size for the API, not GPU memory (since it's only used to fill prog-UniformRemapTable). I spent some time this morning looking at this code a bit more closely, and I think you're right. The problem is that GL overloads locations both to mean the number of handles a uniform gets and to mean amount of storage space the uniform occupies. For some things this is the same (simple variables, arrays, structures), but for other things it is not (matrices, samplers, other opaque types). I think it's weird for uniform_locations to call component_slots at all. Looking at the OpenGL 4.2 spec, it seems that everything except atomic counters and subroutines (not yet supported by Mesa) should have at least one location per array element. Here's what I think would be better: have uniform_locations mirror the structure of component_slots. Eliminate the is_matrix() check at the top, and just have all the basic types (e.g., GLSL_TYPE_FLOAT) return 1. Keeping functions separate that count separate kinds of things seems like a good idea. On Sat, Aug 23, 2014 at 2:07 PM, Micael kam1k...@gmail.com mailto:kam1k...@gmail.com wrote: On Fri, Aug 22, 2014 at 9:23 PM, Ian Romanick i...@freedesktop.org mailto:i...@freedesktop.org wrote: I'm not sure this is correct, and I think we need a more complex fix. As Dave points out, bindless will make it even more complex. In a non-bindless, static-sampler-array-indexing world, applications assume that samplers will use zero uniform locations. The sampler information is baked into the instructions, and it isn't represented anywhere else in GPU memory. As a result, I would not be surprised to see previously working applications fail to link (due to using too many uniforms) with this change. It seems that the only time a sampler needs non-zero space is when it is part of any array samplers (or array of structures containing samplers, etc.) or is bindless. In the array cases, it is probably only necessary when the index is dynamic. I think the array splitting and structure splitting passes may make that moot. Did you miss the case of assigning an explicit location to a sampler, or did you ommit on purpose? I'm not very familiar with mesa source, but as far as I could analyze it, only reserve_explicit_locations() and validate_explicit_location() call glsl_type::uniform_locations(), everything else uses glsl_type::component_slots(). Now I agree with you that simply returning 1 from glsl_type::uniform_locations() for samplers can be misleading. How about if glsl_type::uniform_locations() takes a bool isExplicitLocation and return at least 1 for every type? I am aware that this won't solve the bindless textures issue, but it would fix this bug (and the integer underflows) for now, I think. The 82921 bug seems to be an orthognal issue. There is a difference between the API visible locations assigned to a uniform and the GPU visible locations where the uniform is stored. My guess is that we're using the same accounting for both in places where we should not. On 08/21/2014 04:25 PM, Micael Dias wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length *
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
On Fri, Aug 22, 2014 at 9:23 PM, Ian Romanick i...@freedesktop.org wrote: I'm not sure this is correct, and I think we need a more complex fix. As Dave points out, bindless will make it even more complex. In a non-bindless, static-sampler-array-indexing world, applications assume that samplers will use zero uniform locations. The sampler information is baked into the instructions, and it isn't represented anywhere else in GPU memory. As a result, I would not be surprised to see previously working applications fail to link (due to using too many uniforms) with this change. It seems that the only time a sampler needs non-zero space is when it is part of any array samplers (or array of structures containing samplers, etc.) or is bindless. In the array cases, it is probably only necessary when the index is dynamic. I think the array splitting and structure splitting passes may make that moot. Did you miss the case of assigning an explicit location to a sampler, or did you ommit on purpose? I'm not very familiar with mesa source, but as far as I could analyze it, only reserve_explicit_locations() and validate_explicit_location() call glsl_type::uniform_locations(), everything else uses glsl_type::component_slots(). Now I agree with you that simply returning 1 from glsl_type::uniform_locations() for samplers can be misleading. How about if glsl_type::uniform_locations() takes a bool isExplicitLocation and return at least 1 for every type? I am aware that this won't solve the bindless textures issue, but it would fix this bug (and the integer underflows) for now, I think. The 82921 bug seems to be an orthognal issue. There is a difference between the API visible locations assigned to a uniform and the GPU visible locations where the uniform is stored. My guess is that we're using the same accounting for both in places where we should not. On 08/21/2014 04:25 PM, Micael Dias wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } -- Micael Dias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
On second thought, the glsl_type::uniform_locations() method comment in the header file says: /** * Calculate the number of unique values from glGetUniformLocation for the * elements of the type. */ Which makes me believe that maybe we should return at least 1 for every case because this is only called for explicit locations and therefore will not make shaders failing to link unless they've explicitely used more locations than they should. Maybe glsl_type::uniform_locations() should be renamed to something that makes it clear it's only used for explicit locations too, or make it clear that it reflects the size for the API, not GPU memory (since it's only used to fill prog-UniformRemapTable). On Sat, Aug 23, 2014 at 2:07 PM, Micael kam1k...@gmail.com wrote: On Fri, Aug 22, 2014 at 9:23 PM, Ian Romanick i...@freedesktop.org wrote: I'm not sure this is correct, and I think we need a more complex fix. As Dave points out, bindless will make it even more complex. In a non-bindless, static-sampler-array-indexing world, applications assume that samplers will use zero uniform locations. The sampler information is baked into the instructions, and it isn't represented anywhere else in GPU memory. As a result, I would not be surprised to see previously working applications fail to link (due to using too many uniforms) with this change. It seems that the only time a sampler needs non-zero space is when it is part of any array samplers (or array of structures containing samplers, etc.) or is bindless. In the array cases, it is probably only necessary when the index is dynamic. I think the array splitting and structure splitting passes may make that moot. Did you miss the case of assigning an explicit location to a sampler, or did you ommit on purpose? I'm not very familiar with mesa source, but as far as I could analyze it, only reserve_explicit_locations() and validate_explicit_location() call glsl_type::uniform_locations(), everything else uses glsl_type::component_slots(). Now I agree with you that simply returning 1 from glsl_type::uniform_locations() for samplers can be misleading. How about if glsl_type::uniform_locations() takes a bool isExplicitLocation and return at least 1 for every type? I am aware that this won't solve the bindless textures issue, but it would fix this bug (and the integer underflows) for now, I think. The 82921 bug seems to be an orthognal issue. There is a difference between the API visible locations assigned to a uniform and the GPU visible locations where the uniform is stored. My guess is that we're using the same accounting for both in places where we should not. On 08/21/2014 04:25 PM, Micael Dias wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } -- Micael Dias -- Micael Dias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
On 08/22/2014 08:31 AM, Tapani Pälli wrote: On 08/22/2014 02:25 AM, Micael Dias wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. Thanks for the patch, I sent a bit different fix to be sure all possible types will consume at least 1 loc, this is just to be future-proof. I was just pointed out that your patch is more correct though :) (Not all opaque types deserve a location, only some.) src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
In the future for ARB_bindless_texture we might want this to be 2, I'm not sure if we should just do that now. Dave. On 22 August 2014 09:25, Micael Dias kam1k...@gmail.com wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
Is this because of the 64 bit handles? If so, I think I agree with you. According to the ARB_bindless_texture spec changes: When used as shader inputs, outputs, uniform block members, or temporaries, the value of the sampler is a 64-bit unsigned integer handle and never refers to a texture image unit. However, should we consume 2 locs only if ARB_bindless_texture is present, or always? On Fri, Aug 22, 2014 at 8:36 AM, Dave Airlie airl...@gmail.com wrote: In the future for ARB_bindless_texture we might want this to be 2, I'm not sure if we should just do that now. Dave. On 22 August 2014 09:25, Micael Dias kam1k...@gmail.com wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev -- Micael Dias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
I'm not sure this is correct, and I think we need a more complex fix. As Dave points out, bindless will make it even more complex. In a non-bindless, static-sampler-array-indexing world, applications assume that samplers will use zero uniform locations. The sampler information is baked into the instructions, and it isn't represented anywhere else in GPU memory. As a result, I would not be surprised to see previously working applications fail to link (due to using too many uniforms) with this change. It seems that the only time a sampler needs non-zero space is when it is part of any array samplers (or array of structures containing samplers, etc.) or is bindless. In the array cases, it is probably only necessary when the index is dynamic. I think the array splitting and structure splitting passes may make that moot. The 82921 bug seems to be an orthognal issue. There is a difference between the API visible locations assigned to a uniform and the GPU visible locations where the uniform is stored. My guess is that we're using the same accounting for both in places where we should not. On 08/21/2014 04:25 PM, Micael Dias wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
--- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location
On 08/22/2014 02:25 AM, Micael Dias wrote: --- If samplers occupy zero locations we can run into a lot of issues. See #82921. I briefly tested this with my own code (which was previously crashing and misbehaving) and also ran other apps and everything seems to work fine. I'm not used to contributing code in this fashion, so please forgive me if I'm making some mistake. Thanks. Thanks for the patch, I sent a bit different fix to be sure all possible types will consume at least 1 loc, this is just to be future-proof. src/glsl/glsl_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 66e9b13..cc05193 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const return size; case GLSL_TYPE_ARRAY: return this-length * this-fields.array-uniform_locations(); + case GLSL_TYPE_SAMPLER: + return 1; default: break; } // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
