Complete.Org: Mailing Lists: Archives: freeciv-dev: December 2001:
[Freeciv-Dev] Re: [PATCH] cleanup to canvas_pos_to_map_pos
Home

[Freeciv-Dev] Re: [PATCH] cleanup to canvas_pos_to_map_pos

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: jdorje@xxxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx, freeciv-dev <freeciv-dev@xxxxxxxxxxx>
Subject: [Freeciv-Dev] Re: [PATCH] cleanup to canvas_pos_to_map_pos
From: "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>
Date: Mon, 31 Dec 2001 13:12:06 -0500

At 06:14 AM 01/12/31 -0500, Jason Short wrote:
>The big disadvantage of this patch is that I'm not sure how to cleanly 
>do integer division.  After the translation, getting the map position 
>out of the canvas position should be as simple as
>   map_x = canvas_translated_x / TRANSLATED_WIDTH;
>   map_y = canvas_translated_y / TRANSLATED_HEIGHT;
>but, this doesn't work because the translated position can be negative, 
>and the rounding won't be done properly in this case (-5/2==-2).  My 
>temporary solution is to add a divide() function, but this isn't really 
>acceptable.  So what should I do?  Surely there's a "real" way to do 
>such calculations?  (Note to Ross: you don't account for this in the 
>corecleanups, so there's an off-by-one error for some tiles.)

Could be ... if you spot a case let me know. But you should look really
closely at the macros that do native/index to map transformations. Also
recognize that things like unnormalize take great pains to make things
positive offsets before they do a lot of subsequent manipulations. If
you run corecleanup_11, you will find there aren't very many problems,
if any.

Incidentally, your functions are still far too complicated. What you do
in 20 lines can be done in an order of magnitude less :-).

Cheers,
RossW
=====




[Prev in Thread] Current Thread [Next in Thread]