kicad-developers team mailing list archive

["Reece R. Pollack" <reece@xxxxxxx>]

I have a question on this commit:

commit e6a200b09e9a38baac4bde53b68d3def6d2d350c
Author: jean-pierre charras <jp.charras@xxxxxxxxxx>
Date:   Thu Feb 14 10:55:57 2019 +0100

    Pcbnew: avoid integer overflow when displaying local coordinates.
    Minor cleanup in code.

This commit changes the calculations of relative coordinates
from integer to floating-point. For example, this calculation:

    int dx = GetCrossHairPosition().x - screen->m_O_Curseur.x;

became this:

    double dx = (double)GetCrossHairPosition().x - (double)screen->m_O_Curseur.x;

There are several other such changes. I don't understand why this
is necessary, and the commit doesn't reference a bug report.  What
circumstances resulted in this commit?

The reason I ask is that bears strongly on the changes I'm working
on to display coordinates relative to a user-selected origin. To
translate coordinates from the internal origin to the user origin I
subtract the user origin coordinates from the internal coordinates.
And, of course, to translate back to the internal origin I add the
user origin coordinates to the displayed coordinates. If this is
likely to overflow then my changes might re-introduce whatever
problem this commit was intended to correct.

The internal representation is in nanometers (1e-9). A signed 32-bit
integer can therefore represent 2^31 (~2.147) meters. Subtracting one
integer coordinate from another could only overflow if the distance
between them is greater than this, which implies one value is positive
and the other is negative. Converting the operands to double before
the subtraction doesn't eliminate the problem, it only extends it to
2^32 or 4.295 meters.

Does KiCad support schematic sheets or PCBs larger than 2 meters? If
so, then we should promote the internal units to "long long" and not
hack around with half-steps like this. If not, then this commit isn't
needed.

-Reece