Add DynamicGeometry and GeometryCollection. #850

awulkiew · 2021-05-17T21:35:55Z

After: #843

This PR adds DynamicGeometry and GeometryCollection and usage in several algorithms.

List of changes:

Added DynamicGeometry concept
- Required specialization of traits::tag<> defining type as dynamic_geometry_tag
- Required specialization of 1-parameter traits::visit<>
- Required specialization of traits::geometry_types<>
- Optional specialization of 2-parameter traits::visit<>
- [mutable] Must be assignable from types specified in traits::geometry_types<>
Added GeometryCollection concept
- Required specialization of traits::tag<> defining type as geometry_collection_tag
- Required adaptation to ForwardRange concept
- Optional specialization of traits::geometry_types<> (by default using traits::geometry_types<> for range_value<>::type)
- Optional specialization of traits::iter_visit<> (by default calling traits::visit<> for range_value<>::type)
- [mutable] Required specialization of traits::emplace_back<> allowing to add objects of types specified in traits::geometry_types<>
- [mutable] Optional specialization of traits::clear<> (by default calling collection.clear())
- May be recursive
Added adaptations of DynamicGeometry
- boost::variant<>
- boost::variant2::variant<>
- boost::any<> (requires additional user specialization of traits::geometry_types<>)
- std::variant<>
- std::any<> (requires additional user specialization of traits::geometry_types<>)
Added GeometryCollection models
- model::geometry_collection<>
Added concept checks for DynamicGeometry and GeometryCollection
- added new util concepts::concept_type<> defining concept of Geometry in a generic way
- refactored concepts::check(), dispatches replaced with concepts::concept_type<>
Added algorithms:
- detail::visit()
- detail::visit_breadth_first()
Added support for DynamicGeometry and GeometryCollection in:
- append() (only DynamicGeometry)
- area()
- clear()
- disjoint() (the algorithm for 2 GeometryCollections has quadratic complexity so should probably be refactored)
- length()
- read_wkt()
- wkt()
- comparable_distance_result<>
- distance_result<>
Other
- Added support for rvalue references to range value_type in traits::push_back and range::push_back to support move semantics (the support for rvalue reference for the Range itself should still be added in a different PR)
- boost::variant workarounds previously implemented for VS2015 now works with all compilers #ifdef BOOST_VARIANT_DO_NOT_USE_VARIADIC_TEMPLATES (in various algorithms)

I'm open to suggestions.

…t in several algorithms. Also add basic support for GeometryCollection.

Replace variant support in wkt write. Add util::sequence_find_if.

include/boost/geometry/algorithms/detail/disjoint/interface.hpp

include/boost/geometry/algorithms/visit.hpp

mloskot

I've skimmed the code trying to understand the core parts and nothing looks suspicious.
Nice feature!

awulkiew · 2021-05-24T22:51:21Z

@mloskot Thanks for the review!

The thing that I don't like about it is the fact that traits::visit has to be specialized for a specific number of geometries, one or two. Do you have a different idea how this trait could be implemented to allow specialization for e.g. any number of arbitrary variants (potentially holding different geometries)?

Do you think that rvalue references should be handled in visit too?

Do you think visit should be able to return a value?

mloskot · 2021-05-25T06:28:02Z

Do you have a different idea how this trait could be implemented to allow specialization for e.g. any number of arbitrary variants

My C++ is getting rusty, but it generally should be feasible (https://stackoverflow.com/q/24687026/151641 or https://bitbashing.io/std-visit.html), but I'm sure you've looked at the variadics.

Do you think that rvalue references should be handled in visit too?
Do you think visit should be able to return a value?

I think it is generally a good idea to keep interfaces aligned with the standard stuff, i.e. similar to std::visit

I wish I could be more helpful, but can't afford any experiments on my own right now.

barendgehrels

Thanks a lot!

I think at least some of the TODO's should be done, or rephrased, and some commented code can go or be uncommented, so I set this to Request changes for now, but apart from that, I'm fine.

include/boost/geometry/algorithms/append.hpp

include/boost/geometry/algorithms/area.hpp

include/boost/geometry/algorithms/detail/disjoint/interface.hpp

include/boost/geometry/extensions/gis/io/shapefile/read.hpp

include/boost/geometry/geometries/adapted/boost_any.hpp

include/boost/geometry/geometries/adapted/boost_variant.hpp

include/boost/geometry/geometries/concepts/check.hpp

include/boost/geometry/geometries/geometry_collection.hpp

Add support for rvalue references in visit traits and algorithms. Move visit algorithms to detail namespace. Rename visit_iterator trait to iter_visit. Add tests for visit traits and algorithms. Remove unneeded comments.

Remove unneeded comments. Replace typedef with using. Change names and formatting.

awulkiew · 2021-05-28T17:13:30Z

@mloskot I added support for rvalue references because it is possible we'll want to move something inside visit. I didn't implement returning values though because:

in some cases the implementation would be complicated and still not 100% bulletproof, e.g. for boost::variant forcing to define return value as template argument of boost::static_visitor<>,
this would be a requirement for the users adapting their classes and this requirement would make this more complicated as well.

So I don't think it's worth it only to be able to save 2 lines of code now and then.

…in wkt read.

Add concepts::concept_type<> utility for generic concept definitions. Replace tag dispatching in concepts::check() with concepts::concept_type<>.

awulkiew · 2021-05-31T17:46:14Z

@barendgehrels I made the PR more complete. I added the list of changes in the description.

…the compiler. Variadic templates can be disabled in Boost.Variant by defining BOOST_VARIANT_DO_NOT_USE_VARIADIC_TEMPLATES. Also support DG and GC in: - comparable_distance_result - default_length_result - distance_result

…ngth().

barendgehrels

Thanks!

include/boost/geometry/geometries/adapted/std_any.hpp

include/boost/geometry/geometries/adapted/boost_variant2.hpp

Add missing curly braces. Remove unneeded comments.

vissarion

Thanks and sorry for late reviewing, I am OK for merging.

include/boost/geometry/algorithms/area_result.hpp

include/boost/geometry/algorithms/append.hpp

include/boost/geometry/strategies/default_length_result.hpp

include/boost/geometry/algorithms/detail/select_geometry_type.hpp

include/boost/geometry/strategies/distance_result.hpp

test/algorithms/clear_multi.cpp

test/algorithms/disjoint/test_disjoint.hpp

…tegies.

awulkiew · 2021-06-03T14:04:32Z

Thanks for the reviews!

awulkiew added 2 commits May 15, 2021 13:21

Support rvalue refs in range utils and use it.

50d1bfe

Add DynamicGeometry and GeometryCollection.

61d25a3

awulkiew added the enhancement label May 17, 2021

awulkiew linked an issue May 17, 2021 that may be closed by this pull request

Proposal: GeometryCollection, DynamicGeometry #843

Closed

[algorithms] Reimplement variant support using DynamicGeometry concep…

52c2723

…t in several algorithms. Also add basic support for GeometryCollection.

awulkiew force-pushed the feature/gc branch from ff8fae9 to 52c2723 Compare May 18, 2021 23:12

awulkiew added 2 commits May 20, 2021 00:42

[io][util] Add support for DynamicGeometry in read_wkt()

a1ccbcd

Replace variant support in wkt write. Add util::sequence_find_if.

[io] Add support for GeometryCollection in WKT read and write.

a5c8258

awulkiew force-pushed the feature/gc branch from d86821c to a5c8258 Compare May 21, 2021 00:00

awulkiew requested review from barendgehrels and vissarion May 24, 2021 19:25

awulkiew added this to the 1.77 milestone May 24, 2021

awulkiew requested a review from mloskot May 24, 2021 19:26

mloskot reviewed May 24, 2021

View reviewed changes

include/boost/geometry/algorithms/detail/disjoint/interface.hpp Outdated Show resolved Hide resolved

mloskot reviewed May 24, 2021

View reviewed changes

include/boost/geometry/algorithms/visit.hpp Outdated Show resolved Hide resolved

mloskot approved these changes May 24, 2021

View reviewed changes

barendgehrels requested changes May 26, 2021

View reviewed changes

awulkiew added 6 commits May 28, 2021 01:48

Improve visit traits and algorithms.

667471e

Add support for rvalue references in visit traits and algorithms. Move visit algorithms to detail namespace. Rename visit_iterator trait to iter_visit. Add tests for visit traits and algorithms. Remove unneeded comments.

Minor changes related to DG and GC support.

d523cb9

Remove unneeded comments. Replace typedef with using. Change names and formatting.

Merge branch 'develop' into feature/gc

c7ae87d

Move algorithms/visit.hpp to algorithms/detail/visit.hpp

251f7d1

Reenable 2-argument visit specializations (disabled for test purposes).

d690364

Add DynamicGeometry adaptation for std::variant.

6e71295

awulkiew force-pushed the feature/gc branch from 07dde60 to 5e6e76e Compare May 29, 2021 00:37

awulkiew added 2 commits May 29, 2021 23:09

Add DynamicGeometry adaptation for std::any.

9849f23

[io] Bring back support of proxy interior rings (e.g. Boost.Polygon) …

a8db924

…in wkt read.

awulkiew force-pushed the feature/gc branch from 5e6e76e to a8db924 Compare May 29, 2021 23:25

[concepts] Implement DynamicGeometry and GeometryCollection concepts.

ef89b4c

Add concepts::concept_type<> utility for generic concept definitions. Replace tag dispatching in concepts::check() with concepts::concept_type<>.

awulkiew force-pushed the feature/gc branch from 66693b6 to 7adb72e Compare June 1, 2021 13:31

[length] Add support for DynamicGeometry and GeometryCollection in le…

e966bdd

…ngth().

barendgehrels approved these changes Jun 2, 2021

View reviewed changes

include/boost/geometry/geometries/adapted/std_any.hpp Show resolved Hide resolved

include/boost/geometry/geometries/adapted/boost_variant2.hpp Outdated Show resolved Hide resolved

awulkiew added 4 commits June 2, 2021 13:54

Merge branch 'develop' into feature/gc

4c84757

Replace typedef with using in trait specializations for DGs and GC.

fd32e7b

Add missing curly braces. Remove unneeded comments.

Check all subgeometry types in DG and GC concept checks.

620e02f

[clear] Add support for GeometryCollection in clear().

b1b29ec

vissarion approved these changes Jun 3, 2021

View reviewed changes

awulkiew added 2 commits June 3, 2021 16:03

[append] Replace typedefs with using.

bac1e43

[test][disjoint] Add cases for DG and GC with explicitly defined stra…

c9edabe

…tegies.

awulkiew merged commit e1571b3 into boostorg:develop Jun 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DynamicGeometry and GeometryCollection. #850

Add DynamicGeometry and GeometryCollection. #850

awulkiew commented May 17, 2021 •

edited

mloskot left a comment

awulkiew commented May 24, 2021

mloskot commented May 25, 2021 •

edited

barendgehrels left a comment

awulkiew commented May 28, 2021

awulkiew commented May 31, 2021

barendgehrels left a comment

vissarion left a comment

awulkiew commented Jun 3, 2021

Add DynamicGeometry and GeometryCollection. #850

Add DynamicGeometry and GeometryCollection. #850

Conversation

awulkiew commented May 17, 2021 • edited

mloskot left a comment

Choose a reason for hiding this comment

awulkiew commented May 24, 2021

mloskot commented May 25, 2021 • edited

barendgehrels left a comment

Choose a reason for hiding this comment

awulkiew commented May 28, 2021

awulkiew commented May 31, 2021

barendgehrels left a comment

Choose a reason for hiding this comment

vissarion left a comment

Choose a reason for hiding this comment

awulkiew commented Jun 3, 2021

awulkiew commented May 17, 2021 •

edited

mloskot commented May 25, 2021 •

edited