[Mesa-dev] [PATCH 13/13] vbo: Make use of _DrawVAO from the dlist code.
Brian Paul
brianp at vmware.com
Tue Feb 20 21:36:40 UTC 2018
AFAICT, the rest of the series looks good. One nit-pick below. Sorry
for being a little slow reviewing the rest. I suspect you'll be posted
an updated v2 series.
-Brian
On 02/15/2018 12:55 PM, Mathias.Froehlich at gmx.net wrote:
> From: Mathias Fröhlich <mathias.froehlich at web.de>
>
> Finally use an internal VAO to execute display
> list draws. Avoid duplicate state validation
> for display list draws. Remove client arrays
> previously used exclusively for display lists.
Your commit messages could use longer lines (72 chars or so is safe).
Same thing for some of the other patches.
>
> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
> ---
> src/mesa/vbo/vbo_save.c | 37 ++----------
> src/mesa/vbo/vbo_save.h | 4 +-
> src/mesa/vbo/vbo_save_api.c | 132 +++++++++++++++++++++++++++++++++++++++++--
> src/mesa/vbo/vbo_save_draw.c | 64 ++++-----------------
> 4 files changed, 144 insertions(+), 93 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_save.c b/src/mesa/vbo/vbo_save.c
> index 19c40ec530..f106cf279a 100644
> --- a/src/mesa/vbo/vbo_save.c
> +++ b/src/mesa/vbo/vbo_save.c
> @@ -27,6 +27,7 @@
>
>
> #include "main/mtypes.h"
> +#include "main/arrayobj.h"
> #include "main/bufferobj.h"
>
> #include "vbo_private.h"
> @@ -44,32 +45,8 @@ void vbo_save_init( struct gl_context *ctx )
>
> vbo_save_api_init( save );
>
> - {
> - struct gl_vertex_array *arrays = save->arrays;
> - unsigned i;
> -
> - memcpy(arrays, &vbo->currval[VBO_ATTRIB_POS],
> - VERT_ATTRIB_FF_MAX * sizeof(arrays[0]));
> - for (i = 0; i < VERT_ATTRIB_FF_MAX; ++i) {
> - struct gl_vertex_array *array;
> - array = &arrays[VERT_ATTRIB_FF(i)];
> - array->BufferObj = NULL;
> - _mesa_reference_buffer_object(ctx, &arrays->BufferObj,
> - vbo->currval[VBO_ATTRIB_POS+i].BufferObj);
> - }
> -
> - memcpy(arrays + VERT_ATTRIB_GENERIC(0),
> - &vbo->currval[VBO_ATTRIB_GENERIC0],
> - VERT_ATTRIB_GENERIC_MAX * sizeof(arrays[0]));
> -
> - for (i = 0; i < VERT_ATTRIB_GENERIC_MAX; ++i) {
> - struct gl_vertex_array *array;
> - array = &arrays[VERT_ATTRIB_GENERIC(i)];
> - array->BufferObj = NULL;
> - _mesa_reference_buffer_object(ctx, &array->BufferObj,
> - vbo->currval[VBO_ATTRIB_GENERIC0+i].BufferObj);
> - }
> - }
> + for (gl_vertex_processing_mode vpm = VP_MODE_FF; vpm < VP_MODE_MAX; ++vpm)
> + save->VAO[vpm] = NULL;
>
> ctx->Driver.CurrentSavePrimitive = PRIM_OUTSIDE_BEGIN_END;
> }
> @@ -79,7 +56,9 @@ void vbo_save_destroy( struct gl_context *ctx )
> {
> struct vbo_context *vbo = vbo_context(ctx);
> struct vbo_save_context *save = &vbo->save;
> - GLuint i;
> +
> + for (gl_vertex_processing_mode vpm = VP_MODE_FF; vpm < VP_MODE_MAX; ++vpm)
> + _mesa_reference_vao(ctx, &save->VAO[vpm], NULL);
>
> if (save->prim_store) {
> if ( --save->prim_store->refcount == 0 ) {
> @@ -93,10 +72,6 @@ void vbo_save_destroy( struct gl_context *ctx )
> save->vertex_store = NULL;
> }
> }
> -
> - for (i = 0; i < VBO_ATTRIB_MAX; i++) {
> - _mesa_reference_buffer_object(ctx, &save->arrays[i].BufferObj, NULL);
> - }
> }
>
>
> diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
> index cb0bff2341..edbce3673d 100644
> --- a/src/mesa/vbo/vbo_save.h
> +++ b/src/mesa/vbo/vbo_save.h
> @@ -66,6 +66,7 @@ struct vbo_save_vertex_list {
> GLenum16 attrtype[VBO_ATTRIB_MAX];
> GLuint offsets[VBO_ATTRIB_MAX];
> GLuint vertex_size; /**< size in GLfloats */
> + struct gl_vertex_array_object *VAO[VP_MODE_MAX];
>
> /* Copy of the final vertex from node->vertex_store->bufferobj.
> * Keep this in regular (non-VBO) memory to avoid repeated
> @@ -140,14 +141,13 @@ struct vbo_save_context {
> struct gl_context *ctx;
> 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 */
> GLenum16 attrtype[VBO_ATTRIB_MAX]; /**< GL_FLOAT, GL_INT, etc */
> GLubyte active_sz[VBO_ATTRIB_MAX]; /**< 1, 2, 3 or 4 */
> GLuint vertex_size; /**< size in GLfloats */
> + struct gl_vertex_array_object *VAO[VP_MODE_MAX];
>
> GLboolean out_of_memory; /**< True if last VBO allocation failed */
>
> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
> index fb51bdb84e..1edf7b9dfa 100644
> --- a/src/mesa/vbo/vbo_save_api.c
> +++ b/src/mesa/vbo/vbo_save_api.c
> @@ -68,6 +68,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
>
>
> #include "main/glheader.h"
> +#include "main/arrayobj.h"
> #include "main/bufferobj.h"
> #include "main/context.h"
> #include "main/dlist.h"
> @@ -79,6 +80,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
> #include "main/vtxfmt.h"
> #include "main/dispatch.h"
> #include "main/state.h"
> +#include "main/varray.h"
> #include "util/bitscan.h"
>
> #include "vbo_noop.h"
> @@ -411,6 +413,112 @@ convert_line_loop_to_strip(struct vbo_save_context *save,
> }
>
>
> +/* Compare the present vao if it has the same setup. */
> +static bool
> +compare_vao(gl_vertex_processing_mode mode,
> + const struct gl_vertex_array_object *vao,
> + const struct gl_buffer_object *bo, GLintptr buffer_offset,
> + GLuint stride, GLbitfield64 vao_enabled,
> + const GLubyte size[VBO_ATTRIB_MAX],
> + const GLenum16 type[VBO_ATTRIB_MAX],
> + const GLuint offset[VBO_ATTRIB_MAX])
> +{
> + if (!vao)
> + return false;
> +
> + /* If the enabled arrays are not the same we are not equal. */
> + if (vao_enabled != vao->_Enabled)
> + return false;
> +
> + /* Check the buffer binding at 0 */
> + if (vao->BufferBinding[0].BufferObj != bo)
> + return false;
> + /* BufferBinding[0].Offset != buffer_offset is checked per attribute */
> + if (vao->BufferBinding[0].Stride != stride)
> + return false;
> + assert(vao->BufferBinding[0].InstanceDivisor == 0);
> +
> + /* Retrieve the mapping from VBO_ATTRIB to VERT_ATTRIB space */
> + const GLubyte *const vao_to_vbo_map = _vbo_attribute_alias_map[mode];
> +
> + /* Now check the enabled arrays */
> + GLbitfield mask = vao_enabled;
> + while (mask) {
> + const int attr = u_bit_scan(&mask);
> + const unsigned char vbo_attr = vao_to_vbo_map[attr];
> + const GLenum16 tp = type[vbo_attr];
> + const GLintptr off = offset[vbo_attr] + buffer_offset;
> + const struct gl_array_attributes *attrib = &vao->VertexAttrib[attr];
> + if (attrib->RelativeOffset + vao->BufferBinding[0].Offset != off)
> + return false;
> + if (attrib->Type != tp)
> + return false;
> + if (attrib->Size != size[vbo_attr])
> + return false;
> + assert(attrib->Format == GL_RGBA);
> + assert(attrib->Enabled == GL_TRUE);
> + assert(attrib->Normalized == GL_FALSE);
> + assert(attrib->Integer == vbo_attrtype_to_integer_flag(tp));
> + assert(attrib->Doubles == vbo_attrtype_to_double_flag(tp));
> + assert(attrib->BufferBindingIndex == 0);
> + }
> +
> + return true;
> +}
> +
> +
> +/* Create or reuse the vao for the vertex processing mode. */
> +static void
> +update_vao(struct gl_context *ctx,
> + gl_vertex_processing_mode mode,
> + struct gl_vertex_array_object **vao,
> + struct gl_buffer_object *bo, GLintptr buffer_offset,
> + GLuint stride, GLbitfield64 vbo_enabled,
> + const GLubyte size[VBO_ATTRIB_MAX],
> + const GLenum16 type[VBO_ATTRIB_MAX],
> + const GLuint offset[VBO_ATTRIB_MAX])
> +{
> + /* Compute the bitmasks of vao_enabled arrays */
> + GLbitfield vao_enabled = _vbo_get_vao_enabled_from_vbo(mode, vbo_enabled);
> +
> + /*
> + * Check if we can possibly reuse the exisiting one.
> + * In the long term we should reset them when something changes.
> + */
> + if (compare_vao(mode, *vao, bo, buffer_offset, stride,
> + vao_enabled, size, type, offset))
> + return;
> +
> + /* The initial refcount is 1 */
> + _mesa_reference_vao(ctx, vao, NULL);
> + *vao = _mesa_new_vao(ctx, ~((GLuint)0));
> +
> + /* Bind the buffer object at binding point 0 */
> + _mesa_bind_vertex_buffer(ctx, *vao, 0, bo, buffer_offset, stride, false);
> +
> + /* Retrieve the mapping from VBO_ATTRIB to VERT_ATTRIB space
> + * Note that the position/generic0 aliasing is done in the VAO.
> + */
> + const GLubyte *const vao_to_vbo_map = _vbo_attribute_alias_map[mode];
> + /* Now set the enable arrays */
> + GLbitfield mask = vao_enabled;
> + while (mask) {
> + const int vao_attr = u_bit_scan(&mask);
> + const GLubyte vbo_attr = vao_to_vbo_map[vao_attr];
> +
> + _vbo_set_attrib_format(ctx, *vao, vao_attr, buffer_offset,
> + size[vbo_attr], type[vbo_attr], offset[vbo_attr]);
> + _mesa_vertex_attrib_binding(ctx, *vao, vao_attr, 0, false);
> + _mesa_enable_vertex_array_attrib(ctx, *vao, vao_attr, false);
> + }
> + assert(vao_enabled == (*vao)->_Enabled);
> + assert((vao_enabled & ~(*vao)->VertexAttribBufferMask) == 0);
> +
> + /* Finalize and freeze the VAO */
> + _mesa_set_vao_immutable(ctx, *vao);
> +}
> +
> +
> /**
> * Insert the active immediate struct onto the display list currently
> * being built.
> @@ -420,6 +528,7 @@ compile_vertex_list(struct gl_context *ctx)
> {
> struct vbo_save_context *save = &vbo_context(ctx)->save;
> struct vbo_save_vertex_list *node;
> + GLintptr buffer_offset = 0;
> GLuint offset;
> unsigned i;
>
> @@ -470,6 +579,20 @@ compile_vertex_list(struct gl_context *ctx)
> node->vertex_store = save->vertex_store;
> node->prim_store = save->prim_store;
>
> + /* Create a pair of VAOs for the possible VERTEX_PROCESSING_MODEs
> + * Note that this may reuse the previous one of possible.
> + */
> + for (gl_vertex_processing_mode vpm = VP_MODE_FF; vpm < VP_MODE_MAX; ++vpm) {
> + /* create or reuse the vao */
> + update_vao(ctx, vpm, &save->VAO[vpm],
> + node->vertex_store->bufferobj, buffer_offset,
> + node->vertex_size*sizeof(GLfloat), node->enabled,
> + node->attrsz, node->attrtype, node->offsets);
> + /* Reference the vao in the dlist */
> + node->VAO[vpm] = NULL;
> + _mesa_reference_vao(ctx, &node->VAO[vpm], save->VAO[vpm]);
> + }
> +
> node->vertex_store->refcount++;
> node->prim_store->refcount++;
>
> @@ -1700,7 +1823,9 @@ 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;
> +
> + for (gl_vertex_processing_mode vpm = VP_MODE_FF; vpm < VP_MODE_MAX; ++vpm)
> + _mesa_reference_vao(ctx, &node->VAO[vpm], NULL);
>
> if (--node->vertex_store->refcount == 0)
> free_vertex_store(ctx, node->vertex_store);
> @@ -1773,7 +1898,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,
> @@ -1785,8 +1909,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 8c58fecf40..5a73eb4613 100644
> --- a/src/mesa/vbo/vbo_save_draw.c
> +++ b/src/mesa/vbo/vbo_save_draw.c
> @@ -129,58 +129,14 @@ playback_copy_to_current(struct gl_context *ctx,
>
>
> /**
> - * Treat the vertex storage as a VBO, define vertex arrays pointing
> - * into it:
> + * Set the appropriate VAO to draw.
> */
> static void
> bind_vertex_list(struct gl_context *ctx,
> const 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 attr;
> - GLbitfield varying_inputs = 0x0;
> -
> - const gl_vertex_processing_mode processing_mode
> - = ctx->VertexProgram._VPMode;
> - const GLubyte * const map = _vbo_attribute_alias_map[processing_mode];
> -
> - /* Grab VERT_ATTRIB_{POS,GENERIC0} from VBO_ATTRIB_POS */
> - const gl_attribute_map_mode mode = ATTRIBUTE_MAP_MODE_POSITION;
> - const GLubyte *const array_map = _mesa_vao_attribute_map[mode];
> - for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) {
> - const GLuint src = map[array_map[attr]];
> - const GLubyte size = node->attrsz[src];
> -
> - if (size == 0) {
> - save->inputs[attr] = &vbo->currval[map[attr]];
> - } else {
> - struct gl_vertex_array *array = &arrays[attr];
> - const GLenum16 type = node->attrtype[src];
> -
> - /* override the default array set above */
> - save->inputs[attr] = array;
> -
> - array->Ptr = (const GLubyte *) NULL + node->offsets[src];
> - array->Size = size;
> - array->StrideB = node->vertex_size * sizeof(GLfloat);
> - array->Type = type;
> - array->Integer = vbo_attrtype_to_integer_flag(type);
> - array->Format = GL_RGBA;
> - array->_ElementSize = size * sizeof(GLfloat);
> - _mesa_reference_buffer_object(ctx,
> - &array->BufferObj,
> - node->vertex_store->bufferobj);
> -
> - assert(array->BufferObj->Name);
> -
> - varying_inputs |= VERT_BIT(attr);
> - }
> - }
> -
> - _mesa_set_varying_vp_inputs(ctx, varying_inputs);
> - ctx->NewDriverState |= ctx->DriverFlags.NewArray;
> + const gl_vertex_processing_mode mode = ctx->VertexProgram._VPMode;
> + _mesa_set_draw_vao(ctx, node->VAO[mode], _vbo_get_vao_filter(mode));
> }
>
>
> @@ -258,6 +214,9 @@ vbo_save_playback_vertex_list(struct gl_context *ctx, void *data)
> goto end;
> }
>
> + bind_vertex_list(ctx, node);
> +
> + /* Need that at least one time. */
> if (ctx->NewState)
> _mesa_update_state(ctx);
>
> @@ -271,14 +230,11 @@ vbo_save_playback_vertex_list(struct gl_context *ctx, void *data)
> return;
> }
>
> - bind_vertex_list(ctx, node);
> -
> - _mesa_set_drawing_arrays(ctx, vbo->save.inputs);
> + /* Finally update the inputs array */
> + _vbo_update_inputs(ctx, &vbo->array);
> + _mesa_set_drawing_arrays(ctx, vbo->array.inputs);
>
> - /* Again...
> - */
> - if (ctx->NewState)
> - _mesa_update_state(ctx);
> + assert(ctx->NewState == 0);
>
> if (node->vertex_count > 0) {
> GLuint min_index = node->start_vertex;
>
More information about the mesa-dev
mailing list