[kstars] kstars: A few minor code optimizations plus adding documentation to clear things up. starData and deepStarData structures must be fully documented to remove any ambiguity. Looks like when opening deep stars catalog, the header is read twice so while it might not

Jasem Mutlaq mutlaqja at ikarustech.com
Mon Dec 5 10:52:23 UTC 2016


Git commit 882d9535be6da64a666600e717a6a2504e4c3142 by Jasem Mutlaq.
Committed on 05/12/2016 at 10:50.
Pushed by mutlaqja into branch 'master'.

A few minor code optimizations plus adding documentation to clear things up. starData and deepStarData structures must be fully documented to remove any ambiguity. Looks like when opening deep stars catalog, the header is read twice so while it might not
have a huge performance hit, it's better to avoid it.

CCMAIL:kstars-devel at kde.org

M  +13   -1    kstars/auxiliary/binfilehelper.cpp
M  +6    -5    kstars/auxiliary/binfilehelper.h
M  +29   -17   kstars/skycomponents/deepstarcomponent.cpp
M  +1    -2    kstars/skycomponents/starcomponent.cpp
M  +5    -4    kstars/skyobjects/deepstardata.h
M  +7    -6    kstars/skyobjects/stardata.h

https://commits.kde.org/kstars/882d9535be6da64a666600e717a6a2504e4c3142

diff --git a/kstars/auxiliary/binfilehelper.cpp b/kstars/auxiliary/binfilehelper.cpp
index 2476a82c5..8ae1e5db8 100644
--- a/kstars/auxiliary/binfilehelper.cpp
+++ b/kstars/auxiliary/binfilehelper.cpp
@@ -87,15 +87,21 @@ enum BinFileHelper::Errors BinFileHelper::__readHeader() {
     dataElement *de;
 
     // Read the preamble
-    if(!fileHandle) 
+    if(!fileHandle)
         return ERR_FILEOPEN;
 
     rewind(fileHandle);
 
+    // Read the first 124 bytes of the binary file which contains a general text about the binary data.
+    // e.g. "KStars Star Data v1.0. To be read using the 32-bit starData structure only"
     fread(ASCII_text, 124, 1, fileHandle);
     ASCII_text[124] = '\0';
     headerText = ASCII_text;
 
+    // Find out endianess from reading "KS" 0x4B53 in the binary file which was encoded on a little endian machine
+    // Therefore, in the binary file it is written as 53 4B (little endian as least significant byte is stored first),
+    // and when read on a little endian machine then it results in 0x4B53 (least significant byte is stored first in memory),
+    // whereas a big endian machine would read it as 0x534B (most significant byte is stored first in memory).
     fread(&endian_id, 2, 1, fileHandle);
     if(endian_id != 0x4B53)
         byteswap = 1;
@@ -112,6 +118,8 @@ enum BinFileHelper::Errors BinFileHelper::__readHeader() {
     for(i = 0; i < nfields; ++i) {
         // FIXME: Valgrind shows 176 bytes lost here in 11 blocks. Why? Investigate
         de = new dataElement;
+
+        // Read 16 byte dataElement that describe each field (name[8], size[1], type[1], scale[4])
         if(!fread(de, sizeof(dataElement), 1, fileHandle))
         {
             delete de;
@@ -142,6 +150,7 @@ enum BinFileHelper::Errors BinFileHelper::__readHeader() {
     quint32 nrecs;
     quint32 prev_nrecs;
 
+    // Find out current offset so far in the binary file (how many bytes we read so far)
     itableOffset = ftell(fileHandle);
 
     prev_offset = 0;
@@ -155,6 +164,9 @@ enum BinFileHelper::Errors BinFileHelper::__readHeader() {
         errorMessage.sprintf( "Zero index size!" );
         return ERR_INDEX_TRUNC;
     }
+
+    // We read each 12-byte index entry (ID[4], Offset[4] within file in Byte, nrec[4] # of Records).
+    // After reading all the indexes, we are (itableOffset + indexSize * 12) bytes within the file
     for(j = 0; j < indexSize; ++j) {
         if(!fread(&ID, 4, 1, fileHandle)) {
             errorMessage.sprintf("Table truncated before expected! Read i = %d index entries so far", j);
diff --git a/kstars/auxiliary/binfilehelper.h b/kstars/auxiliary/binfilehelper.h
index 604be8822..d4ea27459 100644
--- a/kstars/auxiliary/binfilehelper.h
+++ b/kstars/auxiliary/binfilehelper.h
@@ -28,13 +28,14 @@
 class QString;
 
 /**
- *@short   A structure describing a data field in the file
+ *@short   A structure describing a data field in the binary file. Its size is 16 bytes.
  */
-typedef struct dataElement {
-    char name[10];
-    qint8 size;
+typedef struct dataElement
+{
+    char name[10];      /**< Field name (eg. RA) */
+    qint8 size;         /**< Field size in bytes (eg. 4) */
     quint8 type;
-    qint32 scale;
+    qint32 scale;       /**< Field scale. The final field value = raw_value * scale */
 } dataElement;
 
 
diff --git a/kstars/skycomponents/deepstarcomponent.cpp b/kstars/skycomponents/deepstarcomponent.cpp
index 7349f7749..aee435aac 100644
--- a/kstars/skycomponents/deepstarcomponent.cpp
+++ b/kstars/skycomponents/deepstarcomponent.cpp
@@ -75,7 +75,9 @@ bool DeepStarComponent::loadStaticStars() {
         return false;
     }
 
-    if( starReader.guessRecordSize() != 16 && starReader.guessRecordSize() != 32 ) {
+    quint8 recordSize = starReader.guessRecordSize();
+    if( recordSize != 16 && recordSize != 32 )
+    {
         qDebug() << "Cannot understand catalog file " << dataFileName << endl;
         return false;
     }
@@ -102,28 +104,34 @@ bool DeepStarComponent::loadStaticStars() {
     if( htm_level != m_skyMesh->level() )
         qDebug() << "WARNING: HTM Level in shallow star data file and HTM Level in m_skyMesh do not match. EXPECT TROUBLE" << endl;
 
-    for(Trixel i = 0; i < (unsigned int)m_skyMesh->size(); ++i) {
-
+    for(Trixel i = 0; i < (unsigned int)m_skyMesh->size(); ++i)
+    {
         Trixel trixel = i;
-        StarBlock *SB = new StarBlock( starReader.getRecordCount( i ) );
+        quint64 records = starReader.getRecordCount( i );
+        StarBlock *SB = new StarBlock( records );
+
         if( !SB )
             qDebug() << "ERROR: Could not allocate new StarBlock to hold shallow unnamed stars for trixel " << trixel << endl;
+
         m_starBlockList.at( trixel )->setStaticBlock( SB );
 
-        for(unsigned long j = 0; j < (unsigned long) starReader.getRecordCount(i); ++j) {
+        for(quint64 j = 0; j < records; ++j)
+        {
             bool fread_success = false;
-            if( starReader.guessRecordSize() == 32 )
+            if( recordSize == 32 )
                 fread_success = fread( &stardata, sizeof( starData ), 1, dataFile );
-            else if( starReader.guessRecordSize() == 16 )
+            else if( recordSize == 16 )
                 fread_success = fread( &deepstardata, sizeof( deepStarData ), 1, dataFile );
 
-            if( !fread_success ) {
+            if( !fread_success )
+            {
                 qDebug() << "ERROR: Could not read starData structure for star #" << j << " under trixel #" << trixel << endl;
             }
 
             /* Swap Bytes when required */
-            if( starReader.getByteSwap() ) {
-                if( starReader.guessRecordSize() == 32 )
+            if( starReader.getByteSwap() )
+            {
+                if( recordSize == 32 )
                     byteSwap( &stardata );
                 else
                     byteSwap( &deepstardata );
@@ -131,7 +139,7 @@ bool DeepStarComponent::loadStaticStars() {
 
             /* Initialize star with data just read. */
             StarObject* star;
-            if( starReader.guessRecordSize() == 32 )
+            if( recordSize == 32 )
         #ifdef KSTARS_LITE
                 star = &(SB->addStar( stardata )->star);
         #else
@@ -143,12 +151,16 @@ bool DeepStarComponent::loadStaticStars() {
         #else
                 star = SB->addStar( deepstardata );
         #endif
-            if( star ) {
-                KStarsData* data = KStarsData::Instance();
-                star->EquatorialToHorizontal( data->lst(), data->geo()->lat() );
-                if( star->getHDIndex() != 0 )
-                    m_CatalogNumber.insert( star->getHDIndex(), star );
-            } else {
+            if( star )
+            {
+                //KStarsData* data = KStarsData::Instance();
+                //star->EquatorialToHorizontal( data->lst(), data->geo()->lat() );
+                //if( star->getHDIndex() != 0 )
+                if (stardata.HD)
+                    m_CatalogNumber.insert( stardata.HD, star );
+            }
+            else
+            {
                 qDebug() << "CODE ERROR: More unnamed static stars in trixel " << trixel << " than we allocated space for!" << endl;
             }
         }
diff --git a/kstars/skycomponents/starcomponent.cpp b/kstars/skycomponents/starcomponent.cpp
index c9e124a5c..a77a32daa 100644
--- a/kstars/skycomponents/starcomponent.cpp
+++ b/kstars/skycomponents/starcomponent.cpp
@@ -377,8 +377,7 @@ void StarComponent::drawLabels()
 bool StarComponent::loadStaticData()
 {
     // We break from Qt / KDE API and use traditional file handling here, to obtain speed.
-    // We also avoid C++ constructors for the same reason.
-    KStarsData* data = KStarsData::Instance();
+    // We also avoid C++ constructors for the same reason.    
     FILE *dataFile, *nameFile;
     bool swapBytes = false, named=false, gnamed=false;
     BinFileHelper dataReader, nameReader;
diff --git a/kstars/skyobjects/deepstardata.h b/kstars/skyobjects/deepstardata.h
index 8faad3fd2..739f7e7e4 100644
--- a/kstars/skyobjects/deepstardata.h
+++ b/kstars/skyobjects/deepstardata.h
@@ -21,13 +21,14 @@
 #include <QtGlobal>
 
 /**
- *@short  Structure that holds star data for really faint stars
+ *@short  A 16-byte structure that holds star data for really faint stars.
  *@author Akarsh Simha
  *@version 1.0
  */
-struct deepStarData {
-    qint32 RA;
-    qint32 Dec;
+struct deepStarData
+{
+    qint32 RA;          /**< Raw signed 32-bit RA value. Needs to be multiplied by the scale (1e6) */
+    qint32 Dec;         /**< Raw signed 32-bit DE value. Needs to be multiplied by the scale (1e6) */
     qint16 dRA;
     qint16 dDec;
     qint16 B;
diff --git a/kstars/skyobjects/stardata.h b/kstars/skyobjects/stardata.h
index 393a38f71..62bdef219 100644
--- a/kstars/skyobjects/stardata.h
+++ b/kstars/skyobjects/stardata.h
@@ -21,18 +21,19 @@
 #include <QtGlobal>
 
 /**
- *@short  Structure that holds star data
+ *@short  A 32-byte Structure that holds star data
  *@author Akarsh Simha
  *@version 1.0
  */
-struct starData {
-    qint32 RA;
-    qint32 Dec;
+struct starData
+{
+    qint32 RA;          /**< Raw signed 32-bit RA value. Needs to be multiplied by the scale (1e6) */
+    qint32 Dec;         /**< Raw signed 32-bit DE value. Needs to be multiplied by the scale (1e6) */
     qint32 dRA;
     qint32 dDec;
     qint32 parallax;
-    qint32 HD;
-    qint16 mag;
+    qint32 HD;          /**< signed? 32-bit Henry Draper Index */
+    qint16 mag;         /**< signed 16-bit raw magnitude. Needs to be divided by the scale (1e2) */
     qint16 bv_index;
     char spec_type[2];
     char flags;


More information about the Kstars-devel mailing list