Re: [HACKERS] [PATCH] Improve geometric types

At Wed, 31 Jan 2018 13:09:09 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180131.130909.210233873.horiguchi.kyotaro@lab.ntt.co.jp>
> At Sun, 21 Jan 2018 21:59:19 +0100, Emre Hasegeli <emre@hasegeli.com> wrote in <CAE2gYzxDYs5tcvc4uErsWaFTb3UTYS0ERt_fFyi-28Ldvs5d4A@mail.gmail.com>
> > New versions are attached including all changes we discussed.
> 
> Thanks for the new version.
> 
> # there's many changes from the previous version..
> 
> About 0001 and 0002.
> 
> 1."COPT=-DGEODEBUG make" complains as follows.
> 
>   | geo_ops.c:2445:62: error: invalid type argument of unary ‘*’ (have ‘float8 {aka double}’)
>   |   printf("dist_ppoly_internal- segment 0/n distance is %f\n", *result);
> 
> 2. line_construct_pm has been renamed to line_construct. I
>    noticed that the patch adds the second block for "(m == 0.0)"
>    (from the ealier versions) but it seems to work exactly as the
>    same to the "else" block. We need a comment about the reason
>    for the seemingly redundant second block.
> 
> 3. point_sl can return -0.0 and that is a thing that this patch
>    intends to avoid. line_invsl has the same problem.
> 
> 4. lseg_interpt_line is doing as follows.
> 
>    >  if (FPeq(lseg->p[0].x, interpt.x) && FPeq(lseg->p[0].y, interpt.y))
>    >    *result = lseg->p[0];
>    >  else if (FPeq(lseg->p[1].x, interpt.x) && FPeq(lseg->p[1].y, interpt.y))
>    >    *result = lseg->p[1];
> 
>    I suppose we can use point_pt_point for this purpose.
> 
>    >  if (point_eq_point(&lseg->p[0], &interpt))
>    >    *result = lseg->p[0];
>    >  else if (point_eq_point(&lseg->p[1], &interpt))
>    >    *result = lseg->p[1];
> 
>    However I'm not sure that adjusting the intersection to the
>    tips of the segment is good or not. Adjusting onto the line
>    can be better in another case. lseg_interpt_lseg, for
>    instance, checks lseg_contain_point on the line parameter of
>    lseg_interpt_line.
> 

> # I'll be back later..

I've been back.

0003: This patch replaces "double" with float and bare arithmetic
    and comparisons on double to special functions accompanied
    with checking against abnormal values.

 - Almost all of them are eliminated with a few safe exceptions.

    - circle_recv(), circle_distance(), dist_pc(), dist_cpoint()
      are using "< 0.0" comparison but it looks fine.

    - pg_hypot is right to use bare arithmetics.

    ! circle_contain_pt() does the following comparison and it
      seems to be out of our current policy.

      point_dt(center, point) <= radius

      I suppose this should use FPle.

      FPle(point_dt(center, point), radius)

      The same is true of circle_contain_pt(), pt_contained_circle .



# Sorry, that' all for today..

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center