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

Submitted by Francisco Jerez on July 21, 2016, 4:49 a.m.

Details

Message ID 20160721044947.14076-10-currojerez@riseup.net
State New
Headers show
Series "Framebuffer fetch." ( rev: 11 10 9 8 7 6 5 4 3 2 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Francisco Jerez July 21, 2016, 4:49 a.m.
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(+)

Patch hide | download patch | download mbox

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;
+   }
+
    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

Comments

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.

>     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
>
Kenneth Graunke <kenneth@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.

>>     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
>>
On Wednesday, July 27, 2016 5:05:39 PM PDT Francisco Jerez wrote:
> Kenneth Graunke <kenneth@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?).

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.

--Ken
Kenneth Graunke <kenneth@whitecape.org> writes:

> On Wednesday, July 27, 2016 5:05:39 PM PDT Francisco Jerez wrote:
>> Kenneth Graunke <kenneth@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.

> 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
Francisco Jerez <currojerez@riseup.net> writes:

> Kenneth Graunke <kenneth@whitecape.org> writes:
>
>> On Wednesday, July 27, 2016 5:05:39 PM PDT Francisco Jerez wrote:
>>> Kenneth Graunke <kenneth@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