On 05/21/2014 08:11 AM, Tapani wrote:
> On 05/21/2014 05:43 PM, Tapani wrote:
>> On 05/21/2014 01:27 AM, Ian Romanick wrote:
>>> On 05/19/2014 10:08 PM, Tapani wrote:
>>>> On 05/19/2014 08:18 PM, Ian Romanick wrote:
>>>>> On 04/09/2014 02:56 AM, Tapani Pälli wrote:
>>>>>> diff --git a/src/glsl/glsl_parser_extras.h
>>>>>> b/src/glsl/glsl_parser_extras.h
>>>>>> index c53c583..20879a0 100644
>>>>>> --- a/src/glsl/glsl_parser_extras.h
>>>>>> +++ b/src/glsl/glsl_parser_extras.h
>>>>>> @@ -152,6 +152,20 @@ struct _mesa_glsl_parse_state {
>>>>>> return true;
>>>>>> }
>>>>>> + bool check_explicit_uniform_location_allowed(YYLTYPE *locp,
>>>>>> + const ir_variable
>>>>>> *var)
>>>>>> + {
>>>>>> + /* Requires OpenGL 3.3 or ARB_explicit_attrib_location. */
>>>>>> + if (ctx->Version < 33 &&
>>>>>> !ctx->Extensions.ARB_explicit_attrib_location) {
>>>>>> + _mesa_glsl_error(locp, this, "%s explicit location
>>>>>> requires "
>>>>>> + "GL_ARB_explicit_attrib_location extension "
>>>>>> + "or OpenGL 3.3", mode_string(var));
>>>>> Many copy-and-paste bugs. :) Explicit uniform locations aren't added
>>>>> until 4.3.
>>>> It may look copy-paste but the specification states that "Requires
>>>> OpenGL 3.3 or ARB_explicit_attrib_location":
>>>>
>>>> https://www.opengl.org/registry/specs/ARB/explicit_uniform_location.txt
>>>>
>>>> Using 4.3 capable driver will pass this check correctly.
>>> Oh right, because it relies on 3.3 or ARB_explicit_attib_location to
>>> add the layout keyword. Some comments explaining that this isn't a
>>> copy-and-paste bug will prevent the next person from also thinking that
>>> it is. :)
>>>
>>> But this code should check the version (and extension) bits set in the
>>> shader, not what's enabled in the context. How about:
>>>
>>> bool check_explicit_attrib_location_allowed(YYLTYPE *locp,
>>> const ir_variable *var)
>>> {
>>> if (!this->has_explicit_attrib_location() ||
>>> !this->ARB_explicit_uniform_location_enable) {
>>> _mesa_glsl_error(locp, this,
>>> "uniform explicit location requires
>>> "GL_ARB_explicit_uniform_location and
>>> either "
>>> "GL_ARB_explicit_attrib_location or GLSL
>>> 330.");
>>> return false;
>>> }
>>>
>>> return true;
>>> }
>>
>> Sure, this is fine by me. I'll send new patches soon.
>>
>
> Or maybe fine with some changes since my piglit tests won't pass with
> this change (for those explicit attrib location is not available for
> some reason (!)), will take a look.
Do the tests enable it via #extension?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
