[Freeciv-Dev] (PR#2960) First step towards eliminating sprite padding

["esr@xxxxxxxxxxx via RT" <rt@xxxxxxxxxxxxxx>]

This patch is about half the work needed to make it possible to re-use
a single sprite with different amounts of framing padding in different
tilesets.  It's the only part of this work that will touch the client code.

Why this is good: it means that the padding for a sprite is no longer
fixed but can be computed at load time based on tileset properties.
That means that we can store one (1) copy of a sprite like tx.village,
cropped to its bounding box, and a dozen different tilesets can place
it on a mapview tile in different ways.

In a previous mail exchange with Jason, I discussed a different
implementation of this concept, one that would carried offset
information with the sprites and reached deep into the client drawing
code.  This way is simpler and better.  It changes the logic of only
one function in each client.

This patch has been tested with GTK, GTK2, and Xaw.  I don't know the MUI
or SDL APIs, so I didn't try to include implementations in those.  The
win32 API is BitBLT on wheels so I was able to guess at the changes
needed with fair confidence, but of course it needs to be tested.

Note 1: This patch does not yet change the behavior of any client, and
will not until compute_sprite_offset() starts returning real offsets.
Right now it contains silly test code which should be removed when the
patch is committed.

Note 2: This patch collides with my data reorg patch.  But if this one
gets committed first *and* the client support is in place, I may be
able to drastically cut the number of sprite files that have to go in the
data directory.

After this patch, the followup steps are:

1. Verify that the win32 implementation works.  Do the needed thing in
   MUI and SDL.

2. Implement real code in compute_sprite_offset() to interpret declarations
   like this:

[placement]

centered = {"ts.","tx.","r."} ; Center all resource, extra, & road/rail tiles.
flagspot = {"f."}             ; Pin all flags to the flag spot
offsets = {"x", "y", "sprite",
           14, 14, "battlemech" "hovercraft"}

3. Write PIL code to compute the declarations equivalent to the current
   tile padding.  Merge these into the tilespec files.

4. Test the new machinery by autocropping all sprites to their bounding boxes 
   as they're loaded, then padding them in accordance with the declarations.
   If this works correctly, the visual appearance will be unchanged.

5. Once that looks sane, strip all padding from most sprites in the data
   directory and drop out the autocropping.

6. Get rid of the flag-cropping cruft in the GTK and GTK2 clients,
   it won't be needed any more.

7. Write a tool to recognize duplicates.  Reorganize the search directories
   so duplicates are eliminated.
-- 
                <a href="http://www.catb.org/~esr/";>Eric S. Raymond</a>

Diffs between last version checked in and current workfile(s):

--- client/gui-gtk/graphics.c.~1.48.~   2003-01-29 00:10:49.000000000 -0500
+++ client/gui-gtk/graphics.c   2003-02-01 00:30:19.000000000 -0500
@@ -163,28 +163,30 @@
 }
 
 /***************************************************************************
-return newly allocated sprite cropped from source
+return newly allocated sprite cropped from source.  sprite may be padded 
+to upper left by specifying an offset in ox, oy.
 ***************************************************************************/
 struct Sprite *crop_sprite(struct Sprite *source,
                           int x, int y,
-                          int width, int height)
+                          int width, int height,
+                          int xoffset, int yoffset)
 {
   GdkPixmap *mypixmap, *mask = NULL;
 
-  mypixmap = gdk_pixmap_new(root_window, width, height, -1);
+  mypixmap = gdk_pixmap_new(root_window, xoffset+width, yoffset+height, -1);
 
-  gdk_draw_pixmap(mypixmap, civ_gc, source->pixmap, x, y, 0, 0,
+  gdk_draw_pixmap(mypixmap, civ_gc, source->pixmap, x, y, xoffset, yoffset,
                  width, height);
 
   if (source->has_mask) {
-    mask = gdk_pixmap_new(mask_bitmap, width, height, 1);
+    mask = gdk_pixmap_new(mask_bitmap, xoffset+width, yoffset+height, 1);
     gdk_draw_rectangle(mask, mask_bg_gc, TRUE, 0, 0, -1, -1 );
 
     gdk_draw_pixmap(mask, mask_fg_gc, source->mask,
-                   x, y, 0, 0, width, height);
+                   x, y, xoffset, yoffset, width, height);
   }
 
-  return ctor_sprite_mask(mypixmap, mask, width, height);
+  return ctor_sprite_mask(mypixmap, mask, xoffset+width, yoffset+height);
 }
 
 
@@ -621,5 +623,5 @@
 
   sprite_get_bounding_box(s, &x1, &y1, &x2, &y2);
 
-  return crop_sprite(s, x1, y1, x2 - x1 + 1, y2 - y1 + 1);
+  return crop_sprite(s, x1, y1, x2 - x1 + 1, y2 - y1 + 1, 0, 0);
 }
--- client/gui-gtk/plrdlg.c.~1.44.~     2003-01-05 15:51:36.000000000 -0500
+++ client/gui-gtk/plrdlg.c     2003-01-31 23:54:05.000000000 -0500
@@ -373,7 +373,7 @@
   assert(flag_w >= MIN_DIMENSION && flag_h >= MIN_DIMENSION);
   
   /* croping */
-  croped = crop_sprite(flag, start_x, start_y, flag_w, flag_h);
+  croped = crop_sprite(flag, start_x, start_y, flag_w, flag_h, 0, 0);
 
   /* scaling */
   newflag_h = GTK_CLIST(players_list)->row_height;
--- client/gui-gtk-2.0/graphics.c.~1.12.~       2003-01-29 00:10:49.000000000 
-0500
+++ client/gui-gtk-2.0/graphics.c       2003-02-01 00:31:20.000000000 -0500
@@ -161,31 +161,32 @@
 }
 
 /***************************************************************************
-return newly allocated sprite cropped from source
+return newly allocated sprite cropped from source.  sprite may be padded 
+to upper left by specifying an offset in ox, oy.
 ***************************************************************************/
 struct Sprite *crop_sprite(struct Sprite *source,
                           int x, int y,
-                          int width, int height)
+                          int width, int height,
+                          int xoffset, int yoffset)
 {
   GdkPixmap *mypixmap, *mask = NULL;
 
-  mypixmap = gdk_pixmap_new(root_window, width, height, -1);
+  mypixmap = gdk_pixmap_new(root_window, xoffset+width, yoffset+height, -1);
 
-  gdk_draw_pixmap(mypixmap, civ_gc, source->pixmap, x, y, 0, 0,
+  gdk_draw_pixmap(mypixmap, civ_gc, source->pixmap, x, y, xoffset, yoffset,
                  width, height);
 
   if (source->has_mask) {
-    mask = gdk_pixmap_new(NULL, width, height, 1);
-    gdk_draw_rectangle(mask, mask_bg_gc, TRUE, 0, 0, -1, -1);
+    mask = gdk_pixmap_new(mask_bitmap, xoffset+width, yoffset+height, 1);
+    gdk_draw_rectangle(mask, mask_bg_gc, TRUE, 0, 0, -1, -1 );
 
     gdk_draw_pixmap(mask, mask_fg_gc, source->mask,
-                   x, y, 0, 0, width, height);
+                   x, y, xoffset, yoffset, width, height);
   }
 
-  return ctor_sprite_mask(mypixmap, mask, width, height);
+  return ctor_sprite_mask(mypixmap, mask, xoffset+width, yoffset+height);
 }
 
-
 /***************************************************************************
 ...
 ***************************************************************************/
@@ -615,5 +616,5 @@
 
   sprite_get_bounding_box(s, &x1, &y1, &x2, &y2);
 
-  return crop_sprite(s, x1, y1, x2 - x1 + 1, y2 - y1 + 1);
+  return crop_sprite(s, x1, y1, x2 - x1 + 1, y2 - y1 + 1, 0, 0);
 }
--- client/gui-mui/graphics.c.~1.30.~   2003-01-10 21:49:09.000000000 -0500
+++ client/gui-mui/graphics.c   2003-02-01 01:06:27.000000000 -0500
@@ -83,12 +83,21 @@
 
 /****************************************************************
  Crop a sprite from a bigger sprite (called from the gui
- independend part)
+ independent part)
  Note that the ImageData is not copied but only referenced,
  this saves memory (because Data of BitMaps are aligned) and
  loading time
-*****************************************************************/
-struct Sprite *crop_sprite(struct Sprite *source, int x, int y, int width, int 
height)
+ FIXME: crop_sprite() does not use the x_offset and y_offset args What
+ needs to happen is that the sprite created is actually
+ (xoffset+width, yoffset+height) in size, with the image upper left at
+ (xoffset, yoffset) on a transparent background.  Without this change,
+ overlay tiles such as units may be placed upwards and to the left of
+ where they should be.
+*****************************************************************/
+struct Sprite *crop_sprite(struct Sprite *source, 
+                          int x, int y, 
+                          int width, int height,
+                          int xoffset, int yoffset)
 {
   struct Sprite *sprite = (struct Sprite *) malloc(sizeof(struct Sprite));
 
--- client/gui-sdl/graphics.c.~1.10.~   2003-01-30 14:09:26.000000000 -0500
+++ client/gui-sdl/graphics.c   2003-02-01 00:44:47.000000000 -0500
@@ -1759,9 +1759,17 @@
 /**************************************************************************
   Create a new sprite by cropping and taking only the given portion of
   the image.
+ FIXME: crop_sprite() does not use the x_offset and y_offset args What
+ needs to happen is that the sprite created is actually
+ (xoffset+width, yoffset+height) in size, with the image upper left at
+ (xoffset, yoffset) on a transparent background.  Without this change,
+ overlay tiles such as units may be placed upwards and to the left of
+ where they should be.
 **************************************************************************/
 struct Sprite *crop_sprite(struct Sprite *source,
-                          int x, int y, int width, int height)
+                          int x, int y, 
+                          int width, int height,
+                          int xoffset, int yoffset)
 {
   SDL_Rect src_rect =
       { (Sint16) x, (Sint16) y, (Uint16) width, (Uint16) height };
--- client/gui-stub/graphics.c.~1.10.~  2002-11-30 14:27:37.000000000 -0500
+++ client/gui-stub/graphics.c  2003-01-31 23:55:05.000000000 -0500
@@ -111,7 +111,9 @@
   the image.
 **************************************************************************/
 struct Sprite *crop_sprite(struct Sprite *source,
-                          int x, int y, int width, int height)
+                          int x, int y, 
+                          int width, int height,
+                          int xoffset, int yoffset)
 {
   /* PORTME */
   return NULL;
--- client/gui-win32/graphics.c.~1.13.~ 2003-01-23 16:39:32.000000000 -0500
+++ client/gui-win32/graphics.c 2003-02-01 01:13:44.000000000 -0500
@@ -159,10 +159,18 @@
 
 /**************************************************************************
 
+ FIXME: This implementation has not been tested for nozero offset values.
+ What needs to happen is that the sprite created is actually
+ (xoffset+width, yoffset+height) in size, with the image upper left at
+ (xoffset, yoffset) on a transparent background.  Without this change,
+ overlay tiles such as units may be placed upwards and to the left of
+ where they should be.
 **************************************************************************/
 struct Sprite *
 crop_sprite(struct Sprite *source,
-                           int x, int y, int width, int height)
+           int x, int y, 
+           int width, int height,
+           int xoffset, int yoffset)
 {
   SPRITE *mysprite;
   HDC hdc;
@@ -194,7 +202,7 @@
     {
       freelog(LOG_FATAL,"CreateCompatibleDC failed");
     }
-  if (!(smallbitmap=CreateCompatibleBitmap(hdc,width,height)))
+  if (!(smallbitmap=CreateCompatibleBitmap(hdc,xoffset+width,yoffset+height)))
     {
       freelog(LOG_FATAL,"CreateCompatibleBitmap failed");
       return NULL;
@@ -202,17 +210,17 @@
   
   bigsave=SelectObject(hdcbig,bigbitmap);
   smallsave=SelectObject(hdcsmall,smallbitmap);
-  BitBlt(hdcsmall,0,0,width,height,hdcbig,x,y,SRCCOPY);
+  BitBlt(hdcsmall,xoffset,yoffset,width,height,hdcbig,x,y,SRCCOPY);
   smallmask=NULL;
 
   if (source->has_mask)
     {
       bigmask=BITMAP2HBITMAP(&source->mask);
       SelectObject(hdcbig,bigmask);
-      if ((smallmask=CreateBitmap(width,height,1,1,NULL)))
+      if ((smallmask=CreateBitmap(xoffset+width,yoffset+height,1,1,NULL)))
         {
           SelectObject(hdcsmall,smallmask);
-          BitBlt(hdcsmall,0,0,width,height,hdcbig,x,y,SRCCOPY);
+          BitBlt(hdcsmall,xoffset,yoffset,width,height,hdcbig,x,y,SRCCOPY);
         }
       SelectObject(hdcbig,bigbitmap);
       DeleteObject(bigmask);
@@ -220,8 +228,8 @@
 
   mysprite=fc_malloc(sizeof(struct Sprite));
   mysprite->cache_id=0;
-  mysprite->width=width;
-  mysprite->height=height;
+  mysprite->width=xoffset+width;
+  mysprite->height=yoffset+height;
   HBITMAP2BITMAP(smallbitmap,&mysprite->bmp);
   mysprite->has_mask=0;
   if (smallmask)
--- client/gui-xaw/graphics.c.~1.46.~   2002-12-17 17:41:04.000000000 -0500
+++ client/gui-xaw/graphics.c   2003-02-01 01:01:32.000000000 -0500
@@ -165,28 +165,31 @@
 return newly allocated sprite cropped from source
 ***************************************************************************/
 struct Sprite *crop_sprite(struct Sprite *source,
-                          int x, int y, int width, int height)
+                          int x, int y, 
+                          int width, int height,
+                          int xoffset, int yoffset)
 {
   Pixmap mypixmap;
 
   mypixmap = XCreatePixmap(display, root_window,
-                          width, height, display_depth);
+                          xoffset+width, yoffset+height, display_depth);
   XCopyArea(display, source->pixmap, mypixmap, civ_gc, 
-           x, y, width, height, 0, 0);
+           x, y, width, height, xoffset, yoffset);
 
   if (source->has_mask) {
     GC plane_gc;
     Pixmap mask;
 
-    mask = XCreatePixmap(display, root_window, width, height, 1);
+    mask = XCreatePixmap(display, root_window, 
+                        xoffset+width, yoffset+height, 1);
 
     plane_gc = XCreateGC(display, mask, 0, NULL);
     XCopyArea(display, source->mask, mask, plane_gc, 
-             x, y, width, height, 0, 0);
+             x, y, width, height, xoffset, yoffset);
     XFreeGC(display, plane_gc);
-    return ctor_sprite_mask(mypixmap, mask, width, height);
+    return ctor_sprite_mask(mypixmap, mask, xoffset+width, yoffset+height);
   } else {
-    return ctor_sprite(mypixmap, width, height);
+    return ctor_sprite(mypixmap, xoffset+width, yoffset+height);
   }
 }
 
--- client/include/graphics_g.h.~1.8.~  2002-11-07 11:04:53.000000000 -0500
+++ client/include/graphics_g.h 2003-01-31 23:55:45.000000000 -0500
@@ -29,7 +29,9 @@
 
 struct Sprite *load_gfxfile(const char *filename);
 struct Sprite *crop_sprite(struct Sprite *source,
-                          int x, int y, int width, int height);
+                          int x, int y, 
+                          int width, int height,
+                          int xoffset, int yoffset);
 void free_sprite(struct Sprite *s);
 
 #endif  /* FC__GRAPHICS_G_H */
--- client/tilespec.c.~1.107.~  2003-01-30 13:30:34.000000000 -0500
+++ client/tilespec.c   2003-02-01 01:31:05.000000000 -0500
@@ -477,6 +477,23 @@
 }
 
 /**********************************************************************
+  Do tileset-specific computation of offsets for the sprite with
+  a given tag.
+***********************************************************************/
+static void compute_sprite_offset(const char *tag, int *ox, int *oy)
+{
+      /* TEST CODE: Remove for production!
+       * This just demonstrates that we can conditionally offset a
+       * sprite.  If it works, the settler units you start with will
+       * be displaced a bit down and to the right, and probably be
+       * incompletely displayed because it's clipped by the tile edge.
+       */
+      if (strncmp(tag, "u.settler", 8) == 0) {
+         *ox = 4; *oy = 8;
+      }
+}
+
+/**********************************************************************
   Load one specfile and specified xpm file; splits xpm into tiles,
   and save sprites in sprite_hash.
 ***********************************************************************/
@@ -554,11 +571,12 @@
 
       x1 = x_top_left + column * dx + (is_pixel_border ? column : 0);
       y1 = y_top_left + row * dy + (is_pixel_border ? row : 0);
-      small_sprite = crop_sprite(big_sprite, x1, y1, dx, dy);
 
-      for(k=0; k<num_tags; k++) {
-       /* don't free old sprite, since could be multiple tags
-          pointing to it */
+      for(k = 0; k < num_tags; k++) {
+       int ox = 0, oy = 0;
+       compute_sprite_offset(tags[k], &ox, &oy);
+       small_sprite = crop_sprite(big_sprite, x1, y1, dx, dy, ox, oy);
+       (void) hash_delete_entry(sprite_hash, tags[k]);
        (void) hash_replace(sprite_hash, mystrdup(tags[k]), small_sprite);
       }
       free(tags);

End of diffs.