Fixed another use of array of gpointer in place of a proper struct, this

* app/undo.c: Fixed another use of array of gpointer in place of a
	proper struct, this time in undo_(push|pop)_channel_mod.

	* base/base-types.h
	* base/tile-manager.h
	* base/tile-manager-private.h
	* base/tile-manager.c: added PixelDataHandle as an abstraction
	on top of tiles.  PixelDataHandles either return a pointer into
	the tile data, or create a temporary buffer so the calling
	function can access data from disparate tiles using a single
	buffer.  This is a step in reducing the dependence of core
	image functions being aware of tiles as well as a step toward
	having a single abstraction for pixel data.

	* app/image_map.c: changed to use read_pixel_data_1
	* app/pixel-region.c: changed to use the read_pixel_data and
	write_pixel_data where practical.

ChangeLog

@@ -1,3 +1,23 @@
+2001-11-28  Kelly Martin  <kmartin@pyrzqxgl.org>
+
+	* app/undo.c: Fixed another use of array of gpointer in place of a
+	proper struct, this time in undo_(push|pop)_channel_mod.
+
+	* base/base-types.h
+	* base/tile-manager.h
+	* base/tile-manager-private.h
+	* base/tile-manager.c: added PixelDataHandle as an abstraction
+	on top of tiles.  PixelDataHandles either return a pointer into
+	the tile data, or create a temporary buffer so the calling
+	function can access data from disparate tiles using a single
+	buffer.  This is a step in reducing the dependence of core
+	image functions being aware of tiles as well as a step toward
+	having a single abstraction for pixel data.
+
+	* app/image_map.c: changed to use read_pixel_data_1
+	* app/pixel-region.c: changed to use the read_pixel_data and
+	write_pixel_data where practical.  
+
 2001-11-28  Michael Natterer  <mitch@gimp.org>
 
 	* app/core/gimpimage-mask.[ch]: s/gimage_mask/gimp_image_mask/g

app/base/base-types.h

@@ -137,6 +137,7 @@ typedef struct _TempBuf             MaskBuf;
 typedef struct _Tile                Tile;
 typedef struct _TileManager         TileManager;
 
+typedef struct _PixelDataHandle     PixelDataHandle;
 
 /*  functions  */
 

app/base/pixel-region.c

@@ -123,35 +123,34 @@ pixel_region_get_row (PixelRegion *PR,
 
   pixel_region_get_async (PR, x, y, end, y);
 
-  while (x < end)
+  tilebpp = tile_manager_bpp (PR->tiles);
+  inc = subsample * tilebpp;
+
+  if (subsample == 1)
     {
-      tile = tile_manager_get_tile (PR->tiles, x, y, TRUE, FALSE);
+      read_pixel_data (PR->tiles, x, y, end, y, data, w);
-      tilebpp = tile_bpp (tile);
+    }
-      tile_data = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
+  else 
-      npixels = tile_ewidth (tile) - (x % TILE_WIDTH);
+    {
-
+      while (x < end)
-      if ((x + npixels) > end) /* make sure we don't write past the end */
-	npixels = end - x;
-
-      if (subsample == 1) /* optimize for the common case */
-	{
-	  memcpy (data, tile_data, tilebpp * npixels);
-	  data += tilebpp * npixels;
-	  x += npixels;
-	}
-      else
 	{
+	  tile = tile_manager_get_tile (PR->tiles, x, y, TRUE, FALSE);
+	  tile_data = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
+	  npixels = tile_ewidth (tile) - (x % TILE_WIDTH);
+	  
+	  if ((x + npixels) > end) /* make sure we don't write past the end */
+	    npixels = end - x;
+	  
 	  boundary = x + npixels;
-	  inc = subsample * tilebpp;
 	  for ( ; x < boundary; x += subsample)
 	    {
 	      for (b = 0; b < tilebpp; b++)
 		*data++ = tile_data[b];
-
+	      
 	      tile_data += inc;
 	    }
+	  tile_release (tile, FALSE);
 	}
-      tile_release (tile, FALSE);
     }
 }
 
@@ -162,32 +161,13 @@ pixel_region_set_row (PixelRegion *PR,
 		      gint         w, 
 		      guchar      *data)
 {
-  Tile   *tile;
-  guchar *tile_data;
   gint    end;
-  gint    npixels;
 
   end = x + w;
 
   pixel_region_get_async (PR, x, y, end, y);
 
-  while (x < end)
+  write_pixel_data (PR->tiles, x, y, end, y, data, w);
-    {
-      tile = tile_manager_get_tile (PR->tiles, x, y, TRUE, TRUE);
-      tile_data = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
-
-      npixels = tile_ewidth (tile) - (x % TILE_WIDTH);
-
-      if ((x + npixels) > end) /* make sure we don't write past the end */
-	npixels = end - x;
-
-      memcpy (tile_data, data, tile_bpp (tile) * npixels);
-
-      data += tile_bpp (tile) * npixels;
-      x += npixels;
-
-      tile_release (tile, TRUE);
-    }
 }
 
 
@@ -211,10 +191,11 @@ pixel_region_get_col (PixelRegion *PR,
 
   pixel_region_get_async (PR, x, y, x, end);
 
+  tilebpp = tile_manager_bpp (PR->tiles);
+
   while (y < end)
     {
       tile = tile_manager_get_tile (PR->tiles, x, y, TRUE, FALSE);
-      tilebpp = tile_bpp (tile);
       tile_data = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
       boundary = y + (tile_eheight(tile) - (y % TILE_HEIGHT));
 
@@ -243,39 +224,16 @@ pixel_region_set_col (PixelRegion *PR,
 		      gint         h, 
 		      guchar      *data)
 {
-  Tile   *tile;
-  guchar *tile_data;
   gint    tilebpp;
-  gint    inc;
   gint    end;
-  gint    boundary;
-  gint    b;
 
   end = y + h;
 
   pixel_region_get_async (PR, x, y, x, end);
 
-  while (y < end)
+  tilebpp = tile_manager_bpp (PR->tiles);
-    {
-      tile = tile_manager_get_tile (PR->tiles, x, y, TRUE, TRUE);
-      tilebpp = tile_bpp (tile);
-      tile_data = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
-      boundary = y + (tile_eheight (tile) - (y % TILE_HEIGHT));
-      inc = tilebpp * tile_ewidth (tile);
 
-      if (boundary > end) /* make sure we don't write past the end */
+  write_pixel_data (PR->tiles, x, y, x, end, data, tilebpp);
-	boundary = end;
-
-      for ( ; y < boundary; y++)
-	{
-	  for (b = 0; b < tilebpp; b++)
-	    tile_data[b] = *data++;
-
-	  tile_data += inc;
-	}
-
-      tile_release (tile, TRUE);
-    }
 }
 
 gboolean

app/base/tile-manager-private.h

@@ -39,5 +39,18 @@ struct _TileManager
   gpointer           user_data;     /*  hook for hanging data off of         */
 };
 
+typedef struct _PixelDataHandlePrivate PixelDataHandlePrivate;
+
+struct _PixelDataHandlePrivate 
+{
+  PixelDataHandle  public;
+  TileManager     *tm;
+  gint		   x1, x2, y1, y2;
+  gboolean	   readable;
+  gboolean         writeable;
+  gboolean         local_buffer;
+  Tile            *tile;
+};
+
 
 #endif /* __TILE_MANAGER_PVT_H__ */

app/base/tile-manager.c

@@ -637,3 +637,184 @@ tile_manager_map_over_tile (TileManager *tm,
 
   tile_manager_map (tm, tl->tile_num, srctile);
 }
+
+PixelDataHandle * 
+request_pixel_data (TileManager *tm,
+		    gint         x1,
+		    gint	 y1,
+		    gint	 x2,
+		    gint	 y2,
+		    gboolean	 wantread,
+		    gboolean     wantwrite)
+{
+  PixelDataHandlePrivate *pdh;
+  guint tile_num_1, tile_num_2;
+  guint w, h;
+
+  pdh = g_new (PixelDataHandlePrivate, 1);
+  pdh->tm = tm;
+  pdh->readable  = wantread;
+  pdh->writeable = wantwrite;
+  pdh->x1 = x1;
+  pdh->y1 = y1;
+  pdh->x2 = x2;
+  pdh->y2 = y2;
+  pdh->public.width = w = (x2-x1)+1;
+  pdh->public.height = h = (y2-y1)+1;
+  
+  tile_num_1 = tile_manager_get_tile_num (tm, x1, y1);
+  tile_num_2 = tile_manager_get_tile_num (tm, x2, y2);
+
+  if (tile_num_1 == tile_num_2) 
+    {
+      pdh->tile = tile_manager_get (tm, tile_num_1, wantread, wantwrite);
+      pdh->public.data = tile_data_pointer (pdh->tile, x1 % TILE_WIDTH, y1 % TILE_HEIGHT);
+      pdh->public.stride = tile_bpp (pdh->tile) * tile_ewidth (pdh->tile);
+      pdh->local_buffer = FALSE;
+    }
+  else 
+    {
+      pdh->public.data = g_new (guchar, w * h * tm->bpp);
+      pdh->public.stride = tm->bpp * w;
+      pdh->local_buffer = TRUE;
+      pdh->tile = NULL;
+
+      if (wantread) 
+	read_pixel_data (tm, x1, y1, x2, y2, 
+			 pdh->public.data, pdh->public.stride);
+    }
+  return (PixelDataHandle *) pdh;
+}
+
+void
+release_pixel_data (PixelDataHandle *xpdh)
+{
+  PixelDataHandlePrivate *pdh = (PixelDataHandlePrivate *)(xpdh);
+
+  if (pdh->local_buffer) 
+    {
+      if (pdh->writeable) 
+	write_pixel_data (pdh->tm, pdh->x1, pdh->y1, pdh->x2, pdh->y2,
+			  pdh->public.data, pdh->public.stride);
+      g_free (pdh->public.data);
+    }
+  else 
+    {
+      tile_release (pdh->tile, pdh->writeable);
+    }
+  g_free (pdh);
+}
+
+void
+read_pixel_data (TileManager *tm,
+		 gint         x1,
+		 gint         y1,
+		 gint         x2,
+		 gint         y2,
+		 guchar      *buffer,
+		 guint        stride)
+{
+  guint x, y;
+  guint rows, cols;
+  guint srcstride;
+  Tile *t;
+  guchar *s, *d;
+
+  for (y = y1; y <= y2; y += TILE_HEIGHT - (y % TILE_HEIGHT)) 
+    for (x = x1; x <= x2; x += TILE_WIDTH - (x % TILE_WIDTH)) 
+      {
+	t = tile_manager_get_tile (tm, x, y, TRUE, FALSE);
+	s = tile_data_pointer (t, x % TILE_WIDTH, y % TILE_HEIGHT);
+	d = buffer + stride * (y - y1) + tm->bpp * (x - x1);
+	rows = tile_eheight (t) - y % TILE_HEIGHT;
+	if (rows > (y2 - y)) 
+	  rows = y2 - y;
+		
+	cols = tile_ewidth (t) - x % TILE_WIDTH;
+	if (cols > (x2 - x))
+	  cols = x2 - x;
+	
+	srcstride = tile_ewidth (t) * tile_bpp (t);
+	
+	while (rows --) 
+	  {
+	    memcpy (d, s, cols * tm->bpp);
+	    s += srcstride;
+	    d += stride;
+	  }
+	
+	tile_release (t, FALSE);
+      }
+}
+  
+void
+write_pixel_data (TileManager *tm,
+		  gint         x1,
+		  gint         y1,
+		  gint         x2,
+		  gint         y2,
+		  guchar      *buffer,
+		  guint        stride)
+{
+  guint x, y;
+  guint rows, cols;
+  guint dststride;
+  Tile *t;
+  guchar *s, *d;
+
+  for (y = y1; y <= y2; y += TILE_HEIGHT - (y % TILE_HEIGHT)) 
+    for (x = x1; x <= x2; x += TILE_WIDTH - (x % TILE_WIDTH)) 
+      {
+	t = tile_manager_get_tile (tm, x, y, TRUE, TRUE);
+	s = buffer + stride * (y - y1) + tm->bpp * (x - x1);
+	d = tile_data_pointer (t, x % TILE_WIDTH, y % TILE_HEIGHT);
+	rows = tile_eheight (t) - y % TILE_HEIGHT;
+	if (rows > (y2 - y)) 
+	  rows = y2 - y;
+	
+	cols = tile_ewidth (t) - x % TILE_WIDTH;
+	if (cols > (x2 - x))
+	  cols = x2 - x;
+	
+	dststride = tile_ewidth (t) * tile_bpp (t);
+	
+	while (rows --) 
+	  {
+	    memcpy (d, s, cols * tm->bpp);
+	    s += stride;
+	    d += dststride;
+	  }
+	
+	tile_release (t, TRUE);
+      }
+}
+
+void
+read_pixel_data_1 (TileManager *tm,
+		   gint	         x,
+		   gint          y,
+		   guchar       *buffer)
+{
+  Tile *t;
+  guchar *s;
+
+  t = tile_manager_get_tile (tm, x, y, TRUE, FALSE);
+  s = tile_data_pointer (t, x % TILE_WIDTH, y % TILE_HEIGHT);
+  memcpy (buffer, s, tm->bpp);
+  tile_release (t, FALSE);
+}
+
+void
+write_pixel_data_1 (TileManager *tm,
+		    gint	  x,
+		    gint	  y,
+		    guchar       *buffer)
+{
+  Tile *t;
+  guchar *d;
+
+  t = tile_manager_get_tile (tm, x, y, TRUE, TRUE);
+  d = tile_data_pointer (t, x % TILE_WIDTH, y % TILE_HEIGHT);
+  memcpy (d, buffer, tm->bpp);
+  tile_release (t, TRUE);
+}  

app/base/tile-manager.h

@@ -19,6 +19,14 @@
 #ifndef __TILE_MANAGER_H__
 #define __TILE_MANAGER_H__
 
+struct _PixelDataHandle 
+{
+  guchar                 *data;
+  gint                    width;
+  gint	                  height;
+  gint                    stride;
+  gint			  bpp;
+};
 
 /* Creates a new tile manager with the specified
  *  width for the toplevel. The toplevel sizes is
@@ -123,5 +131,40 @@ void          tile_manager_map_over_tile        (TileManager *tm,
 						 Tile        *tile,
 						 Tile        *srctile);
 
+PixelDataHandle * request_pixel_data (TileManager *tm,
+				      gint         x1,
+				      gint	   y1,
+				      gint	   x2,
+				      gint	   y2,
+				      gboolean     wantread,
+				      gboolean     wantwrite);
+
+void              release_pixel_data (PixelDataHandle *pdh);
+
+void              read_pixel_data (TileManager *tm,
+				   gint         x1,
+				   gint         y1,
+				   gint         x2,
+				   gint         y2,
+				   guchar      *buffer,
+				   guint        stride);
+
+void              write_pixel_data (TileManager *tm,
+				    gint         x1,
+				    gint         y1,
+				    gint         x2,
+				    gint         y2,
+				    guchar      *buffer,
+				    guint        stride);
+
+void		  read_pixel_data_1 (TileManager *tm,
+				     gint	  x,
+				     gint	  y,
+				     guchar      *buffer);
+
+void		  write_pixel_data_1 (TileManager *tm,
+				      gint	   x,
+				      gint	   y,
+				      guchar      *buffer);
 
 #endif /* __TILE_MANAGER_H__ */

app/core/gimpimage-undo-push.c

@@ -1907,31 +1907,39 @@ undo_free_channel (UndoState  state,
 /*********************************/
 /*  Channel Mod Undo               */
 
+typedef struct _ChannelModUndo ChannelModUndo;
+
+struct _ChannelModUndo
+{
+  GimpChannel  *channel;
+  TileManager  *tiles;
+};
+
 gboolean
 undo_push_channel_mod (GimpImage *gimage,
 		       gpointer   channel_ptr)
 {
-  GimpChannel *channel;
+  GimpChannel    *channel;
-  TileManager *tiles;
+  TileManager    *tiles;
-  Undo        *new;
+  Undo           *new;
-  gpointer    *data;
+  ChannelModUndo *data;
-  gint         size;
+  gint            size;
 
   channel = (GimpChannel *) channel_ptr;
 
   tiles = GIMP_DRAWABLE (channel)->tiles;
   size  = GIMP_DRAWABLE (channel)->width * GIMP_DRAWABLE (channel)->height +
-    sizeof (gpointer) * 2;
+    sizeof (ChannelModUndo);
 
   if ((new = undo_push (gimage, size, CHANNEL_MOD, TRUE)))
     {
-      data           = g_new (gpointer, 2);
+      data           = g_new (ChannelModUndo, 1);
       new->data      = data;
       new->pop_func  = undo_pop_channel_mod;
       new->free_func = undo_free_channel_mod;
 
-      data[0] = channel_ptr;
+      data->channel = channel;
-      data[1] = (gpointer) tiles;
+      data->tiles   = tiles;
 
       return TRUE;
     }
@@ -1949,15 +1957,14 @@ undo_pop_channel_mod (GimpImage *gimage,
 		      UndoType   type,
 		      gpointer   data_ptr)
 {
-  gpointer    *data;
+  ChannelModUndo *data;
-  TileManager *tiles;
+  TileManager    *tiles;
-  TileManager *temp;
+  TileManager    *temp;
-  GimpChannel *channel;
+  GimpChannel    *channel;
 
-  data = (gpointer *) data_ptr;
+  data = (ChannelModUndo *) data_ptr;
-  channel = (GimpChannel *) data[0];
+  channel = data->channel;
-
+  tiles = data->tiles;
-  tiles = (TileManager *) data[1];
 
   /*  Issue the first update  */
   gimp_drawable_update (GIMP_DRAWABLE (channel),
@@ -1974,7 +1981,7 @@ undo_pop_channel_mod (GimpImage *gimage,
                                    reflect previous tile set */
 
   /*  Set the new buffer  */
-  data[1] = temp;
+  data->tiles = temp;
 
   /*  Issue the second update  */
   gimp_drawable_update (GIMP_DRAWABLE (channel),

app/core/gimpimagemap.c

@@ -331,8 +331,7 @@ image_map_get_color_at (ImageMap *image_map,
 			gint      x, 
 			gint      y)
 {
-  Tile      *tile;
+  guchar    src[5];
-  guchar    *src;
   guchar    *dest;
 
   g_return_val_if_fail (image_map != NULL, NULL);
@@ -355,11 +354,8 @@ image_map_get_color_at (ImageMap *image_map,
 	}
 
       dest = g_new (guchar, 5);
-
+      
-      tile = tile_manager_get_tile (image_map->undo_tiles, x, y,
+      read_pixel_data_1 (image_map->undo_tiles, x, y, src);
-				    TRUE, FALSE);
-
-      src = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
 
       gimp_image_get_color (gimp_drawable_gimage (image_map->drawable),
 			    gimp_drawable_type (image_map->drawable),
@@ -375,8 +371,6 @@ image_map_get_color_at (ImageMap *image_map,
       else
 	dest[4] = 0;
 
-      tile_release (tile, FALSE);
-
       return dest;
     }
   else /* out of bounds error */

app/image_map.c

@@ -331,8 +331,7 @@ image_map_get_color_at (ImageMap *image_map,
 			gint      x, 
 			gint      y)
 {
-  Tile      *tile;
+  guchar    src[5];
-  guchar    *src;
   guchar    *dest;
 
   g_return_val_if_fail (image_map != NULL, NULL);
@@ -355,11 +354,8 @@ image_map_get_color_at (ImageMap *image_map,
 	}
 
       dest = g_new (guchar, 5);
-
+      
-      tile = tile_manager_get_tile (image_map->undo_tiles, x, y,
+      read_pixel_data_1 (image_map->undo_tiles, x, y, src);
-				    TRUE, FALSE);
-
-      src = tile_data_pointer (tile, x % TILE_WIDTH, y % TILE_HEIGHT);
 
       gimp_image_get_color (gimp_drawable_gimage (image_map->drawable),
 			    gimp_drawable_type (image_map->drawable),
@@ -375,8 +371,6 @@ image_map_get_color_at (ImageMap *image_map,
       else
 	dest[4] = 0;
 
-      tile_release (tile, FALSE);
-
       return dest;
     }
   else /* out of bounds error */

app/undo.c

@@ -1907,31 +1907,39 @@ undo_free_channel (UndoState  state,
 /*********************************/
 /*  Channel Mod Undo               */
 
+typedef struct _ChannelModUndo ChannelModUndo;
+
+struct _ChannelModUndo
+{
+  GimpChannel  *channel;
+  TileManager  *tiles;
+};
+
 gboolean
 undo_push_channel_mod (GimpImage *gimage,
 		       gpointer   channel_ptr)
 {
-  GimpChannel *channel;
+  GimpChannel    *channel;
-  TileManager *tiles;
+  TileManager    *tiles;
-  Undo        *new;
+  Undo           *new;
-  gpointer    *data;
+  ChannelModUndo *data;
-  gint         size;
+  gint            size;
 
   channel = (GimpChannel *) channel_ptr;
 
   tiles = GIMP_DRAWABLE (channel)->tiles;
   size  = GIMP_DRAWABLE (channel)->width * GIMP_DRAWABLE (channel)->height +
-    sizeof (gpointer) * 2;
+    sizeof (ChannelModUndo);
 
   if ((new = undo_push (gimage, size, CHANNEL_MOD, TRUE)))
     {
-      data           = g_new (gpointer, 2);
+      data           = g_new (ChannelModUndo, 1);
       new->data      = data;
       new->pop_func  = undo_pop_channel_mod;
       new->free_func = undo_free_channel_mod;
 
-      data[0] = channel_ptr;
+      data->channel = channel;
-      data[1] = (gpointer) tiles;
+      data->tiles   = tiles;
 
       return TRUE;
     }
@@ -1949,15 +1957,14 @@ undo_pop_channel_mod (GimpImage *gimage,
 		      UndoType   type,
 		      gpointer   data_ptr)
 {
-  gpointer    *data;
+  ChannelModUndo *data;
-  TileManager *tiles;
+  TileManager    *tiles;
-  TileManager *temp;
+  TileManager    *temp;
-  GimpChannel *channel;
+  GimpChannel    *channel;
 
-  data = (gpointer *) data_ptr;
+  data = (ChannelModUndo *) data_ptr;
-  channel = (GimpChannel *) data[0];
+  channel = data->channel;
-
+  tiles = data->tiles;
-  tiles = (TileManager *) data[1];
 
   /*  Issue the first update  */
   gimp_drawable_update (GIMP_DRAWABLE (channel),
@@ -1974,7 +1981,7 @@ undo_pop_channel_mod (GimpImage *gimage,
                                    reflect previous tile set */
 
   /*  Set the new buffer  */
-  data[1] = temp;
+  data->tiles = temp;
 
   /*  Issue the second update  */
   gimp_drawable_update (GIMP_DRAWABLE (channel),