[Mesa-dev] [PATCH] llvmpipe: handle NULL color buffer pointers

Brian Paul brianp at vmware.com
Thu Jan 16 08:20:04 PST 2014


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



More information about the mesa-dev mailing list