Ticket #413 (closed enhancement: fixed)

Opened 5 years ago

Last modified 3 years ago

ST_OffsetCurve

Reported by: rafalmag Owned by: strk
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: trunk
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

postgis-1.5.0-RafalMagda.patch Download (5.2 KB) - added by rafalmag 5 years ago.
Usage: tar xvfz postgis-1.5.0.tar.gz && patch -p0 < postgis-1.5.0-RafalMagda?.patch
postgis-offsetCurve-RafalMagda.patch Download (6.4 KB) - added by rafalmag 5 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 Download (21.5 KB) - added by sholl 3 years ago.
Stacktrace for comment28 of a segfaulting postgres-backend.

Change History

Changed 5 years ago by rafalmag

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

  Changed 5 years ago by strk

"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 :(

  Changed 5 years ago by pramsey

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

  Changed 5 years ago by strk

  • summary changed from ST_Single_Sided_Buffer added to ST_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 ?

  Changed 5 years ago by rafalmag

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?

  Changed 5 years ago by strk

  • owner changed from pramsey to strk
  • status changed from new to assigned

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).

  Changed 5 years ago by pramsey

  • milestone set to PostGIS 2.0.0

Changed 5 years ago by rafalmag

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

in reply to: ↑ description   Changed 5 years ago by rafalmag

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.

  Changed 5 years ago by strk

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.

  Changed 5 years ago by rafalmag

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 ?

  Changed 5 years ago by strk

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

  Changed 5 years ago by rafalmag

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?

  Changed 5 years ago by strk

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.

  Changed 4 years ago by sholl

  • cc sholl added

  Changed 4 years ago by bj

  • 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?

  Changed 4 years ago by strk

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

  Changed 4 years ago by strk

Patch applies to trunk, still pending: - Agreement on signature (ST_OffsetCurveLeft/ST_OffsetCurveRight vs. string parameter) - Documentation - Testcase

  Changed 4 years ago by sholl

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

  Changed 4 years ago by sholl

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

follow-up: ↓ 20   Changed 4 years ago by 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

in reply to: ↑ 19   Changed 4 years ago by sholl

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

  Changed 4 years ago by robe

  • version changed from 1.5.X to trunk

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)

  Changed 4 years ago by strk

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

  Changed 4 years ago by sholl

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

  Changed 4 years ago by robe

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

  Changed 4 years ago by strk

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)

  Changed 3 years ago by strk

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.

  Changed 3 years ago by sholl

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

  Changed 3 years ago by strk

No clue w/out a backtrace.

  Changed 3 years ago by sholl

strk,

I will create a backtrace and post ASAP.

TIA.

Changed 3 years ago by sholl

Stacktrace for comment28 of a segfaulting postgres-backend.

  Changed 3 years ago by sholl

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

  Changed 3 years ago by strk

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

  Changed 3 years ago by strk

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.

  Changed 3 years ago by strk

  • keywords history added
  • status changed from assigned to closed
  • resolution set to fixed

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.