GeoTools

dateline wrapping misses small geometries

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.7.2, 8.0-M1
  • Fix Version/s: 2.7.3, 8.0-M2
  • Component/s: render
  • Labels:
    None
  • Testcase included:
    yes

Description

Small projected geometries that wrap the dateline are incorrectly "unwrapped" resulting in artifacts.

The heuristic for detecting coordinates crossing the dateline misses small geometries as their unprojected width will exceed the heuristic's upper limit. As a fallback, the patch checks for the lower limit and if the upper one is crossed, checks the projected line segment.

I would apply this myself, but wanted your review.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andrea Aime added a comment -
Ian, can you please avoid adding those "private" markers? The classes are already package private, I don't like having the code one bit more verbose than it has to really be. If it's a matter of big importance make a GSIP to state this in the coding conventions
Show
Andrea Aime added a comment - Ian, can you please avoid adding those "private" markers? The classes are already package private, I don't like having the code one bit more verbose than it has to really be. If it's a matter of big importance make a GSIP to state this in the coding conventions
Hide
Permalink
Andrea Aime added a comment -
The patch also fails to follow the java coding conventions when it comes to argument calls, e.g.:
            geometry.apply(new WrappingCoordinateFilter(radius, radius * 2,mt));
should have been
            geometry.apply(new WrappingCoordinateFilter(radius, radius * 2, mt));
Show
Andrea Aime added a comment - The patch also fails to follow the java coding conventions when it comes to argument calls, e.g.:             geometry.apply(new WrappingCoordinateFilter(radius, radius * 2,mt)); should have been             geometry.apply(new WrappingCoordinateFilter(radius, radius * 2, mt));
Hide
Permalink
Andrea Aime added a comment - - edited
Ian, the patch looks good, tested it with the original use case for this (sponsored by FAO) which indeed used sea partitions, so very big polygons.
Thanks for contributing it.

Please fix the code convention things before committing it (ah, there are also unused imports in the test class), if you feel strongly about the other code style things (forcing private declaration, creating dummy variables to store short calculations) let's talk about them in public and make a decision that is to be applied library wise.
If not please avoid changing the code to suit your coding style, it does not necessarily fit other people preference and makes it harder to review the patches
Show
Andrea Aime added a comment - - edited Ian, the patch looks good, tested it with the original use case for this (sponsored by FAO) which indeed used sea partitions, so very big polygons. Thanks for contributing it. Please fix the code convention things before committing it (ah, there are also unused imports in the test class), if you feel strongly about the other code style things (forcing private declaration, creating dummy variables to store short calculations) let's talk about them in public and make a decision that is to be applied library wise. If not please avoid changing the code to suit your coding style, it does not necessarily fit other people preference and makes it harder to review the patches
Hide
Permalink
Ian Schneider added a comment -
Fixed on 2.7.x and trunk.

No contention with aforementioned coding conventions.
Show
Ian Schneider added a comment - Fixed on 2.7.x and trunk. No contention with aforementioned coding conventions.

People

  • Assignee:
    Ian Schneider
    Reporter:
    Ian Schneider
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: