[Mesa-dev] [PATCH 1/4] llvmpipe: fix using wrong format with MRT in blend code
Roland Scheidegger
sroland at vmware.com
Mon Jan 14 10:30:44 PST 2013
On 01/14/2013 09:44 AM, Brian Paul wrote:
> IIRC, we can have a mixture of color buffer formats when there's
> multiple render targets. This change doesn't effect that, right?
Well it does - it should fix some edge cases there.
We actually did some changes to the color mask of the blend state
depending on the color channels present in the color buffer (and now
disabling blend for integer color buffers too), however with MRTs and
non-independent blending we never used the respective blend state later.
And the second (really separate) bug meant that when looking at the
color buffer format in the blend code itself, we used the one from color
buffer 0 instead.
So just always using independent blend should fix both issues.
Roland
>
> 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