[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