[Mesa-dev] [PATCH 1/2] mesa: add NV_image_formats extension support
Ilia Mirkin
imirkin at alum.mit.edu
Fri Nov 11 18:39:09 UTC 2016
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.)
> 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
More information about the mesa-dev
mailing list