Opened 14 years ago

Closed 13 years ago

#413 closed enhancement (fixed)

ST_OffsetCurve

Reported by: rafalmag Owned by: strk
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: history Cc: sholl, bj

Description

I needed some new functionality and found topic: http://postgis.refractions.net/pipermail/postgis-users/2009-June/023717.html

In that topic there was link to: http://trac.osgeo.org/geos/ticket/215

It was exactly what I was looking for so I added this to PostGIS.

Example of use:

select st_astext(ST_Single_Sided_Buffer(
ST_GeomFromText('LINESTRING(10 10,10 20, 20 20 )'),
1,'join=mitre mitre_limit=5.0',0));
--------------
LINESTRING(20 19,11 19,11 10)

select st_astext(ST_Single_Sided_Buffer(
ST_GeomFromText('LINESTRING(10 10,10 20, 20 20 )'),
1,'join=mitre mitre_limit=5.0',1));
--------------
LINESTRING(9 10,9 21,20 21)

Attachments (3)

postgis-1.5.0-RafalMagda.patch (5.2 KB ) - added by rafalmag 14 years ago.
Usage: tar xvfz postgis-1.5.0.tar.gz && patch -p0 < postgis-1.5.0-RafalMagda.patch
postgis-offsetCurve-RafalMagda.patch (6.4 KB ) - added by rafalmag 14 years ago.
usage: svn co http://svn.osgeo.org/postgis/trunk/ -r 5389 && patch -p0 < postgis-offsetCurve-RafalMagda.patch
postgres_geos_OffsetCurve_stacktrace-20110510.txt (21.5 KB ) - added by sholl 13 years ago.
Stacktrace for comment28 of a segfaulting postgres-backend.

Download all attachments as: .zip

Change History (36)

by rafalmag, 14 years ago

Usage: tar xvfz postgis-1.5.0.tar.gz && patch -p0 < postgis-1.5.0-RafalMagda.patch

comment:1 by strk, 14 years ago

"SingleSidedBuffer" isn't really a good name for this function. We found a good one being "OffsetCurve" but eventually didn't handle to do the rename. (I would expect an area returned by a "SingleSidedBuffer").

That said, it seems a bit late to rename, at least for the GEOS side of things :(

comment:2 by pramsey, 14 years ago

ST_OffsetCurve() sounds great. BTW, the standard format is now ST_CamelCaseYourFunction()

comment:3 by strk, 14 years ago

Summary: ST_Single_Sided_Buffer addedST_OffsetCurve

Also, I'd move the buffer parameters string as third and optional argument. Rafal: do you agree ? Feel like making these modifications and ensure the patch applies to trunk ?

comment:4 by rafalmag, 14 years ago

strk: It should stay similar to ST_Buffer, so yes "buffer parameters string" should be an optional, last option.

After a while I think that "0" and "1" could be changed from integer to boolean (or added as another option/use case). What do you think?

There are still TODO: what to do if user do not have GEOS in 3.2.0 version (I saw how to do in ST_Buffer)? Should it compile as "null returning" function ?

I created this patch for PostGIS 1.5.0, but I think it should merge into trunk version. Should I create a final patch and post it here? Can I commit something into trunk?

comment:5 by strk, 14 years ago

Owner: changed from pramsey to strk
Status: newassigned

If GEOS version isn't new enough the function should exit with an exception [ elog(ERROR, "message") ] Please make the message useful by specifying which version of GEOS is required and which postgis was built against.

Please attach a patch which applies cleanly to trunk.

Oh, another thing. I guess the third argument is left or right, what about making that part of the ascii-parameter argument (arg 2) with label 'side' ? side=left or side=right. May come handy whenever someone will implement true single-sided buffer (area-returning).

comment:6 by pramsey, 14 years ago

Milestone: PostGIS 2.0.0

by rafalmag, 14 years ago

usage: svn co http://svn.osgeo.org/postgis/trunk/ -r 5389 && patch -p0 < postgis-offsetCurve-RafalMagda.patch

in reply to:  description comment:7 by rafalmag, 14 years ago

Ok. I produced a new patch (postgis-offsetCurve-RafalMagda.patch). It applies to latest trunk.

arguments:

  • geom
  • distance (float)
  • 'right' or 'left'
  • (optional) parameters 'join=' and 'mitre_limit=' (like in St_Buffer (without endcap) )

Example of use:

select ST_AsText(ST_OffsetCurve(
ST_GeomFromText('LINESTRING(10 10,10 20, 20 20 )'),
1,'right', 'join=mitre mitre_limit=5.0'));
--------------
LINESTRING(20 19,11 19,11 10)

select ST_AsText(ST_OffsetCurve(
ST_GeomFromText('LINESTRING(10 10,10 20, 20 20 )'),
1,'left', 'join=mitre mitre_limit=5.0'));
--------------
LINESTRING(9 10,9 21,20 21)

PS: Tested build with GEOS 3.2.0. Untested build with GEOS 3.1 - see code.

comment:8 by strk, 14 years ago

Almost there ! I'd merge 3rd and 4th arguments into a single one: "parameters" Examples:

select ST_AsText(ST_OffsetCurve(
ST_GeomFromText('LINESTRING(10 10,10 20, 20 20 )'),
1,'side=right join=mitre mitre_limit=5.0'));
--------------
LINESTRING(20 19,11 19,11 10)

select ST_AsText(ST_OffsetCurve(
ST_GeomFromText('LINESTRING(10 10,10 20, 20 20 )'),
1,'side=left join=mitre mitre_limit=5.0'));
--------------
LINESTRING(9 10,9 21,20 21)

But it'd be also nice to hear from others about that.

comment:9 by rafalmag, 14 years ago

I thought about that, but I think that side parameter is so crucial, that it should be separated from optional join related parameters. Maybe better way would be include side in function name, e.g.: ST_OffsetCurveLeft(), ST_OffsetCurveRight() or ST_OffsetLeftCurve(), ST_OffsetRightCurve() (but in alphabetic order these names would separate their positions…).

What do you think ?

comment:10 by strk, 14 years ago

By "crucial" you mean there's no good default for it ? Do you see a use for a "both" value ?

comment:11 by rafalmag, 14 years ago

Exactly. I can not imagine a situation when somebody want to get both lines and have no need to know, which line is left/right.

BTW: In my project use case I also need to preserve direction of line. So I have to reverse 'right' line from the example. Is only 'right' line always reversed ? Or is it random? Maybe in PostGIS version it should always preserve direction?

comment:12 by strk, 14 years ago

About directionality I guess it'd be worth making it consisten from GEOS itself (to respect original line direction). Currently it's unspecified, but would likely always be consistent ('right' reversed).

I'm fine with your ST_OffsetCurveLeft/ST_OffsetCurveRight. Let's just give a couple of days for others to express their opinion.

comment:13 by sholl, 14 years ago

Cc: sholl added

comment:14 by bj, 14 years ago

Cc: bj added

Hej folks, i'm wondering what is the status of this patch? We are interested in getting this patch upstream. Is there any progress? Will it land in the next upstream release of postgis? I didn't found any hints of the progress neither in the postgis mailing-lists nor in the svn trunk. Does a show stopper exist? What will be the next steps to get this patch in the next upstream release?

comment:15 by strk, 14 years ago

Please also add a testcase for the regress/ dir, and make sure your patch applies to current trunk.

comment:16 by strk, 14 years ago

Patch applies to trunk, still pending:

  • Agreement on signature (ST_OffsetCurveLeft/ST_OffsetCurveRight vs. string parameter)
  • Documentation
  • Testcase

comment:17 by sholl, 14 years ago

nice to see the activity in here. If there is any (non-programming) tasks in here I am happy to provide some help to make this patch into trunk (and in a later release!)

TIA

Stephan

comment:18 by sholl, 13 years ago

Ping, is there anything I can help to push this function into trunk to get it in PostGIS?

We find this function really usefull for displaying colored bands around line-features though.

Any comment from a dev would be helpful gain momentum here.

Best

Stephan

comment:19 by strk, 13 years ago

See two comments up: documentation + testcase missing. Given all this time elapsed, chances are the code will also need to be reviewed for applicability.

NOTE: there's a mirror on github now, if you'd find it more useful: https://github.com/strk/postgis

in reply to:  19 comment:20 by sholl, 13 years ago

Replying to strk:

See two comments up: documentation + testcase missing. Given all this time elapsed, chances are the code will also need to be reviewed for applicability.

NOTE: there's a mirror on github now, if you'd find it more useful: https://github.com/strk/postgis

OK, good to know.

At least I can document things for this function, but I am unsure which format will suffice. Are there some How-to-document-correct-guidelines somewhere available?

Perhaps the original author of the (very useful) patch is able to update the patch and provide a testcase? Rafal?

Thanks.

Stephan

comment:21 by robe, 13 years ago

Version: 1.5.Xtrunk

Stephan,

Please refer to our wiki section on Patch Submission.

http://trac.osgeo.org/postgis/wiki/DevWikiMain

Let us know if that doesn't answer all your questions.

Thanks, Regina (robe)

comment:22 by strk, 13 years ago

mmm.. wiki page seems to be missing the regress/ (postgresql-based) testing.

comment:23 by sholl, 13 years ago

Regina,

excellent, answers all questions except the test-stuff (as strk mentioned). I will wait with the docs till the patch is appyling to trunk again, is final and write the docs accordingly then. Hopefully the original author is still somewhere arround for doing that.

Best

Stephan

comment:24 by robe, 13 years ago

strk,

I added a PostgreSQL regress section. Can you check it and make sure I didn't make any misstatements.

http://trac.osgeo.org/postgis/wiki/DevWikiPGRegress

comment:25 by strk, 13 years ago

The production of expected results might be not 100% correct. To be strong what I do is touch <thetest>_expected and make check, then copy the <whatever>_out file left by make check near <whatever>_diff on failure.

Still not very friendly (might try to do a 'make expected' rule one day)

comment:26 by strk, 13 years ago

FYI: upcoming GEOS-3.3.0 will have offset curve generation refactored. Part of this is due to JTS introducing proper single sided buffers (areal). The C-API function for offset curves will be named GEOSOffsetCurve deprecating GEOSBufferSingleSided (to avoid confusion with areal single sided buffers).

An interesting interface found in JTS (for the right/left discourse) was to use the sign of buffer distance (negative distance == right side). Such interface might be found in the new GEOSOffsetCurve too.

comment:27 by sholl, 13 years ago

strk,

thanks for your information about the upcoming functions in GEOS3.3.0. I would like to see a PostGIS-Function which makes use of the GEOS-functions within PostGIS.

Anyway, having GEOS 3.2.0 and this patch segfaults PostgreSQL badly. There is no debug-messages given, but the following SQL-Code reproduces the problem with a patched PostGIS 1.5.0

CREATE TABLE foo (id serial, the_geom geometry); 
INSERT INTO foo (the_geom) VALUES(st_geometryfromtext('LINESTRING(33282908.7621248 6005055.4430171,33282900.0339 6005050.3959,33282892.9684 6005042.9596,33282876.3992 6005007.904,33282863.9699 6004982.5979,33282866.676 6004971.788,33282876.9948 6004975.9552,33282967.0854 6005018.4214,33282999.3340629 6005031.56008227)') ); 
INSERT INTO foo (the_geom) VALUES(st_geometryfromtext('LINESTRING(33283146.1378102 6005228.92500223,33283034.0626 6005132.1377,33282927.0484 6005066.0171,33282908.7621248 6005055.4430171)') );
-- works:
SELECT id, st_astext(ST_OffsetCurve(the_geom,42,'left', 'join=round')) as the_geom FROM foo;

-- segfaults:
SELECT id, st_astext(ST_OffsetCurve(the_geom,44,'left', 'join=round')) as the_geom FROM foo;

It seems that the resulting geometry is not valid though.

Is this a known issue? strk, do you have a clue?

TIA

Stephan

comment:28 by strk, 13 years ago

No clue w/out a backtrace.

comment:29 by sholl, 13 years ago

strk,

I will create a backtrace and post ASAP.

TIA.

by sholl, 13 years ago

Stacktrace for comment28 of a segfaulting postgres-backend.

comment:30 by sholl, 13 years ago

strk,

I have added the backtrace (postgres_geos_OffsetCurve_stacktrace-20110510.txt) with debugging-symbols of GEOS 3.2.0, PostGIS1.5.0 with above patch and Postgresql 8.3.11 produced with the given sample-data from comment#27.

Perhaps you have an idea how to solve this issue. I think this is more a GEOS-bug than a postgis-bug though.

Thanks

Stephan

comment:31 by strk, 13 years ago

Looks like GEOS is just returning NULL from that OffsetCurve. And the PostGIS side isn't properly handling that.

comment:32 by strk, 13 years ago

I've tweaked the GEOS part to return LINESTRING EMPTY rather than NULL when there's a collapse. This should give you something somewhat more acceptable than a segfaut.

In any case the patch should be rewritten to encode side in the width parameter (positive for left, negative for right) and to use the GEOSOffsetCurve API.

comment:33 by strk, 13 years ago

Keywords: history added
Resolution: fixed
Status: assignedclosed

r7534 contains the new ST_OffsetCurve signature (single function with optional last argument), documentation and regress testing.

It works with GEOS-3.2 too but it's known to be bogus with that version, see http://trac.osgeo.org/geos/ticket/455#comment:1

Note: See TracTickets for help on using tickets.