On Fri, Apr 20, 2018 at 11:33 AM, Tapani Pälli <tapani.pa...@intel.com> wrote:
> Patch enables use of short and unsigned short data for texture uploads,
> rendering and reading of framebuffers within the restrictions specified
> in GL_EXT_texture_norm16 spec.
>
> Patch also enables those 16bit format layout qualifiers listed in
> GL_NV_image_formats that depend on EXT_texture_norm16.
>
> v2: expose extension with dummy_true
> fix layout qualifier map changes (Ilia Mirkin)
> v3: use _mesa_has_EXT_texture_norm16, other fixes
> and cleanup (Ilia Mirkin)
>
> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
> ---
>
> Note, Piglit test "ext_texture_norm16-render" passes with these changes.
>
> src/compiler/glsl/glsl_parser.yy | 12 ++++----
> src/mesa/main/extensions_table.h | 1 +
> src/mesa/main/genmipmap.c | 2 +-
> src/mesa/main/glformats.c | 61
> +++++++++++++++++++++++++++++++++++++++-
> src/mesa/main/glformats.h | 3 +-
> src/mesa/main/readpix.c | 14 +++++++--
> src/mesa/main/shaderimage.c | 7 ++---
> 7 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_parser.yy
> b/src/compiler/glsl/glsl_parser.yy
> index e5ea41d4df..b4951a258a 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -1340,18 +1340,18 @@ layout_qualifier_id:
> { "r32i", GL_R32I, GLSL_TYPE_INT, 130, 310, false },
> { "r16i", GL_R16I, GLSL_TYPE_INT, 130, 0, true },
> { "r8i", GL_R8I, GLSL_TYPE_INT, 130, 0, true },
> - { "rgba16", GL_RGBA16, GLSL_TYPE_FLOAT, 130, 0, false },
> + { "rgba16", GL_RGBA16, GLSL_TYPE_FLOAT, 130, 0, true },
> { "rgb10_a2", GL_RGB10_A2, GLSL_TYPE_FLOAT, 130, 0, true },
> { "rgba8", GL_RGBA8, GLSL_TYPE_FLOAT, 130, 310, false },
> - { "rg16", GL_RG16, GLSL_TYPE_FLOAT, 130, 0, false },
> + { "rg16", GL_RG16, GLSL_TYPE_FLOAT, 130, 0, true },
> { "rg8", GL_RG8, GLSL_TYPE_FLOAT, 130, 0, true },
> - { "r16", GL_R16, GLSL_TYPE_FLOAT, 130, 0, false },
> + { "r16", GL_R16, GLSL_TYPE_FLOAT, 130, 0, true },
> { "r8", GL_R8, GLSL_TYPE_FLOAT, 130, 0, true },
> - { "rgba16_snorm", GL_RGBA16_SNORM, GLSL_TYPE_FLOAT, 130, 0,
> false },
> + { "rgba16_snorm", GL_RGBA16_SNORM, GLSL_TYPE_FLOAT, 130, 0,
> true },
> { "rgba8_snorm", GL_RGBA8_SNORM, GLSL_TYPE_FLOAT, 130, 310,
> false },
> - { "rg16_snorm", GL_RG16_SNORM, GLSL_TYPE_FLOAT, 130, 0, false
> },
> + { "rg16_snorm", GL_RG16_SNORM, GLSL_TYPE_FLOAT, 130, 0, true
> },
> { "rg8_snorm", GL_RG8_SNORM, GLSL_TYPE_FLOAT, 130, 0, true },
> - { "r16_snorm", GL_R16_SNORM, GLSL_TYPE_FLOAT, 130, 0, false },
> + { "r16_snorm", GL_R16_SNORM, GLSL_TYPE_FLOAT, 130, 0, true },
> { "r8_snorm", GL_R8_SNORM, GLSL_TYPE_FLOAT, 130, 0, true }
> };
>
> diff --git a/src/mesa/main/extensions_table.h
> b/src/mesa/main/extensions_table.h
> index 492f7c3d20..aec17750d5 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -283,6 +283,7 @@ EXT(EXT_texture_format_BGRA8888 , dummy_true
> EXT(EXT_texture_integer , EXT_texture_integer
> , GLL, GLC, x , x , 2006)
> EXT(EXT_texture_lod_bias , dummy_true
> , GLL, x , ES1, x , 1999)
> EXT(EXT_texture_mirror_clamp , EXT_texture_mirror_clamp
> , GLL, GLC, x , x , 2004)
> +EXT(EXT_texture_norm16 , dummy_true
> , x , x , x , 31, 2014)
> EXT(EXT_texture_object , dummy_true
> , GLL, x , x , x , 1995)
> EXT(EXT_texture_rectangle , NV_texture_rectangle
> , GLL, x , x , x , 2004)
> EXT(EXT_texture_rg , ARB_texture_rg
> , x , x , x , ES2, 2011)
> diff --git a/src/mesa/main/genmipmap.c b/src/mesa/main/genmipmap.c
> index 488c32f810..fd20ea2806 100644
> --- a/src/mesa/main/genmipmap.c
> +++ b/src/mesa/main/genmipmap.c
> @@ -93,7 +93,7 @@
> _mesa_is_valid_generate_texture_mipmap_internalformat(struct gl_context *ctx,
> internalformat == GL_LUMINANCE_ALPHA ||
> internalformat == GL_LUMINANCE || internalformat == GL_ALPHA ||
> internalformat == GL_BGRA_EXT ||
> - (_mesa_is_es3_color_renderable(internalformat) &&
> + (_mesa_is_es3_color_renderable(ctx, internalformat) &&
> _mesa_is_es3_texture_filterable(ctx, internalformat));
> }
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 1e797c24c2..5473ec02df 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2857,6 +2857,17 @@ _mesa_es3_error_check_format_and_type(const struct
> gl_context *ctx,
> return GL_INVALID_OPERATION;
> break;
>
> + case GL_UNSIGNED_SHORT:
> + if (!_mesa_has_EXT_texture_norm16(ctx) || internalFormat !=
> GL_RGBA16)
> + return GL_INVALID_OPERATION;
> + break;
> +
> + case GL_SHORT:
> + if (!_mesa_has_EXT_texture_norm16(ctx) ||
> + internalFormat != GL_RGBA16_SNORM)
> + return GL_INVALID_OPERATION;
> + break;
> +
> case GL_UNSIGNED_SHORT_4_4_4_4:
> switch (internalFormat) {
> case GL_RGBA:
> @@ -2984,6 +2995,17 @@ _mesa_es3_error_check_format_and_type(const struct
> gl_context *ctx,
> return GL_INVALID_OPERATION;
> break;
>
> + case GL_UNSIGNED_SHORT:
> + if (!_mesa_has_EXT_texture_norm16(ctx) || internalFormat !=
> GL_RGB16)
> + return GL_INVALID_OPERATION;
> + break;
> +
> + case GL_SHORT:
> + if (!_mesa_has_EXT_texture_norm16(ctx) ||
> + internalFormat != GL_RGB16_SNORM)
> + return GL_INVALID_OPERATION;
> + break;
> +
> case GL_UNSIGNED_SHORT_5_6_5:
> switch (internalFormat) {
> case GL_RGB:
> @@ -3115,6 +3137,17 @@ _mesa_es3_error_check_format_and_type(const struct
> gl_context *ctx,
> return GL_INVALID_OPERATION;
> break;
>
> + case GL_UNSIGNED_SHORT:
> + if (!_mesa_has_EXT_texture_norm16(ctx) || internalFormat != GL_RG16)
> + return GL_INVALID_OPERATION;
> + break;
> +
> + case GL_SHORT:
> + if (!_mesa_has_EXT_texture_norm16(ctx) ||
> + internalFormat != GL_RG16_SNORM)
> + return GL_INVALID_OPERATION;
> + break;
> +
> case GL_HALF_FLOAT:
> case GL_HALF_FLOAT_OES:
> switch (internalFormat) {
> @@ -3205,6 +3238,16 @@ _mesa_es3_error_check_format_and_type(const struct
> gl_context *ctx,
> return GL_INVALID_OPERATION;
> break;
>
> + case GL_UNSIGNED_SHORT:
> + if (ctx->Version < 31 || internalFormat != GL_R16)
!_mesa_has_EXT_texture_norm16(ctx) (and below)
> + return GL_INVALID_OPERATION;
> + break;
> +
> + case GL_SHORT:
> + if (ctx->Version < 31 || internalFormat != GL_R16_SNORM)
> + return GL_INVALID_OPERATION;
> + break;
> +
> case GL_HALF_FLOAT:
> case GL_HALF_FLOAT_OES:
> switch (internalFormat) {
> @@ -3704,7 +3747,8 @@ _mesa_tex_format_from_format_and_type(const struct
> gl_context *ctx,
> * is marked "Color Renderable" in Table 8.10 of the ES 3.2 specification.
> */
> bool
> -_mesa_is_es3_color_renderable(GLenum internal_format)
> +_mesa_is_es3_color_renderable(const struct gl_context *ctx,
> + GLenum internal_format)
> {
> switch (internal_format) {
> case GL_R8:
> @@ -3743,6 +3787,11 @@ _mesa_is_es3_color_renderable(GLenum internal_format)
> case GL_RGBA32I:
> case GL_RGBA32UI:
> return true;
> + case GL_R16:
> + case GL_RG16:
> + case GL_RGBA16:
> + if (_mesa_has_EXT_texture_norm16(ctx))
> + return true;
> default:
> return false;
> }
> @@ -3778,6 +3827,16 @@ _mesa_is_es3_texture_filterable(const struct
> gl_context *ctx,
> case GL_R11F_G11F_B10F:
> case GL_RGB9_E5:
> return true;
> + case GL_R16:
> + case GL_R16_SNORM:
> + case GL_RG16:
> + case GL_RG16_SNORM:
> + case GL_RGB16:
> + case GL_RGB16_SNORM:
> + case GL_RGBA16:
> + case GL_RGBA16_SNORM:
> + if (_mesa_has_EXT_texture_norm16(ctx))
> + return true;
Here and above, you fall through if it's false. Is that desirable?
> case GL_R32F:
> case GL_RG32F:
> case GL_RGB32F:
> diff --git a/src/mesa/main/glformats.h b/src/mesa/main/glformats.h
> index 844f1e270c..5a21317159 100644
> --- a/src/mesa/main/glformats.h
> +++ b/src/mesa/main/glformats.h
> @@ -155,7 +155,8 @@ _mesa_tex_format_from_format_and_type(const struct
> gl_context *ctx,
> GLenum gl_format, GLenum type);
>
> extern bool
> -_mesa_is_es3_color_renderable(GLenum internal_format);
> +_mesa_is_es3_color_renderable(const struct gl_context *ctx,
> + GLenum internal_format);
>
> extern bool
> _mesa_is_es3_texture_filterable(const struct gl_context *ctx,
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index 6ce340ddf9..ff0647327e 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -901,7 +901,7 @@ _mesa_readpixels(struct gl_context *ctx,
>
>
> static GLenum
> -read_pixels_es3_error_check(GLenum format, GLenum type,
> +read_pixels_es3_error_check(struct gl_context *ctx, GLenum format, GLenum
> type,
> const struct gl_renderbuffer *rb)
> {
> const GLenum internalFormat = rb->InternalFormat;
> @@ -927,6 +927,16 @@ read_pixels_es3_error_check(GLenum format, GLenum type,
> return GL_NO_ERROR;
> if (internalFormat == GL_RGB10_A2UI && type == GL_UNSIGNED_BYTE)
> return GL_NO_ERROR;
> + if (type == GL_UNSIGNED_SHORT) {
> + switch (internalFormat) {
> + case GL_R16:
> + case GL_RG16:
> + case GL_RGB16:
> + case GL_RGBA16:
> + if (_mesa_has_EXT_texture_norm16(ctx))
This should be indented in. (i.e. the if should be indented, the case
should remain as-is.)
With these minor fixes, this is
Acked-by: Ilia Mirkin <imir...@alum.mit.edu>
(I haven't done a thorough review of what all this should affect, but
the changes that you do have seem fine.)
> + return GL_NO_ERROR;
> + }
> + }
> break;
> case GL_BGRA:
> /* GL_EXT_read_format_bgra */
> @@ -1049,7 +1059,7 @@ read_pixels(GLint x, GLint y, GLsizei width, GLsizei
> height, GLenum format,
> }
> }
> } else {
> - err = read_pixels_es3_error_check(format, type, rb);
> + err = read_pixels_es3_error_check(ctx, format, type, rb);
> }
>
> if (err != GL_NO_ERROR) {
> diff --git a/src/mesa/main/shaderimage.c b/src/mesa/main/shaderimage.c
> index 596eadd4f8..ac4985d8b7 100644
> --- a/src/mesa/main/shaderimage.c
> +++ b/src/mesa/main/shaderimage.c
> @@ -430,9 +430,8 @@ _mesa_is_shader_image_format_supported(const struct
> gl_context *ctx,
> * 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.
> + * Following formats are supported by GLES 3.1 with GL_NV_image_formats &
> + * GL_EXT_texture_norm16 extensions.
> */
> case GL_RGBA16:
> case GL_RGBA16_SNORM:
> @@ -440,7 +439,7 @@ _mesa_is_shader_image_format_supported(const struct
> gl_context *ctx,
> case GL_RG16_SNORM:
> case GL_R16:
> case GL_R16_SNORM:
> - return _mesa_is_desktop_gl(ctx);
> + return (_mesa_is_desktop_gl(ctx) || _mesa_has_EXT_texture_norm16(ctx));
>
> default:
> return false;
> --
> 2.13.6
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
