[prev in list] [next in list] [prev in thread] [next in thread]
List: postgis-users
Subject: Re: [postgis-users] Bug? Strange code in lwline_crossing_direction()
From: Darafei_"Komяpa"_Praliaskouski <me () komzpa ! net>
Date: 2020-04-20 6:46:17
Message-ID: CAC8Q8tL-AX+k41-uuhU3E=BZ-HcE0nstAWdMo=s7h-7-GdHDug () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
The code was born in 2009 by Paul Ramsey.
https://github.com/postgis/postgis/commit/6adec1b2be11e7d0fe5c482e50ce0872c400cfd6#diff-5705dd09236e9a6e8fa456b7b9599ed7R205-R211
Either a comment is needed on this, or a patch. Easiest way to check a
hypothesis that this may be a bug is to change L526 (github - Edit - change
- pull request) and see that CI says that everything failed (then it needs
docs) or that everything passed (then a trac ticket and a fix, which would
be a test that pins the correct behavior of that branch).
On Mon, Apr 20, 2020 at 7:42 AM Kohei KaiGai <kaigai@heterodb.com> wrote:
> Hello Darafei,
>
> It looks to me the first_cross should be SEG_CROSS_RIGHT, not
> SEG_CROSS_LEFT here.
> If it is expected, L574 shall never become true. Right?
>
> > at L526:
> > if ( this_cross == SEG_CROSS_RIGHT )
> > {
> > LWDEBUG(4,"this_cross == SEG_CROSS_RIGHT");
> > cross_right++;
> > if ( ! first_cross )
> > first_cross = SEG_CROSS_LEFT;
> > }
>
> Thanks,
>
> 2020年4月19日(日) 23:31 Darafei "Komяpa" Praliaskouski <me@komzpa.net>:
> >
> > Hi,
> >
> > the link you have is setting variable this_cross, and L526 sets
> > first_cross from it if it was not set and there is LEFT or RIGHT cross
> > (not no-cross and not COLLINEAR). Looks expected to me.
> >
> > On Sat, Apr 18, 2020 at 6:32 PM Kohei KaiGai <kaigai@heterodb.com>
> wrote:
> > >
> > > Hello,
> > >
> > > I noticed a strange code at liblwgeom/lwalgorithm.c
> > >
> https://github.com/postgis/postgis/blob/master/liblwgeom/lwalgorithm.c#L509
> > >
> > > lwline_crossing_direction() is the main logic of
> ST_LineCrossingDirection.
> > > It walks on all the edges of the supplied geometry (line), and calls
> > > lw_segment_intersects to check intersections.
> > >
> > > It looks to me the local variable 'first_cross' is used to remember
> the first
> > > result from the lw_segment_intersects, however, it saves different
> value
> > > on the second if-block at L526.
> > >
> > > Is it expected? Or, just mistake by copy & paste?
> > >
> > > Best regards,
> > > --
> > > HeteroDB, Inc / The PG-Strom Project
> > > KaiGai Kohei <kaigai@heterodb.com>
> > > _______________________________________________
> > > postgis-users mailing list
> > > postgis-users@lists.osgeo.org
> > > https://lists.osgeo.org/mailman/listinfo/postgis-users
> >
> >
> >
> > --
> > Darafei Praliaskouski
> > Support me: http://patreon.com/komzpa
> > _______________________________________________
> > postgis-users mailing list
> > postgis-users@lists.osgeo.org
> > https://lists.osgeo.org/mailman/listinfo/postgis-users
>
>
>
> --
> HeteroDB, Inc / The PG-Strom Project
> KaiGai Kohei <kaigai@heterodb.com>
> _______________________________________________
> postgis-users mailing list
> postgis-users@lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/postgis-users
--
Darafei Praliaskouski
Support me: http://patreon.com/komzpa
[Attachment #5 (text/html)]
<div dir="ltr">The code was born in 2009 by Paul Ramsey.<br><a \
href="https://github.com/postgis/postgis/commit/6adec1b2be11e7d0fe5c482e50ce0872c400cf \
d6#diff-5705dd09236e9a6e8fa456b7b9599ed7R205-R211">https://github.com/postgis/postgis/ \
commit/6adec1b2be11e7d0fe5c482e50ce0872c400cfd6#diff-5705dd09236e9a6e8fa456b7b9599ed7R205-R211</a> \
<div><br></div><div>Either a comment is needed on this, or a patch. Easiest way to \
check a hypothesis that this may be a bug is to change L526 (github - Edit - change - \
pull request) and see that CI says that everything failed (then it needs docs) or \
that everything passed (then a trac ticket and a fix, which would be a test that pins \
the correct behavior of that branch).</div></div><br><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 20, 2020 at 7:42 AM \
Kohei KaiGai <<a href="mailto:kaigai@heterodb.com">kaigai@heterodb.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Darafei,<br> \
<br> It looks to me the first_cross should be SEG_CROSS_RIGHT, not<br>
SEG_CROSS_LEFT here.<br>
If it is expected, L574 shall never become true. Right?<br>
<br>
> at L526:<br>
> if ( this_cross == SEG_CROSS_RIGHT )<br>
> {<br>
> LWDEBUG(4,"this_cross == SEG_CROSS_RIGHT");<br>
> cross_right++;<br>
> if ( ! first_cross )<br>
> first_cross = SEG_CROSS_LEFT;<br>
> }<br>
<br>
Thanks,<br>
<br>
2020年4月19日(日) 23:31 Darafei "Komяpa" Praliaskouski <<a \
href="mailto:me@komzpa.net" target="_blank">me@komzpa.net</a>>:<br> ><br>
> Hi,<br>
><br>
> the link you have is setting variable this_cross, and L526 sets<br>
> first_cross from it if it was not set and there is LEFT or RIGHT cross<br>
> (not no-cross and not COLLINEAR). Looks expected to me.<br>
><br>
> On Sat, Apr 18, 2020 at 6:32 PM Kohei KaiGai <<a \
href="mailto:kaigai@heterodb.com" target="_blank">kaigai@heterodb.com</a>> \
wrote:<br> > ><br>
> > Hello,<br>
> ><br>
> > I noticed a strange code at liblwgeom/lwalgorithm.c<br>
> > <a href="https://github.com/postgis/postgis/blob/master/liblwgeom/lwalgorithm.c#L509" \
rel="noreferrer" target="_blank">https://github.com/postgis/postgis/blob/master/liblwgeom/lwalgorithm.c#L509</a><br>
> ><br>
> > lwline_crossing_direction() is the main logic of \
ST_LineCrossingDirection.<br> > > It walks on all the edges of the supplied \
geometry (line), and calls<br> > > lw_segment_intersects to check \
intersections.<br> > ><br>
> > It looks to me the local variable 'first_cross' is used to remember \
the first<br> > > result from the lw_segment_intersects, however, it saves \
different value<br> > > on the second if-block at L526.<br>
> ><br>
> > Is it expected? Or, just mistake by copy & paste?<br>
> ><br>
> > Best regards,<br>
> > --<br>
> > HeteroDB, Inc / The PG-Strom Project<br>
> > KaiGai Kohei <<a href="mailto:kaigai@heterodb.com" \
target="_blank">kaigai@heterodb.com</a>><br> > > \
_______________________________________________<br> > > postgis-users mailing \
list<br> > > <a href="mailto:postgis-users@lists.osgeo.org" \
target="_blank">postgis-users@lists.osgeo.org</a><br> > > <a \
href="https://lists.osgeo.org/mailman/listinfo/postgis-users" rel="noreferrer" \
target="_blank">https://lists.osgeo.org/mailman/listinfo/postgis-users</a><br> \
><br> ><br>
><br>
> --<br>
> Darafei Praliaskouski<br>
> Support me: <a href="http://patreon.com/komzpa" rel="noreferrer" \
target="_blank">http://patreon.com/komzpa</a><br> > \
_______________________________________________<br> > postgis-users mailing \
list<br> > <a href="mailto:postgis-users@lists.osgeo.org" \
target="_blank">postgis-users@lists.osgeo.org</a><br> > <a \
href="https://lists.osgeo.org/mailman/listinfo/postgis-users" rel="noreferrer" \
target="_blank">https://lists.osgeo.org/mailman/listinfo/postgis-users</a><br> <br>
<br>
<br>
-- <br>
HeteroDB, Inc / The PG-Strom Project<br>
KaiGai Kohei <<a href="mailto:kaigai@heterodb.com" \
target="_blank">kaigai@heterodb.com</a>><br> \
_______________________________________________<br> postgis-users mailing list<br>
<a href="mailto:postgis-users@lists.osgeo.org" \
target="_blank">postgis-users@lists.osgeo.org</a><br> <a \
href="https://lists.osgeo.org/mailman/listinfo/postgis-users" rel="noreferrer" \
target="_blank">https://lists.osgeo.org/mailman/listinfo/postgis-users</a></blockquote></div><br \
clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div \
dir="ltr"><div><div>Darafei Praliaskouski</div><div>Support me: <a \
href="http://patreon.com/komzpa" \
target="_blank">http://patreon.com/komzpa</a></div></div></div></div>
[Attachment #6 (text/plain)]
_______________________________________________
postgis-users mailing list
postgis-users@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/postgis-users
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic