[Mesa-dev] [PATCH 10/17] glsl: Define a gl_LastFragData built-in for GLSL versions that have gl_FragData.

[Francisco Jerez]

Francisco Jerez <currojerez at riseup.net> writes:

> Kenneth Graunke <kenneth at whitecape.org> writes:
>
>> On Wednesday, July 27, 2016 5:05:39 PM PDT Francisco Jerez wrote:
>>> Kenneth Graunke <kenneth at whitecape.org> writes:
>>> 
>>> > On Wednesday, July 20, 2016 9:49:40 PM PDT Francisco Jerez wrote:
>>> >> The EXT_shader_framebuffer_fetch extension defines alternative
>>> >> language for GLES2 shaders where user-defined fragment outputs are not
>>> >> allowed.  Instead of using inout user-defined fragment outputs the
>>> >> shader is expected to read from the gl_LastFragData built-in array.
>>> >> In addition this allows using the same language on desktop GLSL
>>> >> versions prior to 4.2 that support the deprecated gl_FragData built-in
>>> >> in preparation for the MESA_shader_framebuffer_fetch desktop GL
>>> >> extension.
>>> >> 
>>> >> Both legacy and user-defined inout outputs have a common
>>> >> representation at the GLSL IR level, so it shouldn't make any
>>> >> difference for optimization passes and back-ends whether the
>>> >> application is using gl_LastFragData or user-defined outputs, all
>>> >> they'll see is a variable dereference of a fragment output at a
>>> >> certain interface location with the fb_fetch_output bit set to one.
>>> >> ---
>>> >>  src/compiler/glsl/builtin_variables.cpp | 10 ++++++++++
>>> >>  1 file changed, 10 insertions(+)
>>> >> 
>>> >> diff --git a/src/compiler/glsl/builtin_variables.cpp b/src/compiler/glsl/builtin_variables.cpp
>>> >> index f63dc3a..6a756ed 100644
>>> >> --- a/src/compiler/glsl/builtin_variables.cpp
>>> >> +++ b/src/compiler/glsl/builtin_variables.cpp
>>> >> @@ -1136,6 +1136,16 @@ builtin_variable_generator::generate_fs_special_vars()
>>> >>                   array(vec4_t, state->Const.MaxDrawBuffers), "gl_FragData");
>>> >>     }
>>> >>  
>>> >> +   if (state->has_framebuffer_fetch() && !state->is_version(420, 300)) {
>>> >> +      ir_variable *const var =
>>> >> +         add_output(FRAG_RESULT_DATA0,
>>> >> +                    array(vec4_t, state->Const.MaxDrawBuffers),
>>> >> +                    "gl_LastFragData");
>>> >> +      var->data.precision = GLSL_PRECISION_MEDIUM;
>>> >> +      var->data.read_only = 1;
>>> >> +      var->data.fb_fetch_output = 1;
>>> >> +   }
>>> >> +
>>> >
>>> > Personally, I'd only create gl_LastFragData in desktop 1.10/1.20,
>>> > and not 1.30+ where it's deprecated.  Sure, you /can/ use it, but
>>> > you can also do the 'inout' syntax that's the preferred way going
>>> > forward, so we may as well just require shader authors to do so.
>>> >
>>> 
>>> My thinking here was that since we need to drop the gl_LastFragData
>>> built-in at some point, it would be the least confusing if we did it at
>>> the same point that Khronos dropped gl_FragData.  It doesn't seem like
>>> it will make much of a difference in practice though, because this
>>> extension has never been available in desktop GL, so backwards
>>> compatibility is not really a concern -- I've changed this series
>>> locally to check for GLSL 1.3 instead of GLSL 4.2 in the code above and
>>> in PATCH 12, and updated my draft spec text so that gl_LastFragData is
>>> not required to be present on GLSL 1.3 and above.
>>
>> Yeah, makes sense.  My thinking was to drop it when gl_FragData was
>> first deprecated (why add new deprecated things), but it's not a big
>> deal either way.  
>>
>> Do you have any specs written up for the MESA_framebuffer_fetch_*
>> extension pair?  I feel a little weird about having extension enable
>> flags for extensions that we don't have a spec for (unless I just
>> missed something?).
>>
> Yeah, I think I'm almost done with it, will likely send the spec text
> for review tomorrow together with other patches in order to expose the
> framebuffer fetch functionality in desktop GL.
>

Ken and I discussed this off-list yesterday, but in case somebody else
is wondering why I haven't sent the spec text to the mailing list, the
reason is that I haven't been able to get in touch with the author of
the original EXT_shader_framebuffer_fetch extension yet (the author is
Benj Lipchak who apparently is no longer working at Apple), and I'm not
certain what its copyright status is (I suspect Apple is still the
copyright holder of the extension) and whether derived work such as my
MESA extensions loosely based on the EXT extension would be allowed by
its license -- Any ideas what I should do would be welcome.

I've sent my current draft to Ken in private to help him make progress
reviewing the series, if anyone else would like to have a look feel free
to ping me off-list.

>> Otherwise, this series looks good to me.  I'll try and send actual
>> R-bs soon...IIRC, there were a few bits I glossed over and should
>> look over again quickly.
>>
> Cool, thanks.
>
>> --Ken
-------------- 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/20160729/6761b735/attachment.sig>