Re: [Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

2019-05-04 Thread Mathias Fröhlich
Hi Brian,

On Friday, 3 May 2019 14:40:26 CEST Brian Paul wrote:
> All your suggested changes look good.
>
> Reviewed-by: Brian Paul 
>
> Thanks.

Pushed Thanks!

best
Mathias


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

Re: [Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

2019-05-03 Thread Brian Paul

On 05/02/2019 11:18 PM, Mathias Fröhlich wrote:

Hi Brian,

On Friday, 3 May 2019 00:17:51 CEST Brian Paul wrote:

On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

In glArrayElement, use the bitmask trick to just walk the enabled
vao arrays. This should be about equivalent in execution time to
walk the prepare aelt_context list. Finally this will allow us to
reduce the _mesa_update_state calls in a few patches.

Signed-off-by: Mathias Fröhlich 
---
   src/mesa/main/api_arrayelt.c | 78 
   1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index d46c8d14b68..62f1e73ca4c 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -1541,32 +1541,76 @@ _ae_update_state(struct gl_context *ctx)
   }


+static inline attrib_func
+func_nv(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsNV[vformat->Normalized][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline attrib_func
+func_arb(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsARB[NORM_IDX(vformat)][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline const void *
+attrib_src(const struct gl_vertex_array_object *vao,
+   const struct gl_array_attributes *array, GLint elt)
+{
+   const struct gl_vertex_buffer_binding *binding =
+  >BufferBinding[array->BufferBindingIndex];
+   const GLubyte *src
+  = ADD_POINTERS(binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
+ _mesa_vertex_attrib_address(array, binding))
+  + elt * binding->Stride;
+   return src;
+}


Could you add some brief comments on those functions to explain what
they do?


Added brief comments:

/*
  * Return VertexAttrib*NV function pointer matching the provided vertex format.
  */
static inline attrib_func
func_nv(const struct gl_vertex_format *vformat)

[...]

/*
  * Return VertexAttrib*ARB function pointer matching the provided vertex 
format.
  */
static inline attrib_func
func_arb(const struct gl_vertex_format *vformat)

[...]

/*
  * Return the address of the array attribute array at elt in the
  * vertex array object vao.
  */
static inline const void *
attrib_src(const struct gl_vertex_array_object *vao,
const struct gl_array_attributes *array, GLint elt)



Otherwise, for the rest of the series,
Reviewed-by: Brian Paul 

Nice work!!


Thanks for looking at the patches!


All your suggested changes look good.

Reviewed-by: Brian Paul 

Thanks.

-Brian



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

Re: [Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

2019-05-02 Thread Mathias Fröhlich
Hi Brian,

On Friday, 3 May 2019 00:17:51 CEST Brian Paul wrote:
> On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:
> > From: Mathias Fröhlich 
> > 
> > In glArrayElement, use the bitmask trick to just walk the enabled
> > vao arrays. This should be about equivalent in execution time to
> > walk the prepare aelt_context list. Finally this will allow us to
> > reduce the _mesa_update_state calls in a few patches.
> > 
> > Signed-off-by: Mathias Fröhlich 
> > ---
> >   src/mesa/main/api_arrayelt.c | 78 
> >   1 file changed, 61 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
> > index d46c8d14b68..62f1e73ca4c 100644
> > --- a/src/mesa/main/api_arrayelt.c
> > +++ b/src/mesa/main/api_arrayelt.c
> > @@ -1541,32 +1541,76 @@ _ae_update_state(struct gl_context *ctx)
> >   }
> > 
> > 
> > +static inline attrib_func
> > +func_nv(const struct gl_vertex_format *vformat)
> > +{
> > +   return AttribFuncsNV[vformat->Normalized][vformat->Size-1]
> > +  [TYPE_IDX(vformat->Type)];
> > +}
> > +
> > +
> > +static inline attrib_func
> > +func_arb(const struct gl_vertex_format *vformat)
> > +{
> > +   return AttribFuncsARB[NORM_IDX(vformat)][vformat->Size-1]
> > +  [TYPE_IDX(vformat->Type)];
> > +}
> > +
> > +
> > +static inline const void *
> > +attrib_src(const struct gl_vertex_array_object *vao,
> > +   const struct gl_array_attributes *array, GLint elt)
> > +{
> > +   const struct gl_vertex_buffer_binding *binding =
> > +  >BufferBinding[array->BufferBindingIndex];
> > +   const GLubyte *src
> > +  = ADD_POINTERS(binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
> > + _mesa_vertex_attrib_address(array, binding))
> > +  + elt * binding->Stride;
> > +   return src;
> > +}
> 
> Could you add some brief comments on those functions to explain what 
> they do?

Added brief comments:

/*
 * Return VertexAttrib*NV function pointer matching the provided vertex format.
 */
static inline attrib_func
func_nv(const struct gl_vertex_format *vformat)

[...]

/*
 * Return VertexAttrib*ARB function pointer matching the provided vertex format.
 */
static inline attrib_func
func_arb(const struct gl_vertex_format *vformat)

[...]

/*
 * Return the address of the array attribute array at elt in the
 * vertex array object vao.
 */
static inline const void *
attrib_src(const struct gl_vertex_array_object *vao,
   const struct gl_array_attributes *array, GLint elt)


> Otherwise, for the rest of the series,
> Reviewed-by: Brian Paul 
> 
> Nice work!!

Thanks for looking at the patches!

best

Mathias


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

Re: [Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

2019-05-02 Thread Brian Paul

On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

In glArrayElement, use the bitmask trick to just walk the enabled
vao arrays. This should be about equivalent in execution time to
walk the prepare aelt_context list. Finally this will allow us to
reduce the _mesa_update_state calls in a few patches.

Signed-off-by: Mathias Fröhlich 
---
  src/mesa/main/api_arrayelt.c | 78 
  1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index d46c8d14b68..62f1e73ca4c 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -1541,32 +1541,76 @@ _ae_update_state(struct gl_context *ctx)
  }


+static inline attrib_func
+func_nv(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsNV[vformat->Normalized][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline attrib_func
+func_arb(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsARB[NORM_IDX(vformat)][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline const void *
+attrib_src(const struct gl_vertex_array_object *vao,
+   const struct gl_array_attributes *array, GLint elt)
+{
+   const struct gl_vertex_buffer_binding *binding =
+  >BufferBinding[array->BufferBindingIndex];
+   const GLubyte *src
+  = ADD_POINTERS(binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
+ _mesa_vertex_attrib_address(array, binding))
+  + elt * binding->Stride;
+   return src;
+}


Could you add some brief comments on those functions to explain what 
they do?


Otherwise, for the rest of the series,
Reviewed-by: Brian Paul 

Nice work!!

-Brian



+
+
  void
  _mesa_array_element(struct gl_context *ctx,
  struct _glapi_table *disp, GLint elt)
  {
-   const AEcontext *actx = AE_CONTEXT(ctx);
+   const struct gl_vertex_array_object *vao = ctx->Array.VAO;
+   GLbitfield mask;

-   if (actx->dirty_state)
-  _ae_update_state(ctx);
+   /* emit conventional arrays elements */
+   mask = (VERT_BIT_FF_ALL & ~VERT_BIT_POS) & vao->Enabled;
+   while (mask) {
+  const gl_vert_attrib attrib = u_bit_scan();
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_nv(>Format)(attrib, src);
+   }

 /* emit generic attribute elements */
-   for (const AEattrib *at = actx->attribs; at->func; at++) {
-  const GLubyte *src
- = ADD_POINTERS(at->binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
-_mesa_vertex_attrib_address(at->array, at->binding))
- + elt * at->binding->Stride;
-  at->func(at->index, src);
+   mask = (VERT_BIT_GENERIC_ALL & ~VERT_BIT_GENERIC0) & vao->Enabled;
+   while (mask) {
+  const gl_vert_attrib attrib = u_bit_scan();
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_arb(>Format)(attrib - VERT_ATTRIB_GENERIC0, src);
 }

-   /* emit conventional arrays elements */
-   for (const AEarray *aa = actx->arrays; aa->offset != -1 ; aa++) {
-  const GLubyte *src
- = ADD_POINTERS(aa->binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
-_mesa_vertex_attrib_address(aa->array, aa->binding))
- + elt * aa->binding->Stride;
-  CALL_by_offset(disp, (array_func), aa->offset, ((const void *) src));
-   }
+   /* finally, vertex position */
+   if (vao->Enabled & VERT_BIT_GENERIC0) {
+  const gl_vert_attrib attrib = VERT_ATTRIB_GENERIC0;
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  /* Use glVertex(v) instead of glVertexAttrib(0, v) to be sure it's
+   * issued as the last (provoking) attribute).
+   */
+  func_nv(>Format)(0, src);
+   } else if (vao->Enabled & VERT_BIT_POS) {
+  const gl_vert_attrib attrib = VERT_ATTRIB_POS;
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_nv(>Format)(0, src);
+}
  }


--
2.20.1



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

[Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

2019-05-02 Thread Mathias . Froehlich
From: Mathias Fröhlich 

In glArrayElement, use the bitmask trick to just walk the enabled
vao arrays. This should be about equivalent in execution time to
walk the prepare aelt_context list. Finally this will allow us to
reduce the _mesa_update_state calls in a few patches.

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/main/api_arrayelt.c | 78 
 1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index d46c8d14b68..62f1e73ca4c 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -1541,32 +1541,76 @@ _ae_update_state(struct gl_context *ctx)
 }


+static inline attrib_func
+func_nv(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsNV[vformat->Normalized][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline attrib_func
+func_arb(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsARB[NORM_IDX(vformat)][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline const void *
+attrib_src(const struct gl_vertex_array_object *vao,
+   const struct gl_array_attributes *array, GLint elt)
+{
+   const struct gl_vertex_buffer_binding *binding =
+  >BufferBinding[array->BufferBindingIndex];
+   const GLubyte *src
+  = ADD_POINTERS(binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
+ _mesa_vertex_attrib_address(array, binding))
+  + elt * binding->Stride;
+   return src;
+}
+
+
 void
 _mesa_array_element(struct gl_context *ctx,
 struct _glapi_table *disp, GLint elt)
 {
-   const AEcontext *actx = AE_CONTEXT(ctx);
+   const struct gl_vertex_array_object *vao = ctx->Array.VAO;
+   GLbitfield mask;

-   if (actx->dirty_state)
-  _ae_update_state(ctx);
+   /* emit conventional arrays elements */
+   mask = (VERT_BIT_FF_ALL & ~VERT_BIT_POS) & vao->Enabled;
+   while (mask) {
+  const gl_vert_attrib attrib = u_bit_scan();
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_nv(>Format)(attrib, src);
+   }

/* emit generic attribute elements */
-   for (const AEattrib *at = actx->attribs; at->func; at++) {
-  const GLubyte *src
- = ADD_POINTERS(at->binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
-_mesa_vertex_attrib_address(at->array, at->binding))
- + elt * at->binding->Stride;
-  at->func(at->index, src);
+   mask = (VERT_BIT_GENERIC_ALL & ~VERT_BIT_GENERIC0) & vao->Enabled;
+   while (mask) {
+  const gl_vert_attrib attrib = u_bit_scan();
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_arb(>Format)(attrib - VERT_ATTRIB_GENERIC0, src);
}

-   /* emit conventional arrays elements */
-   for (const AEarray *aa = actx->arrays; aa->offset != -1 ; aa++) {
-  const GLubyte *src
- = ADD_POINTERS(aa->binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
-_mesa_vertex_attrib_address(aa->array, aa->binding))
- + elt * aa->binding->Stride;
-  CALL_by_offset(disp, (array_func), aa->offset, ((const void *) src));
-   }
+   /* finally, vertex position */
+   if (vao->Enabled & VERT_BIT_GENERIC0) {
+  const gl_vert_attrib attrib = VERT_ATTRIB_GENERIC0;
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  /* Use glVertex(v) instead of glVertexAttrib(0, v) to be sure it's
+   * issued as the last (provoking) attribute).
+   */
+  func_nv(>Format)(0, src);
+   } else if (vao->Enabled & VERT_BIT_POS) {
+  const gl_vert_attrib attrib = VERT_ATTRIB_POS;
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_nv(>Format)(0, src);
+}
 }


--
2.20.1

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