On 04/06/2014 10:27 PM, Tapani Pälli wrote:
> On 04/05/2014 12:58 AM, Ian Romanick wrote:
>> On 04/04/2014 02:42 PM, Ian Romanick wrote:
>>> On 03/27/2014 11:45 PM, Tapani Pälli wrote:
>>>> Patch adds a preprocessor define for the extension and stores
>>>> explicit location data for uniforms during AST->HIR conversion.
>>>> It also sets layout token to be available when having the
>>>> extension in place.
>>>>
>>>> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
>>>> ---
>>>> src/glsl/ast_to_hir.cpp | 13 +++++++++++++
>>>> src/glsl/glcpp/glcpp-parse.y | 3 +++
>>>> src/glsl/glsl_lexer.ll | 1 +
>>>> 3 files changed, 17 insertions(+)
>>>>
>>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>>> index 8f6e901..3bfad02 100644
>>>> --- a/src/glsl/ast_to_hir.cpp
>>>> +++ b/src/glsl/ast_to_hir.cpp
>>>> @@ -2123,6 +2123,19 @@ validate_explicit_location(const struct
>>>> ast_type_qualifier *qual,
>>>> {
>>>> bool fail = false;
>>>>
>>>> + /* Checks for GL_ARB_explicit_uniform_location. */
>>>> + if (qual->flags.q.uniform) {
>>>> +
>>>> + if (qual->index < 0) {
>>>> + _mesa_glsl_error(loc, state,
>>>> + "explicit location < 0 for uniform %s",
>>>> var->name);
>>> We also need to check that the explicit location isn't too large. I
>>> don't think that check can happen here because (I think) it will depend
>>> on the size of the uniform. The spec isn't completely clear, but I think
>>>
>>> // assume GL_MAX_UNIFORM_LOCATIONS is 65536
>>> layout(location=65535) float foo[1];
>>>
>>> should fail because foo[1] would have location 65536 (larger than
>>> GL_MAX_UNIFORM_LOCATIONS-1).
>>>
>>> Given the other rules in the spec about counting and use of locations, I
>>> think ast_to_hir is the right time to do the upper bound check. I
>>> believe we can figure out how many locations a structure or array
>>> uniform will maximally use at that point.
>> I now see that you added that check in patch 8. Maybe just put a
>> comment here explaining why the check can't happen here.
>
> OK, I'll try to move check for bounds to happen while parsing to be able
> to fail earlier. I originally preferred to have all checks in one place
> and location assignment felt like the right spot.
I think the check is fine where it is. I just want a comment here so
that the next person doesn't say, "Hey! We need to check the other
bound too!" :)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
