Use XCopyArea when possible, for optimization and bug workaround.

Exercise XCopyArea, XRenderComposite, and XSetTile/XFillRectangle paths for _cairo_xlib_surface_composite

11 files changed, 255 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 508293e80..7a3cbfe76 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2005-07-15  Vladimir Vukicevic  <vladimir@pobox.com>
+
+	* src/cairo-xlib-surface.c: (_cairo_xlib_surface_composite,
+	_recategorize_composite_repeat): Use XCopyArea when
+	possible, for optimization and bug workaround.
+
+	* test/composite-integer-translate-{source,over,over-repeat}.c:
+	Exercise XCopyArea, XRenderComposite, and XSetTile/XFillRectangle
+	paths for _cairo_xlib_surface_composite
+
 2005-07-15  Carl Worth  <cworth@cworth.org>
 
 	* test/Makefile.am: Add new check-valgrind target for running the
diff --git a/ROADMAP b/ROADMAP
index 9b79e9521..88fad3087 100644
--- a/ROADMAP
+++ b/ROADMAP
@@ -10,7 +10,7 @@ cairo 0.5.2
 	 0.1.4 which is already fixed in libpixman 0.1.5)
 
   Add a workaround for Render's overlapping source/dest bug
-	Use XCopyArea when possible (integer translation)
+✓	Use XCopyArea when possible (integer translation)
 	Otherwise make a copy of the source
 
   Fix the cache lock deadlocking problems.
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 95df4d298..caec2b9dd 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -829,17 +829,39 @@ _surface_has_alpha (cairo_xlib_surface_t *surface)
     }
 }
 
+/* Returns true if the given operator and source-alpha combination
+ * requires alpha compositing to complete.
+ */
+static cairo_bool_t
+_operator_needs_alpha_composite (cairo_operator_t operator,
+				 cairo_bool_t     surface_has_alpha)
+{
+    if (operator == CAIRO_OPERATOR_SOURCE ||
+	(!surface_has_alpha &&
+	 (operator == CAIRO_OPERATOR_OVER ||
+	  operator == CAIRO_OPERATOR_ATOP ||
+	  operator == CAIRO_OPERATOR_IN)))
+	return FALSE;
+
+    return TRUE;
+}
+
 /* There is a bug in most older X servers with compositing using a repeating
  * source pattern when the source is in off-screen video memory. When that
  * bug could be triggered, we need a fallback: in the common case where we have no
  * transformation and the source and destination have the same format/visual,
  * we can do the operation using the core protocol, otherwise, we need
  * a software fallback.
+ *
+ * We can also often optimize a compositing operation by calling XCopyArea
+ * for some common cases where there is no alpha compositing to be done.
+ * We figure that out here as well.
  */
 typedef enum {
-    DO_RENDER,			/* use render */
-    DO_XLIB,			/* core protocol fallback */
-    DO_UNSUPPORTED		/* software fallback */
+    DO_RENDER,		/* use render */
+    DO_XCOPYAREA,	/* core protocol XCopyArea optimization/fallback */
+    DO_XTILE,		/* core protocol XSetTile optimization/fallback */
+    DO_UNSUPPORTED	/* software fallback */
 } composite_operation_t;
 
 /* Initial check for the bug; we need to recheck after we turn
@@ -850,10 +872,10 @@ typedef enum {
  * hit the bug and won't be able to use a core protocol fallback.
  */
 static composite_operation_t
-_categorize_composite_repeat (cairo_xlib_surface_t *dst,
-			      cairo_operator_t	    operator,
-			      cairo_pattern_t      *src_pattern,
-			      cairo_bool_t          have_mask)
+_categorize_composite_operation (cairo_xlib_surface_t *dst,
+				 cairo_operator_t      operator,
+				 cairo_pattern_t      *src_pattern,
+				 cairo_bool_t	       have_mask)
 			      
 {
     if (!dst->buggy_repeat)
@@ -892,27 +914,43 @@ _categorize_composite_repeat (cairo_xlib_surface_t *dst,
  * If we end up returning DO_UNSUPPORTED here, we're throwing away work we
  * did to turn gradients into a pattern, but most of the time we can handle
  * that case with core protocol fallback.
+ *
+ * Also check here if we can just use XCopyArea, instead of going through
+ * Render.
  */
 static composite_operation_t
-_recategorize_composite_repeat (cairo_xlib_surface_t       *dst,
-				cairo_operator_t	    operator,
-				cairo_xlib_surface_t       *src,
-				cairo_surface_attributes_t *src_attr,
-				cairo_bool_t                have_mask)
+_recategorize_composite_operation (cairo_xlib_surface_t	      *dst,
+				   cairo_operator_t	       operator,
+				   cairo_xlib_surface_t	      *src,
+				   cairo_surface_attributes_t *src_attr,
+				   cairo_bool_t		       have_mask)
 {
+    cairo_bool_t is_integer_translation =
+	_cairo_matrix_is_integer_translation (&src_attr->matrix, NULL, NULL);
+    cairo_bool_t needs_alpha_composite =
+	_operator_needs_alpha_composite (operator, _surface_has_alpha (src));
+
+    if (!have_mask &&
+	is_integer_translation &&
+	src_attr->extend == CAIRO_EXTEND_NONE &&
+	!needs_alpha_composite &&
+	_surfaces_compatible(src, dst))
+    {
+	return DO_XCOPYAREA;
+    }
+
     if (!dst->buggy_repeat)
 	return DO_RENDER;
     
-    if (_cairo_matrix_is_integer_translation (&src_attr->matrix, NULL, NULL) &&
+    if (is_integer_translation &&
 	src_attr->extend == CAIRO_EXTEND_REPEAT &&
 	(src->width != 1 || src->height != 1))
     {
 	if (!have_mask &&
-	    (operator == CAIRO_OPERATOR_SOURCE ||
-	     (operator == CAIRO_OPERATOR_OVER && !_surface_has_alpha (src))))
+	    !needs_alpha_composite &&
+	    _surfaces_compatible (dst, src))
 	{
-	    if (_surfaces_compatible (dst, src))
-		return DO_XLIB;
+	    return DO_XTILE;
 	}
 	
 	return DO_UNSUPPORTED;
@@ -981,12 +1019,13 @@ _cairo_xlib_surface_composite (cairo_operator_t		operator,
     cairo_xlib_surface_t	*mask;
     cairo_int_status_t		status;
     composite_operation_t       operation;
+    int				itx, ity;
 
     if (!CAIRO_SURFACE_RENDER_HAS_COMPOSITE (dst))
 	return CAIRO_INT_STATUS_UNSUPPORTED;
 
-    operation = _categorize_composite_repeat (dst, operator, src_pattern,
-					      mask_pattern != NULL);
+    operation = _categorize_composite_operation (dst, operator, src_pattern,
+						 mask_pattern != NULL);
     if (operation == DO_UNSUPPORTED)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
     
@@ -1001,8 +1040,8 @@ _cairo_xlib_surface_composite (cairo_operator_t		operator,
     if (status)
 	return status;
     
-    operation = _recategorize_composite_repeat (dst, operator, src, &src_attr,
-						mask_pattern != NULL);
+    operation = _recategorize_composite_operation (dst, operator, src, &src_attr,
+						   mask_pattern != NULL);
     if (operation == DO_UNSUPPORTED) {
 	status = CAIRO_INT_STATUS_UNSUPPORTED;
 	goto FAIL;
@@ -1012,8 +1051,9 @@ _cairo_xlib_surface_composite (cairo_operator_t		operator,
     if (status)
 	goto FAIL;
 
-    if (operation == DO_RENDER)
+    switch (operation)
     {
+    case DO_RENDER:
 	_cairo_xlib_surface_ensure_dst_picture (dst);
 	if (mask) {
 	    status = _cairo_xlib_surface_set_attributes (mask, &mask_attr);
@@ -1041,17 +1081,28 @@ _cairo_xlib_surface_composite (cairo_operator_t		operator,
 			      dst_x, dst_y,
 			      width, height);
 	}
-    }
-    else			/* DO_XLIB */
-    {
+	break;
+
+    case DO_XCOPYAREA:
+	_cairo_xlib_surface_ensure_gc (dst);
+	XCopyArea (dst->dpy,
+		   src->drawable,
+		   dst->drawable,
+		   dst->gc,
+		   src_x + src_attr.x_offset,
+		   src_y + src_attr.y_offset,
+		   width, height,
+		   dst_x, dst_y);
+	break;
+
+    case DO_XTILE:
 	/* This case is only used for bug fallbacks, though it is theoretically
 	 * applicable to the case where we don't have the RENDER extension as
 	 * well.
 	 *
 	 * We've checked that we have a repeating unscaled source in
-	 * _recategorize_composite_repeat.
+	 * _recategorize_composite_operation.
 	 */
-	int itx, ity;
 
 	_cairo_xlib_surface_ensure_gc (dst);
 	_cairo_matrix_is_integer_translation (&src_attr.matrix, &itx, &ity);
@@ -1063,6 +1114,10 @@ _cairo_xlib_surface_composite (cairo_operator_t		operator,
 
 	XFillRectangle (dst->dpy, dst->drawable, dst->gc,
 			dst_x, dst_y, width, height);
+	break;
+
+    default:
+	ASSERT_NOT_REACHED;
     }
 
  FAIL:
@@ -1127,7 +1182,7 @@ _cairo_xlib_surface_composite_trapezoids (cairo_operator_t	operator,
     if (!CAIRO_SURFACE_RENDER_HAS_TRAPEZOIDS (dst))
 	return CAIRO_INT_STATUS_UNSUPPORTED;
 
-    operation = _categorize_composite_repeat (dst, operator, pattern, TRUE);
+    operation = _categorize_composite_operation (dst, operator, pattern, TRUE);
     if (operation == DO_UNSUPPORTED)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
     
@@ -1138,7 +1193,7 @@ _cairo_xlib_surface_composite_trapezoids (cairo_operator_t	operator,
     if (status)
 	return status;
 
-    operation = _recategorize_composite_repeat (dst, operator, src, &attributes, TRUE);
+    operation = _recategorize_composite_operation (dst, operator, src, &attributes, TRUE);
     if (operation == DO_UNSUPPORTED) {
 	status = CAIRO_INT_STATUS_UNSUPPORTED;
 	goto FAIL;
@@ -1959,7 +2014,7 @@ _cairo_xlib_surface_show_glyphs (cairo_scaled_font_t    *scaled_font,
     if (!CAIRO_SURFACE_RENDER_HAS_COMPOSITE_TEXT (self) || !self->format)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
 
-    operation = _categorize_composite_repeat (self, operator, pattern, TRUE);
+    operation = _categorize_composite_operation (self, operator, pattern, TRUE);
     if (operation == DO_UNSUPPORTED)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
     
@@ -1970,7 +2025,7 @@ _cairo_xlib_surface_show_glyphs (cairo_scaled_font_t    *scaled_font,
     if (status)
 	return status;
 
-    operation = _recategorize_composite_repeat (self, operator, src, &attributes, TRUE);
+    operation = _recategorize_composite_operation (self, operator, src, &attributes, TRUE);
     if (operation == DO_UNSUPPORTED) {
 	status = CAIRO_INT_STATUS_UNSUPPORTED;
 	goto FAIL;
diff --git a/test/.cvsignore b/test/.cvsignore
index 5ce9bbd3a..00ed2feca 100644
--- a/test/.cvsignore
+++ b/test/.cvsignore
@@ -4,6 +4,9 @@ Makefile
 Makefile.in
 clip-nesting
 clip-twice
+composite-integer-translate-source
+composite-integer-translate-over
+composite-integer-translate-over-repeat
 coverage
 create-from-png
 fill-and-stroke
diff --git a/test/Makefile.am b/test/Makefile.am
index 118b298f0..ee8d1b98d 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -2,6 +2,9 @@
 TESTS = 			\
 clip-nesting			\
 clip-twice			\
+composite-integer-translate-source \
+composite-integer-translate-over \
+composite-integer-translate-over-repeat \
 create-from-png			\
 fill-and-stroke			\
 fill-rule			\
@@ -52,6 +55,9 @@ endif
 EXTRA_DIST =				\
 clip-nesting-ref.png			\
 clip-twice-ref.png			\
+composite-integer-translate-source-ref.png		\
+composite-integer-translate-over-ref.png		\
+composite-integer-translate-over-repeat-ref.png		\
 create-from-png-ref.png			\
 fill-and-stroke-ref.png			\
 fill-rule-ref.png			\
@@ -133,6 +139,9 @@ LDADDS = libcairotest.la $(top_builddir)/src/libcairo.la
 # from autogen.sh. My, but this is painful...
 clip_nesting_LDADD = $(LDADDS)
 clip_twice_LDADD = $(LDADDS)
+composite_integer_translate_source_LDADD = $(LDADDS)
+composite_integer_translate_over_LDADD = $(LDADDS)
+composite_integer_translate_over_repeat_LDADD = $(LDADDS)
 create_from_png_LDADD = $(LDADDS)
 fill_and_stroke_LDADD = $(LDADDS)
 fill_rule_LDADD = $(LDADDS)
@@ -181,4 +190,4 @@ CLEANFILES =					\
 	pdf-clip.pdf
 
 check-valgrind:
-	TESTS_ENVIRONMENT="libtool --mode=execute valgrind --tool=memcheck --leak-check=yes --show-reachable=yes" $(MAKE) check
\ No newline at end of file
+	TESTS_ENVIRONMENT="libtool --mode=execute valgrind --tool=memcheck --leak-check=yes --show-reachable=yes" $(MAKE) check
diff --git a/test/composite-integer-translate-over-ref.png b/test/composite-integer-translate-over-ref.png
new file mode 100644
index 000000000..1fbaaaa3b
--- /dev/null
+++ b/test/composite-integer-translate-over-ref.png
diff --git a/test/composite-integer-translate-over-repeat-ref.png b/test/composite-integer-translate-over-repeat-ref.png
new file mode 100644
index 000000000..c04db2631
--- /dev/null
+++ b/test/composite-integer-translate-over-repeat-ref.png
diff --git a/test/composite-integer-translate-over-repeat.c b/test/composite-integer-translate-over-repeat.c
new file mode 100644
index 000000000..b681ac9c7
--- /dev/null
+++ b/test/composite-integer-translate-over-repeat.c
@@ -0,0 +1,61 @@
+#include <math.h>
+#include "cairo-test.h"
+#include <stdio.h>
+
+#define SIZE 100
+#define SIZE2 20
+#define OFFSET 10
+
+cairo_test_t test = {
+    "composite-integer-translate-over-repeat",
+    "Test simple compositing: integer-translation 32->32 OVER, with repeat",
+    SIZE, SIZE
+};
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    cairo_surface_t *image;
+    cairo_pattern_t *pat;
+    cairo_t *cr2;
+
+    image = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, SIZE2, SIZE2);
+
+    cr2 = cairo_create (image);
+    cairo_set_source_rgba (cr2, 1, 0, 0, 1);
+    cairo_rectangle (cr2, 0, 0, SIZE2/2, SIZE2/2);
+    cairo_fill (cr2);
+    cairo_set_source_rgba (cr2, 0, 1, 0, 1);
+    cairo_rectangle (cr2, SIZE2/2, 0, SIZE2/2, SIZE2/2);
+    cairo_fill (cr2);
+    cairo_set_source_rgba (cr2, 0, 0, 1, 1);
+    cairo_rectangle (cr2, 0, SIZE2/2, SIZE2/2, SIZE2/2);
+    cairo_fill (cr2);
+    cairo_set_source_rgba (cr2, 1, 1, 0, 1);
+    cairo_rectangle (cr2, SIZE2/2, SIZE2/2, SIZE2/2, SIZE2/2);
+    cairo_fill (cr2);
+    cairo_destroy (cr2);
+
+    pat = cairo_pattern_create_for_surface (image);
+    cairo_pattern_set_extend (pat, CAIRO_EXTEND_REPEAT);
+
+    cairo_set_source_rgba (cr, 0, 0, 0, 1);
+    cairo_rectangle (cr, 0, 0, SIZE, SIZE);
+    cairo_fill (cr);
+
+    cairo_translate (cr, OFFSET, OFFSET);
+    cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
+    cairo_set_source (cr, pat);
+    cairo_rectangle (cr, 0, 0, SIZE - OFFSET, SIZE - OFFSET);
+    cairo_fill (cr);
+
+    cairo_surface_destroy (image);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+int
+main (void)
+{
+    return cairo_test (&test, draw);
+}
diff --git a/test/composite-integer-translate-over.c b/test/composite-integer-translate-over.c
new file mode 100644
index 000000000..867448889
--- /dev/null
+++ b/test/composite-integer-translate-over.c
@@ -0,0 +1,42 @@
+#include <math.h>
+#include "cairo-test.h"
+#include <stdio.h>
+
+#define SIZE 100
+#define OFFSET 5.5
+#define SCALE 1.5
+const char	png_filename[]	= "romedalen.png";
+
+cairo_test_t test = {
+    "composite-integer-translate-over",
+    "Test simple compositing: integer-translation 32->32 OVER",
+    SIZE, SIZE
+};
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    cairo_surface_t *image;
+
+    image = cairo_image_surface_create_from_png (png_filename);
+
+    cairo_set_source_rgba (cr, 0, 0, 0, 1);
+    cairo_rectangle (cr, 0, 0, SIZE, SIZE);
+    cairo_fill (cr);
+
+    cairo_translate (cr, OFFSET, OFFSET);
+    cairo_set_operator (cr, CAIRO_OPERATOR_OVER);
+    cairo_set_source_surface (cr, image, 0, 0);
+    cairo_rectangle (cr, 0, 0, (SIZE-OFFSET), (SIZE-OFFSET));
+    cairo_fill (cr);
+
+    cairo_surface_destroy (image);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+int
+main (void)
+{
+    return cairo_test (&test, draw);
+}
diff --git a/test/composite-integer-translate-source-ref.png b/test/composite-integer-translate-source-ref.png
new file mode 100644
index 000000000..da9a3986a
--- /dev/null
+++ b/test/composite-integer-translate-source-ref.png
diff --git a/test/composite-integer-translate-source.c b/test/composite-integer-translate-source.c
new file mode 100644
index 000000000..a467f089a
--- /dev/null
+++ b/test/composite-integer-translate-source.c
@@ -0,0 +1,42 @@
+#include <math.h>
+#include "cairo-test.h"
+#include <stdio.h>
+
+#define SIZE 100
+#define OFFSET 10
+
+const char	png_filename[]	= "romedalen.png";
+
+cairo_test_t test = {
+    "composite-integer-translate-source",
+    "Test simple compositing: integer-translation 32->32 SOURCE",
+    SIZE, SIZE
+};
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    cairo_surface_t *image;
+
+    image = cairo_image_surface_create_from_png (png_filename);
+
+    cairo_set_source_rgba (cr, 0, 0, 0, 1);
+    cairo_rectangle (cr, 0, 0, SIZE, SIZE);
+    cairo_fill (cr);
+
+    cairo_translate (cr, OFFSET, OFFSET);
+    cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
+    cairo_set_source_surface (cr, image, 0, 0);
+    cairo_rectangle (cr, 0, 0, SIZE - OFFSET, SIZE - OFFSET);
+    cairo_fill (cr);
+
+    cairo_surface_destroy (image);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+int
+main (void)
+{
+    return cairo_test (&test, draw);
+}