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

[Ian Romanick]

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.

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.

>> 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);
>>  }
>>