RegionOps.constructAllXYRegionLoops breaking changes between minor versions

[piatrashkakanstantinas]

I'm migrating our codebase from itwinjs 5.0.0 to the latest and noticed that some of our code broke because of changes to RegionOps.constructAllXYRegionLoops method in @itwin/core-geometry.

This PR introduced optional addBridges parameter and set it to true by default (previously no bridges were added). Our code depends on no bridges being added and therefore requires corrections.

At this point, I'm not sure if changing it back is a good approach, but wanted to report the breaking change.

[piatrashkakanstantinas]

@ben-polinsky @dassaf4

[dassaf4]

Thanks for your feedback, @piatrashkakanstantinas! We decided on this default value because the previous behavior (no bridges) was considered less desirable, akin to a bug. Can you provide us with an example of your usage of this method: inputs and expected outputs in json format? This will provide us with another data point for how this method is being used and may also point to an alternate solution better suited for your inputs and expectations.

[piatrashkakanstantinas]

In our case we explicitly depended on output loops not being bridged as we used them to define a face with inner and outer loop being output from the method.

[dassaf4]

I see. Without bridges, this method returns a SignedLoops object for each loop it computes, inner and outer. As I understand it, you are not necessarily interested in the smallest number of returned SignedLoops objects (that addBridges=true ensures for typical usage) because you run your own containment computation logic afterwards.

FWIW, you could throw the positive area loops output by RegionOps.constructAllXYRegionLoops with addBridges=false into RegionOps.sortOuterAndHoleLoopsXY to compute loop containment. In the case of an inner and outer Loop input, for example, this method will return a ParityRegion from which you can directly construct your faces with inner/outer loops. If your input is already Loops and they don't intersect, then I think you can bypass RegionOps.constructAllXYRegionLoops altogether and just call RegionOps.sortOuterAndHoleLoopsXY to compute your faces.

If you can supply example typical input and expected output for RegionOps.constructAllXYRegionLoops, we might be able to suggest better methods to use.