[Mesa-stable] [Mesa-dev] [PATCH 13/15] Revert "mesa: Make GL_TEXTURE_CUBE_MAP valid in FramebufferTextureLayer"

[Fredrik Höglund]

On Tuesday 19 May 2015, Ian Romanick wrote:
> On 05/19/2015 09:22 AM, Fredrik Höglund wrote:
> > On Monday 18 May 2015, Ian Romanick wrote:
> >> From: Ian Romanick <ian.d.romanick at intel.com>
> >>
> >> This reverts commit 03420eac0c53280beae5f72783e52950fd1e3fba.
> > 
> > Did you mean to revert this commit?
> 
> I did, but that may have been in error.  I saw that it depended on
> gl_extensions::ARB_direct_state_access and gave it the ax.  After
> looking a little closer, I think there are some issues with the original
> patch.  I think the only change necessary is to vold the
> GL_TEXTURE_CUBE_MAP case (in the second hunk) in with the "true" cases.
>  I think I'll also add some comments explaining why it and
> GL_TEXTURE_CUBE_MAP_ARRAY don't need additional version or extension checks.

But GL_TEXTURE_CUBE_MAP is not supposed to be valid
in the compatibility profile.

> I'm quite concerned that reverting this patch did not affect any piglit
> tests.  I think we need a test for both the DSA version using
> GL_TEXTURE_CUBE_MAP and the non-DSA version using GL_TEXTURE_CUBE_MAP
> when DSA is available.

That's because no one has reviewed the piglit test I posted,
so it's still sitting on the mailing list.  It tests both the DSA and
the non-DSA version.

> >> Cc: "10.6" <mesa-stable at lists.freedesktop.org>
> >> ---
> >>  src/mesa/main/fbobject.c | 36 +++++-------------------------------
> >>  1 file changed, 5 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> >> index 5b8c52a..ce1c0a6 100644
> >> --- a/src/mesa/main/fbobject.c
> >> +++ b/src/mesa/main/fbobject.c
> >> @@ -2704,9 +2704,9 @@ static bool
> >>  check_texture_target(struct gl_context *ctx, GLenum target,
> >>                       const char *caller)
> >>  {
> >> -   /* We're being called by glFramebufferTextureLayer().
> >> -    * The only legal texture types for that function are 3D,
> >> -    * cube-map, and 1D/2D/cube-map array textures.
> >> +   /* We're being called by glFramebufferTextureLayer() and
> >> +    * textarget is not used.  The only legal texture types for
> >> +    * that function are 3D and 1D/2D arrays textures.
> >>      */
> >>     switch (target) {
> >>     case GL_TEXTURE_3D:
> >> @@ -2715,11 +2715,6 @@ check_texture_target(struct gl_context *ctx, GLenum target,
> >>     case GL_TEXTURE_CUBE_MAP_ARRAY:
> >>     case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> >>        return true;
> >> -   case GL_TEXTURE_CUBE_MAP:
> >> -      /* This target is valid in TextureLayer when ARB_direct_state_access
> >> -       * or OpenGL 4.5 is supported.
> >> -       */
> >> -      return ctx->Extensions.ARB_direct_state_access;
> >>     }
> >>  
> >>     _mesa_error(ctx, GL_INVALID_OPERATION,
> >> @@ -2852,13 +2847,6 @@ check_layer(struct gl_context *ctx, GLenum target, GLint layer,
> >>           return false;
> >>        }
> >>     }
> >> -   else if (target == GL_TEXTURE_CUBE_MAP) {
> >> -      if (layer >= 6) {
> >> -         _mesa_error(ctx, GL_INVALID_VALUE,
> >> -                     "%s(layer %u >= 6)", caller, layer);
> >> -         return false;
> >> -      }
> >> -   }
> >>  
> >>     return true;
> >>  }
> >> @@ -3047,7 +3035,6 @@ _mesa_FramebufferTextureLayer(GLenum target, GLenum attachment,
> >>     GET_CURRENT_CONTEXT(ctx);
> >>     struct gl_framebuffer *fb;
> >>     struct gl_texture_object *texObj;
> >> -   GLenum textarget = 0;
> >>  
> >>     const char *func = "glFramebufferTextureLayer";
> >>  
> >> @@ -3073,15 +3060,9 @@ _mesa_FramebufferTextureLayer(GLenum target, GLenum attachment,
> >>  
> >>        if (!check_level(ctx, texObj->Target, level, func))
> >>           return;
> >> -
> >> -      if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
> >> -         assert(layer >= 0 && layer < 6);
> >> -         textarget = GL_TEXTURE_CUBE_MAP_POSITIVE_X + layer;
> >> -         layer = 0;
> >> -      }
> >>     }
> >>  
> >> -   _mesa_framebuffer_texture(ctx, fb, attachment, texObj, textarget, level,
> >> +   _mesa_framebuffer_texture(ctx, fb, attachment, texObj, 0, level,
> >>                               layer, GL_FALSE, func);
> >>  }
> >>  
> >> @@ -3093,7 +3074,6 @@ _mesa_NamedFramebufferTextureLayer(GLuint framebuffer, GLenum attachment,
> >>     GET_CURRENT_CONTEXT(ctx);
> >>     struct gl_framebuffer *fb;
> >>     struct gl_texture_object *texObj;
> >> -   GLenum textarget = 0;
> >>  
> >>     const char *func = "glNamedFramebufferTextureLayer";
> >>  
> >> @@ -3115,15 +3095,9 @@ _mesa_NamedFramebufferTextureLayer(GLuint framebuffer, GLenum attachment,
> >>  
> >>        if (!check_level(ctx, texObj->Target, level, func))
> >>           return;
> >> -
> >> -      if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
> >> -         assert(layer >= 0 && layer < 6);
> >> -         textarget = GL_TEXTURE_CUBE_MAP_POSITIVE_X + layer;
> >> -         layer = 0;
> >> -      }
> >>     }
> >>  
> >> -   _mesa_framebuffer_texture(ctx, fb, attachment, texObj, textarget, level,
> >> +   _mesa_framebuffer_texture(ctx, fb, attachment, texObj, 0, level,
> >>                               layer, GL_FALSE, func);
> >>  }
> >>  
> 
>