From 04b834506c93eab8d1bf5be445410f6bca32c7d0 Mon Sep 17 00:00:00 2001 From: Menelaos Karavelas Date: Mon, 12 Oct 2015 11:37:29 +0300 Subject: [PATCH 1/2] [test][algorithms][is_valid] add test cases for checking spikes in areal geometries --- test/algorithms/is_valid.cpp | 61 ++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/algorithms/is_valid.cpp b/test/algorithms/is_valid.cpp index 7ec61eece..60dbd4883 100644 --- a/test/algorithms/is_valid.cpp +++ b/test/algorithms/is_valid.cpp @@ -713,6 +713,67 @@ inline void test_open_polygons() 0.7808688094430304 -0.6246950475544243,\ 0.7808688094430304 -0.6246950475544243))", AllowDuplicates); + + + // MySQL report on Sep 30, 2015 + test::apply + ("pg077", + "POLYGON((72.8714768817168 -167.0048853643874,9274.40641550926 3433.5957427942167,-58.09039811390054 187.50989457746405,-81.09039811390053 179.50989457746405,-207.99999999999997 135.36742435621204,-208 1,-208 0,-208 -276.9111154485375,49.8714768817168 -176.0048853643874))", + true); + + test::apply("pg077-simplified", + "POLYGON((-200 0,-207.99999999999997 135.36742435621204,-208 1,-208 0,-208 -276.9111154485375))", + true); + + test::apply + ("pg078", + "POLYGON((0 10,-10 0,0 0,10 0))", + true); + + test::apply + ("pg078spike1", + "POLYGON((0 10,-10 0,0 0,-10 0,10 0))", + false); + + test::apply + ("pg078spike2", + "POLYGON((0 10,-10 0,0 0,-8 0,10 0))", + false); + + test::apply + ("pg078spike3", + "POLYGON((0 10,-10 0,0 0,-11 0,10 0))", + false); + + test::apply + ("pg078reversed", + "POLYGON((0 10,10 0,0 0,-10 0))", + false); + + test::apply + ("pg079", + "POLYGON((10 0,0 10,0 0,0 -10))", + true); + + test::apply + ("pg079spike1", + "POLYGON((10 0,0 10,0 0,0 10,0 -10))", + false); + + test::apply + ("pg079spike2", + "POLYGON((10 0,0 10,0 0,0 8,0 -10))", + false); + + test::apply + ("pg079spike3", + "POLYGON((10 0,0 10,0 0,0 11,0 -10))", + false); + + test::apply + ("pg079reversed", + "POLYGON((10 0,0 -10,0 0,0 10))", + false); } template From b029641604f777012e1617caed4721cc20a022f5 Mon Sep 17 00:00:00 2001 From: Menelaos Karavelas Date: Mon, 12 Oct 2015 11:48:30 +0300 Subject: [PATCH 2/2] [algorithms][detail][point_is_spike_or_equal] fix inconsistency between sideness/collinearity test and check for direction of vectors; Problem: when checking whether a point q creates a spike or not with respect to two (ordered) points a and b, the first check performed is whether q, a and b are collinear; if so, then it is determined whether the vectors q-b and b-a have the same direction or not; for points with floating-point coordinates, due to rounding errors as well as due to the fact that equality to zero is checked using some tolerance, the three points may be detected as collinear; however, when the directions of the two vectors are checked, computations are done without taking into account any tolerance and this can lead to inconsistent results; Fix: when checking the directions of the vectors q-b and b-a, compute signs of differences using numerical tolerances, that is using math::equals() instead of plain comparison operators; --- .../detail/point_is_spike_or_equal.hpp | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/include/boost/geometry/algorithms/detail/point_is_spike_or_equal.hpp b/include/boost/geometry/algorithms/detail/point_is_spike_or_equal.hpp index 9db1ef8e0..399c8fbef 100644 --- a/include/boost/geometry/algorithms/detail/point_is_spike_or_equal.hpp +++ b/include/boost/geometry/algorithms/detail/point_is_spike_or_equal.hpp @@ -1,9 +1,14 @@ // Boost.Geometry (aka GGL, Generic Geometry Library) -// Copyright (c) 2007-2013 Barend Gehrels, Amsterdam, the Netherlands. -// Copyright (c) 2008-2013 Bruno Lalande, Paris, France. -// Copyright (c) 2009-2013 Mateusz Loskot, London, UK. -// Copyright (c) 2013 Adam Wulkiewicz, Lodz, Poland. +// Copyright (c) 2007-2015 Barend Gehrels, Amsterdam, the Netherlands. +// Copyright (c) 2008-2015 Bruno Lalande, Paris, France. +// Copyright (c) 2009-2015 Mateusz Loskot, London, UK. +// Copyright (c) 2013-2015 Adam Wulkiewicz, Lodz, Poland. + +// This file was modified by Oracle on 2015. +// Modifications copyright (c) 2015 Oracle and/or its affiliates. + +// Contributed and/or modified by Menelaos Karavelas, on behalf of Oracle // Use, modification and distribution is subject to the Boost Software License, // Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -12,8 +17,6 @@ #ifndef BOOST_GEOMETRY_ALGORITHMS_DETAIL_POINT_IS_EQUAL_OR_SPIKE_HPP #define BOOST_GEOMETRY_ALGORITHMS_DETAIL_POINT_IS_EQUAL_OR_SPIKE_HPP -#include -#include #include #include #include @@ -28,6 +31,17 @@ namespace boost { namespace geometry namespace detail { +template +inline int sign_of_difference(Point1 const& point1, Point2 const& point2) +{ + return + math::equals(geometry::get(point1), geometry::get(point2)) + ? + 0 + : + (geometry::get(point1) > geometry::get(point2) ? 1 : -1); +} + // Checks if a point ("last_point") causes a spike w.r.t. // the specified two other points (segment_a, segment_b) // @@ -35,7 +49,9 @@ namespace detail // a lp b // // Above, lp generates a spike w.r.t. segment(a,b) -// So specify last point first, then (a,b) (this is unordered, so unintuitive) +// So specify last point first, then (a,b) +// The segment's orientation does matter: if lp is to the right of b +// no spike is reported template static inline bool point_is_spike_or_equal(Point1 const& last_point, Point2 const& segment_a, @@ -46,29 +62,21 @@ static inline bool point_is_spike_or_equal(Point1 const& last_point, typename cs_tag::type >::type side_strategy; - typedef Point1 vector_type; - int const side = side_strategy::apply(last_point, segment_a, segment_b); if (side == 0) { // Last point is collinear w.r.t previous segment. // Check if it is equal - vector_type diff1; - conversion::convert_point_to_point(last_point, diff1); - geometry::subtract_point(diff1, segment_b); - int const sgn_x1 = math::sign(geometry::get<0>(diff1)); - int const sgn_y1 = math::sign(geometry::get<1>(diff1)); + int const sgn_x1 = sign_of_difference<0>(last_point, segment_b); + int const sgn_y1 = sign_of_difference<1>(last_point, segment_b); if (sgn_x1 == 0 && sgn_y1 == 0) { return true; } // Check if it moves forward - vector_type diff2; - conversion::convert_point_to_point(segment_b, diff2); - geometry::subtract_point(diff2, segment_a); - int const sgn_x2 = math::sign(geometry::get<0>(diff2)); - int const sgn_y2 = math::sign(geometry::get<1>(diff2)); + int const sgn_x2 = sign_of_difference<0>(segment_b, segment_a); + int const sgn_y2 = sign_of_difference<1>(segment_b, segment_a); return sgn_x1 != sgn_x2 || sgn_y1 != sgn_y2; }