[Mesa-dev] [PATCH 1/2] glsl: add support for textureSamples function

[Ilia Mirkin]

On Wed, Aug 12, 2015 at 2:34 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
> Hi Ilia;
>
> Some comments below;
>
>
> On 08/12/2015 04:51 AM, Ilia Mirkin wrote:
>>
>> ---
>>   src/glsl/builtin_functions.cpp             | 32
>> +++++++++++++++++++++++++++++-
>>   src/glsl/glcpp/glcpp-parse.y               |  3 +++
>>   src/glsl/glsl_parser_extras.cpp            |  1 +
>>   src/glsl/glsl_parser_extras.h              |  2 ++
>>   src/glsl/ir.cpp                            |  5 +++--
>>   src/glsl/ir.h                              |  3 ++-
>>   src/glsl/ir_clone.cpp                      |  1 +
>>   src/glsl/ir_equals.cpp                     |  1 +
>>   src/glsl/ir_hv_accept.cpp                  |  1 +
>>   src/glsl/ir_print_visitor.cpp              |  6 ++++--
>>   src/glsl/ir_reader.cpp                     |  6 +++++-
>>   src/glsl/ir_rvalue_visitor.cpp             |  1 +
>>   src/glsl/nir/glsl_to_nir.cpp               |  5 +++++
>>   src/glsl/nir/nir.h                         |  4 +++-
>>   src/glsl/nir/nir_print.c                   |  3 +++
>>   src/glsl/opt_tree_grafting.cpp             |  1 +
>>   src/mesa/main/mtypes.h                     |  1 +
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  2 ++
>>   18 files changed, 70 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/glsl/builtin_functions.cpp
>> b/src/glsl/builtin_functions.cpp
>> index 2175c66..1a71d74 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -399,6 +399,13 @@ shader_image_load_store(const _mesa_glsl_parse_state
>> *state)
>>   }
>>
>>   static bool
>> +shader_samples(const _mesa_glsl_parse_state *state)
>> +{
>> +   return state->is_version(450, 0) ||
>> +      state->ARB_shader_texture_image_samples_enable;
>
>
> According to the extension spec, 4.30 should be enough for this.

The extension spec says what GLSL version it's based on, not what GLSL
version is needed to have the functionality auto-enabled. imageSamples
and textureSamples are part of GLSL 4.50, but the extension spec is
written against GLSL 4.30.

>
>> +}
>> +
>> +static bool
>>   gs_streams(const _mesa_glsl_parse_state *state)
>>   {
>>      return gpu_shader5(state) && gs_only(state);
>> @@ -630,10 +637,10 @@ private:
>>      B1(any);
>>      B1(all);
>>      B1(not);
>> -   B2(textureSize);
>
>
> unrelated cleanup?

Yeah... actually that should probably all just become BA2(textureSize)
or something. I guess I should do that separately.

>
>
>>      ir_function_signature *_textureSize(builtin_available_predicate
>> avail,
>>                                          const glsl_type *return_type,
>>                                          const glsl_type *sampler_type);
>> +   B1(textureSamples);
>>
>>   /** Flags to _texture() */
>>   #define TEX_PROJECT 1
>> @@ -1372,6 +1379,16 @@ builtin_builder::create_builtins()
>>                   _textureSize(texture_multisample, glsl_type::ivec3_type,
>> glsl_type::usampler2DMSArray_type),
>>                   NULL);
>>
>> +   add_function("textureSamples",
>> +                _textureSamples(glsl_type::sampler2DMS_type),
>> +                _textureSamples(glsl_type::isampler2DMS_type),
>> +                _textureSamples(glsl_type::usampler2DMS_type),
>> +
>> +                _textureSamples(glsl_type::sampler2DMSArray_type),
>> +                _textureSamples(glsl_type::isampler2DMSArray_type),
>> +                _textureSamples(glsl_type::usampler2DMSArray_type),
>> +                NULL);
>> +
>>      add_function("texture",
>>                   _texture(ir_tex, v130, glsl_type::vec4_type,
>> glsl_type::sampler1D_type,  glsl_type::float_type),
>>                   _texture(ir_tex, v130, glsl_type::ivec4_type,
>> glsl_type::isampler1D_type, glsl_type::float_type),
>> @@ -4079,6 +4096,19 @@
>> builtin_builder::_textureSize(builtin_available_predicate avail,
>>   }
>>
>>   ir_function_signature *
>> +builtin_builder::_textureSamples(const glsl_type *sampler_type)
>> +{
>> +   ir_variable *s = in_var(sampler_type, "sampler");
>> +   MAKE_SIG(glsl_type::int_type, shader_samples, 1, s);
>> +
>> +   ir_texture *tex = new(mem_ctx) ir_texture(ir_texture_samples);
>> +   tex->set_sampler(new(mem_ctx) ir_dereference_variable(s),
>> glsl_type::int_type);
>> +   body.emit(ret(tex));
>> +
>> +   return sig;
>> +}
>> +
>> +ir_function_signature *
>>   builtin_builder::_texture(ir_texture_opcode opcode,
>>                             builtin_available_predicate avail,
>>                             const glsl_type *return_type,
>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>> index dd5ec2a..52cae08 100644
>> --- a/src/glsl/glcpp/glcpp-parse.y
>> +++ b/src/glsl/glcpp/glcpp-parse.y
>> @@ -2478,6 +2478,9 @@
>> _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t
>> versio
>>               if (extensions->ARB_shader_image_load_store)
>>                  add_builtin_define(parser,
>> "GL_ARB_shader_image_load_store", 1);
>>
>> +             if (extensions->ARB_shader_texture_image_samples)
>> +                add_builtin_define(parser,
>> "GL_ARB_shader_texture_image_samples", 1);
>> +
>
>
> Now that we have GL_ARB_shader_image_load_store, before exposing this
> extension we should have imageSamples() supported too. Otherwise these
> changes look good for me.

I'm not exposing the extension... *if* the extension is exposed, I'm
adding the define. But no driver sets that to true... yet.

I was hoping to get feedback on this before doing the
(trivial-for-i965) imageSamples impl. Also it would conflict trivially
with some in-flight stuff from Martin.