Review move sematics and convert the featuresource iterator to a stac…

…k object

src/apps/rocky_demo/Demo_LineFeatures.h

@@ -45,9 +45,9 @@ auto Demo_LineFeatures = [](Application& app)
             FeatureView& feature_view = app.entities.emplace<FeatureView>(entity);
 
             auto iter = data->fs->iterate(app.instance.io());
-            while (iter->hasMore())
+            while (iter.hasMore())
             {
-                auto feature = iter->next();
+                auto feature = iter.next();
                 if (feature.valid())
                 {
                     // convert anything we find to lines:

src/apps/rocky_demo/Demo_PolygonFeatures.h

@@ -48,9 +48,9 @@ auto Demo_PolygonFeatures = [](Application& app)
                 feature_view.features.reserve(data->fs->featureCount());
 
             auto iter = data->fs->iterate(app.instance.io());
-            while (iter->hasMore())
+            while (iter.hasMore())
             {
-                auto feature = iter->next();
+                auto feature = iter.next();
                 if (feature.valid())
                 {
                     feature.interpolation = GeodeticInterpolation::RhumbLine;

src/rocky/Feature.cpp

@@ -478,7 +478,7 @@ OGRFeatureSource::~OGRFeatureSource()
     close();
 }
 
-std::shared_ptr<FeatureSource::iterator>
+FeatureSource::iterator
 OGRFeatureSource::iterate(IOOptions& io)
 {
     OGRDataSourceH dsHandle = nullptr;
@@ -507,14 +507,14 @@ OGRFeatureSource::iterate(IOOptions& io)
         }
     }
 
+    auto i = new iterator_impl();
+
     if (layerHandle)
     {
-        auto i = std::make_shared<iterator>();
         i->_source = this;
         i->_dsHandle = dsHandle;
         i->_layerHandle = layerHandle;
         i->init();
-        return i;
     }
     else
     {
@@ -523,18 +523,16 @@ OGRFeatureSource::iterate(IOOptions& io)
             OGRReleaseDataSource(dsHandle);
         }
     }
-    return { };
 
+    return iterator(i);
 }
 
 
-OGRFeatureSource::iterator::iterator()
-{
-}
-
 void
-OGRFeatureSource::iterator::init()
+OGRFeatureSource::iterator_impl::init()
 {
+    _resultSetEndReached = false;
+
     if (_dsHandle)
     {
         std::string from = OGR_FD_GetName(OGR_L_GetLayerDefn(_layerHandle));
@@ -564,17 +562,16 @@ OGRFeatureSource::iterator::init()
     readChunk();
 }
 
-OGRFeatureSource::iterator::~iterator()
+OGRFeatureSource::iterator_impl::~iterator_impl()
 {
-
     if (_nextHandleToQueue)
     {
         OGR_F_Destroy(_nextHandleToQueue);
     }
 }
 
 void
-OGRFeatureSource::iterator::readChunk()
+OGRFeatureSource::iterator_impl::readChunk()
 {
     if (!_resultSetHandle)
         return;
@@ -617,13 +614,13 @@ OGRFeatureSource::iterator::readChunk()
 }
 
 bool
-OGRFeatureSource::iterator::hasMore() const
+OGRFeatureSource::iterator_impl::hasMore() const
 {
     return _resultSetHandle && !_queue.empty();
 };
 
 const Feature&
-OGRFeatureSource::iterator::next()
+OGRFeatureSource::iterator_impl::next()
 {
     if (!hasMore())
         return _lastFeatureReturned;

src/rocky/Feature.h

@@ -63,6 +63,11 @@ namespace ROCKY_NAMESPACE
 
         //! Copy constructor
         Geometry(const Geometry& rhs) = default;
+        Geometry& operator=(const Geometry& rhs) = default;
+
+        //! Move constructor
+        Geometry(Geometry&& rhs) noexcept = default;
+        Geometry& operator=(Geometry&& rhs) noexcept = default;
 
         //! Contruct a typed geometry by copying points from an existing range
         //! @param type Geometry type
@@ -209,7 +214,14 @@ namespace ROCKY_NAMESPACE
 
         //! Construct an empty feature object.
         Feature() = default;
+
+        //! Copy constructor
         Feature(const Feature& rhs) = default;
+        Feature& operator =(const Feature& rhs) = default;
+
+        //! Move constructor
+        Feature(Feature&& rhs) noexcept = default;
+        Feature& operator =(Feature&& rhs) noexcept = default;
 
         //! Whether the feature is valid
         bool valid() const {
@@ -230,7 +242,9 @@ namespace ROCKY_NAMESPACE
         void dirtyExtent();
     };
 
-
+    /**
+    * Extent, SRS, and possibly the tiling profile of a feautre source
+    */
     struct FeatureProfile
     {
         GeoExtent extent;
@@ -246,15 +260,26 @@ namespace ROCKY_NAMESPACE
         class iterator
         {
         public:
-            virtual bool hasMore() const = 0;
-            virtual const Feature& next() = 0;
+            struct implementation {
+                virtual bool hasMore() const = 0;
+                virtual const Feature& next() = 0;
+            };
+
+        public:
+            bool hasMore() const { return _impl->hasMore(); }
+            const Feature& next() { return _impl->next(); }
+
+            iterator(implementation* impl) : _impl(impl) { }
+
+        private:
+            std::unique_ptr<implementation> _impl;
         };
 
         //! Number of features, or -1 if the count isn't available
         virtual int featureCount() const = 0;
 
         //! Creates a feature iterator
-        virtual std::shared_ptr<iterator> iterate(IOOptions& io) = 0;
+        virtual iterator iterate(IOOptions& io) = 0;
     };
 
 
@@ -284,7 +309,7 @@ namespace ROCKY_NAMESPACE
         void close();
 
         //! Create an interator to read features from the source
-        std::shared_ptr<FeatureSource::iterator> iterate(IOOptions& io) override;
+        FeatureSource::iterator iterate(IOOptions& io) override;
 
         //! Number of features, or -1 if the count isn't available
         int featureCount() const override;
@@ -302,28 +327,28 @@ namespace ROCKY_NAMESPACE
         FeatureProfile _featureProfile;
         std::string _source;
 
-        class ROCKY_EXPORT iterator : public FeatureSource::iterator
+        class ROCKY_EXPORT iterator_impl : public FeatureSource::iterator::implementation
         {
         public:
-            iterator();
-            void init();
-            ~iterator();
+            ~iterator_impl();
             bool hasMore() const override;
             const Feature& next() override;
         private:
             std::queue<Feature> _queue;
             Feature _lastFeatureReturned;
-            void readChunk();
-            friend class OGRFeatureSource;
             OGRFeatureSource* _source = nullptr;
             void* _dsHandle = nullptr;
             void* _layerHandle = nullptr;
             void* _resultSetHandle = nullptr;
             void* _spatialFilterHandle = nullptr;
             void* _nextHandleToQueue = nullptr;
-            bool _resultSetEndReached = false;
+            bool _resultSetEndReached = true;
             const std::size_t _chunkSize = 500;
             Feature::ID _idGenerator = 1;
+
+            void init();
+            void readChunk();
+            friend class OGRFeatureSource;
         };
     };
 

src/rocky/GeoExtent.cpp

@@ -29,18 +29,6 @@ GeoExtent::GeoExtent() :
     //NOP - invalid
 }
 
-GeoExtent&
-GeoExtent::operator =(GeoExtent&& rhs)
-{
-    _srs = rhs._srs;
-    _west = rhs._west;
-    _width = rhs._width;
-    _south = rhs._south;
-    _height = rhs._height;
-    rhs._srs = { };
-    return *this;
-}
-
 GeoExtent::GeoExtent(const SRS& srs) :
     _srs(srs),
     _west(0.0),

src/rocky/GeoExtent.h

@@ -22,8 +22,8 @@ namespace ROCKY_NAMESPACE
         GeoExtent();
         GeoExtent(const GeoExtent& rhs) = default;
         GeoExtent& operator=(const GeoExtent&) = default;
-        GeoExtent(GeoExtent&& rhs) { *this = rhs; }
-        GeoExtent& operator=(GeoExtent&&);
+        GeoExtent(GeoExtent&& rhs) noexcept = default;
+        GeoExtent& operator=(GeoExtent&&) noexcept = default;
 
         /** Contructs a valid extent */
         GeoExtent(
@@ -39,9 +39,6 @@ namespace ROCKY_NAMESPACE
             const SRS& srs,
             const Box& bounds);
 
-        /** dtor */
-        virtual ~GeoExtent() { }
-
         //! Set from the SW and NE corners.
         void set(double west, double south, double east, double north);
 

src/rocky/GeoHeightfield.cpp

@@ -18,17 +18,21 @@ GeoHeightfield::GeoHeightfield() :
 }
 
 GeoHeightfield&
-GeoHeightfield::operator=(GeoHeightfield&& rhs)
+GeoHeightfield::operator=(GeoHeightfield&& rhs) noexcept
 {
-    *this = (const GeoHeightfield&)rhs;
+    if (this != &rhs)
+    {
+        _hf = std::move(rhs._hf);
+        _extent = std::move(rhs._extent);
+        _minHeight = rhs._minHeight;
+        _maxHeight = rhs._maxHeight;
+        _resolution = rhs._resolution;
+    }
     rhs._extent = GeoExtent::INVALID;
     return *this;
 }
 
-GeoHeightfield::GeoHeightfield(
-    shared_ptr<Heightfield> heightField,
-    const GeoExtent&  extent) :
-
+GeoHeightfield::GeoHeightfield(shared_ptr<Heightfield> heightField, const GeoExtent& extent) :
     _hf(heightField),
     _extent(extent),
     _minHeight(FLT_MAX),

src/rocky/GeoHeightfield.h

@@ -22,8 +22,8 @@ namespace ROCKY_NAMESPACE
         GeoHeightfield();
         GeoHeightfield(const GeoHeightfield&) = default;
         GeoHeightfield& operator=(const GeoHeightfield&) = default;
-        GeoHeightfield(GeoHeightfield&& rhs) { *this = rhs; }
-        GeoHeightfield& operator=(GeoHeightfield&& rhs);
+        GeoHeightfield(GeoHeightfield&& rhs) noexcept { *this = rhs; }
+        GeoHeightfield& operator=(GeoHeightfield&& rhs) noexcept;
 
         //! Constructs a new georeferenced heightfield.
         GeoHeightfield(

src/rocky/GeoImage.cpp

@@ -545,9 +545,13 @@ GeoImage::GeoImage() :
 }
 
 GeoImage&
-GeoImage::operator=(GeoImage&& rhs)
+GeoImage::operator=(GeoImage&& rhs) noexcept
 {
-    *this = (const GeoImage&)rhs;
+    if (this != &rhs)
+    {
+        _image = std::move(rhs._image);
+        _extent = std::move(rhs._extent);
+    }
     rhs._image = nullptr;
     rhs._extent = GeoExtent::INVALID;
     return *this;

src/rocky/GeoImage.h

@@ -25,8 +25,8 @@ namespace ROCKY_NAMESPACE
         GeoImage();
         GeoImage(const GeoImage&) = default;
         GeoImage& operator=(const GeoImage&) = default;
-        GeoImage(const GeoImage&& rhs) { *this = rhs; }
-        GeoImage& operator=(GeoImage&&);
+        GeoImage(const GeoImage&& rhs) noexcept { *this = rhs; }
+        GeoImage& operator=(GeoImage&&) noexcept;
 
         //! Constructs a new goereferenced image.
         GeoImage(shared_ptr<Image> image, const GeoExtent& extent);

src/rocky/Geocoder.cpp

@@ -29,9 +29,9 @@ Geocoder::geocode(const std::string& location, IOOptions& io) const
             fs->externalLayerHandle = layerHandle;
             fs->externalSRS = SRS::WGS84;
             auto iter = fs->iterate(io);
-            while (iter->hasMore())
+            while (iter.hasMore())
             {
-                result.emplace_back(iter->next());
+                result.emplace_back(iter.next());
             }
             OGRGeocodeFreeResult(layerHandle);
         }

src/rocky/Heightfield.h

@@ -37,11 +37,11 @@ namespace ROCKY_NAMESPACE
 
         //! Visits each height in the field with a user-provided function
         //! that takes "float" or "float&" as an argument.
-        template<typename FUNC>
-        void forEachHeight(FUNC func);
+        template<typename CALLABLE>
+        inline void forEachHeight(CALLABLE&& func);
 
-        template<typename FUNC>
-        void forEachHeight(FUNC func) const;
+        template<typename CALLABLE>
+        inline void forEachHeight(CALLABLE&& func) const;
 
         //! Interpolated height at a normalized (u,v) location
         float heightAtUV(
@@ -70,16 +70,16 @@ namespace ROCKY_NAMESPACE
         return data<float>(c, r);
     }    
 
-    template<typename FUNC>
-    void Heightfield::forEachHeight(FUNC func)
+    template<typename CALLABLE>
+    void Heightfield::forEachHeight(CALLABLE&& func)
     {
         float* ptr = data<float>();
         for (auto i = 0u; i < sizeInPixels(); ++i, ++ptr)
             func(*ptr);
     }
 
-    template<typename FUNC>
-    void Heightfield::forEachHeight(FUNC func) const
+    template<typename CALLABLE>
+    void Heightfield::forEachHeight(CALLABLE&& func) const
     {
         const float* ptr = data<float>();
         for (auto i = 0u; i < sizeInPixels(); ++i, ++ptr)