Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve OffsetCurve behaviour and add Joined mode #956

Merged
merged 4 commits into from
Feb 24, 2023

Conversation

dr-jts
Copy link
Contributor

@dr-jts dr-jts commented Feb 22, 2023

The OffsetCurve class added in #810 had an intentional design for the generated output. It only provides a single line output. This semantic was motivated by discussions with the GEOS community (see libgeos/geos#477).

However, it is recognized that in some cases this behaviour doesn't fully represent the linework which can be considered to be part of an offset curve. In particular, input lines with self-intersections produce curves which are shorter than desirable in some cases. Also, it doesn't match the output generated by the original GEOS/PostGIS OffsetCurve implementation. This is discussed in libgeos/geos#816.

It seems like a good thing to produce as much offset linework as feasible. This PR enhances the OffsetCurve class to return as much as possible of the linework which can form an offset curve. It also adds a "Joined mode" to produce continuous lines.

Fixes #957.

Enhanced Output

The output contains as much linework as possible that can be determined to be in the offset curve (based on the buffer boundary of the input). This will be a single line in simple cases (which matches the previous implementation, and the old GEOS code). For more complex cases containing "narrow corridors" or self-intersections the output will be a MultiLineString containing the various disjoint curves which lie at the given offset distance from the input line. The output is structured to make it possible to reduce if necessary; in particular, the line elements are ordered along the raw offset curve as much as possible, with the main or outer curve appearing first.

Examples

Linework with "necks" narrower than twice the offset distance produce additional ring elements in the offset curve:
image

Self-intersections can produce ring elements in the offset curve:

Offset Distance = 8
image

Offset Distance = -5
image

Self-intersections may produce linear elements as well as rings in the offset curve:
image

Example showing offsets on both sides:
image

Joined Mode

The class also adds a "Joined mode", which joins computed offset curve sections into a single continuous line. (See below for examples). In cases of self-intersecting input this produces output which may better correspond to expectations of what the offset curve should look like. (Note that this mode may cause the output to contain "flattening" artifacts at large offset distances.)

Examples of Joined Mode

image

image

An example of a flattening artifact in Join mode:
image

@dr-jts dr-jts changed the title Improve OffsetCurve behaviour, add Joined mode Improve OffsetCurve behaviour and add Joined mode Feb 22, 2023
@dr-jts
Copy link
Contributor Author

dr-jts commented Feb 22, 2023

@nyalldawson @garci66 you may be interested in reviewing this.

@nyalldawson
Copy link

Thanks @dr-jts !!

Linework with "necks" narrower than twice the offset distance produce additional ring elements in the offset curve:

To me this particular one is a regression for certain use cases. (Specifically, generating an offset line of that shape to use as a baseline for map labels). Do you think introducing a flag to control the behaviour would be suitable, so that callers can decide which class of results they need?

@dr-jts
Copy link
Contributor Author

dr-jts commented Feb 23, 2023

To me this particular one is a regression for certain use cases. (Specifically, generating an offset line of that shape to use as a baseline for map labels). Do you think introducing a flag to control the behaviour would be suitable, so that callers can decide which class of results they need?

@nyalldawson I thought you might have some concerns... :).

Is libgeos/geos#477 (comment) an example of what you are expecting? E.g.

If so, as mentioned above the new output is structured so that it is relatively easy to obtain the single "primary" line (i.e. by simply extracting the first element if the output is a MultiLineString). But I guess "easy" depends on the client system. Is that QGIS in your case? if so, then how should this be reconciled with libgeos/geos#816, which I think also comes from QGIS requirements?

I'm loathe to introduce a flag unless it's really needed. Although I guess that does have the benefit of making the behaviour options clear.

Signed-off-by: Martin Davis <[email protected]>
@nyalldawson
Copy link

Is libgeos/geos#477 (comment) an example of what you are expecting? E.g.

Yes -- for label baselines that's the correct result.

If so, as mentioned above the new output is structured so that it is relatively easy to obtain the single "primary" line (i.e. by simply extracting the first element if the output is a MultiLineString). But I guess "easy" depends on the client system. Is that QGIS in your case? if so, then how should this be reconciled with libgeos/geos#816, which I think also comes from QGIS requirements?

For labeling this one:

image

The red line is the desired result. It's not a great example, but if that self intersection section were smaller then it's clear that the portion should be ignored when drawing a curved label for the line.

Maybe generating label baselines is an abuse of this operation, and needs some dedicated special thing. Looking at this example I think the desired label baseline would follow around the outside of the curve and ignore the actual direction of the self intersection:

image

I'd love to hear your thoughts here -- I'm sure you have some novel ideas about what we could be doing here! 👍 😃

Signed-off-by: Martin Davis <[email protected]>
@garci66
Copy link

garci66 commented Feb 23, 2023

@nyalldawson @garci66 you may be interested in reviewing this.

Thanks!.. for my use case, which is to offset "cable runs" in maps that would normally overlap, the joined mode solution seems to work great for me.

Even the "neck" or the broken / multi-polygons would be acceptable as right now, I lose any self intersecting loops, regardless of size. I think I can even live with the "flattening artifcat" you mention as, at least in my case, I use relatively small offsets compared to loop sizes (think few meters offsets vs city-block-sized loops.

Thanks for the effort... I can't comment on the code itself, but at least at first sight, it looks like a very good step!

But from what I understand, even without the joined mode, the black line seen in the image below: strange loop issue

would be solved? (notice it gets truncated very early)

I would also be a bit against a flag (at least for the basic behavior) cause, that would mean API changes on GEOS as well and then further hamper / complicate the adoption in QGIS (which is where I would need it)

The "joined mode" would be even better for my case, and while indeed that's a new function / option, I think it might warrant enough interest in QGIS to expose it as an offset option in the layer rendering settings as it can give very interesting results. (IMO, "better" results for most "graphical" offset definitions.

@dr-jts
Copy link
Contributor Author

dr-jts commented Feb 23, 2023

@nyalldawson thanks for the feedback.

Maybe generating label baselines is an abuse of this operation, and needs some dedicated special thing. Looking at this example I think the desired label baseline would follow around the outside of the curve and ignore the actual direction of the self intersection

Yes, maybe baselines are different to offset curves. It is possible to produce a curve like the one you show, by matching the ends of the line to the buffer boundary and then extracting the section between them. But what happens if the ends are not distinct (i.e. they only stick out by a very small amount, or don't overlap at all)? is it possible you are wanting a sort of "outer offset curve", which would always wrap around the "outside" of a geometry? But then what is the behaviour for straighter lines?

@dr-jts
Copy link
Contributor Author

dr-jts commented Feb 23, 2023

@garci66 thanks for the comments. It sounds like this is going to meet your needs better, which is good.

the black line seen in the image would be solved? (notice it gets truncated very early)

I think so, if I understand your use case. The result is the same as the old implementation, anyway. Here's what it will look like:
image

Even the "neck" or the broken / multi-polygons would be acceptable as right now, I lose any self intersecting loops, regardless of size.

Here's what the offset curve of a narrow neck looks like in Joined mode:
image

I would also be a bit against a flag

This will be the default behaviour, so no flag required

The "joined mode" might warrant enough interest in QGIS to expose it as an offset option

Excellent. Hopefully the artifacts won't be too much of a problem. It probably needs some extensive testing with different use cases. i expect this will be exposed by a flag (mode type code) on a new GEOS API function (perhaps GEOSOffsetCurveWithMode). That leaves room for other offset curve modes, if they can be specified and implemented.

Signed-off-by: Martin Davis <[email protected]>
@dr-jts dr-jts merged commit 41c7d92 into locationtech:master Feb 24, 2023
@dr-jts dr-jts deleted the improve-offset-curve branch February 24, 2023 22:25
pramsey added a commit to libgeos/geos that referenced this pull request Mar 6, 2023
Port of locationtech/jts#956

The output contains as much linework as possible that can be determined to be in the offset curve (based on the buffer boundary of the input). This will be a single line in simple cases (which matches the previous implementation, and the old GEOS code). For more complex cases containing "narrow corridors" or self-intersections the output will be a MultiLineString containing the various disjoint curves which lie at the given offset distance from the input line. The output is structured to make it possible to reduce if necessary; in particular, the line elements are ordered along the raw offset curve as much as possible, with the main or outer curve appearing first.
@jodygarnett jodygarnett added this to the 1.20.0 milestone Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OffsetCurve does not handle repeated points
4 participants