[Olena-patches] 1769: Fix mln::line_graph_image<P> initializations and associated entitites.

https://svn.lrde.epita.fr/svn/oln/trunk/milena Index: ChangeLog from Roland Levillain &lt;roland(a)lrde.epita.fr&gt; Fix mln::line_graph_image<P> initializations and associated entitites. * mln/core/line_graph_image.hh: Add some more documentation. (mln::init_(tag::image_t, line_graph_image<P,V>&, const line_graph_image<P,V>): Turn the signature of this routine into... (mln::init_(tag::image_t, line_graph_image<P,V>&, const line_graph_image<P,W>): ...this. Don't copy values from the model. Instead, initialize with a vector of the same size. (mln::line_graph_image<P, V>::line_graph_image(const p_line_graph<P>&)): s/g.npoints()/g.nnodes()/. s/g.nlines()/g.nedges()/. (mln::line_graph_image<P, V>::init_): Ensure there are no data before initializing new ones. * mln/core/internal/image_morpher.hh: Remove spurious FIXMEs. * mln/core/line_graph_psite.hh (mln::line_graph_psite<P>::point): Set typedef to P. (mln::line_graph_psite<P>::operator[]): Adjust return type. (mln::line_graph_psite<P>::line_graph_psite): New ctor. (mln::line_graph_psite<P>::plg_): Change the type of this member from const p_line_graph<P>& to const p_line_graph<P>*. (line_graph_psite(const p_line_graph<P>&, util::node_id)) (mln::line_graph_psite<P>::operator= (const mln::line_graph_psite<P>&)) (mln::line_graph_psite<P>::to_point) (line_graph_psite<P>::is_valid_): Adjust. (mln::line_graph_psite<P>::plg): Likewise: ensure member plg_ is not null. (mln::line_graph_psite<P>::id): Fix return type. (mln::line_graph_psite<P>::first) (mln::line_graph_psite<P>::second): New accessors. (mln::line_graph_psite<P>::update_): Remove. (mln::line_graph_psite<P>::operator P): Remove. (operator<<(std::ostream&, const line_graph_psite<P>&)): Adjust implementation accordingly. (operator==(const line_graph_psite<P>&, const line_graph_psite<P>&)): New. internal/image_morpher.hh | 4 - line_graph_image.hh | 36 ++++++--------- line_graph_psite.hh | 106 +++++++++++++++++++++++++++++----------------- 3 files changed, 82 insertions(+), 64 deletions(-) Index: mln/core/line_graph_image.hh --- mln/core/line_graph_image.hh (revision 1768) +++ mln/core/line_graph_image.hh (working copy) @@ -107,21 +107,22 @@ } // end of namespace mln::trait - /*! \brief Kind of image based on graph structure. - * - */ + + /// \brief Image based on a graph. + /// + /// Values are stored on the vertices of the graph. template <typename P, typename V> struct line_graph_image : public internal::image_primary_< p_line_graph<P>, line_graph_image<P, V> > { - + /// Super type. typedef mln::internal::image_base_< p_line_graph<P>, line_graph_image<P, V> > super_; /// Value associated type. typedef V value; - /// Return type of read-write access. + /// \brief Return type of read-write access. /// /// We use the associated type \c reference instead of a plain /// reference on th value type (\v V), because it's the only way @@ -135,7 +136,6 @@ /// Value set associated type. typedef mln::value::set<value> vset; - /// Skeleton. typedef line_graph_image< tag::psite_<P>, tag::value_<V> > skeleton; @@ -184,10 +184,10 @@ }; // Fwd decl. - template <typename P, typename V> + template <typename P, typename V, typename W> void init_(tag::image_t, line_graph_image<P, V>& target, - const line_graph_image<P, V>& model); + const line_graph_image<P, W>& model); # ifndef MLN_INCLUDE_ONLY @@ -196,17 +196,15 @@ | Initialization. | `-----------------*/ - /* FIXME: We should check the expected behavior of init_ here. As - far as I (Roland) can recall, it was not meant to initialize - data/values. We should probably just create vectors of values of - the same size, not copy the values. */ - template <typename P, typename V> + template <typename P, typename V, typename W> inline void init_(tag::image_t, line_graph_image<P, V>& target, - const line_graph_image<P, V>& model) + const line_graph_image<P, W>& model) { - target.init_(model.domain(), model.node_values(), model.edge_values()); + target.init_(model.domain(), + std::vector<V>(model.node_values().size()), + std::vector<V>(model.edge_values().size())); } /*-------. @@ -242,7 +240,7 @@ inline line_graph_image<P, V>::line_graph_image(const p_line_graph<P>& g) { - init_(g, std::vector<V>(g.npoints()), std::vector<V>(g.nlines())); + init_(g, std::vector<V>(g.nnodes()), std::vector<V>(g.nedges())); } template <typename P, typename V> @@ -261,11 +259,7 @@ const std::vector<V>& node_val, const std::vector<V>& edge_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! */ + mln_precondition(! this->has_data()); this->data_ = new internal::data_< line_graph_image<P, V> >(g, node_val, edge_val); } Index: mln/core/internal/image_morpher.hh --- mln/core/internal/image_morpher.hh (revision 1768) +++ mln/core/internal/image_morpher.hh (working copy) @@ -55,13 +55,9 @@ typedef I delegatee; - /* FIXME: Change the interface of this method: return a reference - instead of a pointer. */ /// Return the delegatee_ pointer; default code. mlc_const(I)* delegatee_() const; - /* FIXME: Change the interface of this method: return a reference - instead of a pointer. */ /// Return the delegatee_ pointer (non-const version); default code. I* delegatee_(); Index: mln/core/line_graph_psite.hh --- mln/core/line_graph_psite.hh (revision 1768) +++ mln/core/line_graph_psite.hh (working copy) @@ -32,7 +32,6 @@ /// \brief Definition of a line graph-based point site. # include <mln/core/p_line_graph.hh> -# include <mln/core/point_pair.hh> /* FIXME: This class shares a lot with graph_psite. Factor as much as possible. */ @@ -52,7 +51,7 @@ typedef line_graph_psite<P> self_; public: - typedef point_pair<P> point; + typedef P point; typedef mln_mesh(point) mesh; enum { dim = point::dim }; typedef mln_dpoint(point) dpoint; @@ -60,9 +59,9 @@ /// Construction and assignment. /// \{ - line_graph_psite(const p_line_graph<P>& plg_, unsigned id); + line_graph_psite(); + line_graph_psite(const p_line_graph<P>& plg, unsigned id); line_graph_psite(const self_& rhs); - /// \pre This psite must have the same graph point set as \a rhs. self_& operator= (const self_& rhs); /// \} @@ -71,25 +70,27 @@ /// Access to point. /// \{ - operator point() const; const point& to_point() const; coord operator[](unsigned id) const; /// \} /// Return the mln::p_line_graph this point site belongs to. const p_line_graph<P>& plg() const; - /// Return the node id of this point site. - util::node_id id() const; + /// Return the edge id of this point site. + util::edge_id id() const; + + /// Return the first associated point (vertex). + P first() const; + /// Return the second associated point (vertex). + P second() const; private: - /// Update \a p_. - void update_(); /// Is this psite valid? bool is_valid_() const; private: /// The p_line_graph this point site belongs to. - const p_line_graph<P>& plg_; + const p_line_graph<P>* plg_; /// The id of the edge this psite is pointing towards. util::edge_id id_; /** \brief The point associated to this psite. @@ -101,9 +102,17 @@ since points associated to edges are computed on the fly (storing them in the graph could be possible, but too costly in space). */ + // FIXME: Actually, this is a dummy value! point p_; }; + /// Compare two mln::line_graph_psite<P> instances. + /* FIXME: Shouldn't this comparison be part of a much general + mechanism? */ + template <typename P> + bool + operator==(const line_graph_psite<P>& lhs, const line_graph_psite<P>& rhs); + /* FIXME: This hand-made delegation is painful. We should rely on the general mechanism provided by Point_Site. But then again, we need to refine/adjust the interface of Point_Site w.r.t. the @@ -123,9 +132,19 @@ template<typename P> inline - line_graph_psite<P>::line_graph_psite(const p_line_graph<P>& g, + line_graph_psite<P>::line_graph_psite() + // Dummy initializations. + : plg_(0), + id_(-1), + p_() + { + } + + template<typename P> + inline + line_graph_psite<P>::line_graph_psite(const p_line_graph<P>& plg, util::edge_id id) - : plg_(g), + : plg_(&plg), id_(id), p_() { @@ -135,7 +154,8 @@ inline line_graph_psite<P>::line_graph_psite(const line_graph_psite<P>& rhs) : plg_(rhs.plg_), - id_(rhs.id_) + id_(rhs.id_), + p_() { } @@ -146,28 +166,17 @@ { if (&rhs == this) return *this; - // Assigning a psite from a line graph point set to a psite from - // another line graph point set is meaningless. - mln_assertion(&plg_ == &rhs.plg_); + plg_ = rhs.plg_; id_ = rhs.id_; - update_(); return *this; } template<typename P> inline - line_graph_psite<P>::operator point_pair<P>() const - { - mln_assertion (is_valid_()); - return p_; - } - - template<typename P> - inline bool line_graph_psite<P>::is_valid_() const { - return id_ < plg_.gr_.nedges(); + return plg_ && id_ < plg_->gr_.nedges(); } template<typename P> @@ -180,15 +189,16 @@ template<typename P> inline - const point_pair<P>& + const P& line_graph_psite<P>::to_point() const { + // Dummy value. return p_; } template<typename P> inline - mln_coord(point_pair<P>) + mln_coord(P) line_graph_psite<P>::operator[](unsigned i) const { mln_assertion (is_valid_()); @@ -197,19 +207,11 @@ template<typename P> inline - void - line_graph_psite<P>::update_() - { - p_ = point(plg_.gr_.node_data(plg_.gr_.edge(id_).n1()), - plg_.gr_.node_data(plg_.gr_.edge(id_).n2())); - } - - template<typename P> - inline const p_line_graph<P>& line_graph_psite<P>::plg() const { - return plg_; + mln_assertion(plg_); + return *plg_; } template<typename P> @@ -220,13 +222,39 @@ return id_; } + template<typename P> + inline + P + line_graph_psite<P>::first() const + { + mln_assertion(is_valid_()); + return plg().gr_.node_data(plg().gr_.edge(id_).n1()); + } + + template<typename P> + inline + P + line_graph_psite<P>::second() const + { + mln_assertion(is_valid_()); + return plg().gr_.node_data(plg().gr_.edge(id_).n2()); + } + + + template <typename P> + bool + operator==(const line_graph_psite<P>& lhs, const line_graph_psite<P>& rhs) + { + return &lhs.plg() == &rhs.plg() && lhs.id() == rhs.id(); + } + template <typename P> inline std::ostream& operator<<(std::ostream& ostr, const line_graph_psite<P>& p) { - return ostr << p.to_point(); + return ostr << '(' << p.first() << " -- " << p.second() << ')'; } # endif // ! MLN_INCLUDE_ONLY