diff --git a/ChangeLog.md b/ChangeLog.md index b489f70d3b..11f60e4650 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -68,12 +68,17 @@ some improvements on bounding box computation with orientation check. (B. Kerautret, [#1123](https://github.com/DGtal-team/DGtal/pull/1123)) +- *Image Package* + - Fixing issue [#779](https://github.com/DGtal-team/DGtal/issues/779) by + storing domain with smart pointer in ImageContainerBySTLMap. + (Roland Denis [#1151](https://github.com/DGtal-team/DGtal/pull/1151)) + - *IO Package* - Display3D: Fix embedder usage when using default constructor in Debug mode. - (Roland Denis [##1143](https://github.com/DGtal-team/DGtal/pull/1143)) + (Roland Denis [#1143](https://github.com/DGtal-team/DGtal/pull/1143)) - Viewer3D: Fix a problem when the show() method was called at the end of the main program (the list creation was not called). - (Bertrand Kerautret [##1138](https://github.com/DGtal-team/DGtal/pull/1138)) + (Bertrand Kerautret [#1138](https://github.com/DGtal-team/DGtal/pull/1138)) - Viewer3D: add three new modes for shape rendering (default, metallic and plastic). The rendering can be changed by using the key M. The user can also choose its own rendering with some setter/getter on the opengl diff --git a/src/DGtal/base/CowPtr.h b/src/DGtal/base/CowPtr.h index 70a1d2ae4f..0cf311de9a 100644 --- a/src/DGtal/base/CowPtr.h +++ b/src/DGtal/base/CowPtr.h @@ -43,6 +43,7 @@ ////////////////////////////////////////////////////////////////////////////// // Inclusions #include +#include #include "DGtal/base/Common.h" #include "DGtal/base/CountedPtr.h" ////////////////////////////////////////////////////////////////////////////// @@ -88,14 +89,38 @@ namespace DGtal const T& operator*() const throw() {return *myPtr;} const T* operator->() const throw() {return myPtr.get();} const T* get() const throw() {return myPtr.get();} - + + /* The following non-const methods are deactivated if T is a const type. + * The goal here is to avoid unecessary copies when it is known that + * the T object will not be modified. + * + * The problem is that C++ uses the non-const methods whenever it's possible + * (ie when CowPtr is non-const), even if the full expression doesn't + * modify the object. A solution is to const_cast the CowPtr before + * using one of these methods to force the usage of the const versions above. + * + * However, in the trivial case when T is a const type, we can simplify this + * by deactivating those methods. + * + * To do that, we use std::enable_if (see http://en.cppreference.com/w/cpp/types/enable_if ) + * in the template parameters (it is also possible in the return type or + * as a parameter but it will change the signature). It depends on + * the constness of T returned by the type trait std::is_const. + * The `typename U = T` is necessary in order that the SFINAE rule works at the overload + * resolution step. + */ + template < typename U = T, typename std::enable_if< ! std::is_const::value >::type* = nullptr > T& operator*() {copy(); return *myPtr;} + + template < typename U = T, typename std::enable_if< ! std::is_const::value >::type* = nullptr > T* operator->() {copy(); return myPtr.get();} + + template < typename U = T, typename std::enable_if< ! std::is_const::value >::type* = nullptr > T* get() {copy(); return myPtr.get();} /** Equality operator == - + @param other any other pointer. @return 'true' if pointed address is equal to \a other. */ @@ -106,7 +131,7 @@ namespace DGtal /** Inequality operator != - + @param other any other pointer. @return 'true' if pointed address is different from \a other. */ diff --git a/src/DGtal/images/ImageContainerBySTLMap.h b/src/DGtal/images/ImageContainerBySTLMap.h index 1c209d3244..f0111bd4b9 100644 --- a/src/DGtal/images/ImageContainerBySTLMap.h +++ b/src/DGtal/images/ImageContainerBySTLMap.h @@ -50,9 +50,9 @@ #include #include "DGtal/base/Common.h" -#include "DGtal/base/ConstAlias.h" -#include "DGtal/base/CountedPtr.h" #include "DGtal/base/BasicFunctors.h" +#include "DGtal/base/CowPtr.h" +#include "DGtal/base/Clone.h" #include "DGtal/images/DefaultConstImageRange.h" #include "DGtal/images/DefaultImageRange.h" #include "DGtal/images/SetValueIterator.h" @@ -78,33 +78,33 @@ namespace DGtal * Once constructed, the image is valid, i.e. every point of the * image domain has a value, which can be read and overwritten. * Note that the default value (returned for points that are not - * stored in the underlying STL map) can be chosen by the user. + * stored in the underlying STL map) can be chosen by the user. * - * As a model of concepts::CImage, this class provides two ways of accessing values: - * - through the range of points returned by the domain() method - * combined with the operator() that takes a point and returns its associated value. - * - through the range of values returned by the range() method, + * As a model of concepts::CImage, this class provides two ways of accessing values: + * - through the range of points returned by the domain() method + * combined with the operator() that takes a point and returns its associated value. + * - through the range of values returned by the range() method, * which can be used to directly iterate over the values of the image * - * This class also provides a setValue() method and an output iterator, - * which is returned by the outputIterator() method for writting purposes. + * This class also provides a setValue() method and an output iterator, + * which is returned by the outputIterator() method for writting purposes. * * @see testImage.cpp */ template - class ImageContainerBySTLMap: + class ImageContainerBySTLMap: public std::map { public: - typedef ImageContainerBySTLMap Self; - typedef std::map Parent; + typedef ImageContainerBySTLMap Self; + typedef std::map Parent; /// domain BOOST_CONCEPT_ASSERT(( concepts::CDomain )); - typedef TDomain Domain; + typedef TDomain Domain; typedef typename Domain::Point Point; typedef typename Domain::Vector Vector; typedef typename Domain::Integer Integer; @@ -113,8 +113,7 @@ namespace DGtal typedef Point Vertex; // Pointer to the (const) Domain given at construction. - typedef const Domain * DomainPtr; - + typedef CowPtr< const Domain > DomainPtr; /// static constants static const typename Domain::Dimension dimension; @@ -122,16 +121,16 @@ namespace DGtal /// range of values BOOST_CONCEPT_ASSERT(( concepts::CLabel )); typedef TValue Value; - typedef DefaultConstImageRange ConstRange; - typedef DefaultImageRange Range; + typedef DefaultConstImageRange ConstRange; + typedef DefaultImageRange Range; /// output iterator - typedef SetValueIterator OutputIterator; + typedef SetValueIterator OutputIterator; /////////////////// Data members ////////////////// - private: + private: - /// Counted pointer on the image domain, + /// Shared pointer on the image domain, /// Since the domain is not mutable, not assignable, /// it is shared by all the copies of *this DomainPtr myDomainPtr; @@ -141,39 +140,41 @@ namespace DGtal /////////////////// standard services ////////////////// - public: + public: - /** - * Constructor from a Domain - * + /** + * Constructor from a pointer to a domain. + * + * If Domain is a heavy type, consider giving instead a smart pointer on the domain (like CountedPtr). + * * @param aDomain the image domain. * @param aValue a default value associated to the domain points * that are not contained in the underlying map. */ - ImageContainerBySTLMap( ConstAlias aDomain, const Value& aValue = 0); + ImageContainerBySTLMap( Clone aDomain, const Value& aValue = 0); - /** + /** * Copy operator - * + * * @param other the object to copy. */ ImageContainerBySTLMap(const ImageContainerBySTLMap& other); - /** + /** * Assignement operator - * + * * @param other the object to copy. * @return this */ ImageContainerBySTLMap& operator=(const ImageContainerBySTLMap& other); - /** + /** * Destructor. * */ ~ImageContainerBySTLMap(); - + /////////////////// Interface ////////////////// /** @@ -196,12 +197,12 @@ namespace DGtal * @param aValue the value. */ void setValue(const Point &aPoint, const Value &aValue); - + /** * @return the domain associated to the image. */ - const Domain &domain() const; + const Domain &domain() const; /** * @return the const range providing constant @@ -238,15 +239,15 @@ namespace DGtal typedef typename std::map::const_iterator ConstIterator; typedef typename std::map::reverse_iterator ReverseIterator; typedef typename std::map::const_reverse_iterator ConstReverseIterator; - + /** - * Construct a Iterator on the image + * Construct a Iterator on the image + * * - * * @return a Iterator */ OutputIterator outputIterator(); - + }; /** @@ -258,7 +259,7 @@ namespace DGtal template inline std::ostream& - operator<< ( std::ostream & out, + operator<< ( std::ostream & out, const ImageContainerBySTLMap & object ) { object.selfDisplay ( out ); diff --git a/src/DGtal/images/ImageContainerBySTLMap.ih b/src/DGtal/images/ImageContainerBySTLMap.ih index f39fa78f80..43653d173a 100644 --- a/src/DGtal/images/ImageContainerBySTLMap.ih +++ b/src/DGtal/images/ImageContainerBySTLMap.ih @@ -43,12 +43,13 @@ template const typename TDomain::Dimension DGtal::ImageContainerBySTLMap::dimension = TDomain::Space::dimension; +//------------------------------------------------------------------------------ template inline DGtal::ImageContainerBySTLMap -::ImageContainerBySTLMap( ConstAlias aDomain, const Value& aValue) - : myDomainPtr( &aDomain ), myDefaultValue( aValue ) +::ImageContainerBySTLMap( Clone aDomain, const Value& aValue) + : myDomainPtr( aDomain ), myDefaultValue( aValue ) { } @@ -57,7 +58,7 @@ template inline DGtal::ImageContainerBySTLMap ::ImageContainerBySTLMap(const ImageContainerBySTLMap& other) - : Parent(other), + : Parent(other), myDomainPtr(other.myDomainPtr), myDefaultValue(other.myDefaultValue) { } @@ -70,11 +71,11 @@ DGtal::ImageContainerBySTLMap { if (this != &other) { - Parent::operator=(other); - myDomainPtr = other.myDomainPtr; - myDefaultValue = other.myDefaultValue; + Parent::operator=(other); + myDomainPtr = other.myDomainPtr; + myDefaultValue = other.myDefaultValue; } - return *this; + return *this; } //------------------------------------------------------------------------------ template @@ -87,9 +88,9 @@ DGtal::ImageContainerBySTLMap::~ImageContainerBySTLMap( ) template inline typename DGtal::ImageContainerBySTLMap::Value -DGtal::ImageContainerBySTLMap::operator()(const Point &aPoint) const +DGtal::ImageContainerBySTLMap::operator()(const Point &aPoint) const { - ASSERT( this->domain().isInside( aPoint ) ); + ASSERT( this->domain().isInside( aPoint ) ); ConstIterator it = this->find( aPoint ); if ( it == this->end() ) return myDefaultValue; @@ -103,11 +104,11 @@ inline void DGtal::ImageContainerBySTLMap::setValue(const Point &aPoint, const Value &aValue) { - ASSERT( this->domain().isInside( aPoint ) ); - std::pair::iterator, bool> - res = this->insert( std::pair(aPoint, aValue) ); + ASSERT( this->domain().isInside( aPoint ) ); + std::pair::iterator, bool> + res = this->insert( std::pair(aPoint, aValue) ); if (res.second == false) - res.first->second = aValue; + res.first->second = aValue; } //------------------------------------------------------------------------------ @@ -161,7 +162,7 @@ inline void DGtal::ImageContainerBySTLMap::selfDisplay ( std::ostream & out ) const { - out << "[Image - STLMap] size=" << this->size() << " valuetype=" + out << "[Image - STLMap] size=" << this->size() << " valuetype=" << sizeof(TValue) << "bytes Domain=" << *myDomainPtr; } diff --git a/tests/geometry/volumes/distance/testPowerMap.cpp b/tests/geometry/volumes/distance/testPowerMap.cpp index 202910c870..484f09b8c1 100644 --- a/tests/geometry/volumes/distance/testPowerMap.cpp +++ b/tests/geometry/volumes/distance/testPowerMap.cpp @@ -53,35 +53,36 @@ bool testPowerMap() { unsigned int nbok = 0; unsigned int nb = 0; - + trace.beginBlock ( "Testing PowerMap2D ..." ); Z2i::Domain domain(Z2i::Point(0,0),Z2i::Point(10,10)); Z2i::Domain domainLarge(Z2i::Point(0,0),Z2i::Point(10,10)); DigitalSetBySTLSet set(domain); - set.insertNew(Z2i::Point(3,3)); - //set.insertNew(Z2i::Point(3,7)); + set.insertNew(Z2i::Point(3,3)); + //set.insertNew(Z2i::Point(3,7)); set.insertNew(Z2i::Point(7,7)); - DigitalSetDomain< DigitalSetBySTLSet > setDomain(set); - - typedef ImageContainerBySTLMap< DigitalSetDomain< DigitalSetBySTLSet > , DGtal::int64_t> Image; - Image image(setDomain); - + + using SetDomain = DigitalSetDomain< DigitalSetBySTLSet >; + using Image = ImageContainerBySTLMap< SetDomain , DGtal::int64_t>; + + Image image( new SetDomain( set ) ); + //Setting some values - image.setValue(Z2i::Point(3,3), 9); - // image.setValue(Z2i::Point(3,7), 0); + image.setValue(Z2i::Point(3,3), 9); + // image.setValue(Z2i::Point(3,7), 0); image.setValue(Z2i::Point(7,7), 16); - + Z2i::L2PowerMetric l2power; PowerMap power(&domainLarge, &image, &l2power); for(unsigned int i=0; i<11; i++) { for(unsigned int j=0; j<11; j++) - if (image.domain().isInside(Z2i::Point(i,j))) - trace.info()<< image(Z2i::Point(i,j))<<" "; - else - trace.info()<< "0 "; + if (image.domain().isInside(Z2i::Point(i,j))) + trace.info()<< image(Z2i::Point(i,j))<<" "; + else + trace.info()<< "0 "; trace.info()<=0) - std::cerr<< "0 "; - else - std::cerr<< "X "; - } + { + Z2i::Point p(i,j); + DGtal::int32_t dist = (i-power(p)[0])*(i-power(p)[0]) + + ( j-power(p)[1])*(j-power(p)[1]) - image(power(p)); + if (dist>=0) + std::cerr<< "0 "; + else + std::cerr<< "X "; + } std::cerr< set(domain); - set.insertNew(Z2i::Point(3,3)); - //set.insertNew(Z2i::Point(3,7)); + set.insertNew(Z2i::Point(3,3)); + //set.insertNew(Z2i::Point(3,7)); set.insertNew(Z2i::Point(7,7)); - DigitalSetDomain > setDomain(set); - - typedef ImageContainerBySTLMap > , DGtal::int64_t> Image; - Image image(setDomain); - + + using SetDomain = DigitalSetDomain< DigitalSetBySTLSet >; + using Image = ImageContainerBySTLMap< SetDomain , DGtal::int64_t>; + Image image( new SetDomain( set ) ); + //Setting some values - image.setValue(Z2i::Point(3,3), 9); - // image.setValue(Z2i::Point(3,7), 0); + image.setValue(Z2i::Point(3,3), 9); + // image.setValue(Z2i::Point(3,7), 0); image.setValue(Z2i::Point(7,7), 16); - + Z2i::L2PowerMetric l2power; PowerMap power(&domainLarge, &image, &l2power); for(unsigned int i=0; i<11; i++) @@ -107,10 +107,10 @@ bool testReducedMedialAxis() std::cerr< >::Type rdma = ReducedMedialAxis< PowerMap >::getReducedMedialAxisFromPowerMap(power); - + //Reconstruction for(unsigned int i=0; i<11; i++) { @@ -121,19 +121,19 @@ bool testReducedMedialAxis() trace.info()<< rdma(p); else trace.info()<< " - "; - + } std::cerr<