[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 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 should also fix the code below for multiple dual-source-blended
>> render targets, which no driver currently supports but we have plans
>> to enable eventually in the i965 driver (the comment saying that no
>> hardware will ever support it seems rather hilarious).
>> ---
>>  src/compiler/glsl/builtin_variables.cpp | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/compiler/glsl/builtin_variables.cpp b/src/compiler/glsl/builtin_variables.cpp
>> index cb5f730..b29f677 100644
>> --- a/src/compiler/glsl/builtin_variables.cpp
>> +++ b/src/compiler/glsl/builtin_variables.cpp
>> @@ -1145,13 +1145,8 @@ builtin_variable_generator::generate_fs_special_vars()
>>     }
>>
>>     if (state->es_shader && state->language_version == 100 && state->EXT_blend_func_extended_enable) {
>> -      /* We make an assumption here that there will only ever be one dual-source draw buffer
>> -       * In case this assumption is ever proven to be false, make sure to assert here
>> -       * since we don't handle this case.
>> -       * In practice, this issue will never arise since no hardware will support it.
>> -       */
>> -      assert(state->Const.MaxDualSourceDrawBuffers <= 1);
>> -      add_index_output(FRAG_RESULT_DATA0, 1, vec4_t, "gl_SecondaryFragColorEXT");
>> +      add_index_output(FRAG_RESULT_COLOR, 1, vec4_t,
>> +                       "gl_SecondaryFragColorEXT");
>>        add_index_output(FRAG_RESULT_DATA0, 1,
>>                         array(vec4_t, state->Const.MaxDualSourceDrawBuffers),
>>                         "gl_SecondaryFragDataEXT");
>> --
>> 2.9.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- 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/5c54486f/attachment.sig>