> However, for either patch, I'm a bit concerned about using FPzero()
> on the inner product result. To the extent that the EPSILON bound
> has any useful meaning at all, it needs to mean a maximum difference
> between two coordinate values. The inner product has units of coordinates
> squared, so it seems like EPSILON isn't an appropriate threshold there.
In this case, I suggest this:
> if (FPzero(l1->A))
> PG_RETURN_BOOL(FPzero(l2->B));
> if (FPzero(l2->A))
> PG_RETURN_BOOL(FPzero(l1->B));
> if (FPzero(l1->B))
> PG_RETURN_BOOL(FPzero(l2->A));
> if (FPzero(l2->B))
> PG_RETURN_BOOL(FPzero(l1->A));
>
> PG_RETURN_BOOL(FPeq((l1->A * l2->A) / (l1->B * l2->B), -1.0));
> Possibly this objection is pointless, because I'm not at all sure that
> the existing code is careful about how it uses FPeq/FPzero; perhaps
> we're applying EPSILON to all manner of numbers that don't have units
> of the coordinate space. But this won't help make it better.
The existing code was probably paying attention to this particular
one, but it fails to apply EPSILON meaningfully to many other places.
For example lseg_parallel() and lseg_perp() applies it 2 times to the
same input. First point_sl() compares x coordinates with FPeq(), and
then the returned slopes are compared again with FPeq().