[Mesa-dev] [PATCH 3/7] mesa: implement glBindTextures()

[Ian Romanick]

On 01/07/2014 12:05 AM, Fredrik Höglund wrote:
> On Friday 03 January 2014, Marek Olšák wrote:
>> On Fri, Jan 3, 2014 at 2:04 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>> On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré
>>> <maxence.ledore at gmail.com> wrote:
>>>> ---
>>>>  src/mesa/main/texobj.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  src/mesa/main/texobj.h |  3 +++
>>>>  2 files changed, 55 insertions(+)
>>>>
>>>> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
>>>> index bddbc50..66e2fb0 100644
>>>> --- a/src/mesa/main/texobj.c
>>>> +++ b/src/mesa/main/texobj.c
>>>> @@ -1686,4 +1686,56 @@ _mesa_InvalidateTexImage(GLuint texture, GLint level)
>>>>     return;
>>>>  }
>>>>
>>>> +/** ARB_multi_bind / OpenGL 4.4 */
>>>> +
>>>> +void GLAPIENTRY
>>>> +_mesa_BindTextures(GLuint first, GLsizei count, const GLuint *textures)
>>>> +{
>>>> +   GET_CURRENT_CONTEXT(ctx);
>>>> +   struct GLuint currentTexUnit = 0;
>>>> +   int i = 0;
>>>> +
>>>> +   currentTexUnit = ctx->Texture.CurrentUnit;
>>>> +
>>>> +   if(first + count > ctx->Const.MaxCombinedTextureImageUnits) {
>>>> +      _mesa_error(ctx, GL_INVALID_OPERATION, "glBindTextures(first+count)");
>>>> +      return;
>>>> +   }
>>>> +
>>>> +   for(i = 0 ; i < count ; i++) {
>>>> +      GLuint texture;
>>>> +      struct gl_texture_object *texObj;
>>>> +      GLenum texTarget;
>>>> +      int j = 0;
>>>> +
>>>> +      if(textures == NULL)
>>>> +        texture = 0;
>>>> +      else
>>>> +        texture = textures[i];
>>>> +
>>>> +      _mesa_ActiveTexture(GL_TEXTURE0 + first + i);
>>>> +      if(texture != 0) {
>>>> +        texObj = _mesa_lookup_texture(ctx, texture);
>>>> +        if(texObj) {
>>>> +          texTarget = texObj->Target;
>>>> +          _mesa_BindTexture(texTarget, texture);
>>>> +        }
>>>> +        else
>>>> +          _mesa_error(ctx, GL_INVALID_OPERATION,
>>>> +                      "glBindTextures(textures[%i])", i);
>>>
>>> This error is set too late. It should be done before changing textures.
>>
>> Note that you make the same mistake in the other patches too. Also
>> please double-check that none of the _mesa_ functions generate errors.
> 
> This is actually not the case with the ARB_multi_bind functions:
> 
>     (11) Typically, OpenGL specifies that if an error is generated by a
>          command, that command has no effect.  This is somewhat unfortunate
>          for multi-bind commands, because it would require a first pass to
>          scan the entire list of bound objects for errors and then a second
>          pass to actually perform the bindings.  Should we have different
>          error semantics?
> 
>       RESOLVED:  Yes.  In this specification, when the parameters for one of
>       the <count> binding points are invalid, that binding point is not
>       updated and an error will be generated.  However, other binding points
>       in the same command will be updated if their parameters are valid and no
>       other error occurs.

The code should reference this spec text.  Otherwise someone will come
along later and try to "fix" it.

> The code is still wrong for a different reason though; when a texture has
> has never been bound, it doesn't have a target.  That case needs to be
> handled correctly.
> 
> Fredrik
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev