[Mesa-dev] [PATCH] mesa: add OES_EGL_image_external_essl3 support

[Ilia Mirkin]

On Mon, Mar 23, 2015 at 3:20 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>
>
> On 03/22/2015 12:07 AM, Ilia Mirkin wrote:
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Not sure what kind of testing is needed here... thought I'd just send
>> this out though, as the extension is simple enough. Wasn't sure
>> whether mesa already handles the requirements re marking the texture
>> as incomplete if the sampler has funny settings.
>
>
> Looks good to me, one thing that might need additional work or maybe just
> comments for now is the interaction with image load store for compute
> shaders, judging from the format checks in shaderimage.c, external textures
> would not work at the moment for BindImageTexture, I'm not sure if this
> extension is expected to work without compute shaders but just using
> GL_ARB_image_load_store (?)

This extension is written against ESSL3 which doesn't have either of
those. In ESSL3.1, both of those would be available. See issue 10 --
they're accessed using image2D. I don't really see anything in the
current shader image logic that would prevent that from working.

>
>
> 2 little things below
>
>>   src/glsl/builtin_functions.cpp  | 17 +++++++++++++++++
>>   src/glsl/builtin_types.cpp      |  3 ++-
>>   src/glsl/glsl_lexer.ll          |  2 +-
>>   src/glsl/glsl_parser_extras.cpp |  1 +
>>   src/glsl/glsl_parser_extras.h   |  2 ++
>>   src/mesa/main/extensions.c      |  1 +
>>   6 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/glsl/builtin_functions.cpp
>> b/src/glsl/builtin_functions.cpp
>> index 84bbdc2..24fd4d6 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -147,6 +147,14 @@ texture_external(const _mesa_glsl_parse_state *state)
>>      return state->OES_EGL_image_external_enable;
>>   }
>>
>> +static bool
>> +texture_external_es3(const _mesa_glsl_parse_state *state)
>> +{
>> +   return state->OES_EGL_image_external_essl3_enable &&
>> +      state->es_shader &&
>> +      state->is_version(0, 300);
>
>
> is_version fits in to previous line

With a few exceptions, this is the style used for the checks in this
file -- 1 per line. I can move it up though.

>
>
>> +}
>> +
>>   /** True if texturing functions with explicit LOD are allowed. */
>>   static bool
>>   lod_exists_in_stage(const _mesa_glsl_parse_state *state)
>> @@ -1387,6 +1395,8 @@ builtin_builder::create_builtins()
>>
>>                   _texture(ir_tex, v130, glsl_type::float_type,
>> glsl_type::sampler2DRectShadow_type, glsl_type::vec3_type),
>>
>> +                _texture(ir_tex, texture_external_es3,
>> glsl_type::vec4_type,  glsl_type::samplerExternalOES_type,
>> glsl_type::vec2_type),
>> +
>>                   _texture(ir_txb, v130_fs_only, glsl_type::vec4_type,
>> glsl_type::sampler1D_type,  glsl_type::float_type),
>>                   _texture(ir_txb, v130_fs_only, glsl_type::ivec4_type,
>> glsl_type::isampler1D_type, glsl_type::float_type),
>>                   _texture(ir_txb, v130_fs_only, glsl_type::uvec4_type,
>> glsl_type::usampler1D_type, glsl_type::float_type),
>> @@ -1546,6 +1556,9 @@ builtin_builder::create_builtins()
>>
>>                   _texture(ir_tex, v130, glsl_type::float_type,
>> glsl_type::sampler2DRectShadow_type, glsl_type::vec4_type, TEX_PROJECT),
>>
>> +                _texture(ir_tex, texture_external_es3,
>> glsl_type::vec4_type,  glsl_type::samplerExternalOES_type,
>> glsl_type::vec3_type, TEX_PROJECT),
>> +                _texture(ir_tex, texture_external_es3,
>> glsl_type::vec4_type,  glsl_type::samplerExternalOES_type,
>> glsl_type::vec4_type, TEX_PROJECT),
>> +
>>                   _texture(ir_txb, v130, glsl_type::vec4_type,
>> glsl_type::sampler1D_type,  glsl_type::vec2_type, TEX_PROJECT),
>>                   _texture(ir_txb, v130, glsl_type::ivec4_type,
>> glsl_type::isampler1D_type, glsl_type::vec2_type, TEX_PROJECT),
>>                   _texture(ir_txb, v130, glsl_type::uvec4_type,
>> glsl_type::usampler1D_type, glsl_type::vec2_type, TEX_PROJECT),
>> @@ -1604,8 +1617,12 @@ builtin_builder::create_builtins()
>>                   _texelFetch(texture_multisample, glsl_type::vec4_type,
>> glsl_type::sampler2DMSArray_type,  glsl_type::ivec3_type),
>>                   _texelFetch(texture_multisample, glsl_type::ivec4_type,
>> glsl_type::isampler2DMSArray_type, glsl_type::ivec3_type),
>>                   _texelFetch(texture_multisample, glsl_type::uvec4_type,
>> glsl_type::usampler2DMSArray_type, glsl_type::ivec3_type),
>> +
>> +                _texelFetch(texture_external_es3, glsl_type::vec4_type,
>> glsl_type::samplerExternalOES_type, glsl_type::ivec2_type),
>> +
>>                   NULL);
>>
>> +
>
>
> remove extra line

Will do.

  -ilia