On 02/28/2018 10:50 PM, mathias.froehl...@gmx.net wrote:
From: Mathias Fröhlich <mathias.froehl...@web.de>

Hi Brian,

that's what I mentioned yesterday.
You may want to test this change against your use cases of your recent
draw primitive optimization.
With one openscenegraph dlist based workload this change improves the
framerates on radeonsi by about 10% compared to without that change.

If it helps, please review...

I'll try to re-check the dlist application later. For now though, this patch looks good. One thing below...



best
Mathias


The change tries to catch more opportunities to reuse the same set
of VAO's when building up display lists. Instead of checking the
offset with respect to the beginning of the vertex buffer object
the change tries to apply this same optimization with respect to the
previous display list node.

Signed-off-by: Mathias Fröhlich <mathias.froehl...@web.de>
---
  src/mesa/vbo/vbo_save_api.c | 17 ++++++++++++++---
  1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
index 47ee355e72..c2a9921bb7 100644
--- a/src/mesa/vbo/vbo_save_api.c
+++ b/src/mesa/vbo/vbo_save_api.c
@@ -542,11 +542,18 @@ compile_vertex_list(struct gl_context *ctx)
/* Duplicate our template, increment refcounts to the storage structs:
      */
+   GLintptr old_offset = 0;
+   if (save->VAO[0]) {
+      old_offset = save->VAO[0]->BufferBinding[0].Offset
+         + save->VAO[0]->VertexAttrib[VERT_ATTRIB_POS].RelativeOffset;
+   }
     const GLsizei stride = save->vertex_size*sizeof(GLfloat);
     GLintptr buffer_offset =
         (save->buffer_map - save->vertex_store->buffer_map) * sizeof(GLfloat);
+   assert(old_offset <= buffer_offset);
+   const GLintptr offset_diff = buffer_offset - old_offset;
     GLuint start_offset = 0;
-   if (0 < buffer_offset && 0 < stride && buffer_offset % stride == 0) {
+   if (0 < offset_diff && 0 < stride && offset_diff % stride == 0) {

Minor nit: I find offset_diff > 0 && stride > 0 to be a little easier to read.

In any case, Reviewed-by: Brian Paul <bri...@vmware.com>


        /* The vertex size is an exact multiple of the buffer offset.
         * This means that we can use zero-based vertex attribute pointers
         * and specify the start of the primitive with the _mesa_prim::start
@@ -558,8 +565,9 @@ compile_vertex_list(struct gl_context *ctx)
        /* We cannot immediately update the primitives as some methods below
         * still need the uncorrected start vertices
         */
-      start_offset = buffer_offset/stride;
-      buffer_offset = 0;
+      start_offset = offset_diff/stride;
+      assert(old_offset == buffer_offset - offset_diff);
+      buffer_offset = old_offset;
     }
     GLuint offsets[VBO_ATTRIB_MAX];
     for (unsigned i = 0, offset = 0; i < VBO_ATTRIB_MAX; ++i) {
@@ -666,6 +674,9 @@ compile_vertex_list(struct gl_context *ctx)
         */
        free_vertex_store(ctx, save->vertex_store);
        save->vertex_store = NULL;
+      /* When we have a new vbo, we will for sure need a new vao */
+      for (gl_vertex_processing_mode vpm = 0; vpm < VP_MODE_MAX; ++vpm)
+         _mesa_reference_vao(ctx, &save->VAO[vpm], NULL);
/* Allocate and map new store:
         */


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

Reply via email to