Buffer inconsistency with regards to v2.5.0

[hahschaa]

Hi there,

I was migrating from version 2.5.0 to 2.6.0, but found that some things started to act differently. I've managed to narrow it down to a single method: Geometry.Buffer(double distance, int quadrantSegments, EndCapStyle endCapStyle).

I've attached an example project that shows the different behavior in both versions:
Nts2.6.0Buffer.zip

I load a polygon from a file, then buffer it. In v2.5.0 the number of points increases as expected. In the newer version the number of points stays the same. This does not hold for all cases, e.g. a simple rectangle does work.

Thanks for your hard work, love the project.

With kind regards,

Harm

[FObermaier]

I assume this has to do with the issue described in this JTS issue.
The fix would be to implement locationtech/jts#1132.

[hahschaa]

Thanks for your answer. Is there anything I can do to circumvent this? Or is the only option to keep using v2.5.0 for now. I'd contribute to the code if I could, but I'm afraid my knowledge of this subject is too limited.

[michaldengusiak]

@FObermaier We also have encountered an exception when using BufferCurveSetBuilder.GetCurves() with a valid polygon geometry and a negative buffer. This was previously working as expected in older versions.

// Input Polygon:
Geometry geometry = new WKTReader(new GeometryFactory())
    .Read("POLYGON ((0 0, 0 -7.439614, 12.134293 -7.439614, 12.134293 0, 0 0))");

// Validity checks:
bool valid = geometry.IsValid; // true
var  isValidOp = new IsValidOp(geometry); // ValidationError: null (geometry is valid)

// Precision model:
var precisionModel = new PrecisionModel(1 / 1E-6);

// Buffer parameters we used some not sure if best one:
var bufferParameters = new BufferParameters
{
    EndCapStyle = EndCapStyle.Square,
    JoinStyle = JoinStyle.Mitre
};

double offset = -0.1;

// This call throws an exception (worked in previous version):
var bufferCurveSetBuilder = new BufferCurveSetBuilder(geometry, offset, precisionModel, bufferParameters);
IEnumerable<NetTopologySuite.Noding.ISegmentString> segmentStrings =bufferCurveSetBuilder.GetCurves(); // <-- Throws exception!

//
// This works fine - SHOLD WE USE AS WORKING ALTERNATIVE?:
var geometry_Temp = geometry.Buffer(offset, bufferParameters);

Is it correct to use geometry.Buffer(offset, bufferParameters) as an alternative to get an offset polygon, or are there important differences/limitations compared to using BufferCurveSetBuilder.GetCurves() directly? Any comment or advice will be appreciated.

[FObermaier]

I have ported just the mentioned fix and I'd like to release it as v2.6.1.
@airbreather:

• How do you feel about starting a service/v2.6 branch for back porting plain bug-fixes like this?
• Would changing <NtsOfficialRelease> tag condition to

  <NtsOfficialRelease Condition=" ('$(GITHUB_REF)' == 'refs/heads/main' Or '$(GITHUB_REF)'.StartsWith('refs/heads/service/v') ) And '$(GITHUB_EVENT_NAME)' == 'push' ">true</NtsOfficialRelease>

  be sufficient for starting the release chain?

[FObermaier]

@hahschaa:

I assume this has to do with the issue described in this JTS issue.

The assumption has proven incorrect. The input polygon is basically a circle with short segments. On that you perform a buffer operation with buffer parameters requesting 71 segments per quadrant for cap generation. This is, for the buffer size you want, limited by a new heuristic to prevent adding a lot of short segments, as this has lead to other problems when processing the geometry later.

So, this is by design and has been introduced with f454244.

[FObermaier]

@michaldengusiak, your issue is totally unrelated but actually revealed a bug in the constructor of the BufferCurveSetBuilder.

I don't know what you need the curves for, but if geometry.Buffer is sufficient for you than you can use that.
The other option would be to pass an ElevationModel to the constructor of the BufferCurveSetBuilder:

var bufferCurveSetBuilder = new BufferCurveSetBuilder(
    geometry, offset, precisionModel, geometry.Factory.ElevationModel,  bufferParameters);

[hahschaa]

@FObermaier, thanks.

It is indeed a circle that is buffered, but if you buffer without adding segments it is no longer a circle. I.e. on a very small scale a dodecagon would be a good approximation of a circle, but if you then buffer it it is no longer a good approximation, and that is exactly the reason why we want to add so many short segments. It has always worked flawlessly for our use case.

As far as I can see in the code that you linked the new behavior is hard-coded and cannot be overruled. Is there a way we can get both the old and the new behavior based on a setting?

[FObermaier]

As far as I can see in the code that you linked the new behavior is hard-coded and cannot be overruled. Is there a way we can get both the old and the new behavior based on a setting?

I'm sorry, that is not possible. You may need to change the way you create the circles. Do you only work with circles? How do you create them?

[hahschaa]

It is not circles only, but a combination of straight lines and circular curves.

We use your code extensively in a process related to PCBA manufacturing. The round shape is an approximation of the outline of a large electrolytic capacitor. But a PCBA can have components in all kinds of shapes. One of the things we use your project for is to determine what area of the PCB is x mm away from all components. So what we do is we convert all the outlines to polygons, then buffer it, subtract it from the shape of the bare board, and apply some curve fitting to get back to lines and curves instead of the large amount of coordinates.

This last step now fails. Instead of getting nice curves, we get hundreds or thousands of straight lines, and for good reason: the fitting needs to determine if something is a either a regular polygon or a curve. That's why I mentioned the dodecahedron, is it a regular polygon, or was it intended as a curve. Obviously this process is never perfect, but by doing some thorough analysis, we have come up with a way of determining how many segments we need before buffering. And like I said, that has always worked flawlessly.

I am more than willing to contribute to your project to get this working again with some kind of setting.

[michaldengusiak]

@michaldengusiak, your issue is totally unrelated but actually revealed a bug in the constructor of the BufferCurveSetBuilder.

I don't know what you need the curves for, but if geometry.Buffer is sufficient for you than you can use that. The other option would be to pass an ElevationModel to the constructor of the BufferCurveSetBuilder:

var bufferCurveSetBuilder = new BufferCurveSetBuilder(
geometry, offset, precisionModel, geometry.Factory.ElevationModel, bufferParameters);

Thanks a lot, we will use: geometry.Buffer

[FObermaier]

I am more than willing to contribute to your project to get this working again with some kind of setting.

That is nice of you. It boils down to an increased value for OffsetSegmentSeparationFactor.

NetTopologySuite/src/NetTopologySuite/Operation/Buffer/OffsetSegmentGenerator.cs

Lines 21 to 28 in 3c7387e

	/// <summary>
	/// Factor controlling how close offset segments can be to
	/// skip adding a fillet or mitre.
	/// This eliminates very short fillet segments,
	/// reduces the number of offset curve vertices.
	/// and improves the robustness of mitre construction.
	/// </summary>
	private const double OffsetSegmentSeparationFactor = 0.05;

Setting it (back?) to 0.01 gives the old result. I'm open to change it from const to static or instance field but this is an internal class and we'd have to think of ways of how to set it accordingliy.

[hahschaa]

I have been looking through the code, and to me it seems that by far the easiest thing to do is to move the constant from OffsetSegmentGenerator to BufferParameters:

develop...hahschaa:NetTopologySuite:feature/allow-setting-buffer-offset-segment-separation-factor

In my mind this makes perfect sense, as it is a parameter for a buffer operation. Please let me know if this also works for you, because then I'll open a PR.

[hahschaa]

Hi @FObermaier, did you see my last comment? I'd like to open a PR if you agree with the approach :)