[Liblas-commits] r1122 - in trunk: include/liblas include/liblas/detail src src/detail

liblas-commits at liblas.org liblas-commits at liblas.org
Thu Mar 19 21:31:43 EDT 2009


Author: mloskot
Date: Thu Mar 19 21:31:42 2009
New Revision: 1122
URL: http://liblas.org/changeset/1122

Log:
Fixed weakness of MakePIMPL by reverting changes applied as r984 and bringing m_pimpl back to constness. There is no need to re-create internal reader object and reassign it to m_pimpl, because reader state can be correctly reset by simple call of LASReader::Init. This fix is also related to Ticket #85. All 87 test cases pass.

Modified:
   trunk/include/liblas/detail/reader.hpp
   trunk/include/liblas/lasreader.hpp
   trunk/src/detail/reader.cpp
   trunk/src/detail/reader10.cpp
   trunk/src/detail/reader11.cpp
   trunk/src/detail/reader12.cpp
   trunk/src/lasreader.cpp

Modified: trunk/include/liblas/detail/reader.hpp
==============================================================================
--- trunk/include/liblas/detail/reader.hpp	(original)
+++ trunk/include/liblas/detail/reader.hpp	Thu Mar 19 21:31:42 2009
@@ -72,6 +72,7 @@
     std::istream& GetStream() const;
     bool ReadVLR(LASHeader& header);
     bool ReadGeoreference(LASHeader& header);
+    void Reset(LASHeader const& header);
     void SetSRS(const LASSRS& srs);
     
 protected:
@@ -80,19 +81,15 @@
     std::streamoff m_offset;
     uint32_t m_size;
     uint32_t m_current;
-    uint32_t m_recordlength;
-
-    void FillPoint(PointRecord& record, LASPoint& point);
-    void Project(LASPoint& point);
-    
+    uint32_t m_recordlength;    
     LASSRS m_out_srs;
-    LASSRS m_in_srs;
-    
-
+    LASSRS m_in_srs;    
     OGRCoordinateTransformationH m_transform;
     OGRSpatialReferenceH m_in_ref;
     OGRSpatialReferenceH m_out_ref;
 
+    void FillPoint(PointRecord& record, LASPoint& point);
+    void Project(LASPoint& point);
 
 private:
 

Modified: trunk/include/liblas/lasreader.hpp
==============================================================================
--- trunk/include/liblas/lasreader.hpp	(original)
+++ trunk/include/liblas/lasreader.hpp	Thu Mar 19 21:31:42 2009
@@ -1,44 +1,44 @@
 /******************************************************************************
- * $Id$
- *
- * Project:  libLAS - http://liblas.org - A BSD library for LAS format data.
- * Purpose:  LAS reader class 
- * Author:   Mateusz Loskot, mateusz at loskot.net
- *
- ******************************************************************************
- * Copyright (c) 2008, Mateusz Loskot
- * Copyright (c) 2008, Phil Vachon
- *
- * All rights reserved.
- * 
- * Redistribution and use in source and binary forms, with or without 
- * modification, are permitted provided that the following 
- * conditions are met:
- * 
- *     * Redistributions of source code must retain the above copyright 
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright 
- *       notice, this list of conditions and the following disclaimer in 
- *       the documentation and/or other materials provided 
- *       with the distribution.
- *     * Neither the name of the Martin Isenburg or Iowa Department 
- *       of Natural Resources nor the names of its contributors may be 
- *       used to endorse or promote products derived from this software 
- *       without specific prior written permission.
- * 
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT 
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS 
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE 
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, 
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, 
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS 
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, 
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT 
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY 
- * OF SUCH DAMAGE.
- ****************************************************************************/
+* $Id$
+*
+* Project:  libLAS - http://liblas.org - A BSD library for LAS format data.
+* Purpose:  LAS reader class 
+* Author:   Mateusz Loskot, mateusz at loskot.net
+*
+******************************************************************************
+* Copyright (c) 2008, Mateusz Loskot
+* Copyright (c) 2008, Phil Vachon
+*
+* All rights reserved.
+* 
+* Redistribution and use in source and binary forms, with or without 
+* modification, are permitted provided that the following 
+* conditions are met:
+* 
+*     * Redistributions of source code must retain the above copyright 
+*       notice, this list of conditions and the following disclaimer.
+*     * Redistributions in binary form must reproduce the above copyright 
+*       notice, this list of conditions and the following disclaimer in 
+*       the documentation and/or other materials provided 
+*       with the distribution.
+*     * Neither the name of the Martin Isenburg or Iowa Department 
+*       of Natural Resources nor the names of its contributors may be 
+*       used to endorse or promote products derived from this software 
+*       without specific prior written permission.
+* 
+* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 
+* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT 
+* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS 
+* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE 
+* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, 
+* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, 
+* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS 
+* OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
+* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, 
+* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT 
+* OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY 
+* OF SUCH DAMAGE.
+****************************************************************************/
 
 #ifndef LIBLAS_LASREADER_HPP_INCLUDED
 #define LIBLAS_LASREADER_HPP_INCLUDED
@@ -63,29 +63,27 @@
 
     LASReader(std::istream& ifs);
     ~LASReader();
-    
+
     std::size_t GetVersion() const;
     LASHeader const& GetHeader() const;
     LASPoint const& GetPoint() const;
     std::vector<LASVLR> const& GetVLRs() const;
-    
+    /// Allow fetching of the stream
+    std::istream& GetStream() const;
+    bool IsEOF() const;
+
     bool ReadNextPoint();
     bool ReadPointAt(std::size_t n);
     bool ReadVLR();
+    void Reset();
 
-    // Reproject data as they are written if the LASWriter's reference is
-    // different than the LASHeader's    
+    /// Reproject data as they are written if the LASWriter's reference is
+    /// different than the LASHeader's    
     bool SetSRS(const LASSRS& ref);
 
-    // The operator is not const because it updates file stream position.
+    /// The operator is not const because it updates file stream position.
     LASPoint const& operator[](std::size_t n);
 
-    // Allow fetching of the stream
-    std::istream& GetStream() const;
-    
-    bool Reset();
-    bool IsEOF() const;
-    
 private:
 
     // Blocked copying operations, declared but not defined.
@@ -93,14 +91,11 @@
     LASReader& operator=(LASReader const& rhs);
 
     void Init(); // throws on error
-    void MakePIMPL(std::istream& ifs);
 
-    // TODO: Consider restoring const keyword and improving idea of re-assignment (see MakePIMPL).
-    std::auto_ptr<detail::Reader> m_pimpl;
+    const std::auto_ptr<detail::Reader> m_pimpl;
     LASHeader m_header;
     LASPoint m_point;
     std::vector<LASVLR> m_vlrs;
-
 };
 
 } // namespace liblas

Modified: trunk/src/detail/reader.cpp
==============================================================================
--- trunk/src/detail/reader.cpp	(original)
+++ trunk/src/detail/reader.cpp	Thu Mar 19 21:31:42 2009
@@ -67,20 +67,26 @@
 
 namespace liblas { namespace detail {
 
-Reader::Reader(std::istream& ifs) : m_ifs(ifs), m_offset(0), m_current(0), m_transform(0), m_in_ref(0), m_out_ref(0)
+Reader::Reader(std::istream& ifs) :
+    m_ifs(ifs), m_offset(0), m_size(0), m_current(0),
+    m_recordlength(0), m_transform(0),
+    m_in_ref(0), m_out_ref(0)
 {
 }
 
 Reader::~Reader()
 {
 #ifdef HAVE_GDAL
-    if (m_transform) {
+    if (m_transform)
+    {
         OCTDestroyCoordinateTransformation(m_transform);
     }
-    if (m_in_ref) {
+    if (m_in_ref)
+    {
         OSRDestroySpatialReference(m_in_ref);
     }
-    if (m_out_ref) {
+    if (m_out_ref)
+    {
         OSRDestroySpatialReference(m_out_ref);
     }
 
@@ -91,6 +97,7 @@
 {
     return m_ifs;
 }
+
 void Reader::FillPoint(PointRecord& record, LASPoint& point) 
 {
     
@@ -98,7 +105,10 @@
     point.SetY(record.y);
     point.SetZ(record.z);
     
-    if (m_transform) Project(point);
+    if (m_transform)
+    {
+        Project(point);
+    }
 
     point.SetIntensity(record.intensity);
     point.SetScanFlags(record.flags);
@@ -108,7 +118,6 @@
     point.SetPointSourceID(record.point_source_id);
 }
 
-
 bool Reader::ReadVLR(LASHeader& header)
 {
     VLRHeader vlrh = { 0 };
@@ -121,10 +130,10 @@
         read_n(vlrh, m_ifs, sizeof(VLRHeader));
 
         uint16_t length = vlrh.recordLengthAfterHeader;
-        if (length < 1) {
+        if (length < 1)
+        {
             throw std::domain_error("VLR record length must be at least 1 byte long");
-        }
-         
+        } 
         std::vector<uint8_t> data;
         data.resize(length);
 
@@ -140,13 +149,11 @@
 
         header.AddVLR(vlr);
     }
-
     return true;
 }
 
 bool Reader::ReadGeoreference(LASHeader& header)
 {
-
     std::vector<LASVLR> vlrs;
     for (uint16_t i = 0; i < header.GetRecordsCount(); ++i)
     {
@@ -154,16 +161,26 @@
         vlrs.push_back(record);
     }
 
-    LASSRS srs(vlrs);
-    
+    LASSRS srs(vlrs);    
     header.SetSRS(srs);
 
     // keep a copy on the reader in case we're going to reproject data 
     // on the way out.
     m_in_srs = srs;
-    
+
     return true;
+}
+
+void Reader::Reset(LASHeader const& header)
+{
+    m_ifs.clear();
+    m_ifs.seekg(0);
 
+    // Reset sizes and set internal cursor to the beginning of file.
+    m_current = 0;
+    m_offset = header.GetDataOffset();
+    m_size = header.GetPointRecordsCount();
+    m_recordlength = header.GetDataRecordLength();
 }
 
 void Reader::SetSRS(const LASSRS& srs)
@@ -205,15 +222,14 @@
     double y = point.GetY();
     double z = point.GetZ();
     
-    ret = OCTTransform(m_transform, 1, &x, &y, &z);
-    
-    if (!ret) {
+    ret = OCTTransform(m_transform, 1, &x, &y, &z);    
+    if (!ret)
+    {
         std::ostringstream msg; 
         msg << "Could not project point for Reader::" << CPLGetLastErrorMsg() << ret;
         std::string message(msg.str());
         throw std::runtime_error(message);
     }
-    
 
     point.SetX(x);
     point.SetY(y);
@@ -223,7 +239,6 @@
 #endif
 }
 
-
 Reader* ReaderFactory::Create(std::istream& ifs)
 {
     if (!ifs)

Modified: trunk/src/detail/reader10.cpp
==============================================================================
--- trunk/src/detail/reader10.cpp	(original)
+++ trunk/src/detail/reader10.cpp	Thu Mar 19 21:31:42 2009
@@ -207,9 +207,7 @@
     header.SetMax(x1, y1, z1);
     header.SetMin(x2, y2, z2);
 
-    m_offset = header.GetDataOffset();
-    m_size = header.GetPointRecordsCount();
-    m_recordlength = header.GetDataRecordLength();
+    Reset(header);
 
     return true;
 }

Modified: trunk/src/detail/reader11.cpp
==============================================================================
--- trunk/src/detail/reader11.cpp	(original)
+++ trunk/src/detail/reader11.cpp	Thu Mar 19 21:31:42 2009
@@ -211,9 +211,7 @@
     header.SetMax(x1, y1, z1);
     header.SetMin(x2, y2, z2);
 
-    m_offset = header.GetDataOffset();
-    m_size = header.GetPointRecordsCount();
-    m_recordlength = header.GetDataRecordLength();
+    Reset(header);
     
     return true;
 }

Modified: trunk/src/detail/reader12.cpp
==============================================================================
--- trunk/src/detail/reader12.cpp	(original)
+++ trunk/src/detail/reader12.cpp	Thu Mar 19 21:31:42 2009
@@ -217,9 +217,7 @@
     header.SetMax(x1, y1, z1);
     header.SetMin(x2, y2, z2);
 
-    m_offset = header.GetDataOffset();
-    m_size = header.GetPointRecordsCount();
-    m_recordlength = header.GetDataRecordLength();
+    Reset(header);
     
     return true;
 }

Modified: trunk/src/lasreader.cpp
==============================================================================
--- trunk/src/lasreader.cpp	(original)
+++ trunk/src/lasreader.cpp	Thu Mar 19 21:31:42 2009
@@ -53,9 +53,10 @@
 namespace liblas
 {
 
-LASReader::LASReader(std::istream& ifs) 
+LASReader::LASReader(std::istream& ifs) :
+    m_pimpl(detail::ReaderFactory::Create(ifs))
 {
-    MakePIMPL(ifs);
+    //MakePIMPL(ifs);
     Init();
 }
 
@@ -82,19 +83,13 @@
 
 bool LASReader::ReadNextPoint()
 {
-    bool ret = false;
-    
-    ret = m_pimpl->ReadNextPoint(m_point, m_header);
-
+    bool ret = m_pimpl->ReadNextPoint(m_point, m_header);
     return ret;
 }
 
 bool LASReader::ReadPointAt(std::size_t n)
 {
-    bool ret = false;
-
-    ret = m_pimpl->ReadPointAt(n, m_point, m_header);
-
+    bool ret = m_pimpl->ReadPointAt(n, m_point, m_header);
     return ret;
 }
 
@@ -105,37 +100,27 @@
         throw std::out_of_range("point subscript out of range");
     }
 
-    bool ret = false;
-
-    ret = m_pimpl->ReadPointAt(n, m_point, m_header);
-
+    bool ret = m_pimpl->ReadPointAt(n, m_point, m_header);
     if (!ret)
     {
         throw std::out_of_range("no point record at given position");
     }
 
-
     return m_point;
 }
 
-void LASReader::MakePIMPL(std::istream& ifs) 
-{
-    m_pimpl = std::auto_ptr<detail::Reader>( detail::ReaderFactory::Create(ifs) );
-}
-
 void LASReader::Init()
 {    
     bool ret = m_pimpl->ReadHeader(m_header);
-
     if (!ret)
         throw std::runtime_error("public header block reading failure");
-        
+
     ret = m_pimpl->ReadVLR(m_header);
     if (!ret)
         throw std::runtime_error("public vlr header block reading failure");
-    
+
     m_pimpl->ReadGeoreference(m_header);
-    
+    m_pimpl->Reset(m_header);
 }
 
 std::istream& LASReader::GetStream() const
@@ -143,14 +128,9 @@
     return m_pimpl->GetStream();
 }
 
-bool LASReader::Reset() 
+void LASReader::Reset() 
 {
-    std::istream& ifs = GetStream();
-    ifs.clear();
-    ifs.seekg(0);
-    MakePIMPL(ifs);
-	Init();
-    return true;
+    Init();
 }
 
 bool LASReader::IsEOF() const


More information about the Liblas-commits mailing list