From 3427466e6dbbb8db7c1ecda6b3859ca1cc5827a3 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 10 Dec 2012 12:31:46 -0700 Subject: [PATCH] llvmpipe: support pipe_resource-based constant buffers Before this we only supported user-based constant buffers. First, we basically plumb pipe_constant_buffer objects through llvmpipe rather than pipe_resource objects. Second, update llvmpipe_set_constant_buffer() and try_update_scene_state() so they understand both resource- and user-based constant buffers. The problem with user constant buffers is the potential for use-after-free, as seen in some WebGL tests. The next patch will flip the switch for resource-based const buffers. Reviewed-by: Jose Fonseca --- src/gallium/drivers/llvmpipe/lp_context.c | 2 +- src/gallium/drivers/llvmpipe/lp_context.h | 2 +- src/gallium/drivers/llvmpipe/lp_setup.c | 29 +++++++++++++------ src/gallium/drivers/llvmpipe/lp_setup.h | 3 +- src/gallium/drivers/llvmpipe/lp_setup_context.h | 2 +- src/gallium/drivers/llvmpipe/lp_state_fs.c | 38 ++++++++++++------------- src/gallium/drivers/llvmpipe/lp_texture.c | 6 +++- 7 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/gallium/drivers/llvmpipe/lp_context.c b/src/gallium/drivers/llvmpipe/lp_context.c index e59ae237..eb454b1 100644 --- a/src/gallium/drivers/llvmpipe/lp_context.c +++ b/src/gallium/drivers/llvmpipe/lp_context.c @@ -83,7 +83,7 @@ static void llvmpipe_destroy( struct pipe_context *pipe ) for (i = 0; i < Elements(llvmpipe->constants); i++) { for (j = 0; j < Elements(llvmpipe->constants[i]); j++) { - pipe_resource_reference(&llvmpipe->constants[i][j], NULL); + pipe_resource_reference(&llvmpipe->constants[i][j].buffer, NULL); } } diff --git a/src/gallium/drivers/llvmpipe/lp_context.h b/src/gallium/drivers/llvmpipe/lp_context.h index 5afa436..b11a3d8 100644 --- a/src/gallium/drivers/llvmpipe/lp_context.h +++ b/src/gallium/drivers/llvmpipe/lp_context.h @@ -72,7 +72,7 @@ struct llvmpipe_context { struct pipe_blend_color blend_color; struct pipe_stencil_ref stencil_ref; struct pipe_clip_state clip; - struct pipe_resource *constants[PIPE_SHADER_TYPES][LP_MAX_TGSI_CONST_BUFFERS]; + struct pipe_constant_buffer constants[PIPE_SHADER_TYPES][LP_MAX_TGSI_CONST_BUFFERS]; struct pipe_framebuffer_state framebuffer; struct pipe_poly_stipple poly_stipple; struct pipe_scissor_state scissor; diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c index 7d40d8c..5aba7a2 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup.c +++ b/src/gallium/drivers/llvmpipe/lp_setup.c @@ -554,7 +554,7 @@ lp_setup_set_fs_variant( struct lp_setup_context *setup, void lp_setup_set_fs_constants(struct lp_setup_context *setup, unsigned num, - struct pipe_resource **buffers) + struct pipe_constant_buffer *buffers) { unsigned i; @@ -563,11 +563,12 @@ lp_setup_set_fs_constants(struct lp_setup_context *setup, assert(num <= Elements(setup->constants)); for (i = 0; i < num; ++i) { - if (setup->constants[i].current != buffers[i]) { - pipe_resource_reference(&setup->constants[i].current, buffers[i]); - setup->dirty |= LP_SETUP_NEW_CONSTANTS; - } + util_copy_constant_buffer(&setup->constants[i].current, &buffers[i]); + } + for (; i < Elements(setup->constants); i++) { + util_copy_constant_buffer(&setup->constants[i].current, NULL); } + setup->dirty |= LP_SETUP_NEW_CONSTANTS; } @@ -873,11 +874,21 @@ try_update_scene_state( struct lp_setup_context *setup ) if (setup->dirty & LP_SETUP_NEW_CONSTANTS) { for (i = 0; i < Elements(setup->constants); ++i) { - struct pipe_resource *buffer = setup->constants[i].current; + struct pipe_resource *buffer = setup->constants[i].current.buffer; + const unsigned current_size = setup->constants[i].current.buffer_size; + const ubyte *current_data = NULL; if (buffer) { - unsigned current_size = buffer->width0; - const void *current_data = llvmpipe_resource_data(buffer); + /* resource buffer */ + current_data = (ubyte *) llvmpipe_resource_data(buffer); + } + else if (setup->constants[i].current.user_buffer) { + /* user-space buffer */ + current_data = (ubyte *) setup->constants[i].current.user_buffer; + } + + if (current_data) { + current_data += setup->constants[i].current.buffer_offset; /* TODO: copy only the actually used constants? */ @@ -1054,7 +1065,7 @@ lp_setup_destroy( struct lp_setup_context *setup ) } for (i = 0; i < Elements(setup->constants); i++) { - pipe_resource_reference(&setup->constants[i].current, NULL); + pipe_resource_reference(&setup->constants[i].current.buffer, NULL); } /* free the scenes in the 'empty' queue */ diff --git a/src/gallium/drivers/llvmpipe/lp_setup.h b/src/gallium/drivers/llvmpipe/lp_setup.h index 55fbd9b..55b710d 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup.h +++ b/src/gallium/drivers/llvmpipe/lp_setup.h @@ -101,8 +101,7 @@ lp_setup_set_fs_variant( struct lp_setup_context *setup, void lp_setup_set_fs_constants(struct lp_setup_context *setup, unsigned num, - struct pipe_resource **buffers); - + struct pipe_constant_buffer *buffers); void lp_setup_set_alpha_ref_value( struct lp_setup_context *setup, diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h b/src/gallium/drivers/llvmpipe/lp_setup_context.h index 60809db..4d20dd3 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup_context.h +++ b/src/gallium/drivers/llvmpipe/lp_setup_context.h @@ -128,7 +128,7 @@ struct lp_setup_context /** fragment shader constants */ struct { - struct pipe_resource *current; + struct pipe_constant_buffer current; unsigned stored_size; const void *stored_data; } constants[LP_MAX_TGSI_CONST_BUFFERS]; diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c b/src/gallium/drivers/llvmpipe/lp_state_fs.c index dced5d2..bf59a43 100644 --- a/src/gallium/drivers/llvmpipe/lp_state_fs.c +++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c @@ -2408,32 +2408,32 @@ llvmpipe_set_constant_buffer(struct pipe_context *pipe, { struct llvmpipe_context *llvmpipe = llvmpipe_context(pipe); struct pipe_resource *constants = cb ? cb->buffer : NULL; - unsigned size; - const void *data; - - if (cb && cb->user_buffer) { - constants = llvmpipe_user_buffer_create(pipe->screen, - (void *) cb->user_buffer, - cb->buffer_size, - PIPE_BIND_CONSTANT_BUFFER); - } - - size = constants ? constants->width0 : 0; - data = constants ? llvmpipe_resource_data(constants) : NULL; assert(shader < PIPE_SHADER_TYPES); assert(index < Elements(llvmpipe->constants[shader])); - if(llvmpipe->constants[shader][index] == constants) - return; + /* note: reference counting */ + util_copy_constant_buffer(&llvmpipe->constants[shader][index], cb); - draw_flush(llvmpipe->draw); + if (shader == PIPE_SHADER_VERTEX || + shader == PIPE_SHADER_GEOMETRY) { + /* Pass the constants to the 'draw' module */ + const unsigned size = cb ? cb->buffer_size : 0; + const ubyte *data; - /* note: reference counting */ - pipe_resource_reference(&llvmpipe->constants[shader][index], constants); + if (constants) { + data = (ubyte *) llvmpipe_resource_data(constants); + } + else if (cb && cb->user_buffer) { + data = (ubyte *) cb->user_buffer; + } + else { + data = NULL; + } + + if (data) + data += cb->buffer_offset; - if(shader == PIPE_SHADER_VERTEX || - shader == PIPE_SHADER_GEOMETRY) { draw_set_mapped_constant_buffer(llvmpipe->draw, shader, index, data, size); } diff --git a/src/gallium/drivers/llvmpipe/lp_texture.c b/src/gallium/drivers/llvmpipe/lp_texture.c index f4d2cb6..31ea7dc 100644 --- a/src/gallium/drivers/llvmpipe/lp_texture.c +++ b/src/gallium/drivers/llvmpipe/lp_texture.c @@ -669,8 +669,12 @@ llvmpipe_transfer_map( struct pipe_context *pipe, } } - if (resource == llvmpipe->constants[PIPE_SHADER_FRAGMENT][0]) + /* Check if we're mapping the current constant buffer */ + if ((usage & PIPE_TRANSFER_WRITE) && + resource == llvmpipe->constants[PIPE_SHADER_FRAGMENT][0].buffer) { + /* constants may have changed */ llvmpipe->dirty |= LP_NEW_CONSTANTS; + } lpt = CALLOC_STRUCT(llvmpipe_transfer); if (!lpt) -- 2.7.4