[Mesa-dev] [PATCH] i965: fix transform feedback with primitive restart

[Ian Romanick]

On 06/29/2012 02:41 PM, Jordan Justen wrote:
> On Fri, Jun 29, 2012 at 2:02 PM, Ian Romanick<idr at freedesktop.org>  wrote:
>> On 06/28/2012 04:12 PM, Jordan Justen wrote:
>>>
>>> When querying GL_PRIMITIVES_GENERATED, if primitive restart
>>> is also used, then take the software primitive restart
>>> path so GL_PRIMITIVES_GENERATED is returned correctly.
>>>
>>> GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN is also updated
>>> since it will also affected by the same issue.
>>>
>>> As noted in brw_primitive_restart.c, with further work we
>>> should be able to move this situation back to a hardware
>>> handled path.
>>>
>>> Signed-off-by: Jordan Justen<jordan.l.justen at intel.com>
>>
>> Do we have piglit tests for this case?
>
> I posted a patch for a piglit test for this issue:
> "[PATCH] ext_transform_feedback: Test Tranform Feedback with Primitive Restart"
>
> It is related to:
> https://bugs.freedesktop.org/show_bug.cgi?id=50925
>
>> Is it safe to assume that they fail
>> without but pass with this patch?
>
> Yep
>
>>> ---
>>>   src/mesa/drivers/dri/i965/brw_context.h           |    2 ++
>>>   src/mesa/drivers/dri/i965/brw_primitive_restart.c |   12 ++++++++++++
>>>   src/mesa/drivers/dri/i965/brw_queryobj.c          |    4 ++++
>>>   3 files changed, 18 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
>>> b/src/mesa/drivers/dri/i965/brw_context.h
>>> index 2c9f5ad..ee860f3 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>>> @@ -1037,7 +1037,9 @@ struct brw_context
>>>         uint32_t svbi_0_max_index;
>>>         uint32_t offset_0_batch_start;
>>>         uint32_t primitives_generated;
>>> +      bool counting_primitives_generated;
>>>         uint32_t primitives_written;
>>> +      bool counting_primitives_written;
>>
>> These should be grouped together for better packing.
>
> Ok, I'll fix that.
>
> Is this Reviewed-by you with that change?

Yes.  Knowing about the piglit test was, however, more important. :)

> Thanks,
>
> -Jordan
>
>>>      } sol;
>>>
>>>      uint32_t render_target_format[MESA_FORMAT_COUNT];
>>> diff --git a/src/mesa/drivers/dri/i965/brw_primitive_restart.c
>>> b/src/mesa/drivers/dri/i965/brw_primitive_restart.c
>>> index 962ff18..02deba4 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_primitive_restart.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_primitive_restart.c
>>> @@ -69,6 +69,18 @@ can_cut_index_handle_prims(struct gl_context *ctx,
>>>                              GLuint nr_prims,
>>>                              const struct _mesa_index_buffer *ib)
>>>   {
>>> +   struct brw_context *brw = brw_context(ctx);
>>> +
>>> +   if (brw->sol.counting_primitives_generated ||
>>> +       brw->sol.counting_primitives_written) {
>>> +      /* Counting primitives generated in hardware is not currently
>>> +       * supported, so take the software path. We need to investigate
>>> +       * the *_PRIMITIVES_COUNT registers to allow this to be handled
>>> +       * entirely in hardware.
>>> +       */
>>> +      return false;
>>> +   }
>>> +
>>>      if (!can_cut_index_handle_restart_index(ctx, ib)) {
>>>         /* The primitive restart index can't be handled, so take
>>>          * the software path
>>> diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c
>>> b/src/mesa/drivers/dri/i965/brw_queryobj.c
>>> index 72b83f4..240fe32 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_queryobj.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_queryobj.c
>>> @@ -171,6 +171,7 @@ brw_begin_query(struct gl_context *ctx, struct
>>> gl_query_object *q)
>>>          * it a software counter.  So just reset the counter.
>>>          */
>>>         brw->sol.primitives_generated = 0;
>>> +      brw->sol.counting_primitives_generated = true;
>>>         break;
>>>
>>>      case GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN:
>>> @@ -178,6 +179,7 @@ brw_begin_query(struct gl_context *ctx, struct
>>> gl_query_object *q)
>>>          * it a software counter.  So just reset the counter.
>>>          */
>>>         brw->sol.primitives_written = 0;
>>> +      brw->sol.counting_primitives_written = true;
>>>         break;
>>>
>>>      default:
>>> @@ -249,6 +251,7 @@ brw_end_query(struct gl_context *ctx, struct
>>> gl_query_object *q)
>>>          * the query object.
>>>          */
>>>         query->Base.Result = brw->sol.primitives_generated;
>>> +      brw->sol.counting_primitives_generated = false;
>>>
>>>         /* And set brw->query.obj to NULL so that this query won't try to
>>> wait
>>>          * for any rendering to complete.
>>> @@ -262,6 +265,7 @@ brw_end_query(struct gl_context *ctx, struct
>>> gl_query_object *q)
>>>          * the query object.
>>>          */
>>>         query->Base.Result = brw->sol.primitives_written;
>>> +      brw->sol.counting_primitives_written = false;
>>>
>>>         /* And set brw->query.obj to NULL so that this query won't try to
>>> wait
>>>          * for any rendering to complete.
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev