[Mesa-dev] [PATCH 1/4] llvmpipe: fix using wrong format with MRT in blend code
Brian Paul
brianp at vmware.com
Mon Jan 14 09:44:28 PST 2013
IIRC, we can have a mixture of color buffer formats when there's
multiple render targets. This change doesn't effect that, right?
Looks OK otherwise. Reviewed-by: Brian Paul <brianp at vmware.com>
On 01/12/2013 05:29 PM, sroland at vmware.com wrote:
> From: Roland Scheidegger<sroland at vmware.com>
>
> We were passing in the rt index however this was always 0 for non-independent
> blend case. (The format was only actually used to decide if the color mask
> covered all channels so this went unnoticed and was discovered by accident.)
> Additionally, there was a second problem because we do fixups in the key based
> on color buffer format we cannot use non-independent blend anyway as the fixed
> up values would never get used.
> So always turn non-independent blending into independent.
> ---
> src/gallium/drivers/llvmpipe/lp_bld_blend.h | 2 +-
> src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c | 6 ++---
> src/gallium/drivers/llvmpipe/lp_state_fs.c | 27 ++++++++++++++++++-----
> src/gallium/drivers/llvmpipe/lp_test_blend.c | 2 +-
> 4 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_bld_blend.h b/src/gallium/drivers/llvmpipe/lp_bld_blend.h
> index 0a1cea1..3a3be81 100644
> --- a/src/gallium/drivers/llvmpipe/lp_bld_blend.h
> +++ b/src/gallium/drivers/llvmpipe/lp_bld_blend.h
> @@ -56,7 +56,7 @@ lp_build_blend(struct lp_build_context *bld,
> LLVMValueRef
> lp_build_blend_aos(struct gallivm_state *gallivm,
> const struct pipe_blend_state *blend,
> - const enum pipe_format *cbuf_format,
> + const enum pipe_format cbuf_format,
> struct lp_type type,
> unsigned rt,
> LLVMValueRef src,
> diff --git a/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c b/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
> index 641c253..65f66e0 100644
> --- a/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
> +++ b/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
> @@ -266,7 +266,7 @@ lp_build_blend_factor(struct lp_build_blend_aos_context *bld,
> * @param blend the blend state of the shader variant
> * @param cbuf_format format of the colour buffer
> * @param type data type of the pixel vector
> - * @param rt rt number
> + * @param rt render target index
> * @param src blend src
> * @param dst blend dst
> * @param mask optional mask to apply to the blending result
> @@ -278,7 +278,7 @@ lp_build_blend_factor(struct lp_build_blend_aos_context *bld,
> LLVMValueRef
> lp_build_blend_aos(struct gallivm_state *gallivm,
> const struct pipe_blend_state *blend,
> - const enum pipe_format *cbuf_format,
> + const enum pipe_format cbuf_format,
> struct lp_type type,
> unsigned rt,
> LLVMValueRef src,
> @@ -298,7 +298,7 @@ lp_build_blend_aos(struct gallivm_state *gallivm,
> unsigned alpha_swizzle = UTIL_FORMAT_SWIZZLE_NONE;
> unsigned i;
>
> - desc = util_format_description(cbuf_format[rt]);
> + desc = util_format_description(cbuf_format);
>
> /* Setup build context */
> memset(&bld, 0, sizeof bld);
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> index 5a8351b..3eae162 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> @@ -1645,12 +1645,21 @@ generate_unswizzled_blend(struct gallivm_state *gallivm,
> /*
> * Blending
> */
> + /* XXX this is broken for RGB8 formats -
> + * they get expanded from 12 to 16 elements (to include alpha)
> + * by convert_to_blend_type then reduced to 15 instead of 12
> + * by convert_from_blend_type (a simple fix though breaks A8...).
> + * R16G16B16 also crashes differently however something going wrong
> + * inside llvm handling npot vector sizes seemingly.
> + * It seems some cleanup could be done here (like skipping conversion/blend
> + * when not needed).
> + */
> convert_to_blend_type(gallivm, out_format_desc, dst_type, row_type, dst, src_count);
>
> for (i = 0; i< src_count; ++i) {
> dst[i] = lp_build_blend_aos(gallivm,
> &variant->key.blend,
> - variant->key.cbuf_format,
> + out_format,
> row_type,
> rt,
> src[i],
> @@ -1999,7 +2008,6 @@ generate_fragment(struct llvmpipe_context *lp,
> LLVMValueRef color_ptr;
> LLVMValueRef stride;
> LLVMValueRef index = lp_build_const_int32(gallivm, cbuf);
> - unsigned rt = key->blend.independent_blend_enable ? cbuf : 0;
>
> boolean do_branch = ((key->depth.enabled
> || key->stencil[0].enabled
> @@ -2016,7 +2024,7 @@ generate_fragment(struct llvmpipe_context *lp,
> LLVMBuildGEP(builder, stride_ptr,&index, 1, ""),
> "");
>
> - generate_unswizzled_blend(gallivm, rt, variant, key->cbuf_format[cbuf],
> + generate_unswizzled_blend(gallivm, cbuf, variant, key->cbuf_format[cbuf],
> num_fs, fs_type, fs_mask, fs_out_color[cbuf],
> context_ptr, color_ptr, stride, partial_mask, do_branch);
> }
> @@ -2505,6 +2513,15 @@ make_variant_key(struct llvmpipe_context *lp,
> }
>
> key->nr_cbufs = lp->framebuffer.nr_cbufs;
> +
> + if (!key->blend.independent_blend_enable) {
> + /* we always need independent blend otherwise the fixups below won't work */
> + for (i = 1; i< key->nr_cbufs; i++) {
> + memcpy(&key->blend.rt[i],&key->blend.rt[0], sizeof(key->blend.rt[0]));
> + }
> + key->blend.independent_blend_enable = 1;
> + }
> +
> 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];
> @@ -2516,8 +2533,6 @@ make_variant_key(struct llvmpipe_context *lp,
> assert(format_desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> format_desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB);
>
> - blend_rt->colormask = lp->blend->rt[i].colormask;
> -
> /*
> * Mask out color channels not present in the color buffer.
> */
> @@ -2539,7 +2554,7 @@ make_variant_key(struct llvmpipe_context *lp,
> * 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]) {
> + format_desc->swizzle[3] == format_desc->swizzle[0]) {
> blend_rt->rgb_src_factor = force_dst_alpha_one(blend_rt->rgb_src_factor);
> blend_rt->rgb_dst_factor = force_dst_alpha_one(blend_rt->rgb_dst_factor);
> blend_rt->alpha_func = blend_rt->rgb_func;
> diff --git a/src/gallium/drivers/llvmpipe/lp_test_blend.c b/src/gallium/drivers/llvmpipe/lp_test_blend.c
> index 9c0a076..754434d 100644
> --- a/src/gallium/drivers/llvmpipe/lp_test_blend.c
> +++ b/src/gallium/drivers/llvmpipe/lp_test_blend.c
> @@ -172,7 +172,7 @@ add_blend_test(struct gallivm_state *gallivm,
> dst = LLVMBuildLoad(builder, dst_ptr, "dst");
> con = LLVMBuildLoad(builder, const_ptr, "const");
>
> - res = lp_build_blend_aos(gallivm, blend,&format, type, rt, src, NULL, dst, NULL, con, NULL, swizzle, 4);
> + res = lp_build_blend_aos(gallivm, blend, format, type, rt, src, NULL, dst, NULL, con, NULL, swizzle, 4);
>
> lp_build_name(res, "res");
>
More information about the mesa-dev
mailing list