On 08/12/2015 03:00 PM, Ilia Mirkin wrote:
On Wed, Aug 12, 2015 at 2:34 AM, Tapani Pälli <[email protected]> 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.
Oh right, this is correct. Either 4.50 or the extension enabled.
+} + +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.
ok
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.
This sounds fine. imageSamples() can be done as follow-up and only then enabling the extension. For this patch;
Reviewed-by: Tapani Pälli <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
