[Mesa-dev] [PATCH 1/2] mesa: add NV_image_formats extension support
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Mon Nov 14 11:55:49 UTC 2016
On 11/11/16 18:39, Ilia Mirkin wrote:
> On Fri, Nov 11, 2016 at 10:40 AM, Lionel Landwerlin
> <llandwerlin at gmail.com> wrote:
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98480
>> ---
>> src/compiler/glsl/glsl_parser.yy | 5 +--
>> src/compiler/glsl/glsl_parser_extras.cpp | 58 ++++++++++++++++++++++++++++++++
>> src/compiler/glsl/glsl_parser_extras.h | 4 +++
>> src/mesa/main/extensions_table.h | 1 +
>> src/mesa/main/mtypes.h | 1 +
>> src/mesa/main/shaderimage.c | 22 ++++++++----
>> 6 files changed, 83 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
>> index 7d709c7..0055c7d 100644
>> --- a/src/compiler/glsl/glsl_parser.yy
>> +++ b/src/compiler/glsl/glsl_parser.yy
>> @@ -1339,8 +1339,9 @@ layout_qualifier_id:
>> };
>>
>> for (unsigned i = 0; i < ARRAY_SIZE(map); i++) {
>> - if (state->is_version(map[i].required_glsl,
>> - map[i].required_essl) &&
>> + if ((state->is_version(map[i].required_glsl,
>> + map[i].required_essl) ||
>> + state->check_additional_es31_image_format(map[i].format)) &&
>> match_layout_qualifier($1, map[i].name, state) == 0) {
>> $$.flags.q.explicit_image_format = 1;
>> $$.image_format = map[i].format;
>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
>> index db659adf..8d639e6 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.cpp
>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
>> @@ -28,6 +28,7 @@
>> #include "main/core.h" /* for struct gl_context */
>> #include "main/context.h"
>> #include "main/debug_output.h"
>> +#include "main/formats.h"
>> #include "main/shaderobj.h"
>> #include "util/u_atomic.h" /* for p_atomic_cmpxchg */
>> #include "util/ralloc.h"
>> @@ -355,6 +356,62 @@ _mesa_glsl_parse_state::check_version(unsigned required_glsl_version,
>> }
>>
>> /**
>> + * Determines whether a given format is supported using the
>> + * GL_NV_image_formats extension.
>> + */
>> +bool
>> +_mesa_glsl_parse_state::check_additional_es31_image_format(GLenum format) const
>> +{
>> + if (!this->NV_image_formats_enable || !is_version(0, 310))
>> + return false;
> NV_image_formats_enable can't be true without ES 3.1. (Note that this
> is an explicit #extension foo:enable action, not just the existence of
> the ext. These enables ensure that there's an appropriate context.)
>
>> +
>> + switch (format) {
>> + case GL_RG32F:
>> + case GL_RG16F:
>> + case GL_R11F_G11F_B10F:
>> + case GL_R16F:
>> + case GL_RGB10_A2:
>> + case GL_RG8:
>> + case GL_RG8_SNORM:
>> + case GL_R8:
>> + case GL_R8_SNORM:
>> + return true;
>> +
>> + case GL_RG32I:
>> + case GL_RG16I:
>> + case GL_RG8I:
>> + case GL_R16I:
>> + case GL_R8I:
>> + return true;
>> +
>> + case GL_RGB10_A2UI:
>> + case GL_RG32UI:
>> + case GL_RG16UI:
>> + case GL_RG8UI:
>> + case GL_R16UI:
>> + case GL_R8UI:
>> + return true;
>> +
>> + case GL_R16:
>> + case GL_R16_SNORM:
>> + case GL_RG16:
>> + case GL_RG16_SNORM:
>> + case GL_RGBA16:
>> + case GL_RGBA16_SNORM:
>> + /**
>> + * GL_NV_image_formats tells us that we should not expose any of the
>> + * normalized 16bits formats unless we have GL_EXT_texture_norm16 or
>> + * equivalent functionality. Since we don't have support for that
>> + * extension, just disable those for now.
>> + */
>> + return false;
>> +
>> + default:
>> + return false;
>> + }
> Why not just add something to the format map where these are parsed,
> to include the info on which formats are allowed with which extra
> extensions. (A single bool for now, maybe a bitfield later when
> EXT_texture_norm16 gets added.)
>
> IMHO that'd be much more compact.
>
>> +}
>> +
>> +/**
>> * Process a GLSL #version directive.
>> *
>> * \param version is the integer that follows the #version token.
>> @@ -687,6 +744,7 @@ static const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = {
>> EXT_AEP(EXT_texture_buffer),
>> EXT_AEP(EXT_texture_cube_map_array),
>> EXT(MESA_shader_integer_functions),
>> + EXT(NV_image_formats),
>> };
>>
>> #undef EXT
>> diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
>> index 53abbbc..a3e1aa5 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.h
>> +++ b/src/compiler/glsl/glsl_parser_extras.h
>> @@ -204,6 +204,8 @@ struct _mesa_glsl_parse_state {
>> return true;
>> }
>>
>> + bool check_additional_es31_image_format(GLenum format) const;
>> +
>> bool has_atomic_counters() const
>> {
>> return ARB_shader_atomic_counters_enable || is_version(420, 310);
>> @@ -765,6 +767,8 @@ struct _mesa_glsl_parse_state {
>> bool MESA_shader_framebuffer_fetch_non_coherent_warn;
>> bool MESA_shader_integer_functions_enable;
>> bool MESA_shader_integer_functions_warn;
>> + bool NV_image_formats_enable;
>> + bool NV_image_formats_warn;
>> /*@}*/
>>
>> /** Extensions supported by the OpenGL implementation. */
>> diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
>> index 2dbd7da..f58f2ad 100644
>> --- a/src/mesa/main/extensions_table.h
>> +++ b/src/mesa/main/extensions_table.h
>> @@ -315,6 +315,7 @@ EXT(NV_depth_clamp , ARB_depth_clamp
>> EXT(NV_draw_buffers , dummy_true , x , x , x , ES2, 2011)
>> EXT(NV_fbo_color_attachments , dummy_true , x , x , x , ES2, 2010)
>> EXT(NV_fog_distance , NV_fog_distance , GLL, x , x , x , 2001)
>> +EXT(NV_image_formats , NV_image_formats , GLL, GLC, x , 31, 2014)
> This is a GLES-only ext. You want "x" in the GLL and GLC spots.
>
> Also, is this strictly necessary? I'd recommend dropping the new
> boolean and just using ARB_shader_image_load_store here. That would
> mean this gets auto-enabled for all ES 3.1-supporting drivers. (Since
> there's no new functionality on top of what
> ARB_shader_image_load_store requires.)
Thanks.
Though I'm a bit perplex with what you're proposing.
ARB_shader_image_load_store is an OpenGL 3.2 extension, how's that
supposed to work with applications using GLES?
>
>> EXT(NV_light_max_exponent , dummy_true , GLL, x , x , x , 1999)
>> EXT(NV_packed_depth_stencil , dummy_true , GLL, GLC, x , x , 2000)
>> EXT(NV_point_sprite , NV_point_sprite , GLL, GLC, x , x , 2001)
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 08f72e0..2485881 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -3924,6 +3924,7 @@ struct gl_extensions
>> GLboolean MESA_ycbcr_texture;
>> GLboolean NV_conditional_render;
>> GLboolean NV_fog_distance;
>> + GLboolean NV_image_formats;
>> GLboolean NV_point_sprite;
>> GLboolean NV_primitive_restart;
>> GLboolean NV_texture_barrier;
>> diff --git a/src/mesa/main/shaderimage.c b/src/mesa/main/shaderimage.c
>> index db36e3b..5d866ea 100644
>> --- a/src/mesa/main/shaderimage.c
>> +++ b/src/mesa/main/shaderimage.c
>> @@ -401,7 +401,7 @@ _mesa_is_shader_image_format_supported(const struct gl_context *ctx,
>>
>> /* Formats supported on unextended desktop GL and the original
>> * ARB_shader_image_load_store extension, c.f. table 3.21 of the OpenGL 4.2
>> - * specification.
>> + * specification or by GLES 3.1 with GL_NV_image_formats extension.
>> */
>> case GL_RG32F:
>> case GL_RG16F:
>> @@ -418,17 +418,27 @@ _mesa_is_shader_image_format_supported(const struct gl_context *ctx,
>> case GL_RG8I:
>> case GL_R16I:
>> case GL_R8I:
>> - case GL_RGBA16:
>> case GL_RGB10_A2:
>> - case GL_RG16:
>> case GL_RG8:
>> - case GL_R16:
>> case GL_R8:
>> + case GL_RG8_SNORM:
>> + case GL_R8_SNORM:
>> + return _mesa_is_desktop_gl(ctx) || ctx->Extensions.NV_image_formats;
> And then this would just become "return true".
>
>> +
>> + /* Formats supported on unextended desktop GL and the original
>> + * ARB_shader_image_load_store extension, c.f. table 3.21 of the OpenGL 4.2
>> + * specification.
>> + *
>> + * These can be supported by GLES 3.1 with GL_NV_image_formats &
>> + * GL_EXT_texture_norm16 extensions but we don't have support for the
>> + * latter in Mesa yet.
>> + */
>> + case GL_RGBA16:
>> case GL_RGBA16_SNORM:
>> + case GL_RG16:
>> case GL_RG16_SNORM:
>> - case GL_RG8_SNORM:
>> + case GL_R16:
>> case GL_R16_SNORM:
>> - case GL_R8_SNORM:
>> return _mesa_is_desktop_gl(ctx);
>>
>> default:
>> --
>> 2.10.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list