[Mesa-dev] [Mesa-stable] [PATCH 1/3] glsl: Allow binding of image variables with 420pack.

[Ian Romanick]

On 12/07/2015 04:12 PM, Matt Turner wrote:
> On Mon, Dec 7, 2015 at 4:10 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 12/07/2015 03:35 PM, Matt Turner wrote:
>>> This interaction was missed in the addition of ARB_image_load_store.
>>>
>>> Cc: "11.0 11.1" <mesa-stable at lists.freedesktop.org>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93266
>>> ---
>>>  src/glsl/ast_to_hir.cpp | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>> index 8fcda63..6d7e40f 100644
>>> --- a/src/glsl/ast_to_hir.cpp
>>> +++ b/src/glsl/ast_to_hir.cpp
>>> @@ -2649,7 +2649,9 @@ apply_explicit_binding(struct _mesa_glsl_parse_state *state,
>>>
>>>           return;
>>>        }
>>> -   } else if (state->is_version(420, 310) && base_type->is_image()) {
>>> +   } else if ((state->is_version(420, 310) ||
>>> +               state->ARB_shading_language_420pack_enable) &&
>>> +              base_type->is_image()) {
>>
>> This is just an open-coded version of has_420pack_or_es31 (added in
>> patch 3), yeah?  If so, I think I'd pull that addition to patch 1 and
>> use it here.
>>
>> With that change, the series is
>>
>> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> 
> It is. I wanted to make this bug fix separate from 3/3 since this is
> tagged for 11.0 and 3/3 isn't.

I figured that.  That's why I didn't suggest just moving patch 3 to the
front.  It just seems a bit icky to leave this open coded in 11.1...
which eventually someone will notice and clean up.  Right?  Either way,
I'd like to see the bug fixed in both branches, and I'm not that hung up
on this issue.

> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable