[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 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.

> DCL OUT[1], COLOR
> DCL CONST[2..3]
> DCL CONST[0..1]
> DCL TEMP[0]
> DCL TEMP[1..2], LOCAL
>
> So clearly something gets confused. I think this needs a bit more
> investigation... without this patch, the header looks more like
>
> FRAG
> PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
> DCL IN[0], POSITION, LINEAR
> DCL OUT[0], COLOR
> DCL OUT[1], COLOR[1]
-------------- 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/711f5674/attachment.sig>