Quantcast

box_range is not a valid range

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

box_range is not a valid range

feverzsj
hi, list
while using box_range with boost.range, it just won't pass the concept check
of boost.range for the box_iterator not being default constructible.

regards, zsj

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
Hi,

> while using box_range with boost.range, it just won't pass the concept
> check of boost.range for the box_iterator not being default
> constructible.

Thanks for the report. Indeed, fixed it (for segment_range it was
already done). Fixed in Trunk.

Regards, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

feverzsj
hi, Barend

>Thanks for the report. Indeed, fixed it (for segment_range it was already
>done). Fixed in Trunk.

Confirmed. Thanks.

While wokring out my own box2d_range/box2d_iterator to work around this, I
found ggl's box_iterator is not so lightwigth to be a iterator, fot it
carrying 4 points everywhere.
It may be better to store the 4 points in the box_range, and only store a
pointer to these points and a index in box_iterator. Similar solution also
applies to segment_range/segment_range_iterator.

regards, zsj

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
Hi,


> While wokring out my own box2d_range/box2d_iterator to work around
> this, I found ggl's box_iterator is not so lightwigth to be a
> iterator, fot it carrying 4 points everywhere.
> It may be better to store the 4 points in the box_range, and only
> store a pointer to these points and a index in box_iterator. Similar
> solution also applies to segment_range/segment_range_iterator.

Right, it is indeed storing the points because it has to return them.
Alternatively it can do what you suggest (making the iterator dependant
on range) or create them on the fly (having to store a reference to the
box inside, seems OK, it is there already in the form of an address).
Making the iterator dependant on the range is not a big problem because
(I think) there is no big need to use it outside.

Did you work out your suggestion in your solution? If so, could you send
it such that we might replace it?

Thanks, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

feverzsj
hi,Barend
 
> Did you work out your suggestion in your solution? If so, could you send
> it such that we might replace it?

That's fairly easy. Almost just copy and paste code of box_rang/box_iterator. I've also removed some unnecessary test since it seems iterator is always compared with other iterator of same range.


namespace ggl = boost::geometry;

template<typename Box>
class box2d_range
{
public :
    typedef typename ggl::point_type<Box>::type point_type;

    struct box2d_iterator
        : public boost::iterator_facade<
            box2d_iterator,
            point_type const,  // read only
            boost::random_access_traversal_tag
        >
    {
        // begin iterator
        box2d_iterator(point_type const* p)
            : m_points(p), m_index(0) {}

        // end iterator
        box2d_iterator()
            : m_points(0), m_index(5) {}

        typedef std::ptrdiff_t difference_type;

    private:
        friend class boost::iterator_core_access;

        point_type const& dereference() const
        {
            assert(m_points);

            if (m_index >= 0 && m_index <= 3)
            {
                return m_points[m_index];
            }
            // If it index is 4, or other, return first point
            return m_points[0];
        }

        bool equal(box2d_iterator const& other) const
        {
            return other.m_index == m_index;
        }

        void increment()
        {
            m_index++;
        }

        void decrement()
        {
            m_index--;
        }

        void advance(difference_type n)
        {
            m_index += n;
        }

        difference_type distance_to(box2d_iterator const& other) const
        {
            return other.m_index - m_index;
        }

        int m_index;
        point_type const* m_points;
    };

    typedef box2d_iterator const_iterator;
    typedef box2d_iterator iterator; // must be defined

    explicit box2d_range(Box const& box)
    {
        init(box);
    }

    const_iterator begin() const { return const_iterator(m_points); }
    const_iterator end() const { return const_iterator(); }

    // It may not be used non-const, so comment this:
    //iterator begin() { return m_begin; }
    //iterator end() { return m_end; }

    void init(Box const& box)
    {
        ggl::detail::assign_box_corners_oriented<false>(box, m_points);
    }

private :
    point_type m_points[4];
};
_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
Hi,
> That's fairly easy. Almost just copy and paste code of box_rang/box_iterator. I've also removed some unnecessary test since it seems iterator is always compared with other iterator of same range.

Thanks for your code. I didn't understand before that you meant making
an inner class of the iterator. Good idea of course.

We're using the box_range internally, and not the iterator. Same for
segment and some others (e.g. closing_iterator). I don't expect the
iterator alone to be important for users, because if you have a range,
you automatically have the iterator functionality.

So we might create private classes of them. Do people agree?

Regards, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
In reply to this post by feverzsj
On 3-5-2011 3:37, zzsj wrote:
> hi,Barend
>
>> Did you work out your suggestion in your solution? If so, could you send
>> it such that we might replace it?
> That's fairly easy. Almost just copy and paste code of box_rang/box_iterator. I've also removed some unnecessary test since it seems iterator is always compared with other iterator of same range.

Thanks again for this - I decided to adopt it, and then processed it a
bit and merged the implementation (now in views/detail/points_view) with
segment_range because they were so similar. And then renamed them to
views, because that was the general idea but not yet done for this. The
iterators are now private for the ranges, it is not the idea to publish
them separately as ranges will fullfil the needs (and can be iterated).
I had to put one thing back for the unit-test, the end-iterator need to
get a pointer to the points as well, to be able to go to end()-1 (think
this is not a common use-case but std::vector works like that, and it
worked like that before).

So views/box_view.hpp and views/segment_view contain the refactored
implementations. Additionally the box_view can now also be defined
counterclockwise (to be specified with an optional template parameter)

If you want you can send me your name to add as a contributor

Regards, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

feverzsj
hi, Barend

> Thanks again for this - I decided to adopt it, and then processed it a
> bit and merged the implementation (now in views/detail/points_view) with
> segment_range because they were so similar. And then renamed them to
> views, because that was the general idea but not yet done for this. The
> iterators are now private for the ranges, it is not the idea to publish
> them separately as ranges will fullfil the needs (and can be iterated).
> I had to put one thing back for the unit-test, the end-iterator need to
> get a pointer to the points as well, to be able to go to end()-1 (think
> this is not a common use-case but std::vector works like that, and it
> worked like that before).
>
> So views/box_view.hpp and views/segment_view contain the refactored
> implementations. Additionally the box_view can now also be defined
> counterclockwise (to be specified with an optional template parameter)
Yes, I've seen the changes. The points_view/copy_policy is a good idea for things like box and segment, except the copy_policy seems to make extra copy of the box/segment while being constructed. Is it intended?

Also, now that box_view is of ring type, why it cannot be an opened ring?
The view concept is a kinda confusing concept. It mixes the range and geometry concepts.
I think the view concept should follow the range concept.
As mentioned in "[ggl] transform linestring into a ring", I found two geometry wrappers very useful in many aspects. Here they are:

// warp X as linestring
// X : could be ring, range
// CloseOpenedRing : close presumed opened ring if set to true, otherwise do nothing
template<class X, bool CloseOpenedRing = true>
struct linestring_wrapper;

// warp X as ring
// X: could be linestring, range
template<class X, bool ClockWise = true, bool Closed = false>
class ring_wrapper;

// convenient generators
as_linestring(X);
as_ring(X);


now, a box_view bv could be either as_linestring(bv) or as_ring(bv), the view concept is then decoupled from geometry concept.
of course, these wrappers could also handle box directly with box_view (since box is ring, we need to know if it is closed/clockwise, so box_view should take care both closure and winding and generate proper range).



regards, ZhouShuangJiang
_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
>
> Yes, I've seen the changes. The points_view/copy_policy is a good idea for things like box and segment, except the copy_policy seems to make extra copy of the box/segment while being constructed. Is it intended?


No, oops, must be by accident. Will be solved.


>
> Also, now that box_view is of ring type, why it cannot be an opened ring?
> The view concept is a kinda confusing concept. It mixes the range and geometry concepts.
> I think the view concept should follow the range concept.
> As mentioned in "[ggl] transform linestring into a ring", I found two geometry wrappers very useful in many aspects. Here they are:
>
> // warp X as linestring
> // X : could be ring, range
> // CloseOpenedRing : close presumed opened ring if set to true, otherwise do nothing
> template<class X, bool CloseOpenedRing = true>
> struct linestring_wrapper;
>
> // warp X as ring
> // X: could be linestring, range
> template<class X, bool ClockWise = true, bool Closed = false>
> class ring_wrapper;
>
> // convenient generators
> as_linestring(X);
> as_ring(X);
>
>
> now, a box_view bv could be either as_linestring(bv) or as_ring(bv), the view concept is then decoupled from geometry concept.
> of course, these wrappers could also handle box directly with box_view (since box is ring, we need to know if it is closed/clockwise, so box_view should take care both closure and winding and generate proper range).

Right, very good points. The general idea is like this, create a view_as function returning the geometry as another concept. Luke has mentioned this on this list as well, and Bruno seconded this idea, and I agree. So box_view is only the first step. view_as<ring_tag>(box) would replace (or make use of) it, and things as orientation and closure must be optionally specified somehow. view_as<polygon_tag>(ring) would make, for example, a polygon from a ring, and things can also be seen as linestring, multi-versions, etc.
However, that will not be included for 1.47

Regards, Barend

>

Sent from iPad.
Barend Gehrels
www.barendgehrels.nl_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
In reply to this post by feverzsj
Hi ZhouShuangJiang,


> Yes, I've seen the changes. The points_view/copy_policy is a good idea for things like box and segment, except the copy_policy seems to make extra copy of the box/segment while being constructed. Is it intended?

Fixed now.

Besides this I also fixed/implemented the point_order on a box_view, if
you specify Reverse it is counter clockwise, as expected. Closure might
complete it (but requires another copy-policy and an adapted approach).

> Also, now that box_view is of ring type, why it cannot be an opened ring?
> The view concept is a kinda confusing concept. It mixes the range and geometry concepts.
> I think the view concept should follow the range concept.
> As mentioned in "[ggl] transform linestring into a ring", I found two geometry wrappers very useful in many aspects. Here they are:

OK, we will think / discuss about this.


Thanks, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

feverzsj
hi, Barend

> Besides this I also fixed/implemented the point_order on a box_view, if
> you specify Reverse it is counter clockwise, as expected. Closure might
> complete it (but requires another copy-policy and an adapted approach).


In the old solution, Closure could be simply achieved by setting the end index. For example:

template<typename Box, bool ClockWise = true, bool Closed = true>
class box2d_range
{
    ...

    const_iterator begin() const { return const_iterator(m_points, 0); }
    const_iterator end() const { return const_iterator(m_points, 4 + Closed); }

    void init(Box const& box)
    {
        ggl::detail::assign_box_corners_oriented<!ClockWise>(box, m_points);
    }

private :
    point_type m_points[4];
}


This could be also possible for points_view.


>> Also, now that box_view is of ring type, why it cannot be an opened ring?
>> The view concept is a kinda confusing concept. It mixes the range and geometry concepts.
>> I think the view concept should follow the range concept.
>> As mentioned in "[ggl] transform linestring into a ring", I found two geometry wrappers very useful in many aspects. Here they are:
>
> OK, we will think / discuss about this.

Thanks. I've attached my ggl helpers. It includes the two wrappers(support box with box2d_range), and some useful functions.


regards, ZhouShuangJiang
_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl

ggl_helpers.h (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: box_range is not a valid range

Barend
Hi ZhouShuangJiang,

> In the old solution, Closure could be simply achieved by setting the end index. For example:
>
> (...)
>
> This could be also possible for points_view.
Sure, I think so. Will do and thanks for the helpers! Will have a close
look soon.

Regards, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl
Loading...