fix all the MosaicFilter bugs

include/pdal/MultiFilterIterator.hpp

@@ -65,6 +65,7 @@ class MultiFilterSequentialIterator : public StageSequentialIterator
     const MultiFilter& m_filter;
     std::vector<StageSequentialIterator*> m_prevIterators;
     StageSequentialIterator* m_prevIterator;
+    boost::uint32_t m_iteratorIndex;
 
 private:
 };

include/pdal/filters/MosaicFilter.hpp

@@ -41,7 +41,6 @@
 
 #include <pdal/MultiFilter.hpp>
 #include <pdal/StageIterator.hpp>
-//#include <pdal/Bounds.hpp>
 
 
 namespace pdal { namespace filters {

include/pdal/filters/MosaicFilterIterator.hpp

@@ -38,9 +38,7 @@
 #include <pdal/pdal.hpp>
 
 #include <vector>
-//#include <pdal/Filter.hpp>
 #include <pdal/MultiFilterIterator.hpp>
-//#include <pdal/Bounds.hpp>
 
 
 namespace pdal { namespace filters {

src/MultiFilterIterator.cpp

@@ -43,6 +43,7 @@ MultiFilterSequentialIterator::MultiFilterSequentialIterator(const MultiFilter&
     : StageSequentialIterator(filter)
     , m_filter(filter)
     , m_prevIterator(NULL)
+    , m_iteratorIndex(0)
 {
     for (size_t i=0; i<filter.getPrevStages().size(); ++i)
     {

src/filters/MosaicFilter.cpp

@@ -53,33 +53,36 @@ void MosaicFilter::initialize()
 {
     MultiFilter::initialize();
 
-    const Stage& prevStage0 = *(getPrevStages()[0]);
+    const std::vector<Stage*>& stages = getPrevStages();
 
-    {
-        setCoreProperties(prevStage0);  // BUG: clearly insufficient
-    }
-
-    boost::uint64_t totalPoints = 0;
-
-    Bounds<double> bigbox(prevStage0.getBounds());
+    const Stage& stage0 = *stages[0];
+    const SpatialReference& srs0 = stage0.getSpatialReference();
+    const Schema& schema0 = stage0.getSchema();
+    PointCountType pointCountType0 = stage0.getPointCountType();
+    boost::uint64_t totalPoints = stage0.getNumPoints();
+    Bounds<double> bigbox(stage0.getBounds());
 
-    for (size_t i=0; i<getPrevStages().size(); i++)
+    // we will only mosaic if all the stages have the same core properties: SRS, schema, etc
+    for (boost::uint32_t i=1; i<stages.size(); i++)
     {
-        const Stage* stage = getPrevStages()[i];
-        if (stage==NULL)
-        {
-            throw pdal_error("bad stage passed to MosaicFilter");
-        }
-        if (stage->getSchema() != this->getSchema())
-        {
-            throw pdal_error("impedance mismatch in MosaicFilter");
-        }
-
-        bigbox.grow(this->getBounds());
-        totalPoints += this->getNumPoints();
+        Stage& stage = *(stages[i]);
+        if (stage.getSpatialReference() != srs0)
+            throw impedance_invalid("mosaicked stages must have same srs");
+        if (stage.getSchema() != schema0)
+            throw impedance_invalid("mosaicked stages must have same schema");
+        if (stage.getPointCountType() == PointCount_Unknown)
+            pointCountType0 = PointCount_Unknown;
+
+        totalPoints += stage.getNumPoints();
+
+        bigbox.grow(stage.getBounds());
     }
 
-    setBounds(bigbox);
+    if (pointCountType0 == PointCount_Unknown)
+        totalPoints = 0;
+
+    setCoreProperties(stage0);
+    setPointCountType(pointCountType0);
     setNumPoints(totalPoints);
 
     return;

src/filters/MosaicFilterIterator.cpp

@@ -53,73 +53,73 @@ MosaicFilterSequentialIterator::~MosaicFilterSequentialIterator()
 }
 
 
+boost::uint64_t MosaicFilterSequentialIterator::skipImpl(boost::uint64_t targetCount)
+{
+    boost::uint64_t count = 0;
 
+    while (count < targetCount)
+    {
+        m_prevIterator = m_prevIterators[m_iteratorIndex];
+
+        // skip as much as we can in the current stage
+        boost::uint64_t thisCount = m_prevIterator->skip(count);
+        count += thisCount;
+
+        if (m_prevIterators[m_iteratorIndex]->atEnd())
+        {
+            ++m_iteratorIndex;
+        }
+        if (m_iteratorIndex == m_prevIterators.size())
+        {
+            // no more points
+            break;
+        }
+    }
 
-boost::uint64_t MosaicFilterSequentialIterator::skipImpl(boost::uint64_t count)
-{
-    // BUG: this is clearly not correct, we need to keep track of which tile we're on
-    m_prevIterators[0]->skip(count);
     return count;
 }
 
 
 bool MosaicFilterSequentialIterator::atEndImpl() const
 {
-    // BUG: this is clearly not correct, we need to keep track of which tile we're on
-    return m_prevIterators[0]->atEnd();
+    if (m_iteratorIndex == m_prevIterators.size())
+        return true;
+    if (m_prevIterators[m_iteratorIndex]->atEnd())
+        return true;
+    return false;
 }
 
 
 boost::uint32_t MosaicFilterSequentialIterator::readBufferImpl(PointBuffer& destData)
 {
-    // BUG: We know that the two prev stage schemas are compatible, 
-    // but we can't be sure the have the same bitfield layouts as 
-    // the buffer we've been given.  We could handle it manually if 
-    // they differ, but that would be a pain for now.  (This affects
-    // all filters, I guess.)
-
-    // BUG: this doesn't account for isValid()
-
     boost::uint32_t totalNumPointsToRead = destData.getCapacity();
     boost::uint32_t totalNumPointsRead = 0;
-
-    boost::uint64_t currentPointIndex = getIndex();
-
-    int destPointIndex = 0;
-    boost::uint64_t stageStartIndex = 0;
+    boost::uint32_t destPointIndex = 0;
 
     // for each stage, we read as many points as we can
-    for (size_t i=0; i<getPrevIterators().size(); i++)
+    while (totalNumPointsRead < totalNumPointsToRead)
     {
-        StageSequentialIterator* iterator = getPrevIterators()[i];
-        const Stage& stage = iterator->getStage();
-
-        const boost::uint64_t stageStopIndex = stageStartIndex + stage.getNumPoints();
-
-        if (currentPointIndex < stageStopIndex)
-        {
-            // we need to read some points from this stage
-
-            boost::uint32_t pointsAvail = (boost::uint32_t)(stageStopIndex - currentPointIndex);
-            boost::uint32_t pointsToGet = std::min(pointsAvail, totalNumPointsToRead);
+        assert(m_iteratorIndex < m_prevIterators.size());
+        m_prevIterator = m_prevIterators[m_iteratorIndex];
 
-            PointBuffer srcData(destData.getSchemaLayout(), pointsToGet);
-            boost::uint32_t pointsGotten = iterator->read(srcData);
+        // read as much as we can into temp buffer
+        PointBuffer tmp(destData.getSchemaLayout(), totalNumPointsToRead-totalNumPointsRead); 
+        boost::uint32_t numRead = m_prevIterator->read(tmp);
+        totalNumPointsRead += numRead;
 
-            for (boost::uint32_t idx=0; idx<pointsGotten; idx++)
-            {
-                destData.copyPointFast(destPointIndex, idx, srcData);
-                destPointIndex++;
-                destData.setNumPoints(idx+1);
-            }
+        // concat the temp buffer on to end of real dest buffer
+        destData.copyPointsFast(destPointIndex, 0, tmp, numRead);
+        destPointIndex += numRead;
+        destData.setNumPoints( destData.getNumPoints() + numRead );
 
-            totalNumPointsRead += pointsGotten;
-            currentPointIndex += pointsGotten;
+        if (m_prevIterator->atEnd())
+        {
+            ++m_iteratorIndex;
+        }
+        if (m_iteratorIndex == m_prevIterators.size())
+        {
+            break;
         }
-
-        stageStartIndex += stage.getNumPoints();
-
-        if (totalNumPointsRead == totalNumPointsToRead) break;
     }
 
     return totalNumPointsRead;