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

2014-01-13 Thread 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 mar...@gmail.com wrote:
 On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré
 maxence.led...@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@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2014-01-07 Thread Fredrik Höglund 
On Friday 03 January 2014, Marek Olšák wrote:
 On Fri, Jan 3, 2014 at 2:04 PM, Marek Olšák mar...@gmail.com wrote:
  On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré
  maxence.led...@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

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2014-01-07 Thread 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.led...@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 fred...@kde.org:
 On Friday 03 January 2014, Marek Olšák wrote:
 On Fri, Jan 3, 2014 at 2:04 PM, Marek Olšák mar...@gmail.com wrote:
  On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré
  maxence.led...@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

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2014-01-03 Thread Marek Olšák 
On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré
maxence.led...@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.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2014-01-03 Thread Marek Olšák 
On Fri, Jan 3, 2014 at 2:04 PM, Marek Olšák mar...@gmail.com wrote:
 On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré
 maxence.led...@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.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2014-01-02 Thread Maxence Le Doré 
---
 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);
+  }
+  else
+for(j = 0 ; j  NUM_TEXTURE_TARGETS ; j++) {
+  GLint index = target_enum_to_index(ctx, j);
+  if(index = 0) {
+texTarget = index_to_target_enum(index);
+_mesa_BindTexture(texTarget, 0);
+  }
+  else
+continue;
+}
+ }
+   _mesa_ActiveTexture(GL_TEXTURE0 + currentTexUnit);
+}
+
 /*@}*/
diff --git a/src/mesa/main/texobj.h b/src/mesa/main/texobj.h
index 55091a6..bbf1e9b 100644
--- a/src/mesa/main/texobj.h
+++ b/src/mesa/main/texobj.h
@@ -179,6 +179,9 @@ _mesa_InvalidateTexSubImage(GLuint texture, GLint level, 
GLint xoffset,
 extern void GLAPIENTRY
 _mesa_InvalidateTexImage(GLuint texture, GLint level);
 
+extern void GLAPIENTRY
+_mesa_BindTextures(GLuint first, GLsizei count, const GLuint *textures);
+
 /*@}*/
 
 
-- 
1.8.5.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev