Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#2755 closed defect (fixed)

4 regression failures for sfcgal

Reported by: robe Owned by: colivier
Priority: blocker Milestone: PostGIS 2.1.4
Component: sfcgal Version: trunk
Keywords: Cc:

Description

Running sfcgal on my 9.4 beta1 windows 32-bit, I see 4 errors:

regress_sfcgal .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_95_diff)

sfcgal/empty .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_96_diff)
                                                         sfcgal/regress_ogc .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_101_diff)

sfcgal/tickets .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_103_diff)

my regress output is also a bit of a mess on screen, but I thin that is probably a different issue.

I've attached the related files.

Attachments (3)

sfcgal_failures_2.2_pg9.4w32.zip (7.5 KB) - added by robe 5 years ago.
2755.patch (17.8 KB) - added by vmo 5 years ago.
2755_1.patch (14.8 KB) - added by vmo 5 years ago.

Download all attachments as: .zip

Change History (19)

Changed 5 years ago by robe

comment:1 Changed 5 years ago by robe

At a quick glance (regress_sfcgal): I honestly can't see a difference so maybe its white space.

--- regress_sfcgal_expected	2014-06-02 18:12:37 -0400
+++ /projects/postgis/tmp/2.2_pg9.4w32/test_95_out	2014-06-05 00:39:04 -0400
@@ -3,7 +3,7 @@
 ST_3DArea|1
 ST_Extrude_point|LINESTRING Z (0 0 0,1 0 0)
 ST_Extrude_line|POLYHEDRALSURFACE Z (((0 0 0,1 0 0,1 1 0,0 1 0,0 0 0)))
-ST_Extrude_surface|POLYHEDRALSURFACE Z (((1 1 0,1 0 0,0 1 0,1 1 0)),((0 1 1,1 0 1,1 1 1,0 1 1)),((1 0 0,0 0 0,0 1 0,1 0 0)),((0 1 1,0 0 1,1 0 1,0 1 1)),((1 0 0,1 1 0,1 1 1,1 0 1,1 0 0)),((1 1 0,0 1 0,0 1 1,1 1 1,1 1 0)),((0 1 0,0 0 0,0 0 1,0 1 1,0 1 0)),((0 0 0,1 0 0,1 0 1,0 0 1,0 0 0)))
+ST_Extrude_surface|POLYHEDRALSURFACE Z (((0 1 0,1 1 0,1 0 0,0 1 0)),((0 1 1,1 0 1,1 1 1,0 1 1)),((0 1 0,1 0 0,0 0 0,0 1 0)),((0 1 1,0 0 1,1 0 1,0 1 1)),((1 0 0,1 1 0,1 1 1,1 0 1,1 0 0)),((1 1 0,0 1 0,0 1 1,1 1 1,1 1 0)),((0 1 0,0 0 0,0 0 1,0 1 1,0 1 0)),((0 0 0,1 0 0,1 0 1,0 0 1,0 0 0)))
 ST_ForceLHR|POLYGON((0 0,1 0,1 1,0 1,0 0))
 ST_Orientation_1|-1
 ST_Orientation_2|1

other something weird with I guess how empty is compared. Might only be a windows issue:

--- sfcgal/empty_expected	2014-06-02 18:12:34 -0400
+++ /projects/postgis/tmp/2.2_pg9.4w32/test_96_out	2014-06-05 00:39:04 -0400
@@ -26,11 +26,11 @@
 ST_Buffer(empty, tolerance) == empty|010300000000000000
 ST_Union(geometry, empty) == geometry|0103000000010000000400000000000000000000000000000000000000000000000000244000000000000000000000000000001440000000000000144000000000000000000000000000000000
 ST_Union(empty, empty) == empty|010300000000000000
-ST_Intersection(geometry, empty) == geometry|010700000000000000
-ST_Intersection(empty, empty) == empty|010700000000000000
+ST_Intersection(geometry, empty) == geometry|010300000000000000
+ST_Intersection(empty, empty) == empty|010300000000000000
 ST_Difference(geometry, empty) == geometry|0103000000010000000400000000000000000000000000000000000000000000000000244000000000000000000000000000001440000000000000144000000000000000000000000000000000
 ST_Difference(empty, geometry) == empty|010300000000000000
-ST_Distance(geometry, empty) == NULL|inf
+ST_Distance(geometry, empty) == NULL|
 ST_DWithin(geometry, empty, tolerance) == FALSE|f
 ST_Within(geometry, empty) == FALSE|f
 ST_Contains(empty, geometry) == FALSE|f

-- regress_ogc_expected - whacked -- there is such a thing as a -0 ? I think my result should be right :)

--- sfcgal/regress_ogc_expected	2014-06-02 18:12:34 -0400
+++ /projects/postgis/tmp/2.2_pg9.4w32/test_101_out	2014-06-05 00:39:05 -0400
@@ -62,7 +62,7 @@
 NOTICE:  Self-intersection
 isvalid|f
 isvalid|t
-intersection|POINT(-0 -0)
+intersection|POINT(0 0)
 difference|MULTILINESTRING((0 10,0 2),(0 -2,0 -10))
 boundary|MULTILINESTRING((0 0,0 10,10 10,10 0,0 0),(2 2,2 4,4 4,4 2,2 2))
 symdifference|GEOMETRYCOLLECTION(LINESTRING(2 2,4 4),LINESTRING(10 10,20 20),POLYGON((0 0,0 10,10 10,10 0,0 0),(4 4,2 4,2 2,4 2,4 4)))

sfcgal_tickets -- don't even know what to make of this:

--- sfcgal/tickets_expected	2014-06-02 18:12:34 -0400
+++ /projects/postgis/tmp/2.2_pg9.4w32/test_103_out	2014-06-05 00:39:06 -0400
@@ -15,7 +15,7 @@
 #73|GEOMETRYCOLLECTION(CIRCULARSTRING(1 1,2 3,4 5,6 7,5 6))
 #80|MULTILINESTRING((0 0,1 1))
 #83|MULTICURVE(CIRCULARSTRING(220268 150415,220227 150505,220227 150406))
-ERROR:  LWGEOM2SFCGAL: Unknown geometry type !
+#85|0
 #112|GEOMETRYCOLLECTION(POINT(-10 50))
 NOTICE:  ST_Locate_Between_Measures and ST_Locate_Along_Measure are deprecated. Use ST_LocateAlong and ST_LocateBetween.
 ERROR:  Geometry argument does not have an 'M' ordinate
@@ -40,7 +40,7 @@
 #179a|
 NOTICE:  No points or linestrings in input array
 #179b|
-#183|CIRCULARSTRING(0 0,0.5 1.2071067812,0 1)
+#183|COMPOUNDCURVE(CIRCULARSTRING(0 0,0.5 1.2071067812,1 0),(1 0,0 1))
 #210a|
 NOTICE:  No points or linestrings in input array
 #210b|
@@ -95,7 +95,7 @@
 #835.11|MULTILINESTRING EMPTY
 #835.12|MULTIPOLYGON EMPTY
 #650|MULTIPOINT(0 0,1 1,2 2)
-#667|SRID=4326;CURVEPOLYGON(CIRCULARSTRING(30 40,-49.2314112161292 32.1963871193548,30 40))
+#667|SRID=4326;CURVEPOLYGON(CIRCULARSTRING(30 40,-50 39.9999999999999,30 40))
 #677|1121395
 #680|01d107000000000000000024c000000000000049400000000000000040
 #681a|
@@ -129,7 +129,7 @@
 #723|0101000020E61000006284F068E33826C00100000000804B40
 #804|<gml:Point srsName="urn:ogc:def:crs:EPSG::4326"><gml:pos srsDimension="2">0 0</gml:pos></gml:Point>
 #845|t
-#834|GEOMETRYCOLLECTION(POINT(0 0),LINESTRING(10 0,10 10))
+#834|GEOMETRYCOLLECTION(POINT(0 0 5),LINESTRING(10 10 5,10 0 5))
 #884|1|f
 #884|2|t
 #938|
@@ -225,8 +225,8 @@
 #1791|4.7
 ERROR:  ST_Segmentize: invalid max_distance 0 (must be >= 0)
 ERROR:  invalid GML representation
-#1957|inf
-#1978|3.1413
+#1957|1
+#1978|3.1416
 #1996|{"type":"Point","coordinates":[]}
 #2001|POLYGON((0 0,0 1,1 1,0 0))
 #2028|TIN(((0 0,0 1,1 1,0 0)))

Anyway if someone else can conrim that these are true errors or bugs in our regress tests itself, that would be appreciated.

I compiled with

select postgis_full_version() || ' ' || version();

POSTGIS="2.2.0dev r12605" GEOS="3.5.0dev-CAPI-1.9.0 r3985" SFCGAL="1.0.4" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.11.0, released 2014/04/16 GDAL_DATA not found" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER

I built SFCGAL using CGAL 4.2, Boost 1.53.0, mpfr 3.1.2, gmp 5.1.2

comment:2 Changed 5 years ago by robe

actually I think the tickets issue might be because of our changes in curve support so maybe the sfcgal_ticket script just needs some updating

comment:3 Changed 5 years ago by vmo

Resolution: fixed
Status: newclosed

comment:4 Changed 5 years ago by vmo

Resolution: fixed
Status: closedreopened

Changed 5 years ago by vmo

Attachment: 2755.patch added

comment:5 Changed 5 years ago by vmo

Part of the problem came from r12350, which is reverted in the patch.

The rest is from sfcgal regression tests that have not been run before.

This patch has been tested against sfcgal 1.0, 1.0.4 and master. Some tests are expected to fail with master due to bugs that will be fixed in 1.0.5.

comment:6 Changed 5 years ago by pramsey

You can't just toss r12350, it's necessary to support extension upgrade, because during upgrade two libpostgis.so libraries are simultaneously loaded and they will collide on the GUC variable and upgrade will halt unless you check and avoid double-defining it.

comment:7 in reply to:  6 Changed 5 years ago by vmo

Replying to pramsey:

You can't just toss r12350, it's necessary to support extension upgrade, because during upgrade two libpostgis.so libraries are simultaneously loaded and they will collide on the GUC variable and upgrade will halt unless you check and avoid double-defining it.

I'm working on it, but as far as I can tell, the proposed solution does not seem to work.

When changing postgis.backend, maybe because the custom variable is already defined as a place holder, r13250 causes the assign hook definition to be bypassed. As a result geos function are called instead of sfcgal ones (some tests failures, like ST_Distance(geom, empty) show that).

I asked on pg-hackers (http://postgresql.1045698.n5.nabble.com/modify-custom-variables-td5808161.html) and I was advised to avoid loading the old postgis.so before upgrade.

I tend to agree: if you skip the definition of the custom variable, the assign hook will still point to the old version of the library, and we will end-up calling sfcgal backend from the previous version of postgis*.so

comment:8 Changed 5 years ago by strk

I confirm reverting r12350 works fine here after r12641. I've committed the revert as r12642, looking at the rest of the patch next

comment:9 Changed 5 years ago by strk

vmo: I still get regression failures in tickets.sql after applying your patch:

--- sfcgal/tickets_expected     2014-06-24 18:03:47.473976272 +0200
+++ /tmp/pgis_reg/test_103_out  2014-06-24 18:04:32.198439334 +0200
@@ -95,7 +95,7 @@
 #835.11|MULTILINESTRING EMPTY
 #835.12|MULTIPOLYGON EMPTY
 #650|MULTIPOINT(0 0,1 1,2 2)
-#667|SRID=4326;CURVEPOLYGON(CIRCULARSTRING(30 40,-49.2314112161292 32.1963871193548,30 40))
+#667|SRID=4326;CURVEPOLYGON(CIRCULARSTRING(30 40,-50 39.9999999999999,30 40))
 #677|1121395
 #680|01d107000000000000000024c000000000000049400000000000000040
 #681a|
@@ -201,7 +201,7 @@
 #1398a|POINT(-119.093153 45.632669)
 #1398b|POINT(-160.137654 77.091608)
 #1543|MULTILINESTRING((0 0,10 0,10 10,0 0),(0 0))|POLYGON((0 0,10 10,10 0,0 0))
-#1578|f|f
+#1578|t|t
 #1580.1|Point[BS]
 ERROR:  transform: couldn't project point (180 90 0): tolerance condition error (-20)
 #1580.3|Point[BS]
@@ -225,7 +225,7 @@
 #1791|4.7
 ERROR:  ST_Segmentize: invalid max_distance 0 (must be >= 0)
 ERROR:  invalid GML representation
-#1957|inf
+#1957|not tested, error otherwise because both case staetements are evaluated
 #1978|3.1416
 #1996|{"type":"Point","coordinates":[]}
 #2001|POLYGON((0 0,0 1,1 1,0 0))

This is with SFCGAL from master branch (1.0.5) and libcgal 4.2-4ubuntu1 (packaged). This is on a 64bit machine.

comment:10 Changed 5 years ago by vmo

That's expected with sfcgal master. It was meant to force fix befrore 1.0.5 is released. I made some fixes in sfcgal, I'll fix the regress test tomorrow.

Changed 5 years ago by vmo

Attachment: 2755_1.patch added

comment:11 Changed 5 years ago by vmo

The patch fixes regress tests for all versions of SFCGAL.

comment:12 Changed 5 years ago by strk

Committed as r12654 -- should it be backported to 2.1 and 2.0 branches ?

comment:13 in reply to:  12 Changed 5 years ago by vmo

Replying to strk:

Committed as r12654 -- should it be backported to 2.1 and 2.0 branches ?

I think so.

comment:14 Changed 5 years ago by strk

r12655 is the backport into 2.1 branch, will leave the 2.0 backport to yourself Vincent, ok ?

comment:15 Changed 5 years ago by strk

Resolution: fixed
Status: reopenedclosed

Actually, SFCGAL support was added in 2.1.0 so all done here.

comment:16 Changed 5 years ago by robe

Milestone: PostGIS 2.2.0PostGIS 2.1.4
Note: See TracTickets for help on using tickets.