Hi Brian,
That one will not work as is.
Display lists are shared objects across contexts.
Means past that change the
const struct gl_vertex_array *inputs[VBO_ATTRIB_MAX];
may be concurrently written to from different threads filling in current value
pointers from the context where the list is executed from. That leaves current
values form a differnet context and concurrency issues at least.
Beside that I am also not sure if the array update logic in bind_vertex_list
works correctly when VBO_ATTRIB* arrays ending in an aliasing VERT_ATTRIB*
slot are both enabled. You probably need to currectly mask out the VBO_ATTRIB
arrays before you distribute them across the inputs[] array. But the the logic
just to store subsequent node->arrays entries to save memory fails when
skipping the masked out arrays.
best
Mathias
On Thursday, 25 January 2018 00:20:35 CET Brian Paul wrote:
> Do more of the vertex array setup at display list compilation time
> via the compile_vertex_list() function.
>
> Then, at draw time, just do final vertex array setup dependent on
> whether we're using fixed function or a vertex shader.
>
> This can help apps which use a lot of short display list drawing.
> ---
> src/mesa/vbo/vbo_save.h | 11 ++-
> src/mesa/vbo/vbo_save_api.c | 93 ++++++++++++++++++++++---
> src/mesa/vbo/vbo_save_draw.c | 157 +++++++++++++
+-----------------------------
> 3 files changed, 147 insertions(+), 114 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
> index 51ea9cc..62a9d54 100644
> --- a/src/mesa/vbo/vbo_save.h
> +++ b/src/mesa/vbo/vbo_save.h
> @@ -85,6 +85,16 @@ struct vbo_save_vertex_list {
>
> struct vbo_save_vertex_store *vertex_store;
> struct vbo_save_primitive_store *prim_store;
> +
> + /* Array [bitcount(enabled)] of gl_vertex_array objects describing
> + * the vertex data contained in this vbo_save_vertex_list.
> + */
> + struct gl_vertex_array *arrays;
> +
> + /* The arrays to actually use at draw time. Some will point at the
> + * 'arrays' field above. The rest will point at the vbo->currval arrays
> + */
> + const struct gl_vertex_array *inputs[VBO_ATTRIB_MAX];
> };
>
>
> @@ -140,7 +150,6 @@ struct vbo_save_context {
> GLvertexformat vtxfmt;
> GLvertexformat vtxfmt_noop; /**< Used if out_of_memory is true */
> struct gl_vertex_array arrays[VBO_ATTRIB_MAX];
> - const struct gl_vertex_array *inputs[VBO_ATTRIB_MAX];
>
> GLbitfield64 enabled; /**< mask of enabled vbo arrays. */
> GLubyte attrsz[VBO_ATTRIB_MAX]; /**< 1, 2, 3 or 4 */
> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
> index 11c40a2..e394d88 100644
> --- a/src/mesa/vbo/vbo_save_api.c
> +++ b/src/mesa/vbo/vbo_save_api.c
> @@ -412,8 +412,80 @@ convert_line_loop_to_strip(struct vbo_save_context
*save,
>
>
> /**
> - * Insert the active immediate struct onto the display list currently
> - * being built.
> + * Prepare the vertex arrays which will be used for drawing the primitives
> + * in the given vertex list. By doing it here, we can avoid doing this
> + * work at display list execution/draw time.
> + */
> +static void
> +setup_vertex_arrays(struct gl_context *ctx,
> + struct vbo_save_vertex_list *node)
> +{
> + struct vbo_context *vbo = vbo_context(ctx);
> + unsigned buffer_offset = node->buffer_offset;
> + unsigned attr;
> + const unsigned num_arrays = _mesa_bitcount_64(node->enabled);
> + unsigned array_count;
> +
> + if (aligned_vertex_buffer_offset(node)) {
> + /* 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
> + * field. This results in issuing several draw calls with identical
> + * vertex attribute information. This can result in fewer state
> + * changes in drivers. In particular, the Gallium CSO module will
> + * filter out redundant vertex buffer changes.
> + */
> + buffer_offset = 0;
> + }
> +
> + assert(!node->arrays);
> + node->arrays = calloc(num_arrays, sizeof(struct gl_vertex_array));
> + if (!node->arrays) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY,
> + "compiling vertex data into display list");
> + /* just turn off all arrays */
> + node->enabled = 0;
> + return;
> + }
> +
> + array_count = 0;
> + for (attr = 0; attr < VBO_ATTRIB_MAX; attr++) {
> + if (node->attrsz[attr]) {
> + /* this vertex array points at actual per-vertex data in a VB */
> + struct gl_vertex_array *array = &node->arrays[array_count];
> +
> + array->Ptr = (const GLubyte *) NULL + buffer_offset;
> + array->Size = node->attrsz[attr];
> + array->StrideB = node->vertex_size * sizeof(GLfloat);
> + array->Type = node->attrtype[attr];
> + array->Integer = vbo_attrtype_to_integer_flag(node-
>attrtype[attr]);
> + array->Format = GL_RGBA;
> + array->_ElementSize = array->Size * sizeof(GLfloat);
> + _mesa_reference_buffer_object(ctx,
> + &array->BufferObj,
> + node->vertex_store->bufferobj);
> +
> + node->inputs[attr] = array;
> +
> + buffer_offset += node->attrsz[attr] * sizeof(GLfloat);
> +
> + array_count++;
> + }
> + else {
> + /* this vertex array points at currval/default values */
> + node->inputs[attr] = &vbo->currval[attr];
> + }
> + }
> +
> + assert(array_count == num_arrays);
> +}
> +
> +
> +/**
> + * Create a new vbo_save_vertex_list object and fill it with the vertices
> + * and primitives accumulated in the vbo_save_context.
> + * The new vbo_save_vertex_list is allocated from display list memory
> + * and will automatically be inserted into the current display list.
> */
> static void
> compile_vertex_list(struct gl_context *ctx)
> @@ -430,6 +502,8 @@ compile_vertex_list(struct gl_context *ctx)
> if (!node)
> return;
>
> + memset(node, 0, sizeof(*node));
> +
> /* Make sure the pointer is aligned to the size of a pointer */
> assert((GLintptr) node % sizeof(void *) == 0);
>
> @@ -568,6 +642,8 @@ compile_vertex_list(struct gl_context *ctx)
> node->start_vertex = 0;
> }
>
> + setup_vertex_arrays(ctx, node);
> +
> /* Reset our structures for the next run of vertices:
> */
> reset_counters(ctx);
> @@ -1679,7 +1755,6 @@ static void
> vbo_destroy_vertex_list(struct gl_context *ctx, void *data)
> {
> struct vbo_save_vertex_list *node = (struct vbo_save_vertex_list *)
data;
> - (void) ctx;
>
> if (--node->vertex_store->refcount == 0)
> free_vertex_store(ctx, node->vertex_store);
> @@ -1688,6 +1763,13 @@ vbo_destroy_vertex_list(struct gl_context *ctx, void
*data)
> free(node->prim_store);
>
> free(node->current_data);
> +
> + const unsigned num_arrays = _mesa_bitcount_64(node->enabled);
> + for (unsigned i = 0; i < num_arrays; i++) {
> + _mesa_reference_buffer_object(ctx, &(node->arrays[i].BufferObj),
NULL);
> + }
> + free(node->arrays);
> +
> node->current_data = NULL;
> }
>
> @@ -1752,7 +1834,6 @@ void
> vbo_save_api_init(struct vbo_save_context *save)
> {
> struct gl_context *ctx = save->ctx;
> - GLuint i;
>
> save->opcode_vertex_list =
> _mesa_dlist_alloc_opcode(ctx,
> @@ -1764,8 +1845,4 @@ vbo_save_api_init(struct vbo_save_context *save)
> vtxfmt_init(ctx);
> current_init(ctx);
> _mesa_noop_vtxfmt_init(&save->vtxfmt_noop);
> -
> - /* These will actually get set again when binding/drawing */
> - for (i = 0; i < VBO_ATTRIB_MAX; i++)
> - save->inputs[i] = &save->arrays[i];
> }
> diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
> index 5f5dcbe..1ef74d7 100644
> --- a/src/mesa/vbo/vbo_save_draw.c
> +++ b/src/mesa/vbo/vbo_save_draw.c
> @@ -125,69 +125,63 @@ playback_copy_to_current(struct gl_context *ctx,
> }
>
>
> -
> /**
> - * Treat the vertex storage as a VBO, define vertex arrays pointing
> - * into it:
> + * Setup the vertex arrays for drawing the primitives described by 'node'.
> + * This populates the node->inputs[] vertex arrays and calls
> + * _mesa_set_drawing_arrays(). Most of the work was previously done in
> + * the setup_vertex_arrays() function when the display list was compiled.
> */
> static void
> bind_vertex_list(struct gl_context *ctx,
> - const struct vbo_save_vertex_list *node)
> + struct vbo_save_vertex_list *node)
> {
> struct vbo_context *vbo = vbo_context(ctx);
> - struct vbo_save_context *save = &vbo->save;
> - struct gl_vertex_array *arrays = save->arrays;
> - GLuint buffer_offset = node->buffer_offset;
> - const GLubyte *map; /** map from VERT_ATTRIB_x to VBO_ATTRIB_x */
> - GLuint attr;
> - GLubyte node_attrsz[VBO_ATTRIB_MAX]; /* copy of node->attrsz[] */
> - GLenum node_attrtype[VBO_ATTRIB_MAX]; /* copy of node->attrtype[] */
> + const enum vp_mode vp_mode = get_vp_mode(ctx);
> + unsigned array_count = 0;
> GLbitfield64 enabled = node->enabled;
>
> - memcpy(node_attrsz, node->attrsz, sizeof(node->attrsz));
> - memcpy(node_attrtype, node->attrtype, sizeof(node->attrtype));
> -
> - if (aligned_vertex_buffer_offset(node)) {
> - /* 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
> - * field. This results in issuing several draw calls with identical
> - * vertex attribute information. This can result in fewer state
> - * changes in drivers. In particular, the Gallium CSO module will
> - * filter out redundant vertex buffer changes.
> - */
> - buffer_offset = 0;
> + if (vp_mode == VP_SHADER) {
> + /* per-vertex materials are to be ignored when using a VS */
> + enabled &= ~VBO_ATTRIBS_MATERIALS;
> }
>
> - /* Install the default (ie Current) attributes first */
> - for (attr = 0; attr < VERT_ATTRIB_FF_MAX; attr++) {
> - save->inputs[attr] = &vbo->currval[VBO_ATTRIB_POS + attr];
> - }
> + /* Loop over the attributes which are present in the vertex buffer
> + * and set the node->input vertex arrays to point to those attributes.
> + */
> + GLbitfield64 enabled_scan = enabled;
> + while (enabled_scan) {
> + const int attr = u_bit_scan64(&enabled_scan);
> + unsigned dst;
>
> - /* Overlay other active attributes */
> - switch (get_vp_mode(ctx)) {
> - case VP_FF:
> - /* Point the generic attributes at the legacy material values */
> - for (attr = 0; attr < MAT_ATTRIB_MAX; attr++) {
> - save->inputs[VERT_ATTRIB_GENERIC(attr)] =
> - &vbo->currval[VBO_ATTRIB_MAT_FRONT_AMBIENT+attr];
> + /* Here is where we resolve whether array[16...] points to legacy
> + * material coefficients or generic vertex attributes.
> + */
> + if (vp_mode == VP_FF) {
> + if (attr >= VBO_ATTRIB_FIRST_MATERIAL) {
> + dst = VBO_ATTRIB_GENERIC0 + attr - VBO_ATTRIB_FIRST_MATERIAL;
> + } else {
> + dst = attr;
> + }
> + } else {
> + assert(vp_mode == VP_SHADER);
> + dst = attr;
> }
> - map = vbo->map_vp_none;
>
> - /* shift material attrib flags into generic attribute positions */
> + assert(dst < VERT_ATTRIB_MAX);
> + assert(node->arrays);
> + node->inputs[dst] = &node->arrays[array_count++];
> + }
> +
> + /* Final vertex array setup depending on whether we're using fixed
> + * function or a vertex shader.
> + */
> + if (vp_mode == VP_FF) {
> + /* Update the enabled attribute bitfield so the material attrib bits
> + * replace the generic attrib bits.
> + */
> enabled = (enabled & VBO_ATTRIBS_LEGACY)
> | ((enabled & VBO_ATTRIBS_MATERIALS) >> VBO_MATERIAL_SHIFT);
> - break;
> - case VP_SHADER:
> - for (attr = 0; attr < VERT_ATTRIB_GENERIC_MAX; attr++) {
> - save->inputs[VERT_ATTRIB_GENERIC(attr)] =
> - &vbo->currval[VBO_ATTRIB_GENERIC0+attr];
> - }
> - map = vbo->map_vp_arb;
> -
> - /* per-vertex materials are to be ignored when using a VS */
> - enabled &= ~VBO_ATTRIBS_MATERIALS;
> -
> + } else {
> /* check if VERT_ATTRIB_POS is not read but VERT_BIT_GENERIC0 is
read.
> * In that case we effectively need to route the data from
> * glVertexAttrib(0, val) calls to feed into the GENERIC0 input.
> @@ -196,64 +190,19 @@ bind_vertex_list(struct gl_context *ctx,
> ctx->VertexProgram._Current->info.inputs_read;
> if ((inputs_read & VERT_BIT_POS) == 0 &&
> (inputs_read & VERT_BIT_GENERIC0)) {
> - save->inputs[VERT_ATTRIB_GENERIC0] = save->inputs[0];
> - node_attrsz[VERT_ATTRIB_GENERIC0] = node_attrsz[0];
> - node_attrtype[VERT_ATTRIB_GENERIC0] = node_attrtype[0];
> - node_attrsz[0] = 0;
> - enabled = (enabled & ~VERT_BIT_POS) | VERT_BIT_GENERIC0;
> - }
> - break;
> - default:
> - unreachable("Bad vertex program mode");
> - }
> -
> -#ifdef DEBUG
> - /* verify the enabled bitmask is consistent with attribute size info */
> - {
> - GLbitfield64 used = 0x0;
> - for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) {
> - const GLuint src = map[attr];
> - if (node_attrsz[src]) {
> - assert(enabled & VERT_BIT(attr));
> - used |= VERT_BIT(attr);
> - } else {
> - assert((enabled & VERT_BIT(attr)) == 0);
> - }
> + node->inputs[VERT_ATTRIB_GENERIC0] = &node-
>arrays[VBO_ATTRIB_POS];
> + node->inputs[VERT_ATTRIB_POS] = &vbo->currval[VBO_ATTRIB_POS];
> + enabled &= ~BITFIELD64_BIT(VBO_ATTRIB_POS);
> + enabled |= BITFIELD64_BIT(VBO_ATTRIB_GENERIC0);
> }
> - assert(used == enabled);
> }
> -#endif
>
> - GLbitfield64 enabled_scan = enabled;
> - while (enabled_scan) {
> - const int attr = u_bit_scan64(&enabled_scan);
> - const GLuint src = map[attr];
> -
> - if (node_attrsz[src]) {
> - struct gl_vertex_array *array = &arrays[attr];
> -
> - /* override the default array set above */
> - save->inputs[attr] = array;
> -
> - array->Ptr = (const GLubyte *) NULL + buffer_offset;
> - array->Size = node_attrsz[src];
> - array->StrideB = node->vertex_size * sizeof(GLfloat);
> - array->Type = node_attrtype[src];
> - array->Integer = vbo_attrtype_to_integer_flag(node_attrtype[src]);
> - array->Format = GL_RGBA;
> - array->_ElementSize = array->Size * sizeof(GLfloat);
> - _mesa_reference_buffer_object(ctx,
> - &array->BufferObj,
> - node->vertex_store->bufferobj);
> -
> - assert(array->BufferObj->Name);
> -
> - buffer_offset += node_attrsz[src] * sizeof(GLfloat);
> - }
> - }
> + /* make sure none of the extra VBO_ATTRIB bits are set */
> + assert((enabled & (GLbitfield64) VERT_BIT_ALL) == enabled);
>
> _mesa_set_varying_vp_inputs(ctx, enabled);
> - ctx->NewDriverState |= ctx->DriverFlags.NewArray;
> +
> + _mesa_set_drawing_arrays(ctx, node->inputs);
> }
>
>
> @@ -292,8 +241,8 @@ loopback_vertex_list(struct gl_context *ctx,
> void
> vbo_save_playback_vertex_list(struct gl_context *ctx, void *data)
> {
> - const struct vbo_save_vertex_list *node =
> - (const struct vbo_save_vertex_list *) data;
> + struct vbo_save_vertex_list *node =
> + (struct vbo_save_vertex_list *) data;
> struct vbo_context *vbo = vbo_context(ctx);
> struct vbo_save_context *save = &vbo->save;
> GLboolean remap_vertex_store = GL_FALSE;
> @@ -346,8 +295,6 @@ vbo_save_playback_vertex_list(struct gl_context *ctx,
void *data)
>
> bind_vertex_list(ctx, node);
>
> - _mesa_set_drawing_arrays(ctx, vbo->save.inputs);
> -
> /* Again...
> */
> if (ctx->NewState)
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
