GeoTools

PerpendicularOffset for LineSymbolizer unsupported; Bounty for render support

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Incomplete
  • Affects Version/s: 2.7.4, 8.0-M2, 8.0-M3
  • Fix Version/s: 8.8
  • Component/s: main, render
  • Labels:
    None
  • Testcase included:
    yes

Description

SE 1.1 describes the possibility to set a PerpendicularOffset for a LineSymbolizer, just like SLD 1.0 does for the LinePlacement. I know that GeoTools is not a reference implementation for SE 1.1, but I know about AtlasStyler users that desire such a function.

In trunk the LineSymbolizer class has a setter and a getter for PerpendicularOffset, but the settings is lost when transforming the Style to and from XML, as well it is lost when the Style is duplicated with the DuplicatingStyleVisitor.

Attached are two patches:
1. Two unit tests showing that LineSymbolizer.set/getPerpendicular settings are lost when transforming/parsing/duplicating the Style.
2. A patch fixing the three cases above

The patches are created again '/trunk/modules/library/main'

What's missing is the implementation in the renderer, as shortly discussed here:
http://www.mail-archive.com/geotools-gt2-users@lists.sourceforge.net/msg13087.html

My symbolic offer for funding the renderer-implementation: I sponser this implementation with 200€ via paypal. I will send the money to any email chosen by the one that implements this in the renderer; or I can donate it to any organization of choice. I will not need any bills, as I fund this privately. In case anybody is willing, please tell me before you start via email

I would be very happy if I could commit the above patches and anybody would take the bounty.

Steve

  1. Text File
    perpendicularOffsetImplementation.patch
    14/Oct/11 8:00 AM
    3 kB
    Stefan Alfons Tzeggai
  2. Text File
    perpendicularOffsetTests.patch
    14/Oct/11 8:00 AM
    3 kB
    Stefan Alfons Tzeggai
  3. File
    perpoff.diff
    28/Jun/12 7:20 AM
    6 kB
    Björn Harrtell
  4. File
    perpoffTest.diff
    30/Jun/12 5:58 AM
    5 kB
    Björn Harrtell
  1. narrowbug.png
    2 kB
    02/Jul/12 2:28 AM
  2. perpoff.png
    1 kB
    02/Jul/12 2:28 AM
  3. perpoff.png
    24 kB
    28/Jun/12 7:20 AM

Activity

Ascending order - Click to sort in descending order
Andrea Aime made changes -
Field Original Value New Value
Assignee Jody Garnett [ jgarnett ] Andrea Aime [ aaime ]
Hide
Permalink
Andrea Aime added a comment -
Hi Stefan, I'll try to have a look at the patches this weekend. As for the line offset algorithm, I already thrown more than 2 days on it without getting anywhere (one year ago). It's really really non trivial. MapServer and GRASS both have implementations that do something, but they don't work if the line has sharp angles or silly small loops (easy to get silly small when you zoom out on any line unfortunately).
Show
Andrea Aime added a comment - Hi Stefan, I'll try to have a look at the patches this weekend. As for the line offset algorithm, I already thrown more than 2 days on it without getting anywhere (one year ago). It's really really non trivial. MapServer and GRASS both have implementations that do something, but they don't work if the line has sharp angles or silly small loops (easy to get silly small when you zoom out on any line unfortunately).
Hide
Permalink
Stefan Alfons Tzeggai added a comment -
In case it is of any help or inspiring, here is how it is solved in deegree3:
http://wald.intevation.org/plugins/scmsvn/viewcvs.php/deegree3/trunk/deegree-core/deegree-core-rendering-2d/src/main/java/org/deegree/rendering/r2d/strokes/OffsetStroke.java?root=deegree&view=markup
Show
Stefan Alfons Tzeggai added a comment - In case it is of any help or inspiring, here is how it is solved in deegree3: http://wald.intevation.org/plugins/scmsvn/viewcvs.php/deegree3/trunk/deegree-core/deegree-core-rendering-2d/src/main/java/org/deegree/rendering/r2d/strokes/OffsetStroke.java?root=deegree&view=markup
Hide
Permalink
Stefan Alfons Tzeggai added a comment -
About the renderer-support for PerpendicularOffset.. If you could be so nice to commit or append a patch that does the "something" like GRASS, MapServer or Deegree, then I would continue there and try hard to find a solution for the edges. But I would need some //comments on where to start degugging/extending.
Show
Stefan Alfons Tzeggai added a comment - About the renderer-support for PerpendicularOffset.. If you could be so nice to commit or append a patch that does the "something" like GRASS, MapServer or Deegree, then I would continue there and try hard to find a solution for the edges. But I would need some //comments on where to start degugging/extending.
Hide
Permalink
Andrea Aime added a comment -
Unfortunately I lost the work I did a couple of years ago on this topic. But the Deegree code may seem a good start.
As for where extending, I would just make a hlper class like GeometryClipper that can be then reused in the rendering code, fliter functions and WPS processes
Show
Andrea Aime added a comment - Unfortunately I lost the work I did a couple of years ago on this topic. But the Deegree code may seem a good start. As for where extending, I would just make a hlper class like GeometryClipper that can be then reused in the rendering code, fliter functions and WPS processes
Hide
Permalink
Stefan Alfons Tzeggai added a comment -
Besindes the renderer-problem where i am still keep to dig by head into: Could we comit the attached patches which do not affect the renderer but only test/fix Parser, Transformer and DuplicatingStyleVisitor. I am afraid the patches will get old otherwise.. ;-) Greetings, Steve
Show
Stefan Alfons Tzeggai added a comment - Besindes the renderer-problem where i am still keep to dig by head into: Could we comit the attached patches which do not affect the renderer but only test/fix Parser, Transformer and DuplicatingStyleVisitor. I am afraid the patches will get old otherwise.. ;-) Greetings, Steve
Hide
Permalink
Andrea Aime added a comment -
I hope to be able to do a round of patch reviews and applications this weekend, but no promises.
Show
Andrea Aime added a comment - I hope to be able to do a round of patch reviews and applications this weekend, but no promises.
Jody Garnett made changes -
Fix Version/s 8.0-RC1 [ 18082 ]
Fix Version/s 8.0-M3 [ 17864 ]
Hide
Permalink
Andrea Aime added a comment -
Patches applied on trunk
Show
Andrea Aime added a comment - Patches applied on trunk
Jody Garnett made changes -
Fix Version/s 8.0-RC1 [ 18150 ]
Fix Version/s 8.0-M4 [ 18082 ]
Hide
Permalink
Stefan Alfons Tzeggai added a comment -
Thanks a lot!
So to make it clear: Render-support is still missing.. We will see what the future brings ;-)
Show
Stefan Alfons Tzeggai added a comment - Thanks a lot! So to make it clear: Render-support is still missing.. We will see what the future brings ;-)
Hide
Permalink
Andrea Aime added a comment -
Yep, confirmed, renderer support is still missing
Show
Andrea Aime added a comment - Yep, confirmed, renderer support is still missing
Jody Garnett made changes -
Fix Version/s 8.0-RC2 [ 18530 ]
Fix Version/s 8.0-RC1 [ 18150 ]
Hide
Permalink
Björn Harrtell added a comment - - edited
Initial render implementation. No doubt need more work to be accepted in trunk but does work as demonstrated by attached image.

Points to improve:
 * Remove dirty hacks (like not having to make shape non final?)
 * Investigate performance implications (perhaps buffer calc should/could be cached?)
 * Make sharp inner corners less ugly (they are because of self intersections?)

Patch is also available in pull request here:
https://github.com/geotools/geotools/pull/1
Show
Björn Harrtell added a comment - - edited Initial render implementation. No doubt need more work to be accepted in trunk but does work as demonstrated by attached image. Points to improve:  * Remove dirty hacks (like not having to make shape non final?)  * Investigate performance implications (perhaps buffer calc should/could be cached?)  * Make sharp inner corners less ugly (they are because of self intersections?) Patch is also available in pull request here: https://github.com/geotools/geotools/pull/1
Björn Harrtell made changes -
Attachment perpoff.diff [ 60407 ]
Attachment perpoff.png [ 60408 ]
Hide
Permalink
Stefan Alfons Tzeggai added a comment - - edited
Great. Thanks!

Let's get it into trunk. Beside the above-mentioned features, JUnit tests are needed.

The test could:
* render a line in to a BufferedImage - once with a first LineSymbolizer
in RED and a second LineSymbolizer in GREEN. The second uses a
perpendicular offset of 3px. The test would then query some pixels in
the BufferedImage and check, that they are Green or Red as expected.

If you write such test, I would see the bounty as finished and I paypal the money directly.
Show
Stefan Alfons Tzeggai added a comment - - edited Great. Thanks! Let's get it into trunk. Beside the above-mentioned features, JUnit tests are needed. The test could: * render a line in to a BufferedImage - once with a first LineSymbolizer in RED and a second LineSymbolizer in GREEN. The second uses a perpendicular offset of 3px. The test would then query some pixels in the BufferedImage and check, that they are Green or Red as expected. If you write such test, I would see the bounty as finished and I paypal the money directly.
Hide
Permalink
Björn Harrtell added a comment -
I also needed a backport of this to 2.7.x for use in GeoServer stable. The patches attached here applies cleanly to 2.7.x branch but my patch require a JTS upgrade from 1.11 to 1.12.
Show
Björn Harrtell added a comment - I also needed a backport of this to 2.7.x for use in GeoServer stable. The patches attached here applies cleanly to 2.7.x branch but my patch require a JTS upgrade from 1.11 to 1.12.
Hide
Permalink
Björn Harrtell added a comment -
I'll try to get a JUnit test done in the near future.
Show
Björn Harrtell added a comment - I'll try to get a JUnit test done in the near future.
Hide
Permalink
Andrea Aime added a comment -
Bjorn, that is a no go, we decided not to use JTS 1.12 on the stable series quite a bit of time ago. You can ask for an upgrade on the ml though.
Show
Andrea Aime added a comment - Bjorn, that is a no go, we decided not to use JTS 1.12 on the stable series quite a bit of time ago. You can ask for an upgrade on the ml though.
Hide
Permalink
Andrea Aime added a comment -
Bjorn, as far as I can see you have added perperdincular offset calculation by using JTS single side buffer code. I was talking about it with Martin Davis during FOSS4G-NA (now added as a watcher of this issue) and afaik that won't produce proper line offsets whenever the line self intersects itself, a proper line offset will also self intersect itself, whilst the single side buffer won't, it will stay on the side of the loop. Also wondering what the behavior of it will be with line rings.

Also looked at DeeGree version, as far as I can see it uses the simple "normals" approach which generates long spikes on acute angles...

Hmm... maybe we should just take Bjorn approach as a tentative one that will work for mostly straight lines, and wait for someone else to implement a true offset line generator?
Show
Andrea Aime added a comment - Bjorn, as far as I can see you have added perperdincular offset calculation by using JTS single side buffer code. I was talking about it with Martin Davis during FOSS4G-NA (now added as a watcher of this issue) and afaik that won't produce proper line offsets whenever the line self intersects itself, a proper line offset will also self intersect itself, whilst the single side buffer won't, it will stay on the side of the loop. Also wondering what the behavior of it will be with line rings. Also looked at DeeGree version, as far as I can see it uses the simple "normals" approach which generates long spikes on acute angles... Hmm... maybe we should just take Bjorn approach as a tentative one that will work for mostly straight lines, and wait for someone else to implement a true offset line generator?
Hide
Permalink
Björn Harrtell added a comment -
Implemented two simple testcases.
Show
Björn Harrtell added a comment - Implemented two simple testcases.
Björn Harrtell made changes -
Attachment perpoffTest.diff [ 60459 ]
Hide
Permalink
Martin Davis added a comment -
The offset code in the patch is using the JTS OffsetCurveBuilder class. This class is used internally in the JTS buffer code to produce *raw* offset curves for buffers. It's not intended for standalone use. It produces ugly linework when there are inside turns which are "narrower" than the offset distance. So I don't think this is suitable for production rendering use, since the failure cases are going to be very obvious when they occur.

On the positive side, since this is not derived from the single-sided buffer code, it will actually work with self-intersecting lines. It doesn't produce closed offset lines for rings, though.

As Andrea observes, this is a tough problem. I've been playing around with a JTS implementation in the lab, but it's not ready for production, and has some blocking issues.
Show
Martin Davis added a comment - The offset code in the patch is using the JTS OffsetCurveBuilder class. This class is used internally in the JTS buffer code to produce *raw* offset curves for buffers. It's not intended for standalone use. It produces ugly linework when there are inside turns which are "narrower" than the offset distance. So I don't think this is suitable for production rendering use, since the failure cases are going to be very obvious when they occur. On the positive side, since this is not derived from the single-sided buffer code, it will actually work with self-intersecting lines. It doesn't produce closed offset lines for rings, though. As Andrea observes, this is a tough problem. I've been playing around with a JTS implementation in the lab, but it's not ready for production, and has some blocking issues.
Hide
Permalink
Björn Harrtell added a comment -
Understandably this is not an ideal implementation, but it works quite well if you avoid the narrow inside turns that Martin mentioned.

Test case is running the symbolizer for visual inspection. Are buffer pixel tests desired? (can't find any preexisiting tests that goes that far)
Show
Björn Harrtell added a comment - Understandably this is not an ideal implementation, but it works quite well if you avoid the narrow inside turns that Martin mentioned. Test case is running the symbolizer for visual inspection. Are buffer pixel tests desired? (can't find any preexisiting tests that goes that far)
Hide
Permalink
Andrea Aime added a comment -
I believe we need to take a decision on whether a known to be buggy implementation is better than no implementation, given that there is no correct implementation in sight. The community has to make this call, Bjorn, can you send a mail to geotools-devel describing the issue?
It would be great to show images of both acceptable behavior and buggy one.
Show
Andrea Aime added a comment - I believe we need to take a decision on whether a known to be buggy implementation is better than no implementation, given that there is no correct implementation in sight. The community has to make this call, Bjorn, can you send a mail to geotools-devel describing the issue? It would be great to show images of both acceptable behavior and buggy one.
Hide
Permalink
Björn Harrtell added a comment - - edited
Adding two images, one with acceptable result and one showing a bugged case. I'll post a message to the list asap.
Show
Björn Harrtell added a comment - - edited Adding two images, one with acceptable result and one showing a bugged case. I'll post a message to the list asap.
Björn Harrtell made changes -
Attachment narrowbug.png [ 60467 ]
Attachment perpoff.png [ 60468 ]
Justin Deoliveira made changes -
Fix Version/s 8.0-RC3 [ 18641 ]
Fix Version/s 8.0-RC2 [ 18530 ]
Hide
Permalink
Martin Davis added a comment -
Bjorn, have you tried a realistic natural feature with a relatively large offset? That's a situation where the raw offset curve generation will exhibit some really ugly artifacts.
Show
Martin Davis added a comment - Bjorn, have you tried a realistic natural feature with a relatively large offset? That's a situation where the raw offset curve generation will exhibit some really ugly artifacts.
Justin Deoliveira made changes -
Fix Version/s 8.1-RC1 [ 18691 ]
Fix Version/s 8.0-RC3 [ 18641 ]
Andrea Aime made changes -
Fix Version/s 8.2 [ 18766 ]
Fix Version/s 8.1 [ 18691 ]
Justin Deoliveira made changes -
Fix Version/s 8.3 [ 18792 ]
Fix Version/s 8.2 [ 18766 ]
Alessio Fabiani made changes -
Fix Version/s 8.4 [ 18895 ]
Fix Version/s 8.3 [ 18792 ]
Justin Deoliveira made changes -
Fix Version/s 8.5 [ 18994 ]
Fix Version/s 8.4 [ 18895 ]
Andrea Aime made changes -
Fix Version/s 8.6 [ 19030 ]
Fix Version/s 8.5 [ 18994 ]
Andrea Aime made changes -
Fix Version/s 8.7 [ 19070 ]
Fix Version/s 8.6 [ 19030 ]
Mauro Bartolomeoli made changes -
Fix Version/s 8.8 [ 19121 ]
Fix Version/s 8.7 [ 19070 ]
Stefan Alfons Tzeggai made changes -
Status Open [ 1 ] Closed [ 6 ]
Resolution Incomplete [ 4 ]

People

  • Assignee:
    Andrea Aime
    Reporter:
    Stefan Alfons Tzeggai
Vote (2)
Watch (7)

Dates

  • Created:
    Updated:
    Resolved: