Fix for cart_intersect and side_by_triangle - inconsistencies on MinGW and more (robustness) #272
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Compare both ratios' potential denominators, corresponding to both segments. Furthermore, take into account the machine epsilon.
Add the support for policy calculating epsilon factor in equals(). Add greatest() algorithm - max() equivalent taking more than 2 parameters. Add specialization of abs() for floating-point numbers.
Compare the floating-point results of cramer's rule (denominators) using the epsilon scaled by the greatest of the differences of coordinates of the segments' endpoints (the input of the determinant formula).
Compare the floating-point results of determinant (denominators) using the epsilon scaled by the greatest of the differences of coordinates of the points passed into the determinant formula.
Instead of expecting invalid results, disable 2 still failing tests.
… in cart_intersect.
Member
Author
|
The tests pass also on Linux GCC 4.8.2 and Clang 3.4. |
To avoid returning reference to temporary object. Add equals_factor_policy "empty" specialization for integral types.
…ed with macro definition.
Collaborator
|
This is a larger changelist. I'm OK with this. I'm curious how it would work, also for in case areal/areal overlay operations do not use integer. But you don't have to test that, I will do that later. I'm OK with merging this. |
awulkiew
added a commit
that referenced
this pull request
Mar 22, 2015
Fix for cart_intersect and side_by_triangle - inconsistencies on MinGW and more (robustness)
mloskot
added a commit
to mloskot/geometry
that referenced
this pull request
Jun 19, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is an attempt to fix the inconsistent results of sides and segment's intersection calculation on various platforms. For now I tested it on msvc10,
msvc13msvc12 and mingw-w64 4.9.1 on which different results was generated before.Before, for floating-point coordinates, the results of determinant (side_by_triangle, cramer's rule) was compared with 0, scaling the machine epsilon by the greatest absolute of compared value. This wasn't a good solution since for nearly collinear segments the results are very small. For all of them factor 1 was always used (raw epsilon) no matter how long were the segments, what coordinates were used, so how great was the error. Furthermore on various platforms different instructions are generated for the determinant expression. For some instructions sets the error of the result is proportional to the input (so a difference of coordinates). The code added in this PR scales the epsilon by the greatest absolute value of the difference of points' coordinates passed into the determinant formula. For this purpose the support for EqualsPolicy was added which can be passed to
math::detail::equals_by_policy()where is used to retrieve the epsilon factor.This PR is related to this one: #259, which was the previous attempt and this ticket: https://svn.boost.org/trac/boost/ticket/8379.
As a side effect some of the tests (e.g. difference) previously failing now passes.
This PR doesn't solve all inconsistencies, there are tests which are still failing (e.g. get_turns, enabled with a macro) but now they're doing it in a more consistent way, i.e. similar ones generates the same, wrong output.
NOTE: The PR: #267 is a part of this one so it should be merged first.