[Mesa-dev] [PATCH] llvmpipe: handle NULL color buffer pointers
Jose Fonseca
jfonseca at vmware.com
Thu Jan 16 08:22:53 PST 2014
Looks good AFAICT. Thanks Brian.
Jose
----- Original Message -----
> Fixes regression from 9baa45f78b8ca7d66280e36009b6a685055d7cd6
>
> v2: incorporate a few small changes suggested by Roland.
> ---
> src/gallium/drivers/llvmpipe/lp_rast.c | 62 ++++++++---
> src/gallium/drivers/llvmpipe/lp_rast_priv.h | 11 +-
> src/gallium/drivers/llvmpipe/lp_scene.c | 23 ++--
> src/gallium/drivers/llvmpipe/lp_setup.c | 2 +-
> src/gallium/drivers/llvmpipe/lp_state_fs.c | 152
> +++++++++++++++------------
> 5 files changed, 156 insertions(+), 94 deletions(-)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c
> b/src/gallium/drivers/llvmpipe/lp_rast.c
> index 6feec94..6ee849b 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
> @@ -110,6 +110,25 @@ lp_rast_tile_begin(struct lp_rasterizer_task *task,
>
>
> /**
> + * Examine a framebuffer object to determine if any of the colorbuffers
> + * use a pure integer format.
> + * XXX this could be a gallium utility function if useful elsewhere.
> + */
> +static boolean
> +is_fb_pure_integer(const struct pipe_framebuffer_state *fb)
> +{
> + unsigned i;
> + for (i = 0; i < fb->nr_cbufs; i++) {
> + if (fb->cbufs[i] &&
> + util_format_is_pure_integer(fb->cbufs[i]->format)) {
> + return TRUE;
> + }
> + }
> + return FALSE;
> +}
> +
> +
> +/**
> * Clear the rasterizer's current color tile.
> * This is a bin command called during bin processing.
> * Clear commands always clear all bound layers.
> @@ -124,7 +143,7 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
> unsigned i;
> union util_color uc;
>
> - if (util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
> + if (is_fb_pure_integer(&scene->fb)) {
> /*
> * We expect int/uint clear values here, though some APIs
> * might disagree (but in any case util_pack_color()
> @@ -174,20 +193,22 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
> clear_color[3]);
>
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> - util_pack_color(arg.clear_color.f,
> - scene->fb.cbufs[i]->format, &uc);
> -
> - util_fill_box(scene->cbufs[i].map,
> - scene->fb.cbufs[i]->format,
> - scene->cbufs[i].stride,
> - scene->cbufs[i].layer_stride,
> - task->x,
> - task->y,
> - 0,
> - task->width,
> - task->height,
> - scene->fb_max_layer + 1,
> - &uc);
> + if (scene->fb.cbufs[i]) {
> + util_pack_color(arg.clear_color.f,
> + scene->fb.cbufs[i]->format, &uc);
> +
> + util_fill_box(scene->cbufs[i].map,
> + scene->fb.cbufs[i]->format,
> + scene->cbufs[i].stride,
> + scene->cbufs[i].layer_stride,
> + task->x,
> + task->y,
> + 0,
> + task->width,
> + task->height,
> + scene->fb_max_layer + 1,
> + &uc);
> + }
> }
> }
> }
> @@ -444,8 +465,15 @@ lp_rast_shade_quads_mask(struct lp_rasterizer_task
> *task,
>
> /* color buffer */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> - stride[i] = scene->cbufs[i].stride;
> - color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
> inputs->layer);
> + if (scene->fb.cbufs[i]) {
> + stride[i] = scene->cbufs[i].stride;
> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x,
> y,
> +
> inputs->layer);
> + }
> + else {
> + stride[i] = 0;
> + color[i] = NULL;
> + }
> }
>
> /* depth buffer */
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> index bc361b6..063a70e 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> @@ -293,8 +293,15 @@ lp_rast_shade_quads_all( struct lp_rasterizer_task
> *task,
>
> /* color buffer */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> - stride[i] = scene->cbufs[i].stride;
> - color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
> inputs->layer);
> + if (scene->fb.cbufs[i]) {
> + stride[i] = scene->cbufs[i].stride;
> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x,
> y,
> +
> inputs->layer);
> + }
> + else {
> + stride[i] = 0;
> + color[i] = NULL;
> + }
> }
>
> if (scene->zsbuf.map) {
> diff --git a/src/gallium/drivers/llvmpipe/lp_scene.c
> b/src/gallium/drivers/llvmpipe/lp_scene.c
> index 0296b79..9ba5f1a 100644
> --- a/src/gallium/drivers/llvmpipe/lp_scene.c
> +++ b/src/gallium/drivers/llvmpipe/lp_scene.c
> @@ -156,6 +156,14 @@ lp_scene_begin_rasterization(struct lp_scene *scene)
>
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> struct pipe_surface *cbuf = scene->fb.cbufs[i];
> +
> + if (!cbuf) {
> + scene->cbufs[i].stride = 0;
> + scene->cbufs[i].layer_stride = 0;
> + scene->cbufs[i].map = NULL;
> + continue;
> + }
> +
> if (llvmpipe_resource_is_texture(cbuf->texture)) {
> scene->cbufs[i].stride = llvmpipe_resource_stride(cbuf->texture,
> cbuf->u.tex.level);
> @@ -171,7 +179,7 @@ lp_scene_begin_rasterization(struct lp_scene *scene)
> struct llvmpipe_resource *lpr = llvmpipe_resource(cbuf->texture);
> unsigned pixstride = util_format_get_blocksize(cbuf->format);
> scene->cbufs[i].stride = cbuf->texture->width0;
> -
> + scene->cbufs[i].layer_stride = 0;
> scene->cbufs[i].map = lpr->data;
> scene->cbufs[i].map += cbuf->u.buf.first_element * pixstride;
> }
> @@ -521,11 +529,14 @@ void lp_scene_begin_binning( struct lp_scene *scene,
> */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> struct pipe_surface *cbuf = scene->fb.cbufs[i];
> - if (llvmpipe_resource_is_texture(cbuf->texture)) {
> - max_layer = MIN2(max_layer, cbuf->u.tex.last_layer -
> cbuf->u.tex.first_layer);
> - }
> - else {
> - max_layer = 0;
> + if (cbuf) {
> + if (llvmpipe_resource_is_texture(cbuf->texture)) {
> + max_layer = MIN2(max_layer,
> + cbuf->u.tex.last_layer -
> cbuf->u.tex.first_layer);
> + }
> + else {
> + max_layer = 0;
> + }
> }
> }
> if (fb->zsbuf) {
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c
> b/src/gallium/drivers/llvmpipe/lp_setup.c
> index 7f22231..2437318 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -882,7 +882,7 @@ lp_setup_is_resource_referenced( const struct
> lp_setup_context *setup,
>
> /* check the render targets */
> for (i = 0; i < setup->fb.nr_cbufs; i++) {
> - if (setup->fb.cbufs[i]->texture == texture)
> + if (setup->fb.cbufs[i] && setup->fb.cbufs[i]->texture == texture)
> return LP_REFERENCED_FOR_READ | LP_REFERENCED_FOR_WRITE;
> }
> if (setup->fb.zsbuf && setup->fb.zsbuf->texture == texture) {
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c
> b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> index eedafa3..abdb6b3 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> @@ -2361,28 +2361,33 @@ generate_fragment(struct llvmpipe_context *lp,
> /* Loop over color outputs / color buffers to do blending.
> */
> for(cbuf = 0; cbuf < key->nr_cbufs; cbuf++) {
> - LLVMValueRef color_ptr;
> - LLVMValueRef stride;
> - LLVMValueRef index = lp_build_const_int32(gallivm, cbuf);
> + if (key->cbuf_format[cbuf] != PIPE_FORMAT_NONE) {
> + LLVMValueRef color_ptr;
> + LLVMValueRef stride;
> + LLVMValueRef index = lp_build_const_int32(gallivm, cbuf);
>
> - boolean do_branch = ((key->depth.enabled
> - || key->stencil[0].enabled
> - || key->alpha.enabled)
> - && !shader->info.base.uses_kill);
> + boolean do_branch = ((key->depth.enabled
> + || key->stencil[0].enabled
> + || key->alpha.enabled)
> + && !shader->info.base.uses_kill);
>
> - color_ptr = LLVMBuildLoad(builder,
> - LLVMBuildGEP(builder, color_ptr_ptr, &index,
> 1, ""),
> - "");
> + color_ptr = LLVMBuildLoad(builder,
> + LLVMBuildGEP(builder, color_ptr_ptr,
> + &index, 1, ""),
> + "");
>
> - lp_build_name(color_ptr, "color_ptr%d", cbuf);
> + lp_build_name(color_ptr, "color_ptr%d", cbuf);
>
> - stride = LLVMBuildLoad(builder,
> - LLVMBuildGEP(builder, stride_ptr, &index, 1,
> ""),
> - "");
> + stride = LLVMBuildLoad(builder,
> + LLVMBuildGEP(builder, stride_ptr, &index, 1,
> ""),
> + "");
>
> - generate_unswizzled_blend(gallivm, cbuf, variant,
> key->cbuf_format[cbuf],
> - num_fs, fs_type, fs_mask, fs_out_color,
> - context_ptr, color_ptr, stride,
> partial_mask, do_branch);
> + generate_unswizzled_blend(gallivm, cbuf, variant,
> + key->cbuf_format[cbuf],
> + num_fs, fs_type, fs_mask, fs_out_color,
> + context_ptr, color_ptr, stride,
> + partial_mask, do_branch);
> + }
> }
>
> LLVMBuildRetVoid(builder);
> @@ -2900,6 +2905,7 @@ make_variant_key(struct llvmpipe_context *lp,
>
> /* alpha test only applies if render buffer 0 is non-integer (or does not
> exist) */
> if (!lp->framebuffer.nr_cbufs ||
> + !lp->framebuffer.cbufs[0] ||
> !util_format_is_pure_integer(lp->framebuffer.cbufs[0]->format)) {
> key->alpha.enabled = lp->depth_stencil->alpha.enabled;
> }
> @@ -2927,64 +2933,74 @@ make_variant_key(struct llvmpipe_context *lp,
> }
>
> for (i = 0; i < lp->framebuffer.nr_cbufs; i++) {
> - enum pipe_format format = lp->framebuffer.cbufs[i]->format;
> struct pipe_rt_blend_state *blend_rt = &key->blend.rt[i];
> - const struct util_format_description *format_desc;
>
> - key->cbuf_format[i] = format;
> + if (lp->framebuffer.cbufs[i]) {
> + enum pipe_format format = lp->framebuffer.cbufs[i]->format;
> + const struct util_format_description *format_desc;
>
> - /*
> - * Figure out if this is a 1d resource. Note that OpenGL allows crazy
> - * mixing of 2d textures with height 1 and 1d textures, so make sure
> - * we pick 1d if any cbuf or zsbuf is 1d.
> - */
> - if (llvmpipe_resource_is_1d(lp->framebuffer.cbufs[0]->texture)) {
> - key->resource_1d = TRUE;
> - }
> + key->cbuf_format[i] = format;
>
> - format_desc = util_format_description(format);
> - assert(format_desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> - format_desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB);
> + /*
> + * Figure out if this is a 1d resource. Note that OpenGL allows
> crazy
> + * mixing of 2d textures with height 1 and 1d textures, so make
> sure
> + * we pick 1d if any cbuf or zsbuf is 1d.
> + */
> + if (llvmpipe_resource_is_1d(lp->framebuffer.cbufs[i]->texture)) {
> + key->resource_1d = TRUE;
> + }
>
> - /*
> - * Mask out color channels not present in the color buffer.
> - */
> - blend_rt->colormask &= util_format_colormask(format_desc);
> + format_desc = util_format_description(format);
> + assert(format_desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> + format_desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB);
>
> - /*
> - * Disable blend for integer formats.
> - */
> - if (util_format_is_pure_integer(format)) {
> - blend_rt->blend_enable = 0;
> - }
> + /*
> + * Mask out color channels not present in the color buffer.
> + */
> + blend_rt->colormask &= util_format_colormask(format_desc);
>
> - /*
> - * Our swizzled render tiles always have an alpha channel, but the
> linear
> - * render target format often does not, so force here the dst alpha to
> be
> - * one.
> - *
> - * This is not a mere optimization. Wrong results will be produced if
> the
> - * dst alpha is used, the dst format does not have alpha, and the
> previous
> - * rendering was not flushed from the swizzled to linear buffer. For
> - * example, NonPowTwo DCT.
> - *
> - * TODO: This should be generalized to all channels for better
> - * performance, but only alpha causes correctness issues.
> - *
> - * Also, force rgb/alpha func/factors match, to make AoS blending
> easier.
> - */
> - if (format_desc->swizzle[3] > UTIL_FORMAT_SWIZZLE_W ||
> - format_desc->swizzle[3] == format_desc->swizzle[0]) {
> - /* Doesn't cover mixed snorm/unorm but can't render to them anyway
> */
> - boolean clamped_zero = !util_format_is_float(format) &&
> - !util_format_is_snorm(format);
> - blend_rt->rgb_src_factor =
> force_dst_alpha_one(blend_rt->rgb_src_factor,
> - clamped_zero);
> - blend_rt->rgb_dst_factor =
> force_dst_alpha_one(blend_rt->rgb_dst_factor,
> - clamped_zero);
> - blend_rt->alpha_func = blend_rt->rgb_func;
> - blend_rt->alpha_src_factor = blend_rt->rgb_src_factor;
> - blend_rt->alpha_dst_factor = blend_rt->rgb_dst_factor;
> + /*
> + * Disable blend for integer formats.
> + */
> + if (util_format_is_pure_integer(format)) {
> + blend_rt->blend_enable = 0;
> + }
> +
> + /*
> + * Our swizzled render tiles always have an alpha channel, but the
> + * linear render target format often does not, so force here the
> dst
> + * alpha to be one.
> + *
> + * This is not a mere optimization. Wrong results will be produced
> if
> + * the dst alpha is used, the dst format does not have alpha, and
> the
> + * previous rendering was not flushed from the swizzled to linear
> + * buffer. For example, NonPowTwo DCT.
> + *
> + * TODO: This should be generalized to all channels for better
> + * performance, but only alpha causes correctness issues.
> + *
> + * Also, force rgb/alpha func/factors match, to make AoS blending
> + * easier.
> + */
> + if (format_desc->swizzle[3] > UTIL_FORMAT_SWIZZLE_W ||
> + format_desc->swizzle[3] == format_desc->swizzle[0]) {
> + /* Doesn't cover mixed snorm/unorm but can't render to them
> anyway */
> + boolean clamped_zero = !util_format_is_float(format) &&
> + !util_format_is_snorm(format);
> + blend_rt->rgb_src_factor =
> + force_dst_alpha_one(blend_rt->rgb_src_factor, clamped_zero);
> + blend_rt->rgb_dst_factor =
> + force_dst_alpha_one(blend_rt->rgb_dst_factor, clamped_zero);
> + blend_rt->alpha_func = blend_rt->rgb_func;
> + blend_rt->alpha_src_factor = blend_rt->rgb_src_factor;
> + blend_rt->alpha_dst_factor = blend_rt->rgb_dst_factor;
> + }
> + }
> + else {
> + /* no color buffer for this fragment output */
> + key->cbuf_format[i] = PIPE_FORMAT_NONE;
> + blend_rt->colormask = 0x0;
> + blend_rt->blend_enable = 0;
> }
> }
>
> --
> 1.7.10.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=HXR%2FlXocPbGHi0CFavPEMSv9OnTWE%2B9AyvXnchZk50Y%3D%0A&s=f42e34bc69530485709e8ecc2654b6957df8893ff9ae7376b3fce8c696e4c2b5
>
More information about the mesa-dev
mailing list