[Mesa-dev] [PATCH 3/3] llvmpipe/draw: Fix texture sampling in geometry shaders
Brian Paul
brianp at vmware.com
Wed Mar 27 08:04:54 PDT 2013
On 03/26/2013 06:56 PM, Zack Rusin wrote:
> We weren't correctly propagating the samplers and sampler views
> when they were related to geometry shaders.
>
> Signed-off-by: Zack Rusin<zackr at vmware.com>
> ---
> src/gallium/auxiliary/draw/draw_context.c | 4 +-
> src/gallium/auxiliary/draw/draw_llvm.c | 83 ++++++++-------
> src/gallium/auxiliary/draw/draw_llvm.h | 31 +++---
> src/gallium/drivers/llvmpipe/lp_context.c | 4 +
> src/gallium/drivers/llvmpipe/lp_context.h | 1 +
> src/gallium/drivers/llvmpipe/lp_draw_arrays.c | 4 +
> src/gallium/drivers/llvmpipe/lp_state.h | 8 ++
> src/gallium/drivers/llvmpipe/lp_state_sampler.c | 127 +++++++++++++++++++++++
> 8 files changed, 205 insertions(+), 57 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_context.c b/src/gallium/auxiliary/draw/draw_context.c
> index d64b82b..ceb74df 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -792,8 +792,8 @@ draw_set_samplers(struct draw_context *draw,
> draw->num_samplers[shader_stage] = num;
>
> #ifdef HAVE_LLVM
> - if (draw->llvm&& shader_stage == PIPE_SHADER_VERTEX)
> - draw_llvm_set_sampler_state(draw);
> + if (draw->llvm)
> + draw_llvm_set_sampler_state(draw, shader_stage);
> #endif
> }
>
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index f857183..3e47452 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -249,17 +249,17 @@ create_gs_jit_context_type(struct gallivm_state *gallivm,
> elem_types[1] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 4),
> DRAW_TOTAL_CLIP_PLANES), 0);
> elem_types[2] = LLVMPointerType(float_type, 0); /* viewport */
> -
> - elem_types[3] = LLVMPointerType(LLVMPointerType(int_type, 0), 0);
> - elem_types[4] = LLVMPointerType(LLVMVectorType(int_type,
> - vector_length), 0);
> - elem_types[5] = LLVMPointerType(LLVMVectorType(int_type,
> - vector_length), 0);
>
> - elem_types[6] = LLVMArrayType(texture_type,
> + elem_types[3] = LLVMArrayType(texture_type,
> PIPE_MAX_SHADER_SAMPLER_VIEWS); /* textures */
> - elem_types[7] = LLVMArrayType(sampler_type,
> + elem_types[4] = LLVMArrayType(sampler_type,
> PIPE_MAX_SAMPLERS); /* samplers */
> +
> + elem_types[5] = LLVMPointerType(LLVMPointerType(int_type, 0), 0);
> + elem_types[6] = LLVMPointerType(LLVMVectorType(int_type,
> + vector_length), 0);
> + elem_types[7] = LLVMPointerType(LLVMVectorType(int_type,
> + vector_length), 0);
>
> context_type = LLVMStructTypeInContext(gallivm->context, elem_types,
> Elements(elem_types), 0);
> @@ -275,18 +275,18 @@ create_gs_jit_context_type(struct gallivm_state *gallivm,
> target, context_type, 1);
> LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, viewport,
> target, context_type, 2);
> - LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, prim_lengths,
> - target, context_type, 3);
> - LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, emitted_vertices,
> - target, context_type, 4);
> - LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, emitted_prims,
> - target, context_type, 5);
> LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, textures,
> target, context_type,
> - DRAW_GS_JIT_CTX_TEXTURES);
> + DRAW_JIT_CTX_TEXTURES);
> LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, samplers,
> target, context_type,
> - DRAW_GS_JIT_CTX_SAMPLERS);
> + DRAW_JIT_CTX_SAMPLERS);
> + LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, prim_lengths,
> + target, context_type, 5);
> + LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, emitted_vertices,
> + target, context_type, 6);
> + LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, emitted_prims,
> + target, context_type, 7);
> LP_CHECK_STRUCT_SIZE(struct draw_gs_jit_context,
> target, context_type);
>
> @@ -1721,33 +1721,36 @@ draw_llvm_set_mapped_texture(struct draw_context *draw,
>
>
> void
> -draw_llvm_set_sampler_state(struct draw_context *draw)
> +draw_llvm_set_sampler_state(struct draw_context *draw,
> + unsigned shader_type)
> {
> unsigned i;
>
> - for (i = 0; i< draw->num_samplers[PIPE_SHADER_VERTEX]; i++) {
> - struct draw_jit_sampler *jit_sam =&draw->llvm->jit_context.samplers[i];
> -
> - if (draw->samplers[i]) {
> - const struct pipe_sampler_state *s
> - = draw->samplers[PIPE_SHADER_VERTEX][i];
> - jit_sam->min_lod = s->min_lod;
> - jit_sam->max_lod = s->max_lod;
> - jit_sam->lod_bias = s->lod_bias;
> - COPY_4V(jit_sam->border_color, s->border_color.f);
> + if (shader_type == PIPE_SHADER_VERTEX) {
> + for (i = 0; i< draw->num_samplers[PIPE_SHADER_VERTEX]; i++) {
> + struct draw_jit_sampler *jit_sam =&draw->llvm->jit_context.samplers[i];
> +
> + if (draw->samplers[i]) {
> + const struct pipe_sampler_state *s
> + = draw->samplers[PIPE_SHADER_VERTEX][i];
> + jit_sam->min_lod = s->min_lod;
> + jit_sam->max_lod = s->max_lod;
> + jit_sam->lod_bias = s->lod_bias;
> + COPY_4V(jit_sam->border_color, s->border_color.f);
> + }
> }
> - }
> -
> - for (i = 0; i< draw->num_samplers[PIPE_SHADER_GEOMETRY]; i++) {
> - struct draw_jit_sampler *jit_sam =&draw->llvm->gs_jit_context.samplers[i];
> -
> - if (draw->samplers[i]) {
> - const struct pipe_sampler_state *s
> - = draw->samplers[PIPE_SHADER_GEOMETRY][i];
> - jit_sam->min_lod = s->min_lod;
> - jit_sam->max_lod = s->max_lod;
> - jit_sam->lod_bias = s->lod_bias;
> - COPY_4V(jit_sam->border_color, s->border_color.f);
> + } else if (shader_type == PIPE_SHADER_GEOMETRY) {
> + for (i = 0; i< draw->num_samplers[PIPE_SHADER_GEOMETRY]; i++) {
> + struct draw_jit_sampler *jit_sam =&draw->llvm->gs_jit_context.samplers[i];
> +
> + if (draw->samplers[i]) {
> + const struct pipe_sampler_state *s
> + = draw->samplers[PIPE_SHADER_GEOMETRY][i];
> + jit_sam->min_lod = s->min_lod;
> + jit_sam->max_lod = s->max_lod;
> + jit_sam->lod_bias = s->lod_bias;
> + COPY_4V(jit_sam->border_color, s->border_color.f);
> + }
> }
> }
It looks like the PIPE_SHADER_VERTEX and PIPE_SHADER_GEOMETRY if/else
cases are identical except for the RHS of the jit_sam assignment. Did
you consider combining that code?
> }
> @@ -1950,6 +1953,8 @@ draw_gs_llvm_generate(struct draw_llvm *llvm,
> &llvm->draw->gs.geometry_shader->info,
> &gs_args);
>
> + sampler->destroy(sampler);
> +
> lp_build_mask_end(&mask);
>
> LLVMBuildRet(builder, lp_build_zero(gallivm, lp_type_uint(32)));
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.h b/src/gallium/auxiliary/draw/draw_llvm.h
> index fc0d2bd..3a0fe45 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.h
> +++ b/src/gallium/auxiliary/draw/draw_llvm.h
> @@ -186,12 +186,14 @@ struct draw_gs_jit_context
> float (*planes) [DRAW_TOTAL_CLIP_PLANES][4];
> float *viewport;
>
> + /* There two need to be exactly at DRAW_JIT_CTX_TEXTURES and
> + * DRAW_JIT_CTX_SAMPLERS positions in the struct */
> + struct draw_jit_texture textures[PIPE_MAX_SHADER_SAMPLER_VIEWS];
> + struct draw_jit_sampler samplers[PIPE_MAX_SAMPLERS];
> +
> int **prim_lengths;
> int *emitted_vertices;
> int *emitted_prims;
> -
> - struct draw_jit_texture textures[PIPE_MAX_SHADER_SAMPLER_VIEWS];
> - struct draw_jit_sampler samplers[PIPE_MAX_SAMPLERS];
> };
>
>
> @@ -204,23 +206,20 @@ struct draw_gs_jit_context
> #define draw_gs_jit_context_viewport(_gallivm, _ptr) \
> lp_build_struct_get(_gallivm, _ptr, 2, "viewport")
>
> +#define draw_gs_jit_context_textures(_gallivm, _ptr) \
> + lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_JIT_CTX_TEXTURES, "textures")
> +
> +#define draw_gs_jit_context_samplers(_gallivm, _ptr) \
> + lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_JIT_CTX_SAMPLERS, "samplers")
> +
> #define draw_gs_jit_prim_lengths(_gallivm, _ptr) \
> - lp_build_struct_get(_gallivm, _ptr, 3, "prim_lengths")
> + lp_build_struct_get(_gallivm, _ptr, 5, "prim_lengths")
>
> #define draw_gs_jit_emitted_vertices(_gallivm, _ptr) \
> - lp_build_struct_get(_gallivm, _ptr, 4, "emitted_vertices")
> + lp_build_struct_get(_gallivm, _ptr, 6, "emitted_vertices")
>
> #define draw_gs_jit_emitted_prims(_gallivm, _ptr) \
> - lp_build_struct_get(_gallivm, _ptr, 5, "emitted_prims")
> -
> -#define DRAW_GS_JIT_CTX_TEXTURES 6
> -#define DRAW_GS_JIT_CTX_SAMPLERS 7
> -
> -#define draw_gs_jit_context_textures(_gallivm, _ptr) \
> - lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_GS_JIT_CTX_TEXTURES, "textures")
> -
> -#define draw_gs_jit_context_samplers(_gallivm, _ptr) \
> - lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_GS_JIT_CTX_SAMPLERS, "samplers")
> + lp_build_struct_get(_gallivm, _ptr, 7, "emitted_prims")
>
>
>
> @@ -478,7 +477,7 @@ draw_llvm_sampler_soa_create(const struct draw_sampler_static_state *static_stat
> LLVMValueRef context_ptr);
>
> void
> -draw_llvm_set_sampler_state(struct draw_context *draw);
> +draw_llvm_set_sampler_state(struct draw_context *draw, unsigned shader_stage);
>
> void
> draw_llvm_set_mapped_texture(struct draw_context *draw,
> diff --git a/src/gallium/drivers/llvmpipe/lp_context.c b/src/gallium/drivers/llvmpipe/lp_context.c
> index 25b1cd0..fa675ea 100644
> --- a/src/gallium/drivers/llvmpipe/lp_context.c
> +++ b/src/gallium/drivers/llvmpipe/lp_context.c
> @@ -81,6 +81,10 @@ static void llvmpipe_destroy( struct pipe_context *pipe )
> pipe_sampler_view_reference(&llvmpipe->sampler_views[PIPE_SHADER_VERTEX][i], NULL);
> }
>
> + for (i = 0; i< Elements(llvmpipe->sampler_views[0]); i++) {
> + pipe_sampler_view_reference(&llvmpipe->sampler_views[PIPE_SHADER_GEOMETRY][i], NULL);
> + }
> +
> for (i = 0; i< Elements(llvmpipe->constants); i++) {
> for (j = 0; j< Elements(llvmpipe->constants[i]); j++) {
> 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 9ccb67b..6ee7b99 100644
> --- a/src/gallium/drivers/llvmpipe/lp_context.h
> +++ b/src/gallium/drivers/llvmpipe/lp_context.h
> @@ -82,6 +82,7 @@ struct llvmpipe_context {
> struct pipe_vertex_buffer vertex_buffer[PIPE_MAX_ATTRIBS];
> struct pipe_index_buffer index_buffer;
> struct pipe_resource *mapped_vs_tex[PIPE_MAX_SHADER_SAMPLER_VIEWS];
> + struct pipe_resource *mapped_gs_tex[PIPE_MAX_SHADER_SAMPLER_VIEWS];
>
> unsigned num_samplers[PIPE_SHADER_TYPES];
> unsigned num_sampler_views[PIPE_SHADER_TYPES];
> diff --git a/src/gallium/drivers/llvmpipe/lp_draw_arrays.c b/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> index eaa9ba3..ae00c49 100644
> --- a/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> +++ b/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> @@ -98,6 +98,9 @@ llvmpipe_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
> llvmpipe_prepare_vertex_sampling(lp,
> lp->num_sampler_views[PIPE_SHADER_VERTEX],
> lp->sampler_views[PIPE_SHADER_VERTEX]);
> + llvmpipe_prepare_geometry_sampling(lp,
> + lp->num_sampler_views[PIPE_SHADER_GEOMETRY],
> + lp->sampler_views[PIPE_SHADER_GEOMETRY]);
>
> /* draw! */
> draw_vbo(draw, info);
> @@ -114,6 +117,7 @@ llvmpipe_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
> draw_set_mapped_so_targets(draw, 0, NULL);
>
> llvmpipe_cleanup_vertex_sampling(lp);
> + llvmpipe_cleanup_geometry_sampling(lp);
>
> /*
> * TODO: Flush only when a user vertex/index buffer is present
> diff --git a/src/gallium/drivers/llvmpipe/lp_state.h b/src/gallium/drivers/llvmpipe/lp_state.h
> index 94a21a0..1f27d70 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state.h
> +++ b/src/gallium/drivers/llvmpipe/lp_state.h
> @@ -141,4 +141,12 @@ void
> llvmpipe_cleanup_vertex_sampling(struct llvmpipe_context *ctx);
>
>
> +void
> +llvmpipe_prepare_geometry_sampling(struct llvmpipe_context *ctx,
> + unsigned num,
> + struct pipe_sampler_view **views);
> +void
> +llvmpipe_cleanup_geometry_sampling(struct llvmpipe_context *ctx);
> +
> +
> #endif
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_sampler.c b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
> index 7441973..5292563 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_sampler.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
> @@ -376,6 +376,133 @@ llvmpipe_cleanup_vertex_sampling(struct llvmpipe_context *ctx)
> }
> }
>
> +
> +/**
> + * Called during state validation when LP_NEW_SAMPLER_VIEW is set.
> + */
> +void
> +llvmpipe_prepare_geometry_sampling(struct llvmpipe_context *lp,
> + unsigned num,
> + struct pipe_sampler_view **views)
> +{
> + unsigned i;
> + uint32_t row_stride[PIPE_MAX_TEXTURE_LEVELS];
> + uint32_t img_stride[PIPE_MAX_TEXTURE_LEVELS];
> + uint32_t mip_offsets[PIPE_MAX_TEXTURE_LEVELS];
> + const void *addr;
> +
> + assert(num<= PIPE_MAX_SHADER_SAMPLER_VIEWS);
> + if (!num)
> + return;
> +
> + for (i = 0; i< PIPE_MAX_SHADER_SAMPLER_VIEWS; i++) {
> + struct pipe_sampler_view *view = i< num ? views[i] : NULL;
> +
> + if (view) {
> + struct pipe_resource *tex = view->texture;
> + struct llvmpipe_resource *lp_tex = llvmpipe_resource(tex);
> + unsigned width0 = tex->width0;
> + unsigned num_layers = tex->depth0;
> + unsigned first_level = 0;
> + unsigned last_level = 0;
> +
> + /* We're referencing the texture's internal data, so save a
> + * reference to it.
> + */
> + pipe_resource_reference(&lp->mapped_gs_tex[i], tex);
> +
> + if (!lp_tex->dt) {
> + /* regular texture - setup array of mipmap level offsets */
> + struct pipe_resource *res = view->texture;
> + int j;
> + void *mip_ptr;
> +
> + if (llvmpipe_resource_is_texture(res)) {
> + first_level = view->u.tex.first_level;
> + last_level = view->u.tex.last_level;
> + assert(first_level<= last_level);
> + assert(last_level<= res->last_level);
> +
> + /* must trigger allocation first before we can get base ptr */
> + /* XXX this may fail due to OOM ? */
> + mip_ptr = llvmpipe_get_texture_image_all(lp_tex, view->u.tex.first_level,
> + LP_TEX_USAGE_READ,
> + LP_TEX_LAYOUT_LINEAR);
> + addr = lp_tex->linear_img.data;
> +
> + for (j = first_level; j<= last_level; j++) {
> + mip_ptr = llvmpipe_get_texture_image_all(lp_tex, j,
> + LP_TEX_USAGE_READ,
> + LP_TEX_LAYOUT_LINEAR);
> + mip_offsets[j] = (uint8_t *)mip_ptr - (uint8_t *)addr;
> + /*
> + * could get mip offset directly but need call above to
> + * invoke tiled->linear conversion.
> + */
> + assert(lp_tex->linear_mip_offsets[j] == mip_offsets[j]);
> + row_stride[j] = lp_tex->row_stride[j];
> + img_stride[j] = lp_tex->img_stride[j];
> + }
> + if (res->target == PIPE_TEXTURE_1D_ARRAY ||
> + res->target == PIPE_TEXTURE_2D_ARRAY) {
> + num_layers = view->u.tex.last_layer - view->u.tex.first_layer + 1;
> + addr = (uint8_t *)addr +
> + view->u.tex.first_layer * lp_tex->img_stride[0];
> + assert(view->u.tex.first_layer<= view->u.tex.last_layer);
> + assert(view->u.tex.last_layer< res->array_size);
> + }
> + }
> + else {
> + unsigned view_blocksize = util_format_get_blocksize(view->format);
> + mip_ptr = lp_tex->data;
> + addr = mip_ptr;
> + /* probably don't really need to fill that out */
> + mip_offsets[0] = 0;
> + row_stride[0] = 0;
> + row_stride[0] = 0;
> +
> + /* everything specified in number of elements here. */
> + width0 = view->u.buf.last_element - view->u.buf.first_element + 1;
> + addr = (uint8_t *)addr + view->u.buf.first_element *
> + view_blocksize;
> + assert(view->u.buf.first_element<= view->u.buf.last_element);
> + assert(view->u.buf.last_element * view_blocksize< res->width0);
> + }
> + }
> + else {
> + /* display target texture/surface */
> + /*
> + * XXX: Where should this be unmapped?
> + */
> + struct llvmpipe_screen *screen = llvmpipe_screen(tex->screen);
> + struct sw_winsys *winsys = screen->winsys;
> + addr = winsys->displaytarget_map(winsys, lp_tex->dt,
> + PIPE_TRANSFER_READ);
> + row_stride[0] = lp_tex->row_stride[0];
> + img_stride[0] = lp_tex->img_stride[0];
> + mip_offsets[0] = 0;
> + assert(addr);
> + }
> + draw_set_mapped_texture(lp->draw,
> + PIPE_SHADER_GEOMETRY,
> + i,
> + width0, tex->height0, num_layers,
> + first_level, last_level,
> + addr,
> + row_stride, img_stride, mip_offsets);
> + }
> + }
> +}
This is a lot of duplication with llvmpipe_prepare_vertex_sampling().
Could they be combined?
> +
> +void
> +llvmpipe_cleanup_geometry_sampling(struct llvmpipe_context *ctx)
> +{
> + unsigned i;
> + for (i = 0; i< Elements(ctx->mapped_gs_tex); i++) {
> + pipe_resource_reference(&ctx->mapped_gs_tex[i], NULL);
> + }
> +}
> +
> void
> llvmpipe_init_sampler_funcs(struct llvmpipe_context *llvmpipe)
> {
Otherwise, for the series:
Reviewed-by: Brian Paul <brianp at vmware.com>
More information about the mesa-dev
mailing list