Complete.Org: Mailing Lists: Archives: freeciv-dev: July 2002:
[Freeciv-Dev] Re: cleanup to canvas_pos_to_map_pos, take II (PR#1183)
Home

[Freeciv-Dev] Re: cleanup to canvas_pos_to_map_pos, take II (PR#1183)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: cleanup to canvas_pos_to_map_pos, take II (PR#1183)
From: jdorje@xxxxxxxxxxxxxxxxxxxxx
Date: Sat, 20 Jul 2002 10:33:58 -0700 (PDT)

Ross W. Wetmore wrote:

Seriously, we just need to find agreement on some of the foolish
stumbling blocks that get in the way here. Most of the code and
algorithmic form with a few notable exceptions are agreed upon.

Indeed, in this case they are identical (except as noted below).

I would prefer to move forward constructively on paths where we can find agreement, and not waste time arm wrestling on points neither of us are going to accept.

Nonetheless, I would like to see this piece of code changed. Waiting to apply a mega-patch to fix all coordinate transformation functions (while introducing many more at the same time) will be much less productive, if history is any guide.

At 12:45 PM 02/07/15 -0700, Jason Short wrote:


This code reduces the operation to its basic mathematical components: a scaling, then a rotation, then another scaling. It is far cleaner and more legible than the current code. It is probably also faster (although I'm not ready to back this up).


The actual operation is a simple transformation. You make it much more
complicated than it really is. That is why it can be done in two lines.

The actual operation consists of a scaling, a rotation, another scaling, and a translation. Whether you use two (long) lines for the whole thing, four shorter lines containing two micro-operations each (which is what you do), or two lines per micro-operation is a matter of preference.

When I originally proposed this change, Ross objected for several reasons:

1. It is really long. In Ross's implementation, the three operations are conducted in a single pair of lines. In this patch I have separated them and added in verbose descriptions of each operation. My goal is to make it more readible for people who don't already have an understanding of the isometric system; other than that I would be happy to condense them.


More reading doesn't necessarily make for better understanding :-).

And sometimes concise code that follows normal patterns is clearer
than piecemeal sections.

I would like to see the operation broken up into its constituent parts, which are fairly normal-looking even to the average reader.

2. Ross claimed the use of the DIVIDE operator was incorrect. He later recanted on this. However, it is still the case that DIVIDE is really ugly here - if anyone has an alternative suggestion, please let me know.


We agree that the fixup handled by your DIVIDE is required, and the operation doesn't need to be done with this piece of ugliness.

The corecleanups use a term of the form "- (*city_x < 0)" to add the negative roundoff factor. There are many others possible that are more
efficient than yours if you simply can't allow Ross code into CVS :-).

The check for <0 is technically incorrect: it introduces an off-by-one error affecting a single pixel. For instance, it gives -64/64 as -2. This is exactly the same issue that was encountered with map_adjust_x.

This may be an acceptable trade-off, but I would rather not introduce something like this that could break in the future (on very small tilesets on PDA's, for instance).

Do you have any other efficient forms that don't have this problem?

And now for the real stumbling blocks you didn't mention :-)

jason
Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.7
diff -u -r1.7 citydlg_common.c
--- client/citydlg_common.c     2002/07/01 21:13:30     1.7
+++ client/citydlg_common.c     2002/07/15 19:26:47
@@ -54,35 +54,38 @@
**************************************************************************/
void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int

*map_y)

The nomenclature here is in dispute.

There are too many pos-es. <type1>_to_<type2>_pos is the standard that
keeps typing cramps to a minimum.

I am not going to rename the functions in this patch. The <type1>_pos_to_<type2>_pos is the standard that was set by the introduction of these functions (mere hours before Vasco proposed the shorter version, as I recall). I would support a change to the shorter version.

{
+  int orig_canvas_x = canvas_x, orig_canvas_y = canvas_y;
 if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
+    /*
+     * The top-left corner is in the center of tile (-2, 2).  The math used
+     * here is identical to that used in canvas_pos_to_map_pos; see that
+     * function for a full explanation.
+     */
+    int flat_canvas_x, flat_canvas_y;


Your use of flat is always a problem.

Unfortunately, there is no good alternative in most cases.

However, I will avoid using the term "flat" in the future. I suppose "non-isometric" it is.

It really needs to be reserved for the unwrapped or flat-earth topologies
and not applied to standard/internal map coordinates or random things it
keeps getting tacked onto.

Flat-view is the biggest problem. Some people call it "standard" view (inaccurate, since isometric is the default), or "overhead" view (incomplete, since isometric view is also overhead). Nobody has suggested a good alternative.

I really, really wish you would quit pushing this when you know it just
won't fly.

I again ask for alternatives.

+    /* Now we've got a square, so it's a simple matter to find the
+       position. */
+    *map_x = -2 + DIVIDE(flat_canvas_x,
+                        NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = 2 + DIVIDE(flat_canvas_y,
+                       NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);


Note the magic "2"s are part of the origin shift, a comment would help and be more in sync with your stated goal.

There is a comment up above; I've added another.

And this is what you needed to do, which apart from some minor differences is pretty close to your code. The compiler will optimize any redundant multiplications, or constant folding,
so except for DIVIDE, we are probably down to the nomenclature
and style issues.

Yes.

Finally, If you add this, then the routine follows a useful standard for all coordinate transformations.

  /* Normalize: return valid map coordinates or FALSE if unreal.
  return is_valid_city_coords(*city_x, *city_y);
*/

I think such a standard is a little ways off.

Here are three alternatives.

(A) The same as before, with the removal of "flat".

(B) The same as before, with the removal of most of the comments. The comments now just name the mathematical operations.

(C) All four micro-operations crammed into one (very long) line.

jason
? foo
? get_data_dirs.diff
? tileset_list.diff
Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.8
diff -u -r1.8 citydlg_common.c
--- client/citydlg_common.c     2002/07/18 19:20:54     1.8
+++ client/citydlg_common.c     2002/07/20 17:14:27
@@ -54,35 +54,39 @@
 **************************************************************************/
 void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int *map_y)
 {
+  int orig_canvas_x = canvas_x, orig_canvas_y = canvas_y;
   if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
+    /*
+     * The top-left corner is in the center of tile (-2, 2).  Aside from
+     * this, the math used here is identical to that used in
+     * canvas_pos_to_map_pos; see that function for a full explanation.
+     */
 
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
+    /* Shift the tile right so the top corner of tile (-2,-2) is at (0,0). */
+    canvas_y += NORMAL_TILE_HEIGHT / 2;
 
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
+    /* Scale to a square. */
+    canvas_x *= NORMAL_TILE_HEIGHT;
+    canvas_y *= NORMAL_TILE_WIDTH;
 
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH / 2)
-      (*map_y)++;
+    /* Rotate by -pi/4. */
+    *map_x = canvas_x + canvas_y;
+    *map_y = canvas_y - canvas_x;
+
+    /* Scale again. */
+    *map_x = DIVIDE(*map_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(*map_y, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+
+    /* Add on the offset of the top-left corner (see above) to get the
+       final coordinates. */
+    *map_x -= 2;
+    *map_y += 2;
   } else {
     *map_x = canvas_x / NORMAL_TILE_WIDTH;
     *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
-  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)", canvas_x, 
canvas_y, *map_x, *map_y);
+  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)",
+         orig_canvas_x, orig_canvas_y, *map_x, *map_y);
 }
 
 /**************************************************************************
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.12
diff -u -r1.12 mapview_common.c
--- client/mapview_common.c     2002/03/19 15:48:38     1.12
+++ client/mapview_common.c     2002/07/20 17:14:28
@@ -187,34 +187,65 @@
                           int map_view_topleft_map_y)
 {
   if (is_isometric) {
-    *map_x = map_view_topleft_map_x;
-    *map_y = map_view_topleft_map_y;
+    /* For another example of this math, see canvas_pos_to_city_pos(). */
 
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
+    /* If the tile width and height are not equal, then it will take very
+     * ugly math to figure anything out about it.  The easiest solution is
+     * just to scale the tile so that it's a perfect (diamond) square.  We
+     * are doing the following transformation to a tile:
+     *
+     *                                 XX  <- height=18, width=18
+     *   XX   <- height=3, width=6
+     * XXXXXX                        XXXXXX
+     *   XX   -----> becomes ----->
+     *                                 XX
+     *
+     * Note that all we really need to do is scale to the LCM of the width
+     * and height (in the above example, 6x6).  But it's easier just to scale
+     * to WIDTH*HEIGHT (18x18).
+     */
+    canvas_x *= NORMAL_TILE_HEIGHT;
+    canvas_y *= NORMAL_TILE_WIDTH;
 
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
+    /*
+     * The "canvas coordinates" we're given are isometric coordinates.  So
+     * we next want to convert them back to "non-isometric" coordinates.  
+     * We'll keep the top-left corner of the canvas as (0,0) so that we 
+     * have an easy frame of reference.  The operation we're doing is thus
+     * just a -pi/4 rotation.   When applied to a square tile, it becomes:
+     *
+     *   1      1 4
+     *  234  ->  3
+     *   5      2 5
+     *
+     * This is the inverse of the rotation from map_pos_to_canvas_pos.
+     */
+    *map_x = canvas_x + canvas_y;
+    *map_y = canvas_y - canvas_x;
 
-    /* We are inside a rectangle, with 2 half tiles starting in the
-       corner, and two tiles further out. Draw a grid to see how this
-       works :). */
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x > canvas_y) {
-      *map_y -= 1;
-    }
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH) {
-      *map_x += 1;
-    }
+    /*
+     * Now the (x0, y0) tile originally had its top corner at (0,0) and
+     * its bottom corner at (0,NORMAL_MAP_HEIGHT).  These are positions 1
+     * and 5 on the diagram above.  We have transformed these two positions:
+     *   (0,0) -> (0,0) -> (0,0)
+     *   (0,H) -> (0,W*H) -> (W*H,W*H)
+     * The tile is now an cardinally-oriented rectangle with these as its
+     * opposite corners.  This makes it easy to tell what tile our
+     * coordinates belong to - we just need to scale the coordinates again.
+     *
+     * Unfortunately, integer division doesn't work so well for negative
+     * numbers; -5/2==-2 while we're looking for -3.  So this is a bit
+     * uglier than it would otherwise be.
+     */
+    *map_x = DIVIDE(*map_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(*map_y, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
   } else {                     /* is_isometric */
-    *map_x = map_view_topleft_map_x + canvas_x / NORMAL_TILE_WIDTH;
-    *map_y = map_view_topleft_map_y + canvas_y / NORMAL_TILE_HEIGHT;
+    *map_x = canvas_x / NORMAL_TILE_WIDTH;
+    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
+
+  *map_x += map_view_topleft_map_x;
+  *map_y += map_view_topleft_map_y;
 
   /*
    * If we are outside the map find the nearest tile, with distance as
Index: common/shared.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/shared.h,v
retrieving revision 1.95
diff -u -r1.95 shared.h
--- common/shared.h     2002/05/25 17:44:08     1.95
+++ common/shared.h     2002/07/20 17:14:30
@@ -52,6 +52,13 @@
 
 #define BOOL_VAL(x) ((x)!=0)
 
+/*
+ * DIVIDE() divides and rounds down, rather than just divides and
+ * rounds toward 0.
+ */
+#define DIVIDE(n, d) \
+    ((n) % (d) < 0 ? (n) / (d) - 1 : (n) / (d))
+
 /* Deletes bit no in val,
    moves all bits larger than no one down,
    and inserts a zero at the top. */
? foo
? get_data_dirs.diff
? tileset_list.diff
Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.8
diff -u -r1.8 citydlg_common.c
--- client/citydlg_common.c     2002/07/18 19:20:54     1.8
+++ client/citydlg_common.c     2002/07/20 17:08:32
@@ -54,35 +54,16 @@
 **************************************************************************/
 void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int *map_y)
 {
+  int orig_canvas_x = canvas_x, orig_canvas_y = canvas_y;
   if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
-
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
-
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
-
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH / 2)
-      (*map_y)++;
+    *map_x = -2 + DIVIDE(canvas_x * NORMAL_TILE_HEIGHT + (canvas_y + 
NORMAL_TILE_HEIGHT / 2) * NORMAL_TILE_WIDTH, NORMAL_TILE_WIDTH * 
NORMAL_TILE_HEIGHT);
+    *map_y = 2 + DIVIDE((canvas_y + NORMAL_TILE_HEIGHT / 2) * 
NORMAL_TILE_WIDTH - canvas_x * NORMAL_TILE_HEIGHT, NORMAL_TILE_WIDTH * 
NORMAL_TILE_HEIGHT);
   } else {
     *map_x = canvas_x / NORMAL_TILE_WIDTH;
     *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
-  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)", canvas_x, 
canvas_y, *map_x, *map_y);
+  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)",
+         orig_canvas_x, orig_canvas_y, *map_x, *map_y);
 }
 
 /**************************************************************************
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.12
diff -u -r1.12 mapview_common.c
--- client/mapview_common.c     2002/03/19 15:48:38     1.12
+++ client/mapview_common.c     2002/07/20 17:08:33
@@ -187,34 +187,15 @@
                           int map_view_topleft_map_y)
 {
   if (is_isometric) {
-    *map_x = map_view_topleft_map_x;
-    *map_y = map_view_topleft_map_y;
-
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
-
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
-
-    /* We are inside a rectangle, with 2 half tiles starting in the
-       corner, and two tiles further out. Draw a grid to see how this
-       works :). */
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x > canvas_y) {
-      *map_y -= 1;
-    }
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH) {
-      *map_x += 1;
-    }
+    *map_x = DIVIDE((canvas_x * NORMAL_TILE_HEIGHT + canvas_y * 
NORMAL_TILE_WIDTH), NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE((canvas_y * NORMAL_TILE_WIDTH - canvas_x * 
NORMAL_TILE_HEIGHT), NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
   } else {                     /* is_isometric */
-    *map_x = map_view_topleft_map_x + canvas_x / NORMAL_TILE_WIDTH;
-    *map_y = map_view_topleft_map_y + canvas_y / NORMAL_TILE_HEIGHT;
+    *map_x = canvas_x / NORMAL_TILE_WIDTH;
+    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
+
+  *map_x += map_view_topleft_map_x;
+  *map_y += map_view_topleft_map_y;
 
   /*
    * If we are outside the map find the nearest tile, with distance as
Index: common/shared.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/shared.h,v
retrieving revision 1.95
diff -u -r1.95 shared.h
--- common/shared.h     2002/05/25 17:44:08     1.95
+++ common/shared.h     2002/07/20 17:08:35
@@ -52,6 +52,13 @@
 
 #define BOOL_VAL(x) ((x)!=0)
 
+/*
+ * DIVIDE() divides and rounds down, rather than just divides and
+ * rounds toward 0.
+ */
+#define DIVIDE(n, d) \
+    ((n) % (d) < 0 ? (n) / (d) - 1 : (n) / (d))
+
 /* Deletes bit no in val,
    moves all bits larger than no one down,
    and inserts a zero at the top. */
? foo
? get_data_dirs.diff
? tileset_list.diff
Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.8
diff -u -r1.8 citydlg_common.c
--- client/citydlg_common.c     2002/07/18 19:20:54     1.8
+++ client/citydlg_common.c     2002/07/20 17:12:59
@@ -54,35 +54,39 @@
 **************************************************************************/
 void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int *map_y)
 {
+  int orig_canvas_x = canvas_x, orig_canvas_y = canvas_y;
   if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
+    /*
+     * The top-left corner is in the center of tile (-2, 2).  Aside from
+     * this, the math used here is identical to that used in
+     * canvas_pos_to_map_pos; see that function for a full explanation.
+     */
 
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
+    /* Shift the tile right so the top corner of tile (-2,-2) is at (0,0). */
+    canvas_y += NORMAL_TILE_HEIGHT / 2;
 
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
+    /* Scale to a square. */
+    canvas_x *= NORMAL_TILE_HEIGHT;
+    canvas_y *= NORMAL_TILE_WIDTH;
 
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH / 2)
-      (*map_y)++;
+    /* Rotate by -pi/4. */
+    *map_x = canvas_x + canvas_y;
+    *map_y = canvas_y - canvas_x;
+
+    /* Scale again. */
+    *map_x = DIVIDE(*map_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(*map_y, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+
+    /* Add on the offset of the top-left corner (see above) to get the
+       final coordinates. */
+    *map_x -= 2;
+    *map_y += 2;
   } else {
     *map_x = canvas_x / NORMAL_TILE_WIDTH;
     *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
-  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)", canvas_x, 
canvas_y, *map_x, *map_y);
+  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)",
+         orig_canvas_x, orig_canvas_y, *map_x, *map_y);
 }
 
 /**************************************************************************
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.12
diff -u -r1.12 mapview_common.c
--- client/mapview_common.c     2002/03/19 15:48:38     1.12
+++ client/mapview_common.c     2002/07/20 17:13:00
@@ -187,34 +187,26 @@
                           int map_view_topleft_map_y)
 {
   if (is_isometric) {
-    *map_x = map_view_topleft_map_x;
-    *map_y = map_view_topleft_map_y;
+    /* For another example of this math, see canvas_pos_to_city_pos(). */
 
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
+    /* Scale to a square. */
+    canvas_x *= NORMAL_TILE_HEIGHT;
+    canvas_y *= NORMAL_TILE_WIDTH;
 
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
+    /* Rotate by -pi/4. */
+    *map_x = canvas_x + canvas_y;
+    *map_y = canvas_y - canvas_x;
 
-    /* We are inside a rectangle, with 2 half tiles starting in the
-       corner, and two tiles further out. Draw a grid to see how this
-       works :). */
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x > canvas_y) {
-      *map_y -= 1;
-    }
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH) {
-      *map_x += 1;
-    }
+    /* Scale again. */
+    *map_x = DIVIDE(*map_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(*map_y, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
   } else {                     /* is_isometric */
-    *map_x = map_view_topleft_map_x + canvas_x / NORMAL_TILE_WIDTH;
-    *map_y = map_view_topleft_map_y + canvas_y / NORMAL_TILE_HEIGHT;
+    *map_x = canvas_x / NORMAL_TILE_WIDTH;
+    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
+
+  *map_x += map_view_topleft_map_x;
+  *map_y += map_view_topleft_map_y;
 
   /*
    * If we are outside the map find the nearest tile, with distance as
Index: common/shared.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/shared.h,v
retrieving revision 1.95
diff -u -r1.95 shared.h
--- common/shared.h     2002/05/25 17:44:08     1.95
+++ common/shared.h     2002/07/20 17:13:02
@@ -52,6 +52,13 @@
 
 #define BOOL_VAL(x) ((x)!=0)
 
+/*
+ * DIVIDE() divides and rounds down, rather than just divides and
+ * rounds toward 0.
+ */
+#define DIVIDE(n, d) \
+    ((n) % (d) < 0 ? (n) / (d) - 1 : (n) / (d))
+
 /* Deletes bit no in val,
    moves all bits larger than no one down,
    and inserts a zero at the top. */

[Prev in Thread] Current Thread [Next in Thread]
  • [Freeciv-Dev] Re: cleanup to canvas_pos_to_map_pos, take II (PR#1183), jdorje <=