[Liblas-commits] libpc: moved isActive into the Field, but it is still isn't a cl...

liblas-commits at liblas.org liblas-commits at liblas.org
Tue Feb 8 19:31:45 EST 2011


details:   http://hg.liblas.orglibpc/rev/3f38e4333773
changeset: 15:3f38e4333773
user:      Michael P. Gerlek <mpg at flaxen.com>
date:      Tue Feb 08 14:48:46 2011 -0800
description:
moved isActive into the Field, but it is still isn't a clean concept; checkpoint here, before removing it
Subject: libpc: removed isActive (I feel better now)

details:   http://hg.liblas.orglibpc/rev/4101f814698f
changeset: 16:4101f814698f
user:      Michael P. Gerlek <mpg at flaxen.com>
date:      Tue Feb 08 14:51:21 2011 -0800
description:
removed isActive (I feel better now)
Subject: libpc: remove unused index

details:   http://hg.liblas.orglibpc/rev/099246b5c27e
changeset: 17:099246b5c27e
user:      Michael P. Gerlek <mpg at flaxen.com>
date:      Tue Feb 08 16:31:41 2011 -0800
description:
remove unused index

diffstat:

 include/libpc/Field.hpp       |   7 ++++++-
 include/libpc/PointData.hpp   |  24 +++++++++++++++++++-----
 include/libpc/PointLayout.hpp |  25 ++++++++++++-------------
 include/libpc/Reader.hpp      |   3 +--
 include/libpc/Stage.hpp       |   4 ++++
 include/libpc/Writer.hpp      |   9 ++++++++-
 src/ColorFilter.cpp           |  10 +++-------
 src/FauxReader.cpp            |  14 ++++----------
 src/Field.cpp                 |   8 ++++----
 src/PointData.cpp             |  43 +++++++++++++++----------------------------
 src/PointLayout.cpp           |  29 +----------------------------
 11 files changed, 77 insertions(+), 99 deletions(-)

diffs (truncated from 448 to 300 lines):

diff -r a7608f689f74 -r 099246b5c27e include/libpc/Field.hpp
--- a/include/libpc/Field.hpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/include/libpc/Field.hpp	Tue Feb 08 16:31:41 2011 -0800
@@ -79,12 +79,17 @@
   Field& operator=(const Field&);
   bool operator==(const Field& other) const;
 
+  // what the field represents
   DataItem getItem() const { return m_item; }
+
+  // the datatype of the field
   DataType getType() const { return m_type; }
 
+  // what byte the field starts at, within the raw bytes of the point
   int getOffset() const { return m_offset; }
   void setOffset(int offset) { m_offset = offset; }
 
+  // number of bytes needed for the datatype
   int getNumBytes() const;
 
   void dump() const;
@@ -92,7 +97,7 @@
 private:
   DataItem m_item;
   DataType m_type;
-  int m_offset;// byte offset within a point buffer
+  int m_offset; // byte offset within a point buffer
 };
 
 #endif
diff -r a7608f689f74 -r 099246b5c27e include/libpc/PointData.hpp
--- a/include/libpc/PointData.hpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/include/libpc/PointData.hpp	Tue Feb 08 16:31:41 2011 -0800
@@ -51,24 +51,35 @@
 class PointData
 {
 public:
+  // note that when we make a PointData object all the fields are initialized to inactive,
+  // regardless of what the passed-in layout says -- this is because the field object 
+  // represents the state within the owning object, which in this case is a completely
+  // empty buffer (similarly, all the points in the buffer are marked "invalid")
   PointData(const PointLayout&, int numPoints);
 
-  byte* getData(int pointIndex) const;
+  // number of points in this buffer
   int getNumPoints() const;
+
+  // layout (number and kinds of fields) for a point in this buffer
   const PointLayout& getLayout() const;
 
+  // "valid" means the data for the point can be used; if invalid, the point should
+  // be ignored or skipped.  (This is done for efficiency; we don't want to have to
+  // modify the buffer's size just to "delete" a point.)
   bool isValid(int pointIndex) const;
   void setValid(int pointIndex, bool value=true);
 
+  // accessors to a particular field of a particular point in this buffer
   byte getField_U8(int pointIndex, int fieldIndex) const;
   float getField_F32(int pointIndex, int fieldIndex) const;
   double getField_F64(int pointIndex, int fieldIndex) const;
 
+  // accessors to a particular field of a particular point in this buffer
   void setField_U8(int pointIndex, int fieldIndex, byte value);
   void setField_F32(int pointIndex, int fieldIndex, float value);
   void setField_F64(int pointIndex, int fieldIndex, double value);
 
-  // some well-known fields
+  // handy functions to get at some some well-known fields
   float getX(int pointIndex) const;
   float getY(int pointIndex) const;
   float getZ(int pointIndex) const;
@@ -85,14 +96,17 @@
   void dump(int index, std::string indent="") const;
 
 private:
-  template<class T> T getField(int index, int itemOffset) const;
-  template<class T> void setField(int index, int itemOffset, T value);
+  // access to the raw memory
+  byte* getData(int pointIndex) const;
+
+  template<class T> T getField(int fieldIndex, int itemOffset) const;
+  template<class T> void setField(int fieldIndex, int itemOffset, T value);
 
   PointLayout m_layout;
   byte* m_data;
   int m_pointSize;
   int m_numPoints;
-  std::vector<bool> m_isValid;
+  std::vector<bool> m_isValid; // one bool for each point
 
   PointData(const PointData&); // not implemented
   PointData& operator=(const PointData&); // not implemented
diff -r a7608f689f74 -r 099246b5c27e include/libpc/PointLayout.hpp
--- a/include/libpc/PointLayout.hpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/include/libpc/PointLayout.hpp	Tue Feb 08 16:31:41 2011 -0800
@@ -46,21 +46,21 @@
 public:
   PointLayout();
   PointLayout(const PointLayout&);
-  PointLayout& operator=(const PointLayout & other);
+  PointLayout& operator=(const PointLayout& other);
 
-  bool operator==(const PointLayout & other) const;
-  bool same(const PointLayout & other, bool ignoreActive=false) const;
+  bool operator==(const PointLayout& other) const;
 
-  // returns the index of the field added
+  // adds a field to the end of the layout, and returns the index of the field added
   int addField(const Field& field);
 
+  // returns a given field
   const Field& getField(int fieldIndex) const { return m_fields[fieldIndex]; }
-  bool isActive(int fieldIndex) const { return m_isActive[fieldIndex]; }
-  void setActive(int fieldIndex, bool value=true) { m_isActive[fieldIndex] = value; }
-  void copyActiveVector(const std::vector<bool>& other) { m_isActive = other; }
-  const std::vector<bool>& getActiveVector() const { return m_isActive; }
+  Field& getField(int fieldIndex) { return m_fields[fieldIndex]; }
 
+  // total num bytes for all fields in the point
   int getSizeInBytes() const;
+
+  // number of fields in the point
   int getNumFields() const;
 
   void dump(std::string indent="") const;
@@ -68,20 +68,19 @@
   // returns -1 if not found
   int findFieldIndex(Field::DataItem item) const;
   bool hasField(Field::DataItem item) const;
-  int findFieldOffset(Field::DataItem item) const;
 
   // some well-known field types are always available
-  // is this worth it?
+  // (is this worth it?)
   int getFieldIndex_X() const { return m_fieldIndex_X; }
   int getFieldIndex_Y() const { return m_fieldIndex_Y; }
   int getFieldIndex_Z() const { return m_fieldIndex_Z; }
 
 private:
-  std::vector<Field> m_fields;
-  std::vector<bool> m_isActive;
+  std::vector<Field> m_fields; // each of the fields
 
-  int m_numBytes;
+  int m_numBytes; // num bytes required to store all fields
 
+  // local cache of handy values
   int m_fieldIndex_X;
   int m_fieldIndex_Y;
   int m_fieldIndex_Z;
diff -r a7608f689f74 -r 099246b5c27e include/libpc/Reader.hpp
--- a/include/libpc/Reader.hpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/include/libpc/Reader.hpp	Tue Feb 08 16:31:41 2011 -0800
@@ -38,13 +38,12 @@
 #include "libpc/Stage.hpp"
 #include "libpc/header.hpp"
 
+
 class Reader : public Stage
 {
 public:
   Reader();
 
-  virtual void readNextPoints(PointData&) = 0;
-
 protected:
   int m_lastPointRead;
 
diff -r a7608f689f74 -r 099246b5c27e include/libpc/Stage.hpp
--- a/include/libpc/Stage.hpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/include/libpc/Stage.hpp	Tue Feb 08 16:31:41 2011 -0800
@@ -45,6 +45,10 @@
 public:
   Stage();
   
+  // The layout of the PointData buffer we are given here might
+  // not match our own header's layout.  That's okay, though: all
+  // that matters is that the buffer we are given has the fields
+  // we need to write into.
   virtual void readNextPoints(PointData&) = 0;
 
   const Header& getHeader() const;
diff -r a7608f689f74 -r 099246b5c27e include/libpc/Writer.hpp
--- a/include/libpc/Writer.hpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/include/libpc/Writer.hpp	Tue Feb 08 16:31:41 2011 -0800
@@ -38,7 +38,8 @@
 #include "libpc/Filter.hpp"
 
 
-// a Writer is a Filter (and so it has a previous stage and a header), but generally they do not implement their readNextPoints method
+// a Writer is a Filter, because it has a previous stage and a header
+// however, Writers generally do not implement the readNextPoints method
 class Writer : public Filter
 {
 public:
@@ -47,11 +48,17 @@
   void write();
 
 protected:
+  // this is called once before the loop with the writeBuffer calls
   virtual void writeBegin() = 0;
+
+  // called repeatedly, until out of data
   virtual void writeBuffer(const PointData&) = 0;
+
+  // called once, after the writeBuffer calls
   virtual void writeEnd() = 0;
 
 private:
+  // not generally used in Writer objects
   virtual void readNextPoints(PointData&) { throw; }
 
   Writer& operator=(const Writer&); // not implemented
diff -r a7608f689f74 -r 099246b5c27e src/ColorFilter.cpp
--- a/src/ColorFilter.cpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/src/ColorFilter.cpp	Tue Feb 08 16:31:41 2011 -0800
@@ -43,13 +43,9 @@
   PointLayout& layout = getHeader().getLayout();
 
   // add the three u8 fields
-  int index;
-  index = layout.addField(Field(Field::Zred, Field::U8));
-  layout.setActive(index);
-  index = layout.addField(Field(Field::Zgreen, Field::U8));
-  layout.setActive(index);
-  index = layout.addField(Field(Field::Zblue, Field::U8));
-  layout.setActive(index);
+  layout.addField(Field(Field::Zred, Field::U8));
+  layout.addField(Field(Field::Zgreen, Field::U8));
+  layout.addField(Field(Field::Zblue, Field::U8));
 
   return;
 }
diff -r a7608f689f74 -r 099246b5c27e src/FauxReader.cpp
--- a/src/FauxReader.cpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/src/FauxReader.cpp	Tue Feb 08 16:31:41 2011 -0800
@@ -50,16 +50,10 @@
   header.setNumPoints(numPoints);
   header.setBounds(bounds);
 
-  int index;
-  
-  index = layout.addField(Field(Field::XPos, Field::F32));
-  layout.setActive(index);
-  index = layout.addField(Field(Field::YPos, Field::F32));
-  layout.setActive(index);
-  index = layout.addField(Field(Field::ZPos, Field::F32));
-  layout.setActive(index);
-  index = layout.addField(Field(Field::Time, Field::F64));
-  layout.setActive(index);
+  layout.addField(Field(Field::XPos, Field::F32));
+  layout.addField(Field(Field::YPos, Field::F32));
+  layout.addField(Field(Field::ZPos, Field::F32));
+  layout.addField(Field(Field::Time, Field::F64));
 
   header.dump();
 
diff -r a7608f689f74 -r 099246b5c27e src/Field.cpp
--- a/src/Field.cpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/src/Field.cpp	Tue Feb 08 16:31:41 2011 -0800
@@ -60,10 +60,10 @@
 }
 
 
-Field::Field(const Field& field)
-  : m_item(field.m_item),
-  m_type(field.m_type),
-  m_offset(field.m_offset)
+Field::Field(const Field& other)
+  : m_item(other.m_item),
+  m_type(other.m_type),
+  m_offset(other.m_offset)
 {
 
   return;
diff -r a7608f689f74 -r 099246b5c27e src/PointData.cpp
--- a/src/PointData.cpp	Tue Feb 08 13:08:59 2011 -0800
+++ b/src/PointData.cpp	Tue Feb 08 16:31:41 2011 -0800
@@ -51,6 +51,7 @@
   m_pointSize = m_layout.getSizeInBytes();
   m_data = new byte[m_pointSize * m_numPoints];
   
+  // the points will all be set to invalid here
   m_isValid.resize(m_numPoints);
 
   return;
@@ -92,8 +93,6 @@
 template <class T>
 void PointData::setField(int pointIndex, int fieldIndex, T value)
 {
-  assert(m_layout.isActive(fieldIndex));
-
   int offset = (pointIndex * m_pointSize) + m_layout.getField(fieldIndex).getOffset();
   assert(offset + (int)sizeof(T) <= m_pointSize * m_numPoints);
   byte* p = m_data + offset;
@@ -105,8 +104,6 @@
 template <class T>
 T PointData::getField(int pointIndex, int fieldIndex) const
 {
-  assert(m_layout.isActive(fieldIndex));


More information about the Liblas-commits mailing list