# [Pixman] [PATCH 10/13] Use floating point combiners for all operators that involve divisions

Søren Sandmann sandmann at cs.au.dk
Wed Dec 11 07:41:59 PST 2013

```Consider a DISJOINT_ATOP operation with the following pixels:

- source:	0xff (8 bits)
- source alpha:	0x01 (8 bits)
- mask alpha:	0x7b (8 bits)
- dest:		0x00 (8 bits)
- dest alpha:	0xff (8 bits)

When (src IN mask) is computed in 8 bits, the resulting alpha channel
is 0 due to rounding:

floor ((0x01 * 0x7b) / 255.0 + 0.5) = floor (0.9823) = 0

which means that since Render defines any division by zero as
infinity, the Fa and Fb for this operator end up as follows:

Fa = max (1 - (1 - 1) / 0, 0) = 0

Fb = min (1, (1 - 0) / 1) = 1

and so since dest is 0x00, the overall result is 0.

However, when computed in full precision, the alpha value no longer
rounds to 0, and so Fa ends up being

Fa = max (1 - (1 - 1) / 0.0001, 0) = 1

and so the result is now

s * ma * Fa + d * Fb

= (1.0 * (0x7b / 255.0) * 1) + d * 0

= 0x7b / 255.0

= 0.4823

so the error in this case ends up being 0.48235294, which is clearly
not something that can be considered acceptable.

In order to avoid this problem, we need to do all arithmetic in such a
way that a multiplication of two tiny numbers can never end up being
zero unless one of the input numbers is itself zero.

This patch makes all computations that involve divisions take place in
floating point, which is sufficient to fix the test cases

This brings the number of failures in pixel-test down to 14.
---
pixman/pixman-general.c | 21 ++++++++++++++++++---
test/blitters-test.c    | 20 ++++++++++----------
test/thread-test.c      | 29 +----------------------------
3 files changed, 29 insertions(+), 41 deletions(-)

diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
index 8bce7c0..7cdea29 100644
--- a/pixman/pixman-general.c
+++ b/pixman/pixman-general.c
@@ -109,6 +109,20 @@ static const op_info_t op_flags[PIXMAN_N_OPERATORS] =

#define SCANLINE_BUFFER_LENGTH 8192

+static pixman_bool_t
+operator_needs_division (pixman_op_t op)
+{
+    static const uint8_t needs_division[] =
+    {
+	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, /* SATURATE */
+	1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, /* DISJOINT */
+	1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, /* CONJOINT */
+	0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0, 1, 1, 1, 1, 0, /* blend ops */
+    };
+
+    return needs_division[op];
+}
+
static void
general_composite_rect  (pixman_implementation_t *imp,
pixman_composite_info_t *info)
@@ -124,9 +138,10 @@ general_composite_rect  (pixman_implementation_t *imp,
int Bpp;
int i;

-    if ((src_image->common.flags & FAST_PATH_NARROW_FORMAT)		    &&
-	(!mask_image || mask_image->common.flags & FAST_PATH_NARROW_FORMAT) &&
-	(dest_image->common.flags & FAST_PATH_NARROW_FORMAT))
+    if ((src_image->common.flags & FAST_PATH_NARROW_FORMAT)		     &&
+	(!mask_image || mask_image->common.flags & FAST_PATH_NARROW_FORMAT)  &&
+	(dest_image->common.flags & FAST_PATH_NARROW_FORMAT)		     &&
+	!(operator_needs_division (op)))
{
width_flag = ITER_NARROW;
Bpp = 4;
diff --git a/test/blitters-test.c b/test/blitters-test.c
index e2a2e7f..026f4b0 100644
--- a/test/blitters-test.c
+++ b/test/blitters-test.c
@@ -122,6 +122,15 @@ static pixman_op_t op_list[] = {
PIXMAN_OP_ATOP_REVERSE,
PIXMAN_OP_XOR,
+    PIXMAN_OP_MULTIPLY,
+    PIXMAN_OP_SCREEN,
+    PIXMAN_OP_OVERLAY,
+    PIXMAN_OP_DARKEN,
+    PIXMAN_OP_LIGHTEN,
+    PIXMAN_OP_HARD_LIGHT,
+    PIXMAN_OP_DIFFERENCE,
+    PIXMAN_OP_EXCLUSION,
+#if 0 /* these use floating point math and are not always bitexact on different platforms */
PIXMAN_OP_SATURATE,
PIXMAN_OP_DISJOINT_CLEAR,
PIXMAN_OP_DISJOINT_SRC,
@@ -147,17 +156,8 @@ static pixman_op_t op_list[] = {
PIXMAN_OP_CONJOINT_ATOP,
PIXMAN_OP_CONJOINT_ATOP_REVERSE,
PIXMAN_OP_CONJOINT_XOR,
-    PIXMAN_OP_MULTIPLY,
-    PIXMAN_OP_SCREEN,
-    PIXMAN_OP_OVERLAY,
-    PIXMAN_OP_DARKEN,
-    PIXMAN_OP_LIGHTEN,
PIXMAN_OP_COLOR_DODGE,
PIXMAN_OP_COLOR_BURN,
-    PIXMAN_OP_HARD_LIGHT,
-    PIXMAN_OP_DIFFERENCE,
-    PIXMAN_OP_EXCLUSION,
-#if 0 /* these use floating point math and are not always bitexact on different platforms */
PIXMAN_OP_SOFT_LIGHT,
PIXMAN_OP_HSL_HUE,
PIXMAN_OP_HSL_SATURATION,
@@ -394,6 +394,6 @@ main (int argc, const char *argv[])
}

return fuzzer_test_main("blitters", 2000000,
-			    0xB1D1F40E,
+			    0xCC21DDF0,
test_composite, argc, argv);
}
index 8fa2098..1c2f040 100644
@@ -38,38 +38,11 @@ static const pixman_op_t operators[] =
PIXMAN_OP_ATOP_REVERSE,
PIXMAN_OP_XOR,
-    PIXMAN_OP_SATURATE,
-    PIXMAN_OP_DISJOINT_CLEAR,
-    PIXMAN_OP_DISJOINT_SRC,
-    PIXMAN_OP_DISJOINT_DST,
-    PIXMAN_OP_DISJOINT_OVER,
-    PIXMAN_OP_DISJOINT_OVER_REVERSE,
-    PIXMAN_OP_DISJOINT_IN,
-    PIXMAN_OP_DISJOINT_IN_REVERSE,
-    PIXMAN_OP_DISJOINT_OUT,
-    PIXMAN_OP_DISJOINT_OUT_REVERSE,
-    PIXMAN_OP_DISJOINT_ATOP,
-    PIXMAN_OP_DISJOINT_ATOP_REVERSE,
-    PIXMAN_OP_DISJOINT_XOR,
-    PIXMAN_OP_CONJOINT_CLEAR,
-    PIXMAN_OP_CONJOINT_SRC,
-    PIXMAN_OP_CONJOINT_DST,
-    PIXMAN_OP_CONJOINT_OVER,
-    PIXMAN_OP_CONJOINT_OVER_REVERSE,
-    PIXMAN_OP_CONJOINT_IN,
-    PIXMAN_OP_CONJOINT_IN_REVERSE,
-    PIXMAN_OP_CONJOINT_OUT,
-    PIXMAN_OP_CONJOINT_OUT_REVERSE,
-    PIXMAN_OP_CONJOINT_ATOP,
-    PIXMAN_OP_CONJOINT_ATOP_REVERSE,
-    PIXMAN_OP_CONJOINT_XOR,
PIXMAN_OP_MULTIPLY,
PIXMAN_OP_SCREEN,
PIXMAN_OP_OVERLAY,
PIXMAN_OP_DARKEN,
PIXMAN_OP_LIGHTEN,
-    PIXMAN_OP_COLOR_DODGE,
-    PIXMAN_OP_COLOR_BURN,
PIXMAN_OP_HARD_LIGHT,
PIXMAN_OP_DIFFERENCE,
PIXMAN_OP_EXCLUSION,
@@ -183,7 +156,7 @@ main (void)

crc32 = compute_crc32 (0, crc32s, sizeof crc32s);

-#define EXPECTED 0xE8D161DF
+#define EXPECTED 0x82C4D9FB

if (crc32 != EXPECTED)
{
--
1.8.3.1

```