From: Soren Sandmann Pedersen <ssp@dhcp83-218.boston.redhat.com>
Date: Fri, 18 May 2007 17:38:26 +0000 (-0400)
Subject: Make pixman_image_set_indexed() not copy its argument
X-Git-Tag: 1.0_branch~1544
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e1328980457ce6f51ce2649d79feefa616408ce9;p=profile%2Fivi%2Fpixman.git

Make pixman_image_set_indexed() not copy its argument
---

diff --git a/TODO b/TODO
index 20b559d..55746c6 100644
--- a/TODO
+++ b/TODO
@@ -6,6 +6,36 @@
 
   - Go through things marked FIXME
 
+  - Test if pseudo color still works. It does, but it also shows that
+    copying a pixman_indexed_t on every composite operation is not
+    going to fly. So, for now set_indexed() does not copy the 
+    indexed table. 
+
+    Also just the malloc() to allocate a pixman image shows up pretty
+    high.
+
+    Options include
+
+      - Make all the setters not copy their arguments
+
+      - Possibly combined with going back to the stack allocated 
+        approach that we already use for regions.
+
+      - Keep a cached pixman_image_t around for every picture. It would
+        have to be kept uptodate every time something changes about the
+        picture.
+
+      - Break the X server ABI and simply have the releveant parameter
+        stored in the pixman image. This would have the additional benefits
+        that:
+
+          - We can get rid of the annoying repeat field which is duplicated
+            elsewhere.
+
+          - We can use pixman_color_t and pixman_gradient_stop_t
+            etc. instead of the types that are defined in
+            renderproto.h
+
   - Reinstate the FbBits conditional typedef? At the moment we don't
     even have the FbBits type; we just use uint32_t everywhere.
 
@@ -16,5 +46,3 @@
         doing things 64-bits at a time is a net loss.  To quickly fix
         this, I suggest just using 32-bit datatypes by setting
         IC_SHIFT to 5 for all machines.
-
-  - Test if pseudo color still works
diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 64a8d23..b48bc81 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -127,8 +127,10 @@ pixman_image_unref (pixman_image_t *image)
 	if (common->alpha_map)
 	    pixman_image_unref ((pixman_image_t *)common->alpha_map);
 
+#if 0
 	if (image->type == BITS && image->bits.indexed)
 	    free (image->bits.indexed);
+#endif
 	
 #if 0
 	memset (image, 0xaa, sizeof (pixman_image_t));
@@ -267,7 +269,9 @@ pixman_image_create_bits (pixman_format_code_t  format,
 {
     pixman_image_t *image;
 
-    return_val_if_fail ((rowstride & 0x3) == 0, NULL); /* must be a multiple of 4 */
+    return_val_if_fail ((rowstride & 0x3) == 0, NULL); /* must be a
+							* multiple of 4
+							*/
 
     image = allocate_image();
 
@@ -279,7 +283,9 @@ pixman_image_create_bits (pixman_format_code_t  format,
     image->bits.width = width;
     image->bits.height = height;
     image->bits.bits = bits;
-    image->bits.rowstride = rowstride / 4; /* we store it in number of uint32_t's */
+    image->bits.rowstride = rowstride / 4; /* we store it in number
+					    * of uint32_t's
+					    */
     image->bits.indexed = NULL;
 
     return image;
@@ -363,24 +369,17 @@ pixman_image_set_filter (pixman_image_t       *image,
     common->n_filter_params = n_params;
 }
 
+/* Unlike all the other property setters, this function does not
+ * copy the content of indexed. Doing this copying is simply
+ * way, way too expensive.
+ */
 void
 pixman_image_set_indexed (pixman_image_t	 *image,
 			  const pixman_indexed_t *indexed)
 {
     bits_image_t *bits = (bits_image_t *)image;
 
-    if (bits->indexed == indexed)
-	return;
-
-    if (bits->indexed)
-	free (bits->indexed);
-    
-    bits->indexed = malloc (sizeof (pixman_indexed_t));
-
-    if (!bits->indexed)
-	return;
-
-    memcpy (bits->indexed, indexed, sizeof (pixman_indexed_t));
+    bits->indexed = indexed;
 }
 
 void