Move ctor or assignment for boost::geometry::model::ring?

[Volker Schöch <vschoech@…>]

Hi, I have this clumsy piece of code:

if( !vecpt.empty() ) {
       emplace_back( polygon_type() );
       static_cast< std::vector< _TPoint< T > >& >( back().outer() ) = std::move(vecpt);
}

which actually should be written like this:

boost::geometry::convert( polygon_type::ring_type( std::move(vecpt) ), *this );

but unfortunately this kind of conversion does not work. Did I make a mistake in my above statement, or is this not (yet) supported? If the latter, do you plan on adding support for this at some point? Thanks!

Volker

[Volker Schöch <vschoech@…>]

Some additional information. *this is a multi-polygon with the following definition:

template<typename T>
class _TPolygon :
	public std::vector< boost::geometry::model::polygon< _TPoint< T >, /*ClockWise*/false, /*Closed*/false > >,
	boost::equality_comparable< _TPolygon<T>,
	boost::additive< _TPolygon<T>, _TSize<T> > >
{ ... }

namespace boost { namespace geometry { namespace traits {

	template<typename T>
	struct tag< _TPolygon<T> >
	{
		typedef multi_polygon_tag type;
	};

} } } // namespace boost::geometry::traits

The polygon definition is based on our _TPoint<T> type which is declared for use with boost::geometry as follows:

namespace boost { namespace geometry { namespace traits {

	template <typename T>
	struct tag< _TPoint<T> >
	{
		typedef point_tag type;
	};

	template <typename T>
	struct coordinate_type< _TPoint<T> >
	{
		typedef T type;
	};

	template <typename T>
	struct coordinate_system< _TPoint<T> >
	{
		typedef cs::cartesian type;
	};


	template <typename T>
	struct dimension< _TPoint<T> >
		: boost::mpl::int_<2>
	{};

	template <typename T, std::size_t Dimension>
	struct access< _TPoint<T>, Dimension >
	{
		static inline T get(_TPoint<T> const& p)
		{
			return p[(EOrientation)Dimension];
		}

		static inline void set(_TPoint<T>& p, T const& value)
		{
			p[(EOrientation)Dimension] = value;
		}
	};

} } } // namespace boost::geometry::traits

Similarly, we have our own _TRect<T> type which we plug into boost::geometry:

namespace boost { namespace geometry { namespace traits {

	template <typename T>
	struct tag< _TRect<T> >
	{
		typedef box_tag type;
	};

	template <typename T>
	struct point_type< _TRect<T> >
	{
		typedef _TPoint<T> type;
	};

	template <typename T, std::size_t Dimension>
	struct indexed_access< _TRect<T>, min_corner, Dimension >
	{
		typedef typename geometry::coordinate_type< _TPoint<T> >::type coordinate_type;

		static inline coordinate_type get(_TRect<T> const& rect)
		{
			return geometry::get<Dimension>( rect.TopLeft() );
		}

		static inline void set(_TRect<T>& rect, coordinate_type const& value)
		{
			geometry::set<Dimension>( rect.TopLeft(), value );
		}
	};

	template <typename T, std::size_t Dimension>
	struct indexed_access< _TRect<T>, max_corner, Dimension >
	{
		typedef typename geometry::coordinate_type< _TPoint<T> >::type coordinate_type;

		static inline coordinate_type get(_TRect<T> const& rect)
		{
			return geometry::get<Dimension>( rect.BottomRight() );
		}

		static inline void set(_TRect<T>& rect, coordinate_type const& value)
		{
			geometry::set<Dimension>( rect.BottomRight(), value );
		}
	};

} } } // namespace boost::geometry::traits

In our code, these classes are instantiated with template parameter T being either int or double. We have a lot of use for int-based geometry calculations.

With these geometry types we successfully call many algorithms from boost::geometry, e.g.,

template<typename T>
_TPolygon< T >::_TPolygon(_TRect< T > const& rect)
{
	boost::geometry::convert( rect, *this );
}

but the following does not seem to work (as of boost 1.52.0):

template<typename T>
_TPolygon< T >::_TPolygon(std::vector< _TPoint< T > > vecpt)
{
	// vecpt:
	// - No need to add end==begin point.
	// - Orientation is mathematically counter-clockwise (i.e., clockwise with the
	//   origin in the upper left as is the case with screen or slide coordinates).
	// - No self-intersection allowed!
	if( !vecpt.empty() ) {
		boost::geometry::convert( polygon_type::ring_type( std::move(vecpt) ), *this );
	}
}

[Mateusz Loskot]

Barend,

The requested move semantic is one issue, but unless I'm missing something, is the ring model supposed to be constructible directly from compatible container of compatible points? The extension constructor in ring takes iterators only, not container.

Obviously, this does not compile:

#include <vector>
#include <boost/geometry.hpp>
#include <boost/geometry/geometries/ring.hpp>
#include <boost/geometry/geometries/point_xy.hpp>

int main()
{
    typedef boost::geometry::model::d2::point_xy<double> point;
    typedef boost::geometry::model::ring<point> ring;


    std::vector<point> r1;
    ring r2;
    boost::geometry::convert(r1, r2);

    return 0;
}

Shall I extend the ring to make it constructible from the container too?

[Barend Gehrels]

OK for me to add a constructor with a container with its point types. It was not there because vector/deque do not have it neither.

Does this solve the whole issue? In the reported problem, std::move seems not necessary in the call to convert...

[Volker Schöch <vschoech@…>]

Replying to mloskot:

The requested move semantic is one issue, but unless I'm missing something, is the ring model supposed to be constructible directly from compatible container of compatible points? The extension constructor in ring takes iterators only, not container.

Replying to barendgehrels:

OK for me to add a constructor with a container with its point types. It was not there because vector/deque do not have it neither.

Although to me this seems to be a desirable extension, I am not saying that this is the only way to solve the issue. Leaving constructors alone, you could alternatively overload the convert algorithm to support conversion from vector-of-points to (Multi-)Polygon:

boost::geometry::convert( std::move(vecpt), *this );

My suggestion to call convert by way of constructing a ring_type first was merely based on my observation that convert already supports conversion from Ring to Polygon.

Replying to barendgehrels:

Does this solve the whole issue?

There are two more niceties I would like to suggest to solve the whole issue:

1. In boost 1.52.0, convert supports Ring to Polygon conversion, but does not support (any Geometry that supports conversion to Polygon) to Multi-Polygon conversion. Is this by design, or is this considered a missing feature?

2. While we are at it, is it intentional that I have to guard the conversion with "!vecpt.empty()"? I don't see a problem with creating a Ring from an empty vector-of-points and convert that to a Polygon that consist of empty vectors.

Replying to barendgehrels:

In the reported problem, std::move seems not necessary in the call to convert...

Can you explain why you think std::move is unnecessary there? It may in fact be useless if there is any link in the chain of calls that does not support move semantics, but from my perspective I should still put it there to enable move semantics where it is supported. There seems to be consensus that ​the compiler must not automatically turn a copy into a move. Thus I think I have to be explicit about my intent to use move semantics.

[Volker Schöch <vschoech@…>]

As far as I understand, I still cannot write an expression like this in 1.57.0:

boost::geometry::convert( polygon_type::ring_type( tc_move(vecpt) ), *this );

I'm not saying that this is a major problem or anything, just updating the ticket.

[vschoech@…]

This is still true in 1.59.0.