sna/gen3: Remove incorrect premultiplication of solid component-alpha mask

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

1 files changed, 68 insertions, 53 deletions

diff --git a/src/sna/gen3_render.c b/src/sna/gen3_render.c
index fdd28058..fcbe9c7d 100644
--- a/src/sna/gen3_render.c
+++ b/src/sna/gen3_render.c
@@ -2271,10 +2271,10 @@ gen3_composite_set_target(struct sna *sna,
 	return TRUE;
 }
 
-static inline uint8_t mult(uint32_t s, uint32_t m, int shift)
+static inline uint8_t multa(uint32_t s, uint32_t m, int shift)
 {
 	s = (s >> shift) & 0xff;
-	m = (m >> shift) & 0xff;
+	m >>= 24;
 	return (s * m) >> 8;
 }
 
@@ -2292,7 +2292,6 @@ static inline bool is_constant_ps(uint32_t type)
 	}
 }
 
-
 static bool
 is_solid(PicturePtr picture)
 {
@@ -2315,6 +2314,7 @@ source_fallback(PicturePtr p)
 
 static bool
 gen3_composite_fallback(struct sna *sna,
+			uint8_t op,
 			PicturePtr src,
 			PicturePtr mask,
 			PicturePtr dst)
@@ -2347,6 +2347,15 @@ gen3_composite_fallback(struct sna *sna,
 		return TRUE;
 	}
 
+	if (mask &&
+	    mask->componentAlpha && PICT_FORMAT_RGB(mask->format) &&
+	    op != PictOpOver)
+	{
+		DBG(("%s: component-alpha mask with op=%d, should fallback\n",
+		     __FUNCTION__, op));
+		return TRUE;
+	}
+
 	/* If anything is on the GPU, push everything out to the GPU */
 	priv = sna_pixmap(dst_pixmap);
 	if (priv && priv->gpu_damage) {
@@ -2482,7 +2491,7 @@ gen3_render_composite(struct sna *sna,
 			      tmp))
 		return TRUE;
 
-	if (gen3_composite_fallback(sna, src, mask, dst))
+	if (gen3_composite_fallback(sna, op, src, mask, dst))
 		return FALSE;
 
 	if (need_tiling(sna, width, height))
@@ -2563,7 +2572,6 @@ gen3_render_composite(struct sna *sna,
 			}
 		}
 		DBG(("%s: mask type=%d\n", __FUNCTION__, tmp->mask.u.gen3.type));
-
 		if (tmp->mask.u.gen3.type == SHADER_ZERO) {
 			if (tmp->src.bo) {
 				kgem_bo_destroy(&sna->kgem,
@@ -2574,53 +2582,60 @@ gen3_render_composite(struct sna *sna,
 			tmp->mask.u.gen3.type = SHADER_NONE;
 		}
 
-		if (tmp->mask.u.gen3.type != SHADER_NONE &&
-		    mask->componentAlpha && PICT_FORMAT_RGB(mask->format)) {
-			/* Check if it's component alpha that relies on a source alpha
-			 * and on the source value.  We can only get one of those
-			 * into the single source value that we get to blend with.
-			 */
-			tmp->has_component_alpha = TRUE;
-			if (tmp->mask.u.gen3.type == SHADER_WHITE) {
-				tmp->mask.u.gen3.type = SHADER_NONE;
-				tmp->has_component_alpha = FALSE;
-			} else if (is_constant_ps(tmp->src.u.gen3.type) &&
-				   is_constant_ps(tmp->mask.u.gen3.type)) {
-				uint32_t a,r,g,b;
-
-				a = mult(tmp->src.u.gen3.mode,
-					 tmp->mask.u.gen3.mode,
-					 24);
-				r = mult(tmp->src.u.gen3.mode,
-					 tmp->mask.u.gen3.mode,
-					 16);
-				g = mult(tmp->src.u.gen3.mode,
-					 tmp->mask.u.gen3.mode,
-					 8);
-				b = mult(tmp->src.u.gen3.mode,
-					 tmp->mask.u.gen3.mode,
-					 0);
-
-				DBG(("%s: combining constant source/mask: %x x %x -> %x\n",
-				     __FUNCTION__,
-				     tmp->src.u.gen3.mode,
-				     tmp->mask.u.gen3.mode,
-				     a << 24 | r << 16 | g << 8 | b));
-
-				tmp->src.u.gen3.type = SHADER_CONSTANT;
-				tmp->src.u.gen3.mode =
-					a << 24 | r << 16 | g << 8 | b;
-
-				tmp->mask.u.gen3.type = SHADER_NONE;
-				tmp->has_component_alpha = FALSE;
-			} else if (gen3_blend_op[op].src_alpha &&
-				   (gen3_blend_op[op].src_blend != BLENDFACT_ZERO)) {
-				if (op != PictOpOver)
-					goto cleanup_mask;
-
-				tmp->need_magic_ca_pass = TRUE;
-				tmp->op = PictOpOutReverse;
-				sna->render.vertex_start = sna->render.vertex_index;
+		if (tmp->mask.u.gen3.type != SHADER_NONE) {
+			if (mask->componentAlpha && PICT_FORMAT_RGB(mask->format)) {
+				/* Check if it's component alpha that relies on a source alpha
+				 * and on the source value.  We can only get one of those
+				 * into the single source value that we get to blend with.
+				 */
+				DBG(("%s: component-alpha mask: %d\n",
+				     __FUNCTION__, tmp->mask.u.gen3.type));
+				tmp->has_component_alpha = TRUE;
+				if (tmp->mask.u.gen3.type == SHADER_WHITE) {
+					tmp->mask.u.gen3.type = SHADER_NONE;
+					tmp->has_component_alpha = FALSE;
+				} else if (gen3_blend_op[op].src_alpha &&
+					   (gen3_blend_op[op].src_blend != BLENDFACT_ZERO)) {
+					if (op != PictOpOver)
+						goto cleanup_mask;
+
+					tmp->need_magic_ca_pass = TRUE;
+					tmp->op = PictOpOutReverse;
+					sna->render.vertex_start = sna->render.vertex_index;
+				}
+			} else {
+				if (tmp->mask.is_opaque) {
+					tmp->mask.u.gen3.type = SHADER_NONE;
+					tmp->has_component_alpha = FALSE;
+				} else if (is_constant_ps(tmp->src.u.gen3.type) &&
+					   is_constant_ps(tmp->mask.u.gen3.type)) {
+					uint32_t a,r,g,b;
+
+					a = multa(tmp->src.u.gen3.mode,
+						  tmp->mask.u.gen3.mode,
+						  24);
+					r = multa(tmp->src.u.gen3.mode,
+						  tmp->mask.u.gen3.mode,
+						  16);
+					g = multa(tmp->src.u.gen3.mode,
+						  tmp->mask.u.gen3.mode,
+						  8);
+					b = multa(tmp->src.u.gen3.mode,
+						  tmp->mask.u.gen3.mode,
+						  0);
+
+					DBG(("%s: combining constant source/mask: %x x %x -> %x\n",
+					     __FUNCTION__,
+					     tmp->src.u.gen3.mode,
+					     tmp->mask.u.gen3.mode,
+					     a << 24 | r << 16 | g << 8 | b));
+
+					tmp->src.u.gen3.type = SHADER_CONSTANT;
+					tmp->src.u.gen3.mode =
+						a << 24 | r << 16 | g << 8 | b;
+
+					tmp->mask.u.gen3.type = SHADER_NONE;
+				}
 			}
 		}
 	}
@@ -3036,7 +3051,7 @@ gen3_render_composite_spans(struct sna *sna,
 		return FALSE;
 	}
 
-	if (gen3_composite_fallback(sna, src, NULL, dst))
+	if (gen3_composite_fallback(sna, op, src, NULL, dst))
 		return FALSE;
 
 	if (need_tiling(sna, width, height))