[Kstars-devel] KDE/kdeedu/kstars/kstars/skycomponents
Alexey Khudyakov
alexey.skladnoy at gmail.com
Thu Jun 18 21:55:39 CEST 2009
SVN commit 983693 by khudyakov:
* Replace memcpy based copying in with more C++ "copying". Instead
pass reference to {starData,deepStarData} into StarBlock::addStar
and reinitialize star with it.
Possible pitfalls. StarObject::init doesn't reset object name(s) but I
believe it's not real problem for stars which are swapped in/out since
they do not have name.
Code is marginally faster when not dominated by I/O.
* Disallow copying/assignment of StarBlock. Just to add some safety
CCMAIL: kstars-devel at kde.org
M +11 -22 deepstarcomponent.cpp
M +37 -54 starblock.cpp
M +27 -45 starblock.h
M +18 -18 starblocklist.cpp
--- trunk/KDE/kdeedu/kstars/kstars/skycomponents/deepstarcomponent.cpp #983692:983693
@@ -46,7 +46,6 @@
bool DeepStarComponent::loadStaticStars() {
FILE *dataFile;
- StarObject plainStarTemplate;
if( !staticStars )
return true;
@@ -114,30 +113,20 @@
byteSwap( &deepstardata );
}
-
- /*
- * CAUTION: We avoid trying to construct StarObjects using the constructors [The C++ way]
- * in order to gain speed. Instead, one template StarObject is constructed and
- * all other unnamed stars are created by doing a raw copy from this using memcpy()
- * and then calling StarObject::init() to replace the default data with the correct
- * data.
- * This means that this section of the code plays around with pointers to a great
- * extent and has a chance of breaking down / causing segfaults.
- */
-
- /* Make a copy of the star template and set up the data in it */
+ /* Initialize star with data just read. */
+ StarObject* star;
if( starReader.guessRecordSize() == 32 )
- plainStarTemplate.init( &stardata );
+ star = SB->addStar( stardata );
else
- plainStarTemplate.init( &deepstardata );
- plainStarTemplate.EquatorialToHorizontal( data()->lst(), data()->geo()->lat() );
- if( !SB->addStar( &plainStarTemplate ) )
+ star = SB->addStar( deepstardata );
+
+ if( star ) {
+ star->EquatorialToHorizontal( data()->lst(), data()->geo()->lat() );
+ if( star->getHDIndex() != 0 )
+ m_CatalogNumber.insert( star->getHDIndex(), star );
+ } else {
kDebug() << "CODE ERROR: More unnamed static stars in trixel " << trixel << " than we allocated space for!" << endl;
- StarObject *star = SB->star( SB->getStarCount() - 1 );
-
- if( star->getHDIndex() != 0 )
- m_CatalogNumber.insert( star->getHDIndex(), star );
-
+ }
}
}
--- trunk/KDE/kdeedu/kstars/kstars/skycomponents/starblock.cpp #983692:983693
@@ -19,33 +19,25 @@
#include "skyobjects/starobject.h"
#include "starcomponent.h"
#include "skyobjects/stardata.h"
+#include "skyobjects/deepstardata.h"
#include <kdebug.h>
-StarObject *StarBlock::plainStarTemplate = NULL;
-int StarBlock::refCount = 0;
-StarBlock::StarBlock() {
- init();
- allocStars( 100 ); // TODO: Make this number user-specifiable
-}
+StarBlock::StarBlock( int nstars ) :
+ faintMag(-5),
+ brightMag(35),
+ parent(0),
+ prev(0),
+ next(0),
+ drawID(0),
+ nStars(0),
+ stars(nstars, StarObject())
+{ }
-StarBlock::StarBlock( int nstars ) {
- init();
- allocStars( nstars );
-}
-void StarBlock::init() {
- parent = NULL;
- prev = next = NULL;
- drawID = 0;
- if( !plainStarTemplate )
- plainStarTemplate = new StarObject;
- refCount++;
- reset();
-}
-
-void StarBlock::reset() {
+void StarBlock::reset()
+{
if( parent )
parent->releaseBlock( this );
parent = NULL;
@@ -54,45 +46,36 @@
nStars = 0;
}
-
-StarBlock::~StarBlock() {
- StarObject *star;
- foreach( star, stars )
- free( star );
- refCount--;
- if(refCount == 0) {
- delete plainStarTemplate;
- plainStarTemplate = NULL;
- }
+StarBlock::~StarBlock()
+{
if( parent )
parent -> releaseBlock( this );
}
-
-int StarBlock::allocStars( int nstars ) {
- StarObject *newStarObject;
- for( int i = 0; i < nstars; ++i ) {
- newStarObject = (StarObject *) malloc( sizeof( StarObject ) );
- if( !newStarObject )
- return i;
- memcpy( newStarObject, plainStarTemplate, sizeof( StarObject ) );
- stars.append( newStarObject );
- }
- return nstars;
+StarObject* StarBlock::addStar(const starData& data)
+{
+ if(isFull())
+ return 0;
+ StarObject& star = stars[nStars++];
+
+ star.init(&data);
+ if( star.mag() > faintMag )
+ faintMag = star.mag();
+ if( star.mag() < brightMag )
+ brightMag = star.mag();
+ return ☆
}
-bool StarBlock::addStar( StarObject *star ) {
-
+StarObject* StarBlock::addStar(const deepStarData& data)
+{
if(isFull())
- return false;
-
- memcpy( stars.at( nStars ), star, sizeof( StarObject ) );
+ return 0;
+ StarObject& star = stars[nStars++];
- if( star->mag() > faintMag )
- faintMag = star->mag();
- if( star->mag() < brightMag )
- brightMag = star->mag();
-
- ++nStars;
- return true;
+ star.init(&data);
+ if( star.mag() > faintMag )
+ faintMag = star.mag();
+ if( star.mag() < brightMag )
+ brightMag = star.mag();
+ return ☆
}
--- trunk/KDE/kdeedu/kstars/kstars/skycomponents/starblock.h #983692:983693
@@ -25,6 +25,8 @@
class StarObject;
class StarBlockList;
+class starData;
+class deepStarData;
/**
*@class StarBlock
@@ -33,41 +35,32 @@
*@author Akarsh Simha
*@version 1.0
*/
-
-class StarBlock {
-
- public:
-
- /**
- * Constructor
- * Initializes values of various parameters and creates a predefined number of stars
+class StarBlock
+{
+public:
+ /** Constructor
+ * Initializes values of various parameters and creates nstars number of stars
+ * @param nstars Number of stars to hold in this StarBlock
*/
- StarBlock();
+ StarBlock( int nstars = 100 );
- /**
- * Constructor
- * Initializes values of various parameters and creates nstars number of stars
- *@param nstars Number of stars to hold in this StarBlock
+ /**
+ * Destructor
+ * Deletes all stored stars
*/
- StarBlock( int nstars );
-
- /**
- * Destructor
- * Deletes any stored stars
- */
~StarBlock();
- /**
- *@short Make a copy of the given StarObject in this StarBlock
+ /** @short Initialize another star with data.
*
- *This method uses memcpy() to copy the given StarObject into the next un-initialized
- *StarObject in this StarBlock. If the capacity of this StarBlock is exceeded, it
- *returns false.
+ * FIXME: StarObject::init doesn't reset object name(s). It
+ * shouldn't be issue since stars which are swapped in/out do not
+ * have names.
*
- *@param star Pointer to a StarObject
- *@return true on success, false on failure
+ *@param data data to initialize star with.
+ *@return pointer to star initialized with data. NULL if block is full.
*/
- bool addStar( StarObject *star );
+ StarObject* addStar(const starData& data);
+ StarObject* addStar(const deepStarData& data);
/**
*@short Returns true if the StarBlock is full
@@ -93,7 +86,7 @@
*@param Index of StarBlock to return
*@return A pointer to the i-th StarObject
*/
- inline StarObject *star( int i ) { return stars.at( i ); }
+ inline StarObject *star( int i ) { return &stars[i]; }
// These methods are there because we might want to make faintMag and brightMag private at some point
/**
@@ -129,25 +122,14 @@
quint32 drawID;
private:
+ // Disallow copying and assignment. Just in case.
+ StarBlock(const StarBlock&);
+ StarBlock& operator = (const StarBlock&);
- /**
- *@short Allocate Space for nstars stars and put the pointers in the stars vector
- *
- *@param nstars Number of stars to allocate space for
- *@return Number of StarObjects successfully allocated
- */
- int allocStars( int nstars );
-
- /**
- *@short Do the real construction work
- */
- void init();
-
+ /** Number of initialized stars in StarBlock. */
int nStars;
- QVector< StarObject *> stars;
-
- static StarObject *plainStarTemplate;
- static int refCount;
+ /** Array of stars. */
+ QVector<StarObject> stars;
};
#endif
--- trunk/KDE/kdeedu/kstars/kstars/skycomponents/starblocklist.cpp #983692:983693
@@ -67,7 +67,6 @@
// TODO: Remove staticity of BinFileHelper
BinFileHelper *dSReader;
StarBlockFactory *SBFactory;
- StarObject star;
starData stardata;
deepStarData deepstardata;
FILE *dataFile;
@@ -122,27 +121,28 @@
}
// TODO: Make this more general
if( dSReader->guessRecordSize() == 32 ) {
- fread( &stardata, sizeof( starData ), 1, dataFile );
- if( dSReader->getByteSwap() ) DeepStarComponent::byteSwap( &stardata );
- readOffset += sizeof( starData );
- star.init( &stardata );
+ fread( &stardata, sizeof( starData ), 1, dataFile );
+ if( dSReader->getByteSwap() )
+ DeepStarComponent::byteSwap( &stardata );
+ readOffset += sizeof( starData );
+ blocks[nBlocks - 1]->addStar(stardata);
}
else {
- fread( &deepstardata, sizeof( deepStarData ), 1, dataFile );
- if( dSReader->getByteSwap() ) DeepStarComponent::byteSwap( &deepstardata );
- readOffset += sizeof( deepStarData );
- star.init( &deepstardata );
+ fread( &deepstardata, sizeof( deepStarData ), 1, dataFile );
+ if( dSReader->getByteSwap() )
+ DeepStarComponent::byteSwap( &deepstardata );
+ readOffset += sizeof( deepStarData );
+ blocks[nBlocks - 1]->addStar(deepstardata);
}
- blocks[nBlocks - 1]->addStar( &star );
- /*
- if( faintMag > -5.0 && fabs(faintMag - blocks[nBlocks - 1]->getFaintMag()) > 0.2 ) {
- kDebug() << "Encountered a jump from mag" << faintMag << "to mag"
- << blocks[nBlocks - 1]->getFaintMag() << "in trixel" << trixel;
- }
- */
- faintMag = blocks[nBlocks - 1]->getFaintMag();
- nStars++;
+ /*
+ if( faintMag > -5.0 && fabs(faintMag - blocks[nBlocks - 1]->getFaintMag()) > 0.2 ) {
+ kDebug() << "Encountered a jump from mag" << faintMag << "to mag"
+ << blocks[nBlocks - 1]->getFaintMag() << "in trixel" << trixel;
+ }
+ */
+ faintMag = blocks[nBlocks - 1]->getFaintMag();
+ nStars++;
}
return ( ( maglim < faintMag ) ? true : false );
More information about the Kstars-devel
mailing list