kicad-developers team mailing list archive

[Lorenzo Marcantonio <l.marcantonio@xxxxxxxxxxxx>]

On Tue, Apr 30, 2013 at 11:13:19AM -0500, Dick Hollenbeck wrote:
> > - When going from double to int (geometrically) a KiROUND should be used
> >   instead of a int cast
> 
> Yes, as soon as you enable arbitrary degrees.  (We've not done that to this point.)

Not only for degrees, but especially for coordinates, like when
computing point on a circle. For angle I reckon it would be the opposite
(i.e. now they could be rounded, after the switch they would not).
However this should be for 'user visible' angles only... it is not
desiderable to round angles used for internal processing (for example
the angles computed during DRC). If I didn't miss anything the only
"user level" angles are: the arc angles, text orientation, pad
orientation and module orientation.

Since these are only passed around and summed/subtracted between them
I think that if there are no fractional angle incoming (from a file for
example) there shouldn't be fractional angles coming out. And since
I didn't touch the dialog (except for ensuring that they were used as
int) the user should only 'see' integer values (integer decidegrees).

> That is a context specific issue, I am reluctant to say yes to that in a general sense.
> Actually fvar is really a double, so adding an integer to it will be fine.  The integer
> gets promoted to double (at least conceptually) with no loss of information yet.

Exactly. That's why I said 'theorically should be the same'; a double
should fit all the digits (plus one decimal digit used for rounding plus
one guard digit) of an int without loss of precision. However
summing two doubles should be evaluated case by case (to avoid biasing).
And even if fvar would be something like a float, promotion rules would
kick in and everything would be done in double precision anyway.

In short:

i1 + KiROUND(d1) -> int + int (d1 is rounded)

KiROUND(i1 + d1) -> double + double, the result is rounded *but* since 
                    i1 was integer anywhat it shouldn't change how the
                    rounding happen (i.e. 0.7 rounds to 1, 0.7 + i would 
                    round anyway to 1 + i, whatever integer i we chose)

KiROUND(d1 + d2) -> Need to be evaluated, but usually this should be the
                    right thing to do (very context dependant)

AFAIK most of the cases are of the kind center + trigvalue, and there is
no double+double sum around being put in an int. Since I like to think
that an int+int is cheaper than a double+double (not necessarily
true these days...) I prefer the first form.

> Yes, if the strangeness is worth the speed.  For something relative, i.e. a rough ranking,
> Thomas Spindler used another technique that I wrapped into close_ness() in spectra_export.
>  It could have been called "ball_park".

I only did one time, in HitTestPoints. The strangeness is something very
relative since is something like the second thing you learn in CG after
matrix operations:P the close_ness thing seems interesting, I'll look
into that

There is a way bigger performance loss in computing unasked distances
during clipping for display (on the queue)

> I have found it better to return the most information possible, the double, and let the
> caller establish usage (rounding) policy on a case by case basis.  Dumbing down a function
> reduces its applicability.

Then it's already fine that way. I think that in fact sometimes is used for full
precision.

> Possibly MSVC++, which is now excluded.

MSVC likes to warn a lot (implicit double to int for sure), probably that was the reason.

> Yes I do, but the function name tells me nothing.  DECI_DEGRESS  is better clarity
> somewhere in the name if that's what you are using.

It's difficult to make mnemonics useful for everyone, I think that
RAD2DECIDEG and DECIDEG2RAD would be good names (DECIDEGREES2RAD seem
a little too verbose)

> If you want further clean up work.  I no longer think we need the BOARD_ITEM::Show() debug
> functions.  It is very easy now to use

Put in queue for later.

-- 
Lorenzo Marcantonio
Logos Srl