Skip to content

Commit

Permalink
Made requested code changes from pull request review
Browse files Browse the repository at this point in the history
  • Loading branch information
@GPayne
GPayne committed Oct 20, 2017
1 parent 47c271a commit e63197c
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 76 deletions.
26 changes: 13 additions & 13 deletions include/sgct/SGCTMpcdi.h
View file
Expand Up @@ -2,7 +2,7 @@
* SGCTMpcdi.h
*
* Created on: Jul 3, 2017
* Author: openspace
* Author: Gene Payne
*/

#ifndef _SGCT_MPCDI
Expand All @@ -24,7 +24,7 @@
namespace sgct_core //simple graphics cluster toolkit
{

struct mpcdiSubFiles {
struct MpcdiSubFiles {
enum mpcdiSubFileTypes {
mpcdiXml = 0,
mpcdiPfm,
Expand All @@ -36,7 +36,7 @@ struct mpcdiSubFiles {
int size[mpcdi_nRequiredFiles];
char* buffer[mpcdi_nRequiredFiles];

mpcdiSubFiles() {
MpcdiSubFiles() {
for (int i = 0; i < mpcdi_nRequiredFiles; ++i) {
hasFound[i] = false;
buffer[i] = nullptr;
Expand All @@ -45,26 +45,26 @@ struct mpcdiSubFiles {
extension[mpcdiPfm] = "pfm";
}

~mpcdiSubFiles() {
~MpcdiSubFiles() {
for (int i = 0; i < mpcdi_nRequiredFiles; ++i) {
if( buffer[i] != nullptr )
delete buffer[i];
}
}
};

struct mpcdiRegion {
struct MpcdiRegion {
std::string id;
};

struct mpcdiWarp {
struct MpcdiWarp {
std::string id;
std::string pathWarpFile;
bool haveFoundPath = false;
bool haveFoundInterpolation = false;
};

struct mpcdiFoundItems {
struct MpcdiFoundItems {
bool haveDisplayElem = false;
bool haveBufferElem = false;
int resolutionX = -1;
Expand All @@ -84,13 +84,13 @@ class SGCTMpcdi
bool readAndParseXML_mpcdi(tinyxml2::XMLDocument& xmlDoc, SGCTNode tmpNode,
sgct::SGCTWindow& tmpWin);
bool readAndParseXML_display(tinyxml2::XMLElement* element[], const char* val[],
SGCTNode tmpNode, sgct::SGCTWindow& tmpWin, mpcdiFoundItems& parsedItems);
SGCTNode tmpNode, sgct::SGCTWindow& tmpWin, MpcdiFoundItems& parsedItems);
bool readAndParseXML_files(tinyxml2::XMLElement* element[], const char* val[],
sgct::SGCTWindow& tmpWin);
bool readAndParseXML_buffer(tinyxml2::XMLElement* element[], const char* val[],
sgct::SGCTWindow& tmpWin, mpcdiFoundItems& parsedItems);
sgct::SGCTWindow& tmpWin, MpcdiFoundItems& parsedItems);
bool readAndParseXML_region(tinyxml2::XMLElement* element[], const char* val[],
sgct::SGCTWindow& tmpWin, mpcdiFoundItems& parsedItems);
sgct::SGCTWindow& tmpWin, MpcdiFoundItems& parsedItems);
bool readAndParseXML_geoWarpFile(tinyxml2::XMLElement* element[],
const char* val[], sgct::SGCTWindow& tmpWin,
std::string filesetRegionId);
Expand All @@ -103,9 +103,9 @@ class SGCTMpcdi
const std::string expectedTag);
void unsupportedFeatureCheck(std::string tag, std::string featureName);

mpcdiSubFiles mMpcdiSubFileContents;
std::vector<mpcdiRegion*> mBufferRegions;
std::vector<mpcdiWarp*> mWarp;
MpcdiSubFiles mMpcdiSubFileContents;
std::vector<MpcdiRegion*> mBufferRegions;
std::vector<MpcdiWarp*> mWarp;
std::string mErrorMsg;
};

Expand Down
6 changes: 3 additions & 3 deletions include/sgct/Viewport.h
View file
Expand Up @@ -27,7 +27,7 @@ For conditions of distribution and use, see copyright notice in sgct.h

namespace sgct_core
{
struct frustumData {
struct FrustumData {
enum elemIdx {
down = 0,
up,
Expand All @@ -41,7 +41,7 @@ struct frustumData {
bool foundElem[numElements];
float value[numElements];

frustumData() {
FrustumData() {
for (bool& i : foundElem)
i = false;
for (float& i : value)
Expand Down Expand Up @@ -94,7 +94,7 @@ class Viewport : public BaseViewport
void parseSphericalMirrorProjection(tinyxml2::XMLElement * element);
void parseFloatFromAttribute(tinyxml2::XMLElement* element, const std::string tag,
float& target);
bool parseFrustumElement(frustumData& frustum, frustumData::elemIdx elemIndex,
bool parseFrustumElement(FrustumData& frustum, FrustumData::elemIdx elemIndex,
tinyxml2::XMLElement* elem, const char* frustumTag);
private:
CorrectionMesh mCM;
Expand Down
1 change: 0 additions & 1 deletion lib/mingw/merge.bat
View file
Expand Up @@ -11,4 +11,3 @@ ar x deps/libvrpn.a
ar x libsgct_light.a
ar r libsgct.a *.obj
DEL *.o *.obj
pause
1 change: 0 additions & 1 deletion lib/mingw_x64/merge.bat
View file
Expand Up @@ -11,4 +11,3 @@ ar x deps/libvrpn.a
ar x libsgct_light.a
ar r libsgct.a *.obj
DEL *.o *.obj
pause
2 changes: 1 addition & 1 deletion src/apps/calibrator/run_mpcdi_example1.bat
View file
@@ -1 +1 @@
calibrator.exe --Ignore-Sync -tilt 0.0 -radius 5.5 -config mpcdi_example1_distorted.xml
calibrator.exe --Ignore-Sync -tilt 0.0 -radius 5.5 -config mpcdi_example1_distorted.xml
2 changes: 1 addition & 1 deletion src/apps/calibrator/run_mpcdi_example2.bat
View file
@@ -1 +1 @@
calibrator.exe --Ignore-Sync -tilt 0.0 -radius 5.5 -config mpcdi_example2_undistorted.xml
calibrator.exe --Ignore-Sync -tilt 0.0 -radius 5.5 -config mpcdi_example2_undistorted.xml
71 changes: 40 additions & 31 deletions src/sgct/SGCTMpcdi.cpp
View file
Expand Up @@ -2,7 +2,7 @@
* SGCTMpcdi.cpp
*
* Created on: Jul 3, 2017
* Author: openspace
* Author: Gene Payne
*/

#include <sgct/ogl_headers.h>
Expand All @@ -23,15 +23,13 @@ sgct_core::SGCTMpcdi::SGCTMpcdi(std::string& parentErrorMessage) : mErrorMsg(par

sgct_core::SGCTMpcdi::~SGCTMpcdi()
{
for (mpcdiWarp* ptr : mWarp)
for (MpcdiWarp* ptr : mWarp)
{
if (ptr)
delete ptr;
delete ptr;
}
for (mpcdiRegion* ptr : mBufferRegions)
for (MpcdiRegion* ptr : mBufferRegions)
{
if (ptr)
delete ptr;
delete ptr;
}
}

Expand All @@ -43,7 +41,8 @@ bool sgct_core::SGCTMpcdi::parseConfiguration(const std::string filenameMpcdi,
unzFile zipfile;
const int MaxFilenameSize_bytes = 500;

if (!openZipFile(cfgFile, filenameMpcdi, &zipfile))
bool fileOpenSuccess = openZipFile(cfgFile, filenameMpcdi, &zipfile);
if (!fileOpenSuccess)
{
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"parseMpcdiConfiguration: Unable to open zip archive file %s\n",
Expand All @@ -52,7 +51,8 @@ bool sgct_core::SGCTMpcdi::parseConfiguration(const std::string filenameMpcdi,
}
// Get info about the zip file
unz_global_info global_info;
if (unzGetGlobalInfo(zipfile, &global_info) != UNZ_OK)
int globalInfoRet = unzGetGlobalInfo(zipfile, &global_info);
if (globalInfoRet != UNZ_OK)
{
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"parseMpcdiConfiguration: Unable to get zip archive info from %s\n",
Expand All @@ -66,32 +66,36 @@ bool sgct_core::SGCTMpcdi::parseConfiguration(const std::string filenameMpcdi,
{
unz_file_info file_info;
char filename[MaxFilenameSize_bytes];
if (unzGetCurrentFileInfo(zipfile, &file_info, filename, MaxFilenameSize_bytes,
NULL, 0, NULL, 0) != UNZ_OK)
int getCurrentFileInfo = unzGetCurrentFileInfo(zipfile, &file_info, filename,
MaxFilenameSize_bytes, NULL, 0, NULL, 0);
if (getCurrentFileInfo != UNZ_OK)
{
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"parseMpcdiConfiguration: Unable to get info on compressed file #%d\n", i);
unzClose(zipfile);
return false;
}

if (!processSubFiles(filename, &zipfile, file_info))
bool isSubFileValid = processSubFiles(filename, &zipfile, file_info);
if (!isSubFileValid)
{
unzClose(zipfile);
return false;
}
if ((i + 1) < global_info.number_entry)
{
if (unzGoToNextFile(zipfile) != UNZ_OK)
int goToNextFileStatus = unzGoToNextFile(zipfile);
if (goToNextFileStatus != UNZ_OK)
{
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_WARNING,
"parseMpcdiConfiguration: Unable to get next file in archive\n");
}
}
}
unzClose(zipfile);
if( !mMpcdiSubFileContents.hasFound[mpcdiSubFiles::mpcdiXml]
|| !mMpcdiSubFileContents.hasFound[mpcdiSubFiles::mpcdiPfm])
bool hasXmlFile = mMpcdiSubFileContents.hasFound[MpcdiSubFiles::mpcdiXml];
bool hasPfmFile = mMpcdiSubFileContents.hasFound[MpcdiSubFiles::mpcdiPfm];
if( !hasXmlFile || !hasPfmFile)
{
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"parseMpcdiConfiguration: mpcdi file %s does not contain xml and/or pfm file\n",
Expand Down Expand Up @@ -140,7 +144,8 @@ bool sgct_core::SGCTMpcdi::processSubFiles(std::string filename, unzFile* zipfil
mMpcdiSubFileContents.hasFound[i] = true;
mMpcdiSubFileContents.size[i] = file_info.uncompressed_size;
mMpcdiSubFileContents.filename[i] = filename;
if( unzOpenCurrentFile(*zipfile) != UNZ_OK )
int openCurrentFile = unzOpenCurrentFile(*zipfile);
if( openCurrentFile != UNZ_OK )
{
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"parseMpcdiConfiguration: Unable to open %s\n", filename);
Expand Down Expand Up @@ -176,11 +181,11 @@ bool sgct_core::SGCTMpcdi::processSubFiles(std::string filename, unzFile* zipfil
bool sgct_core::SGCTMpcdi::readAndParseXMLString(SGCTNode& tmpNode, sgct::SGCTWindow& tmpWin)
{
bool mpcdiParseResult = false;
if (mMpcdiSubFileContents.buffer[mpcdiSubFiles::mpcdiXml] != nullptr)
if (mMpcdiSubFileContents.buffer[MpcdiSubFiles::mpcdiXml] != nullptr)
{
tinyxml2::XMLDocument xmlDoc;
tinyxml2::XMLError result = xmlDoc.Parse(mMpcdiSubFileContents.buffer[mpcdiSubFiles::mpcdiXml],
mMpcdiSubFileContents.size[mpcdiSubFiles::mpcdiXml]);
tinyxml2::XMLError result = xmlDoc.Parse(mMpcdiSubFileContents.buffer[MpcdiSubFiles::mpcdiXml],
mMpcdiSubFileContents.size[MpcdiSubFiles::mpcdiXml]);

if (result != tinyxml2::XML_NO_ERROR)
{
Expand Down Expand Up @@ -219,23 +224,27 @@ bool sgct_core::SGCTMpcdi::readAndParseXML_mpcdi(tinyxml2::XMLDocument& xmlDoc,
element[i] = NULL;
const char * val[MAX_XML_DEPTH];

if (!checkAttributeForExpectedValue(XMLroot, "profile", "MPCDI profile", "3d")) {
bool hasExpectedValue;
hasExpectedValue = checkAttributeForExpectedValue(XMLroot, "profile", "MPCDI profile", "3d");
if (!hasExpectedValue) {
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"readAndParseXML_mpcdi: Problem with 'MPCDI' attribute in XML\n");
return false;
}
if (!checkAttributeForExpectedValue(XMLroot, "geometry", "MPCDI geometry level", "1")) {
hasExpectedValue = checkAttributeForExpectedValue(XMLroot, "geometry", "MPCDI geometry level", "1");
if (!hasExpectedValue) {
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"readAndParseXML_mpcdi: Problem with 'geometry' attribute in XML\n");
return false;
}
if (!checkAttributeForExpectedValue(XMLroot, "version", "MPCDI version", "2.0")) {
hasExpectedValue = checkAttributeForExpectedValue(XMLroot, "version", "MPCDI version", "2.0");
if (!hasExpectedValue) {
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
"readAndParseXML_mpcdi: Problem with 'version' attribute in XML\n");
return false;
}

mpcdiFoundItems parsedItems;
MpcdiFoundItems parsedItems;
element[0] = XMLroot->FirstChildElement();
while( element[0] != NULL )
{
Expand Down Expand Up @@ -263,7 +272,7 @@ bool sgct_core::SGCTMpcdi::readAndParseXML_display(tinyxml2::XMLElement* element
const char* val[],
SGCTNode tmpNode,
sgct::SGCTWindow& tmpWin,
mpcdiFoundItems& parsedItems)
MpcdiFoundItems& parsedItems)
{
if( parsedItems.haveDisplayElem ) {
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
Expand Down Expand Up @@ -334,7 +343,7 @@ bool sgct_core::SGCTMpcdi::readAndParseXML_geoWarpFile(tinyxml2::XMLElement* ele
sgct::SGCTWindow& tmpWin,
std::string filesetRegionId)
{
mWarp.push_back(new mpcdiWarp);
mWarp.push_back(new MpcdiWarp);
mWarp.back()->id = filesetRegionId;
element[3] = element[2]->FirstChildElement();
while( element[3] != NULL )
Expand Down Expand Up @@ -372,12 +381,12 @@ bool sgct_core::SGCTMpcdi::readAndParseXML_geoWarpFile(tinyxml2::XMLElement* ele
{
std::string currRegion_warpFilename = mWarp.back()->pathWarpFile;
std::string matchingMpcdiDataFile
= mMpcdiSubFileContents.filename[mpcdiSubFiles::mpcdiPfm];
= mMpcdiSubFileContents.filename[MpcdiSubFiles::mpcdiPfm];
if( currRegion_warpFilename.compare(matchingMpcdiDataFile) == 0 )
{
tmpWin.getViewport(r)->setMpcdiWarpMesh(
mMpcdiSubFileContents.buffer[mpcdiSubFiles::mpcdiPfm],
mMpcdiSubFileContents.size[mpcdiSubFiles::mpcdiPfm]);
mMpcdiSubFileContents.buffer[MpcdiSubFiles::mpcdiPfm],
mMpcdiSubFileContents.size[MpcdiSubFiles::mpcdiPfm]);
foundMatchingPfmBuffer = true;
}
}
Expand All @@ -402,7 +411,7 @@ bool sgct_core::SGCTMpcdi::readAndParseXML_geoWarpFile(tinyxml2::XMLElement* ele
bool sgct_core::SGCTMpcdi::readAndParseXML_buffer(tinyxml2::XMLElement* element[],
const char* val[],
sgct::SGCTWindow& tmpWin,
mpcdiFoundItems& parsedItems)
MpcdiFoundItems& parsedItems)
{
if( parsedItems.haveBufferElem ) {
sgct::MessageHandler::instance()->print(sgct::MessageHandler::NOTIFY_ERROR,
Expand Down Expand Up @@ -450,15 +459,15 @@ bool sgct_core::SGCTMpcdi::readAndParseXML_buffer(tinyxml2::XMLElement* element[
bool sgct_core::SGCTMpcdi::readAndParseXML_region(tinyxml2::XMLElement* element[],
const char* val[],
sgct::SGCTWindow& tmpWin,
mpcdiFoundItems& parsedItems)
MpcdiFoundItems& parsedItems)
{
//Require an 'id' attribute for each region. These will be compared later to the
// fileset, in which there must be a matching 'id'. The mBufferRegions vector is
// intended for use with MPCDI files containing multiple regions, but currently
// only is tested with single region files.
if( element[2]->Attribute("id") != NULL )
{
mBufferRegions.push_back(new mpcdiRegion);
mBufferRegions.push_back(new MpcdiRegion);
mBufferRegions.back()->id = element[2]->Attribute("id");
}
else
Expand Down

0 comments on commit e63197c

Please sign in to comment.