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

[Maxence Le Doré]

Thanks Fredrik,
I was nearly sure this was ok from what I read of the specs. And
you're also right again : I totally forget a case where a texture has
never been bound !

2014/1/7 Maxence Le Doré <maxence.ledore at gmail.com>:
> Thanks Fredrik,
> I was nearly sure this was ok from what I read of the specs. And
> you're also right again : I totally forget a case where a texture has
> never been bound !
>
> 2014/1/7 Fredrik Höglund <fredrik at kde.org>:
>> 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 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
>>