[Freedreno] [PATCH 1/8] freedreno/a3xx: fix alpha-blending on RGBX formats
Ilia Mirkin
imirkin at alum.mit.edu
Thu Dec 4 08:46:48 PST 2014
On Thu, Dec 4, 2014 at 11:37 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Wed, Dec 3, 2014 at 9:57 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> Expert debugging assistance provided by Chris Forbes.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>> src/gallium/drivers/freedreno/a3xx/fd3_emit.c | 21 +++++++++---
>> src/gallium/drivers/freedreno/freedreno_util.c | 46 ++++++++++++++++++++++++++
>> src/gallium/drivers/freedreno/freedreno_util.h | 1 +
>> 3 files changed, 63 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/gallium/drivers/freedreno/a3xx/fd3_emit.c b/src/gallium/drivers/freedreno/a3xx/fd3_emit.c
>> index 44f8ac4..7a6557a 100644
>> --- a/src/gallium/drivers/freedreno/a3xx/fd3_emit.c
>> +++ b/src/gallium/drivers/freedreno/a3xx/fd3_emit.c
>> @@ -571,11 +571,12 @@ fd3_emit_state(struct fd_context *ctx, struct fd_ringbuffer *ring,
>> uint32_t i;
>>
>> for (i = 0; i < ARRAY_SIZE(blend->rb_mrt); i++) {
>> - bool is_float = util_format_is_float(
>> - pipe_surface_format(ctx->framebuffer.cbufs[i]));
>> - bool is_int = util_format_is_pure_integer(
>> - pipe_surface_format(ctx->framebuffer.cbufs[i]));
>> + enum pipe_format format = pipe_surface_format(ctx->framebuffer.cbufs[i]);
>> + bool is_float = util_format_is_float(format);
>> + bool is_int = util_format_is_pure_integer(format);
>> + bool has_alpha = util_format_has_alpha(format);
>> uint32_t control = blend->rb_mrt[i].control;
>> + uint32_t blend_control = blend->rb_mrt[i].blend_control;
>>
>> if (is_int) {
>> control &= (A3XX_RB_MRT_CONTROL_COMPONENT_ENABLE__MASK |
>> @@ -583,11 +584,21 @@ fd3_emit_state(struct fd_context *ctx, struct fd_ringbuffer *ring,
>> control |= A3XX_RB_MRT_CONTROL_ROP_CODE(ROP_COPY);
>> }
>>
>
> hmm, this getting a bit sad all the fixup we have to do in emit_state...
>
> Maybe we should populate the blend state objects w/ alternatives? Ie,
> blend->rb_mrt[i].blend_control_a and blend->rb_mrt[i].blend_control_x
> and then emit_state code just has to pick the right one?
Sounds reasonable to me.
>
>
>> + if (!has_alpha) {
>> + blend_control &= ~(
>> + A3XX_RB_MRT_BLEND_CONTROL_RGB_SRC_FACTOR__MASK |
>> + A3XX_RB_MRT_BLEND_CONTROL_RGB_DEST_FACTOR__MASK);
>> + blend_control |=
>> + A3XX_RB_MRT_BLEND_CONTROL_RGB_SRC_FACTOR(fd_blend_factor_no_dst_alpha(blend->base.rt[i].rgb_src_factor)) |
>> + A3XX_RB_MRT_BLEND_CONTROL_RGB_DEST_FACTOR(fd_blend_factor_no_dst_alpha(blend->base.rt[i].rgb_dst_factor));
>> + control &= ~A3XX_RB_MRT_CONTROL_BLEND2;
>> + }
>> +
>> OUT_PKT0(ring, REG_A3XX_RB_MRT_CONTROL(i), 1);
>> OUT_RING(ring, control);
>>
>> OUT_PKT0(ring, REG_A3XX_RB_MRT_BLEND_CONTROL(i), 1);
>> - OUT_RING(ring, blend->rb_mrt[i].blend_control |
>> + OUT_RING(ring, blend_control |
>> COND(!is_float, A3XX_RB_MRT_BLEND_CONTROL_CLAMP_ENABLE));
>> }
>> }
>> diff --git a/src/gallium/drivers/freedreno/freedreno_util.c b/src/gallium/drivers/freedreno/freedreno_util.c
>> index 9892b05..30bcf03 100644
>> --- a/src/gallium/drivers/freedreno/freedreno_util.c
>> +++ b/src/gallium/drivers/freedreno/freedreno_util.c
>> @@ -111,6 +111,52 @@ fd_blend_factor(unsigned factor)
>> }
>> }
>>
>> +enum adreno_rb_blend_factor
>> +fd_blend_factor_no_dst_alpha(unsigned factor)
>
> maybe instead just map pipe blend factor to pipe blend factor, so for
> no-alpha we do
It'd have to be a nodst_alpha_factor(factor) -- src_alpha is fine.
It's just dst_alpha that needs to be 1 for blending, since that is the
implicit A value there. (It turns out that it's perfectly legal to do
alpha-blending onto a no-alpha render target... who'd-a-thunk it.)
>
> fd_blend_factor(noalpha_factor(factor))
>
> vs
>
> fd_blend_factor(factor)
>
> ?
>
> that should make it a shorter switch statement, plus seems kinda like
> the thing others would need (so candidate to put in aux/util instead?)
I guess I should check if it's there already. I think people's
blenders tend to know what's going on though... the nvidia one
definitely does, and has separate RGBA and RGBX render formats.
>
> BR,
> -R
>
>> +{
>> + switch (factor) {
>> + case PIPE_BLENDFACTOR_ONE:
>> + return FACTOR_ONE;
>> + case PIPE_BLENDFACTOR_SRC_COLOR:
>> + return FACTOR_SRC_COLOR;
>> + case PIPE_BLENDFACTOR_SRC_ALPHA:
>> + return FACTOR_SRC_ALPHA;
>> + case PIPE_BLENDFACTOR_DST_ALPHA:
>> + return FACTOR_ONE;
>> + case PIPE_BLENDFACTOR_DST_COLOR:
>> + return FACTOR_DST_COLOR;
>> + case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
>> + return FACTOR_SRC_ALPHA_SATURATE;
>> + case PIPE_BLENDFACTOR_CONST_COLOR:
>> + return FACTOR_CONSTANT_COLOR;
>> + case PIPE_BLENDFACTOR_CONST_ALPHA:
>> + return FACTOR_CONSTANT_ALPHA;
>> + case PIPE_BLENDFACTOR_ZERO:
>> + case 0:
>> + return FACTOR_ZERO;
>> + case PIPE_BLENDFACTOR_INV_SRC_COLOR:
>> + return FACTOR_ONE_MINUS_SRC_COLOR;
>> + case PIPE_BLENDFACTOR_INV_SRC_ALPHA:
>> + return FACTOR_ONE_MINUS_SRC_ALPHA;
>> + case PIPE_BLENDFACTOR_INV_DST_ALPHA:
>> + return FACTOR_ZERO;
>> + case PIPE_BLENDFACTOR_INV_DST_COLOR:
>> + return FACTOR_ONE_MINUS_DST_COLOR;
>> + case PIPE_BLENDFACTOR_INV_CONST_COLOR:
>> + return FACTOR_ONE_MINUS_CONSTANT_COLOR;
>> + case PIPE_BLENDFACTOR_INV_CONST_ALPHA:
>> + return FACTOR_ONE_MINUS_CONSTANT_ALPHA;
>> + case PIPE_BLENDFACTOR_INV_SRC1_COLOR:
>> + case PIPE_BLENDFACTOR_INV_SRC1_ALPHA:
>> + case PIPE_BLENDFACTOR_SRC1_COLOR:
>> + case PIPE_BLENDFACTOR_SRC1_ALPHA:
>> + /* I don't think these are supported */
>> + default:
>> + DBG("invalid blend factor: %x", factor);
>> + return 0;
>> + }
>> +}
>> +
>> enum adreno_pa_su_sc_draw
>> fd_polygon_mode(unsigned mode)
>> {
>> diff --git a/src/gallium/drivers/freedreno/freedreno_util.h b/src/gallium/drivers/freedreno/freedreno_util.h
>> index 0abc53f..111f45a 100644
>> --- a/src/gallium/drivers/freedreno/freedreno_util.h
>> +++ b/src/gallium/drivers/freedreno/freedreno_util.h
>> @@ -46,6 +46,7 @@
>> enum adreno_rb_depth_format fd_pipe2depth(enum pipe_format format);
>> enum pc_di_index_size fd_pipe2index(enum pipe_format format);
>> enum adreno_rb_blend_factor fd_blend_factor(unsigned factor);
>> +enum adreno_rb_blend_factor fd_blend_factor_no_dst_alpha(unsigned factor);
>> enum adreno_pa_su_sc_draw fd_polygon_mode(unsigned mode);
>> enum adreno_stencil_op fd_stencil_op(unsigned op);
>>
>> --
>> 2.0.4
>>
> _______________________________________________
> Freedreno mailing list
> Freedreno at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/freedreno
More information about the Freedreno
mailing list