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

[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 <samuel.pitoiset at gmail.com>
>> ---
>>   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)
>>   {
>>