GeoTools
  1. GeoTools
  2. GEOT-3912

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. perpendicularOffsetImplementation.patch
        3 kB
        Stefan Alfons Tzeggai
      2. perpendicularOffsetTests.patch
        3 kB
        Stefan Alfons Tzeggai
      3. perpoff.diff
        6 kB
        Björn Harrtell
      4. perpoffTest.diff
        5 kB
        Björn Harrtell
      1. narrowbug.png
        2 kB
      2. perpoff.png
        1 kB
      3. perpoff.png
        24 kB

        Activity

        Hide
        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).
        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
        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
        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
        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
        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.
        Hide
        Andrea Aime added a comment -
        Patches applied on trunk
        Show
        Andrea Aime added a comment - Patches applied on trunk
        Hide
        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
        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
        Hide
        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
        Hide
        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
        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
        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
        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
        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
        Björn Harrtell added a comment -
        Implemented two simple testcases.
        Show
        Björn Harrtell added a comment - Implemented two simple testcases.
        Hide
        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
        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
        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
        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.
        Hide
        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.

          People

          • Assignee:
            Andrea Aime
            Reporter:
            Stefan Alfons Tzeggai
          • Votes:
            2 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: