Re: [Mesa-dev] [PATCH v3] mesa: GL_EXT_texture_norm16 extension plumbing

Tapani Pälli Sat, 21 Apr 2018 00:26:13 -0700


On 20.04.2018 19:15, Ilia Mirkin wrote:

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)


will fix

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


yes, then we will return false like should happen without the extension

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


will fix

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

Thanks, it is about glTeximage2D, glGenerateMipmap, glReadPixels,rendering to fbo with these formats and GL_EXT_copy_image. Those aretested by the Piglit test I've sent. Parser changes get tested byNV_image_formats tests in Piglit.

+            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

Re: [Mesa-dev] [PATCH v3] mesa: GL_EXT_texture_norm16 extension plumbing

Reply via email to