[Olena-patches] 1765: Fix bgraph_image construction bug.

https://svn.lrde.epita.fr/svn/oln/trunk/milena Index: ChangeLog from Nicolas Ballas &lt;ballas(a)lrde.epita.fr&gt; Fix bgraph_image construction bug. * tests/core/bgraph_image.cc, * tests/core/p_bgraph.cc: update the tests. * mln/core/p_bgraph_piter.hh, * mln/core/bgraph_image.hh, * mln/core/p_bgraph.hh, * mln/core/bgraph_psite.hh: fix the bug (thanks to Theo). mln/core/bgraph_image.hh | 29 ++++++++++------------------- mln/core/bgraph_psite.hh | 4 +--- mln/core/p_bgraph.hh | 37 ++++++++++++++++++------------------- mln/core/p_bgraph_piter.hh | 22 ++++++++++++---------- tests/core/bgraph_image.cc | 23 ++++++++++++++++++----- tests/core/p_bgraph.cc | 2 +- 6 files changed, 60 insertions(+), 57 deletions(-) Index: tests/core/bgraph_image.cc --- tests/core/bgraph_image.cc (revision 1764) +++ tests/core/bgraph_image.cc (working copy) @@ -29,8 +29,13 @@ /// \brief Tests on mln::bgraph_image. #include <vector> -#include <mln/core/point2d.hh> + +#include <mln/value/int_u8.hh> +#include <mln/core/image2d.hh> #include <mln/core/bgraph_image.hh> +#include <mln/level/fill.hh> +#include <mln/level/paste.hh> +#include <mln/debug/println.hh> @@ -56,7 +61,7 @@ const int num_edges = sizeof(edge_array)/sizeof(edge_array[0]); // declare a graph object - Graph g(num_vertices); + Graph& g = *new Graph(num_vertices); // add the edges to the graph object for (int i = 0; i < num_edges; ++i) @@ -85,10 +90,18 @@ values.push_back(4); values.push_back(5); - ima_type ima(g, values); + ima_type ima(&g, values); mln_piter_(ima_type) p(ima.domain()); for_all(p) - std::cout << ima(p) << std::endl; + std::cout << p << ' ' << ima(p) << std::endl; + + { + using value::int_u8; + image2d<int_u8> ima_(ima.bbox()); + level::fill(ima_, 0); + level::paste(ima, ima_); + debug::println(ima_); + } - return 0; + delete &g; } Index: tests/core/p_bgraph.cc --- tests/core/p_bgraph.cc (revision 1764) +++ tests/core/p_bgraph.cc (working copy) @@ -73,7 +73,7 @@ `-----------*/ /// Creation - p_bgraph<point2d> pset(g); + p_bgraph<point2d> pset(&g); /// Iterator p_bgraph_piter_<point2d> p(pset); Index: mln/core/p_bgraph_piter.hh --- mln/core/p_bgraph_piter.hh (revision 1764) +++ mln/core/p_bgraph_piter.hh (working copy) @@ -106,6 +106,7 @@ private: iterators_type_ iterators_; + typename p_bgraph<P>::node_iterator cur_; }; @@ -120,9 +121,7 @@ psite_(pg, pg_.npoints()), p_() { - // Initialize the iterators - start(); - + iterators_ = boost::vertices(pg_.to_graph()); // Invalidate id_. invalidate(); } @@ -140,7 +139,7 @@ bool p_bgraph_piter_<P>::is_valid() const { - return iterators_.first != iterators_.second; + return cur_ != iterators_.second; } template<typename P> @@ -148,7 +147,7 @@ void p_bgraph_piter_<P>::invalidate() { - iterators_.first == iterators_.second; + cur_ = iterators_.second; // Past the end iterator. } template<typename P> @@ -156,8 +155,9 @@ void p_bgraph_piter_<P>::start() { + cur_ = iterators_.first; /// FIXME: Hide implementation details? - iterators_ = boost::vertices(pg_.to_graph()); + if (is_valid()) update_(); } @@ -166,7 +166,8 @@ void p_bgraph_piter_<P>::next_() { - ++iterators_.first; + ++cur_; + if (is_valid()) update_(); } @@ -176,9 +177,9 @@ p_bgraph_piter_<P>::update_() { // Update psite_. - psite_ = bgraph_psite<P>(pg_, *iterators_.first); + psite_ = bgraph_psite<P>(pg_, *cur_); // Update p_. - p_ = pg_.point_from_id(*iterators_.first); + p_ = pg_.point_from_id(*cur_); } template<typename P> @@ -233,7 +234,8 @@ p_bgraph_piter_<P>::operator bgraph_psite<P>() const { mln_precondition(is_valid()); - return psite_; + bgraph_psite<P> tmp(pg_, *cur_); + return tmp; // psite_; } # endif // ! MLN_INCLUDE_ONLY Index: mln/core/bgraph_image.hh --- mln/core/bgraph_image.hh (revision 1764) +++ mln/core/bgraph_image.hh (working copy) @@ -53,15 +53,15 @@ template <typename P, typename V> struct data_< bgraph_image<P, V> > { + /// Data stores a **copy** of the pset g. + /// But, the pset g and the data_ copy will shared the same + /// underlaying graph. data_(const p_bgraph<P>& g, const std::vector<V>& val); + + p_bgraph<P> pg_; + std::vector<V> val_; - /* The graph point set is handled by address, so that we can - check the compatibility of images w.r.t. to their point - sites. We could use a safer (and more complex) facility to - ensure (memory) equality of line graph point sets, but using - addresses is simple and efficient enough for the moment. */ - const p_bgraph<P>& pg_; }; } // end of namespace mln::internal @@ -76,7 +76,6 @@ { typedef trait::image::category::primary category; - // FIXME: Is that right? typedef trait::image::access::random access; typedef typename trait::image::space_from_point<P>::ret space; typedef trait::image::size::regular size; @@ -85,7 +84,6 @@ typedef trait::image::border::none border; typedef trait::image::data::stored data; typedef trait::image::io::read_write io; - // FIXME: Is that right? typedef trait::image::speed::fast speed; }; @@ -150,8 +148,6 @@ const std::vector<V>& node_values() const; /// \} - /* FIXME: Do we want to provide these two methods? (at least, in - the interface of the class? */ /// Return the point of the first node adjacent to the edge with /// id \a e. @@ -191,8 +187,8 @@ inline data_< bgraph_image<P, V> >::data_(const p_bgraph<P>& g, const std::vector<V>& val) - : val_ (val), - pg_ (g) + : pg_ (g), + val_ (val) { } @@ -228,11 +224,6 @@ void bgraph_image<P, V>::init_(const p_bgraph<P>& g, const std::vector<V>& val) { - /* FIXME: We leak memory here: calling init_ twice loses the - previous content pointed by data_. - - We should definitely write down formal guidelines on - initialization and memory management in general! */ this->data_ = new internal::data_< bgraph_image<P, V> > (g, val); } @@ -245,7 +236,7 @@ typename bgraph_image<P, V>::rvalue bgraph_image<P, V>::operator()(const bgraph_psite<P>& p) const { - mln_precondition(&p.pg() == &this->data_->pg_); + mln_precondition(&p.pg().to_graph() == &this->data_->pg_.to_graph()); mln_precondition(p.id() < this->data_->val_.size()); return this->data_->val_[p.id()]; } @@ -255,7 +246,7 @@ typename bgraph_image<P, V>::lvalue bgraph_image<P, V>::operator()(const bgraph_psite<P>& p) { - mln_precondition(&p.pg() == &this->data_->pg_); + mln_precondition(&p.pg().to_graph() == &this->data_->pg_.to_graph()); mln_precondition(p.id() < this->data_->val_.size()); return this->data_->val_[p.id()]; } Index: mln/core/p_bgraph.hh --- mln/core/p_bgraph.hh (revision 1764) +++ mln/core/p_bgraph.hh (working copy) @@ -53,9 +53,6 @@ { typedef util::internal::boost_graph<P, util::empty> graph; - /// Construct a graph psite set from a graph of points. - p_bgraph (graph& gr); - /// Point_Site associated type. typedef bgraph_psite<P> psite; @@ -75,6 +72,11 @@ typedef std::pair<node_iterator, node_iterator> node_iterators; + /// Construct a graph psite set from a graph of points. + /// \p gr is a pointer on a boost graph. + /// This boost graph must have been allocated dynamically. + p_bgraph (graph* gr); + /// Return The number of points (i.e., nodes) in the graph. std::size_t npoints() const; @@ -103,11 +105,7 @@ private: - graph gr_; - // FIXME: (Roland) Is it really useful/needed? - /* 2007-12-19: It seems so, since graph_image must implement a method - named bbox(). Now the question is: should each image type have a - bounding box? */ + graph* gr_; box_<P> bb_; }; @@ -115,17 +113,18 @@ template<typename P> inline - p_bgraph<P>::p_bgraph (typename p_bgraph<P>::graph& gr) + p_bgraph<P>::p_bgraph (typename p_bgraph<P>::graph* gr) : gr_ (gr) { + mln_precondition(gr != 0); // FIXME: Warning: if the underlying graph is updated later, this // won't be taken into account by this p_bgraph! accu::bbox<P> a; - for (node_iterators iter = boost::vertices(gr_); + for (node_iterators iter = boost::vertices(*gr_); iter.first != iter.second; ++iter.first) - a.take(gr_[*iter.first]); + a.take((*gr_)[*iter.first]); bb_ = a.to_result(); } @@ -134,7 +133,7 @@ std::size_t p_bgraph<P>::npoints() const { - return boost::num_vertices((gr_)); + return boost::num_vertices(*gr_); } template<typename P> @@ -142,7 +141,7 @@ std::size_t p_bgraph<P>::nlines() const { - return boost::num_edges(gr_.nedges()); + return boost::num_edges(gr_->nedges()); } template<typename P> @@ -170,14 +169,14 @@ const P& p_bgraph<P>::point_from_id(const typename p_bgraph<P>::node_id& id) const { - return this->gr_[id]; + return this->gr_->operator[](id); } template <typename P> P& p_bgraph<P>::point_from_id(const typename p_bgraph<P>::node_id& id) { - return this->gr_[id]; + return this->gr_->operator[](id); } @@ -187,7 +186,7 @@ const P& p_bgraph<P>::node1(const typename p_bgraph<P>::edge_id& e) const { - return this->point_from_id(source(e, this->gr_)); + return this->point_from_id(source(e, *this->gr_)); } /// FIXME: same as node1... @@ -195,21 +194,21 @@ const P& p_bgraph<P>::node2(const typename p_bgraph<P>::edge_id& e) const { - return this->point_from_id(target(e, this->gr_)); + return this->point_from_id(target(e, *this->gr_)); } template <typename P> const typename p_bgraph<P>::graph& p_bgraph<P>::to_graph() const { - return this->gr_; + return *this->gr_; } template <typename P> typename p_bgraph<P>::graph& p_bgraph<P>::to_graph() { - return this->gr_; + return *this->gr_; } Index: mln/core/bgraph_psite.hh --- mln/core/bgraph_psite.hh (revision 1764) +++ mln/core/bgraph_psite.hh (working copy) @@ -85,8 +85,6 @@ typename p_bgraph<P>::node_id id_; }; - - # ifndef MLN_INCLUDE_ONLY template<typename P> @@ -118,7 +116,7 @@ return *this; // Assigning a psite from a graph point set to a psite from // another graph point set is meaningless. - mln_assertion(&pg_ == &rhs.pg_); + mln_assertion(&pg_.to_graph() == &rhs.pg_.to_graph()); id_ = rhs.id_; return *this; }