[Mesa-dev] [PATCH 2/3] glsl: Fix incorrect hard-coded location of the gl_SecondaryFragColorEXT built-in.

[Francisco Jerez]

Ilia Mirkin <imirkin at alum.mit.edu> writes:

> On Tue, Aug 23, 2016 at 12:05 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Ilia Mirkin <imirkin at alum.mit.edu> writes:
>>
>>> On Mon, Aug 22, 2016 at 10:55 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>> Ilia Mirkin <imirkin at alum.mit.edu> writes:
>>>>
>>>>> On Mon, Aug 22, 2016 at 9:59 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>>>> gl_SecondaryFragColorEXT should have the same location as gl_FragColor
>>>>>> for the secondary fragment color to be replicated to all fragment
>>>>>> outputs.  The incorrect location of gl_SecondaryFragColorEXT would
>>>>>> cause the linker to mark both FRAG_RESULT_COLOR and FRAG_RESULT_DATA0
>>>>>> as being written to, which isn't allowed by the spec and would
>>>>>> ultimately lead to an assertion failure in
>>>>>> fs_visitor::emit_fb_writes() on my i965-fb-fetch branch.
>>>>>
>>>>> My recollection was that it didn't work with COLOR for "stupid"
>>>>> reasons. Can you confirm that
>>>>> bin/arb_blend_func_extended-fbo-extended-blend-pattern_gles2 -auto
>>>>> passes with this patch?
>>>>>
>>>> Yes, it does, in fact
>>>> arb_blend_func_extended-fbo-extended-blend-pattern_gles2 hits the i965
>>>> assertion failure I mentioned above unless this patch is applied.
>>>
>>> This causes the test in question to fail on nouveau... the TGSI shader
>>> generated starts with
>>>
>>> FRAG
>>> PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
>>> DCL IN[0], POSITION, LINEAR
>>> DCL OUT[0], SAMPLEMASK
>>
>> Heh, this smells a lot like the bug fixed in PATCH 1, but somewhere in
>> the mesa state tracker.  st_glsl_to_tgsi.cpp:2422 does:
>>
>> | entry = new(mem_ctx) variable_storage(var,
>> |                                       PROGRAM_OUTPUT,
>> |                                       var->data.location
>> |                                       + var->data.index);
>>
>> which is obviously bogus, e.g. for var->data.location ==
>> FRAG_RESULT_COLOR and var->data.index == 1 you get
>> FRAG_RESULT_SAMPLE_MASK which explains the sample mask declaration
>> above.
>
> Right, because having FRAG_RESULT_COLOR and index != 0 was never
> possible prior to this. That might be why Ryan stuck it into
> FRAG_RESULT_DATA0 [I may have been the one to suggest that].

Heh, so I guess that's the "stupid" reason you were referring to,
working around this mesa state tracker bug in the GLSL front-end.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160822/89eeda0d/attachment.sig>