geometry icon indicating copy to clipboard operation
geometry copied to clipboard

Prevent integer overflow during segment touch ratio calculation

Open mcquay239 opened this issue 8 years ago • 8 comments

I use boost/geometry clipping operations after snap rounding (to prevent robustness issues). So I have no intersecting segments in my areals, segments are intersecting with their endpoints. Integers are large (they can be about 2**30), but I use 64-bit integer coordinate type. So sides (side_info) are calculating perfectly, but when segment ratio calculation leads to integer overflow.

But we don't actually need to calculate these ratios, they are trivial.

mcquay239 avatar Jul 26 '16 18:07 mcquay239

Thanks @mcquay239 ! The compilation fixes can certainly be applied and will help some regressions in the regression matrix. The other fix can most probably be applied too - (have to spend more time on it to confirm, which I now don't have) but to be sure, did you run the unit tests?

barendgehrels avatar Jul 26 '16 20:07 barendgehrels

I did and I cought a regression in multiline/multiline intersection. Unfortunately I noticed it after the pull request, because there already were several test failures. I beleive it's something easy-to-fix and I'll try to debug it.

вт, 26 июля 2016 г., 23:11 Barend Gehrels [email protected]:

Thanks @mcquay239 https://github.com/mcquay239 ! The compilation fixes can certainly be applied and will help some regressions in the regression matrix. The other fix can most probably be applied too - (have to spend more time on it to confirm, which I now don't have) but to be sure, did you run the unit tests?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boostorg/geometry/pull/354#issuecomment-235389671, or mute the thread https://github.com/notifications/unsubscribe-auth/AAxnDP8t82irmxL2CiM1vPxRheDiy4eIks5qZmnpgaJpZM4JVb1u .

mcquay239 avatar Jul 26 '16 21:07 mcquay239

OK - thanks again. If necessary we can also split this PR

barendgehrels avatar Jul 27 '16 15:07 barendgehrels

I've just fixed tests. The only notice is that a linestring, consisting of an isolated point (two equal points, actually), is not simple (really?). But as I understand it's an expected behaviour.

Anyway all tests except broken earlier are passed.

mcquay239 avatar Jul 28 '16 15:07 mcquay239

I don't know if it's necessary to split this PR: #353 is equivalent to https://github.com/boostorg/geometry/pull/354/commits/b5c07c67ba53c3916d59ade6b005806acde75325, you can apply #353 and then think about this one.

mcquay239 avatar Jul 28 '16 15:07 mcquay239

Updated PR, checked tests once more, please apply @barendgehrels

mcquay239 avatar Jun 02 '17 11:06 mcquay239

@barendgehrels Considering you've made many changes in the strategy since 2016 is this PR still relevant?

awulkiew avatar Jun 03 '21 15:06 awulkiew