From 0b8a704ae95da0698e0c5f8b56c358e8e370a8ec Mon Sep 17 00:00:00 2001 From: Adam Wulkiewicz Date: Wed, 15 Nov 2017 21:23:49 +0100 Subject: [PATCH 1/2] [core][overlay] Improve/fix add_rings(). Support passing of area strategy into add_rings() and pass it from overlay, buffer and dissolve. Add invalid_output_exception and if preprocesor definition is defined throw it if an intermediate result in add_rings() has negative area which means that in non-cartesian CS too big polygon is the result. If area of the intermediate result is 0 ignore it. --- .../buffer/buffered_piece_collection.hpp | 3 +- .../algorithms/detail/overlay/add_rings.hpp | 43 ++++++++++++++----- .../algorithms/detail/overlay/overlay.hpp | 20 ++++++--- include/boost/geometry/core/exception.hpp | 23 +++++++++- .../extensions/algorithms/dissolve.hpp | 5 +-- 5 files changed, 71 insertions(+), 23 deletions(-) diff --git a/include/boost/geometry/algorithms/detail/buffer/buffered_piece_collection.hpp b/include/boost/geometry/algorithms/detail/buffer/buffered_piece_collection.hpp index 635ee2577..b5ebb4db4 100644 --- a/include/boost/geometry/algorithms/detail/buffer/buffered_piece_collection.hpp +++ b/include/boost/geometry/algorithms/detail/buffer/buffered_piece_collection.hpp @@ -1471,7 +1471,8 @@ struct buffered_piece_collection // negative rings (negative child with negative parent) detail::overlay::assign_parents(offsetted_rings, traversed_rings, selected, m_intersection_strategy, true, false); - return detail::overlay::add_rings(selected, offsetted_rings, traversed_rings, out); + return detail::overlay::add_rings(selected, offsetted_rings, traversed_rings, out, + m_area_strategy); } }; diff --git a/include/boost/geometry/algorithms/detail/overlay/add_rings.hpp b/include/boost/geometry/algorithms/detail/overlay/add_rings.hpp index 317bacc78..f64eb0b06 100644 --- a/include/boost/geometry/algorithms/detail/overlay/add_rings.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/add_rings.hpp @@ -62,6 +62,13 @@ inline void convert_and_add(GeometryOut& result, } } +enum add_rings_error_handling +{ + add_rings_ignore_unordered, + add_rings_add_unordered, + add_rings_throw_if_reversed +}; + template < typename GeometryOut, @@ -69,17 +76,18 @@ template typename Geometry1, typename Geometry2, typename RingCollection, - typename OutputIterator + typename OutputIterator, + typename AreaStrategy > inline OutputIterator add_rings(SelectionMap const& map, Geometry1 const& geometry1, Geometry2 const& geometry2, RingCollection const& collection, OutputIterator out, - bool require_positive_area = true) + AreaStrategy const& area_strategy, + add_rings_error_handling error_handling = add_rings_ignore_unordered) { typedef typename SelectionMap::const_iterator iterator; - typedef typename SelectionMap::mapped_type property_type; - typedef typename property_type::area_type area_type; + typedef typename AreaStrategy::return_type area_type; area_type const zero = 0; std::size_t const min_num_points = core_detail::closure::minimum_ring_size @@ -123,11 +131,22 @@ inline OutputIterator add_rings(SelectionMap const& map, // Only add rings if they satisfy minimal requirements. // This cannot be done earlier (during traversal), not // everything is figured out yet (sum of positive/negative rings) - if (geometry::num_points(result) >= min_num_points - && (! require_positive_area - || math::larger(geometry::area(result), zero))) + if (geometry::num_points(result) >= min_num_points) { - *out++ = result; + area_type const area = geometry::area(result, area_strategy); + // Ignore if area is 0 + if (! math::equals(area, zero)) + { + if (error_handling == add_rings_add_unordered + || area > zero) + { + *out++ = result; + } + else if (error_handling == add_rings_throw_if_reversed) + { + BOOST_THROW_EXCEPTION(invalid_output_exception()); + } + } } } } @@ -141,15 +160,17 @@ template typename SelectionMap, typename Geometry, typename RingCollection, - typename OutputIterator + typename OutputIterator, + typename AreaStrategy > inline OutputIterator add_rings(SelectionMap const& map, Geometry const& geometry, RingCollection const& collection, - OutputIterator out) + OutputIterator out, + AreaStrategy const& area_strategy) { Geometry empty; - return add_rings(map, geometry, empty, collection, out); + return add_rings(map, geometry, empty, collection, out, area_strategy); } diff --git a/include/boost/geometry/algorithms/detail/overlay/overlay.hpp b/include/boost/geometry/algorithms/detail/overlay/overlay.hpp index 57855f1f3..b98448a86 100644 --- a/include/boost/geometry/algorithms/detail/overlay/overlay.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/overlay.hpp @@ -234,7 +234,8 @@ inline OutputIterator return_if_one_input_is_empty(Geometry1 const& geometry1, select_rings(geometry1, geometry2, empty, all_of_one_of_them, strategy); ring_container_type rings; assign_parents(geometry1, geometry2, rings, all_of_one_of_them, strategy); - return add_rings(all_of_one_of_them, geometry1, geometry2, rings, out); + return add_rings(all_of_one_of_them, geometry1, geometry2, rings, out, + strategy.template get_area_strategy()); } @@ -370,9 +371,8 @@ std::cout << "traverse" << std::endl; selected_ring_properties, strategy); // Add rings created during traversal + area_strategy_type const area_strategy = strategy.template get_area_strategy(); { - area_strategy_type const area_strategy = strategy.template get_area_strategy(); - ring_identifier id(2, 0, -1); for (typename boost::range_iterator::type it = boost::begin(rings); @@ -390,10 +390,18 @@ std::cout << "traverse" << std::endl; // NOTE: There is no need to check result area for union because // as long as the polygons in the input are valid the resulting // polygons should be valid as well. - // This also solves the issue with non-cartesian CSes, the result may - // be too big so the area is negative but it's returned anyway. + // This also solves the issue with non-cartesian CSes. The result may + // be too big, so the area is negative. In this case either it's returned + // anyway (default) or an exception is thrown (based on #define). return add_rings(selected_ring_properties, geometry1, geometry2, rings, out, - OverlayType != overlay_union); + area_strategy, + OverlayType == overlay_union ? +#ifdef BOOST_GEOMETRY_UNION_THROW_INVALID_OUTPUT_EXCEPTION + add_rings_throw_if_reversed +#else + add_rings_add_unordered +#endif + : add_rings_ignore_unordered); } template diff --git a/include/boost/geometry/core/exception.hpp b/include/boost/geometry/core/exception.hpp index 21abbd577..72bc598b2 100644 --- a/include/boost/geometry/core/exception.hpp +++ b/include/boost/geometry/core/exception.hpp @@ -4,8 +4,8 @@ // Copyright (c) 2008-2015 Bruno Lalande, Paris, France. // Copyright (c) 2009-2015 Mateusz Loskot, London, UK. -// This file was modified by Oracle on 2015. -// Modifications copyright (c) 2015 Oracle and/or its affiliates. +// This file was modified by Oracle on 2015, 2017. +// Modifications copyright (c) 2015-2017 Oracle and/or its affiliates. // Contributed and/or modified by Adam Wulkiewicz, on behalf of Oracle @@ -83,6 +83,25 @@ public: } }; +/*! +\brief Invalid Output Exception +\ingroup core +\details The invalid_output_exception is thrown if valid output cannot be + generated by a function even if arguments are valid, e.g. union of + geographic polygons covering more than half of the area of the globe. + */ +class invalid_output_exception : public geometry::exception +{ +public: + + inline invalid_output_exception() {} + + virtual char const* what() const throw() + { + return "Boost.Geometry Invalid-Output exception"; + } +}; + }} // namespace boost::geometry diff --git a/include/boost/geometry/extensions/algorithms/dissolve.hpp b/include/boost/geometry/extensions/algorithms/dissolve.hpp index 26621982e..422015df6 100644 --- a/include/boost/geometry/extensions/algorithms/dissolve.hpp +++ b/include/boost/geometry/extensions/algorithms/dissolve.hpp @@ -246,9 +246,8 @@ struct dissolve_ring_or_polygon detail::overlay::select_rings(geometry, turn_info_per_ring, selected, strategy); // Add intersected rings + area_strategy_type const area_strategy = strategy.template get_area_strategy(); { - area_strategy_type const area_strategy = strategy.template get_area_strategy(); - ring_identifier id(2, 0, -1); for (typename boost::range_iterator const>::type it = boost::begin(rings); @@ -264,7 +263,7 @@ struct dissolve_ring_or_polygon // children with negative parents detail::overlay::assign_parents(geometry, rings, selected, strategy, true, true); - return detail::overlay::add_rings(selected, geometry, rings, out); + return detail::overlay::add_rings(selected, geometry, rings, out, area_strategy); } }; From a2d889ca055111dba5bfe936dec3254613ab063f Mon Sep 17 00:00:00 2001 From: Adam Wulkiewicz Date: Wed, 15 Nov 2017 21:34:37 +0100 Subject: [PATCH 2/2] [test][union] Conditionally check union's invalid output exception. --- .../set_operations/union/union_aa_geo.cpp | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/test/algorithms/set_operations/union/union_aa_geo.cpp b/test/algorithms/set_operations/union/union_aa_geo.cpp index a2d573be7..3797757df 100644 --- a/test/algorithms/set_operations/union/union_aa_geo.cpp +++ b/test/algorithms/set_operations/union/union_aa_geo.cpp @@ -58,21 +58,36 @@ void test_geographic_one(std::string const& wkt1, std::string const& wkt2, boost::geometry::read_wkt(wkt2, p2); multipolygon result; - boost::geometry::union_(p1, p2, result, is); - double result_area = bg::area(result, as); +#ifdef BOOST_GEOMETRY_UNION_THROW_INVALID_OUTPUT_EXCEPTION + if (expected_area >= 0) + { +#else + { +#endif + boost::geometry::union_(p1, p2, result, is); - std::size_t result_count = boost::size(result); - std::size_t result_exterior_points = std::for_each(boost::begin(result), - boost::end(result), - exterior_points_counter()).count; - std::size_t result_interiors = std::for_each(boost::begin(result), - boost::end(result), - interiors_counter()).count; - BOOST_CHECK_EQUAL(result_count, count); - BOOST_CHECK_EQUAL(result_exterior_points, exterior_points_count); - BOOST_CHECK_EQUAL(result_interiors, interiors_count); - BOOST_CHECK_CLOSE(result_area, expected_area, 0.001); + double result_area = bg::area(result, as); + + std::size_t result_count = boost::size(result); + std::size_t result_exterior_points = std::for_each(boost::begin(result), + boost::end(result), + exterior_points_counter()).count; + std::size_t result_interiors = std::for_each(boost::begin(result), + boost::end(result), + interiors_counter()).count; + BOOST_CHECK_EQUAL(result_count, count); + BOOST_CHECK_EQUAL(result_exterior_points, exterior_points_count); + BOOST_CHECK_EQUAL(result_interiors, interiors_count); + BOOST_CHECK_CLOSE(result_area, expected_area, 0.001); + } +#ifdef BOOST_GEOMETRY_UNION_THROW_INVALID_OUTPUT_EXCEPTION + else + { + BOOST_CHECK_THROW(boost::geometry::union_(p1, p2, result, is), + bg::invalid_output_exception); + } +#endif }