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

[Tapani Pälli]

On 03/23/2015 01:43 PM, Ilia Mirkin wrote:
> 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.

OK, the thing puzzling me is that how the format check in 
BindImageTexture will work out if you would call BindImageTexture with 
external texture that has some exotic format?


>>
>>
>> 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
>