Re: [Mesa-dev] [PATCH 03/14] mesa: add begin_transform_feedback() helper

2017-08-25 Thread Timothy Arceri 

On 25/08/17 19:22, Samuel Pitoiset wrote:

On 08/25/2017 02:36 AM, Timothy Arceri wrote:

On 24/08/17 23:21, Samuel Pitoiset wrote:

Signed-off-by: Samuel Pitoiset 
---
  src/mesa/main/transformfeedback.c | 49 
---

  1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/src/mesa/main/transformfeedback.c 
b/src/mesa/main/transformfeedback.c

index 307728c399..b217d0d84a 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -381,22 +381,22 @@ get_xfb_source(struct gl_context *ctx)
  }
-void GLAPIENTRY
-_mesa_BeginTransformFeedback(GLenum mode)
+static ALWAYS_INLINE void
+begin_transform_feedback(struct gl_context *ctx, GLenum mode, bool 
no_error)

  {
 struct gl_transform_feedback_object *obj;
 struct gl_transform_feedback_info *info = NULL;
+   struct gl_program *source;
 GLuint i;
 unsigned vertices_per_prim;
-   GET_CURRENT_CONTEXT(ctx);
 obj = ctx->TransformFeedback.CurrentObject;
 /* Figure out what pipeline stage is the source of data for 
transform

  * feedback.
  */
-   struct gl_program *source = get_xfb_source(ctx);
-   if (source == NULL) {
+   source = get_xfb_source(ctx);



Personally I would rather this left as is. We no longer have to bend 
to MCVSs refusal to support C99 for so many years.


It was for consistency actually, not for C99.


Sure, I get that but the reason it was done that way in the first place 
was because it needed to be to work with MSVC. IMO is just results code 
that is harder to follow and we are generally moving away from this 
style in newer code, it seems a bit backwards to be actively changing 
code back to this style.








+   if (!no_error && source == NULL) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glBeginTransformFeedback(no program active)");
return;
@@ -404,7 +404,7 @@ _mesa_BeginTransformFeedback(GLenum mode)
 info = source->sh.LinkedTransformFeedback;
-   if (info->NumOutputs == 0) {
+   if (!no_error && info->NumOutputs == 0) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glBeginTransformFeedback(no varyings to record)");
return;
@@ -421,23 +421,26 @@ _mesa_BeginTransformFeedback(GLenum mode)
vertices_per_prim = 3;
break;
 default:
-  _mesa_error(ctx, GL_INVALID_ENUM, 
"glBeginTransformFeedback(mode)");

+  if (!no_error)
+ _mesa_error(ctx, GL_INVALID_ENUM, 
"glBeginTransformFeedback(mode)");


You should be able to make this something like:

if (!no_error) {
_mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");
return;
} else {
/* Stop compiler warnings */
unreachable("Error in API use when using KHR_no_error");
}


Okay, looks good to me!

Thanks for the review.






return;
 }
-   if (obj->Active) {
-  _mesa_error(ctx, GL_INVALID_OPERATION,
-  "glBeginTransformFeedback(already active)");
-  return;
-   }
+   if (!no_error) {
+  if (obj->Active) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glBeginTransformFeedback(already active)");
+ return;
+  }
-   for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
-  if ((info->ActiveBuffers >> i) & 1) {
- if (obj->BufferNames[i] == 0) {
-_mesa_error(ctx, GL_INVALID_OPERATION,
-"glBeginTransformFeedback(binding point %d 
does not "

-"have a buffer object bound)", i);
-return;
+  for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
+ if ((info->ActiveBuffers >> i) & 1) {
+if (obj->BufferNames[i] == 0) {
+   _mesa_error(ctx, GL_INVALID_OPERATION,
+   "glBeginTransformFeedback(binding point 
%d does not "

+   "have a buffer object bound)", i);
+   return;
+}
   }
}
 }
@@ -472,6 +475,14 @@ _mesa_BeginTransformFeedback(GLenum mode)
  }
+void GLAPIENTRY
+_mesa_BeginTransformFeedback(GLenum mode)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   begin_transform_feedback(ctx, mode, false);
+}
+
+
  void GLAPIENTRY
  _mesa_EndTransformFeedback(void)
  {


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

Re: [Mesa-dev] [PATCH 03/14] mesa: add begin_transform_feedback() helper

2017-08-25 Thread Samuel Pitoiset 



On 08/25/2017 02:36 AM, Timothy Arceri wrote:

On 24/08/17 23:21, Samuel Pitoiset wrote:

Signed-off-by: Samuel Pitoiset 
---
  src/mesa/main/transformfeedback.c | 49 
---

  1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/src/mesa/main/transformfeedback.c 
b/src/mesa/main/transformfeedback.c

index 307728c399..b217d0d84a 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -381,22 +381,22 @@ get_xfb_source(struct gl_context *ctx)
  }
-void GLAPIENTRY
-_mesa_BeginTransformFeedback(GLenum mode)
+static ALWAYS_INLINE void
+begin_transform_feedback(struct gl_context *ctx, GLenum mode, bool 
no_error)

  {
 struct gl_transform_feedback_object *obj;
 struct gl_transform_feedback_info *info = NULL;
+   struct gl_program *source;
 GLuint i;
 unsigned vertices_per_prim;
-   GET_CURRENT_CONTEXT(ctx);
 obj = ctx->TransformFeedback.CurrentObject;
 /* Figure out what pipeline stage is the source of data for 
transform

  * feedback.
  */
-   struct gl_program *source = get_xfb_source(ctx);
-   if (source == NULL) {
+   source = get_xfb_source(ctx);



Personally I would rather this left as is. We no longer have to bend to 
MCVSs refusal to support C99 for so many years.


It was for consistency actually, not for C99.





+   if (!no_error && source == NULL) {
    _mesa_error(ctx, GL_INVALID_OPERATION,
    "glBeginTransformFeedback(no program active)");
    return;
@@ -404,7 +404,7 @@ _mesa_BeginTransformFeedback(GLenum mode)
 info = source->sh.LinkedTransformFeedback;
-   if (info->NumOutputs == 0) {
+   if (!no_error && info->NumOutputs == 0) {
    _mesa_error(ctx, GL_INVALID_OPERATION,
    "glBeginTransformFeedback(no varyings to record)");
    return;
@@ -421,23 +421,26 @@ _mesa_BeginTransformFeedback(GLenum mode)
    vertices_per_prim = 3;
    break;
 default:
-  _mesa_error(ctx, GL_INVALID_ENUM, 
"glBeginTransformFeedback(mode)");

+  if (!no_error)
+ _mesa_error(ctx, GL_INVALID_ENUM, 
"glBeginTransformFeedback(mode)");


You should be able to make this something like:

if (!no_error) {
    _mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");
    return;
} else {
    /* Stop compiler warnings */
    unreachable("Error in API use when using KHR_no_error");
}


Okay, looks good to me!

Thanks for the review.






    return;
 }
-   if (obj->Active) {
-  _mesa_error(ctx, GL_INVALID_OPERATION,
-  "glBeginTransformFeedback(already active)");
-  return;
-   }
+   if (!no_error) {
+  if (obj->Active) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glBeginTransformFeedback(already active)");
+ return;
+  }
-   for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
-  if ((info->ActiveBuffers >> i) & 1) {
- if (obj->BufferNames[i] == 0) {
-    _mesa_error(ctx, GL_INVALID_OPERATION,
-    "glBeginTransformFeedback(binding point %d 
does not "

-    "have a buffer object bound)", i);
-    return;
+  for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
+ if ((info->ActiveBuffers >> i) & 1) {
+    if (obj->BufferNames[i] == 0) {
+   _mesa_error(ctx, GL_INVALID_OPERATION,
+   "glBeginTransformFeedback(binding point %d 
does not "

+   "have a buffer object bound)", i);
+   return;
+    }
   }
    }
 }
@@ -472,6 +475,14 @@ _mesa_BeginTransformFeedback(GLenum mode)
  }
+void GLAPIENTRY
+_mesa_BeginTransformFeedback(GLenum mode)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   begin_transform_feedback(ctx, mode, false);
+}
+
+
  void GLAPIENTRY
  _mesa_EndTransformFeedback(void)
  {


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

Re: [Mesa-dev] [PATCH 03/14] mesa: add begin_transform_feedback() helper

2017-08-24 Thread Timothy Arceri 

On 24/08/17 23:21, Samuel Pitoiset wrote:

Signed-off-by: Samuel Pitoiset 
---
  src/mesa/main/transformfeedback.c | 49 ---
  1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/src/mesa/main/transformfeedback.c 
b/src/mesa/main/transformfeedback.c
index 307728c399..b217d0d84a 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -381,22 +381,22 @@ get_xfb_source(struct gl_context *ctx)
  }
  
  
-void GLAPIENTRY

-_mesa_BeginTransformFeedback(GLenum mode)
+static ALWAYS_INLINE void
+begin_transform_feedback(struct gl_context *ctx, GLenum mode, bool no_error)
  {
 struct gl_transform_feedback_object *obj;
 struct gl_transform_feedback_info *info = NULL;
+   struct gl_program *source;
 GLuint i;
 unsigned vertices_per_prim;
-   GET_CURRENT_CONTEXT(ctx);
  
 obj = ctx->TransformFeedback.CurrentObject;
  
 /* Figure out what pipeline stage is the source of data for transform

  * feedback.
  */
-   struct gl_program *source = get_xfb_source(ctx);
-   if (source == NULL) {
+   source = get_xfb_source(ctx);



Personally I would rather this left as is. We no longer have to bend to 
MCVSs refusal to support C99 for so many years.




+   if (!no_error && source == NULL) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glBeginTransformFeedback(no program active)");
return;
@@ -404,7 +404,7 @@ _mesa_BeginTransformFeedback(GLenum mode)
  
 info = source->sh.LinkedTransformFeedback;
  
-   if (info->NumOutputs == 0) {

+   if (!no_error && info->NumOutputs == 0) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glBeginTransformFeedback(no varyings to record)");
return;
@@ -421,23 +421,26 @@ _mesa_BeginTransformFeedback(GLenum mode)
vertices_per_prim = 3;
break;
 default:
-  _mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");
+  if (!no_error)
+ _mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");


You should be able to make this something like:

if (!no_error) {
   _mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");
   return;
} else {
   /* Stop compiler warnings */
   unreachable("Error in API use when using KHR_no_error");
}




return;
 }
  
-   if (obj->Active) {

-  _mesa_error(ctx, GL_INVALID_OPERATION,
-  "glBeginTransformFeedback(already active)");
-  return;
-   }
+   if (!no_error) {
+  if (obj->Active) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glBeginTransformFeedback(already active)");
+ return;
+  }
  
-   for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {

-  if ((info->ActiveBuffers >> i) & 1) {
- if (obj->BufferNames[i] == 0) {
-_mesa_error(ctx, GL_INVALID_OPERATION,
-"glBeginTransformFeedback(binding point %d does not "
-"have a buffer object bound)", i);
-return;
+  for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
+ if ((info->ActiveBuffers >> i) & 1) {
+if (obj->BufferNames[i] == 0) {
+   _mesa_error(ctx, GL_INVALID_OPERATION,
+   "glBeginTransformFeedback(binding point %d does not 
"
+   "have a buffer object bound)", i);
+   return;
+}
   }
}
 }
@@ -472,6 +475,14 @@ _mesa_BeginTransformFeedback(GLenum mode)
  }
  
  
+void GLAPIENTRY

+_mesa_BeginTransformFeedback(GLenum mode)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   begin_transform_feedback(ctx, mode, false);
+}
+
+
  void GLAPIENTRY
  _mesa_EndTransformFeedback(void)
  {


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

[Mesa-dev] [PATCH 03/14] mesa: add begin_transform_feedback() helper

2017-08-24 Thread Samuel Pitoiset 
Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/transformfeedback.c | 49 ---
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/src/mesa/main/transformfeedback.c 
b/src/mesa/main/transformfeedback.c
index 307728c399..b217d0d84a 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -381,22 +381,22 @@ get_xfb_source(struct gl_context *ctx)
 }
 
 
-void GLAPIENTRY
-_mesa_BeginTransformFeedback(GLenum mode)
+static ALWAYS_INLINE void
+begin_transform_feedback(struct gl_context *ctx, GLenum mode, bool no_error)
 {
struct gl_transform_feedback_object *obj;
struct gl_transform_feedback_info *info = NULL;
+   struct gl_program *source;
GLuint i;
unsigned vertices_per_prim;
-   GET_CURRENT_CONTEXT(ctx);
 
obj = ctx->TransformFeedback.CurrentObject;
 
/* Figure out what pipeline stage is the source of data for transform
 * feedback.
 */
-   struct gl_program *source = get_xfb_source(ctx);
-   if (source == NULL) {
+   source = get_xfb_source(ctx);
+   if (!no_error && source == NULL) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "glBeginTransformFeedback(no program active)");
   return;
@@ -404,7 +404,7 @@ _mesa_BeginTransformFeedback(GLenum mode)
 
info = source->sh.LinkedTransformFeedback;
 
-   if (info->NumOutputs == 0) {
+   if (!no_error && info->NumOutputs == 0) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "glBeginTransformFeedback(no varyings to record)");
   return;
@@ -421,23 +421,26 @@ _mesa_BeginTransformFeedback(GLenum mode)
   vertices_per_prim = 3;
   break;
default:
-  _mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");
+  if (!no_error)
+ _mesa_error(ctx, GL_INVALID_ENUM, "glBeginTransformFeedback(mode)");
   return;
}
 
-   if (obj->Active) {
-  _mesa_error(ctx, GL_INVALID_OPERATION,
-  "glBeginTransformFeedback(already active)");
-  return;
-   }
+   if (!no_error) {
+  if (obj->Active) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glBeginTransformFeedback(already active)");
+ return;
+  }
 
-   for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
-  if ((info->ActiveBuffers >> i) & 1) {
- if (obj->BufferNames[i] == 0) {
-_mesa_error(ctx, GL_INVALID_OPERATION,
-"glBeginTransformFeedback(binding point %d does not "
-"have a buffer object bound)", i);
-return;
+  for (i = 0; i < ctx->Const.MaxTransformFeedbackBuffers; i++) {
+ if ((info->ActiveBuffers >> i) & 1) {
+if (obj->BufferNames[i] == 0) {
+   _mesa_error(ctx, GL_INVALID_OPERATION,
+   "glBeginTransformFeedback(binding point %d does not 
"
+   "have a buffer object bound)", i);
+   return;
+}
  }
   }
}
@@ -472,6 +475,14 @@ _mesa_BeginTransformFeedback(GLenum mode)
 }
 
 
+void GLAPIENTRY
+_mesa_BeginTransformFeedback(GLenum mode)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   begin_transform_feedback(ctx, mode, false);
+}
+
+
 void GLAPIENTRY
 _mesa_EndTransformFeedback(void)
 {
-- 
2.14.1

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