google test merge request feedback reviewed. addressed all necessary major and...

google test merge request feedback reviewed. addressed all necessary major and minor issues that were pointed out; and hopefully fixed my stupidity in LinearMapping::operator==

core/datastructures/imagemappinginformation.cpp

@@ -83,7 +83,7 @@ namespace campvis {
         return (this->_offset == obj._offset)
             && (this->_size == obj._size)
             && (this->_voxelSize == obj._voxelSize)
-            //&& (this->_realWorldValueMapping == obj._realWorldValueMapping)
+            && (this->_realWorldValueMapping == obj._realWorldValueMapping)
             && (this->_textureToWorldTransformation == obj._textureToWorldTransformation)
             && (this->_voxelToWorldTransformation == obj._voxelToWorldTransformation)
             && (this->_worldToTextureTransformation == obj._worldToTextureTransformation)

core/tools/glreduction.h

@@ -45,7 +45,7 @@ namespace campvis {
     class FaceGeometry;
 
     /**
-     * Class performing a reduction of 2D image data using OpenGL.
+     * Class performing a reduction of 2D or 3D image data using OpenGL.
      * \note    All methods need to be called from a valid OpenGL context.
      */
     class CAMPVIS_CORE_API GlReduction {

core/tools/mapping.h

@@ -64,6 +64,10 @@ namespace campvis {
             return (value - _shift) / _scale;
         }
 
+        bool operator==(const LinearMapping<T>& obj) const{
+            return this->_shift == obj._shift && this->_scale == obj._scale;
+        }
+
         T _shift;   ///< Shifting coefficient of mapping
         T _scale;   ///< Scaling coefficient of mapping
 

core/tools/stringutils.cpp

@@ -103,18 +103,20 @@ namespace campvis {
             // check whether there are quotes
             if (str[strpos] == quotes) {
                 // find position of closing quotes
-                endpos = str.find_first_of('"', strpos + 1);
+                endpos = str.find_first_of(quotes, strpos + 1);
 
                 std::string toPush = str.substr(strpos + 1 , endpos - strpos - 1);
                 // ensure we haven't found double quotes ("") which shall be resolved to one double quote in resulting string
-                while ((endpos != std::string::npos) && (endpos + 1 < str.length()) && (str[endpos + 1] == '"')) {
+                while ((endpos != std::string::npos) && (endpos + 1 < str.length()) && (str[endpos + 1] == quotes)) {
                     strpos = endpos + 1;
-                    endpos = str.find_first_of('"', endpos + 2);
+                    endpos = str.find_first_of(quotes, endpos + 2);
                     toPush.append(str.substr(strpos, endpos - strpos));
                 }
 
                 // push string in quotes onto toReturn
-                toReturn.push_back(StringUtils::trim(toPush, whitespace));
+                toReturn.push_back(toPush);
+                // use the following if whitespace trimming is necessary in between <quote>s
+                //toReturn.push_back(StringUtils::trim(toPush, whitespace));
 
                 // ignore everything until next delimiter
                 endpos = str.find_first_of(delimiter, endpos);

core/tools/stringutils.h

@@ -97,7 +97,7 @@ namespace campvis {
          * \param whitespace    Set of whitespace characters which shall be removed during trimming.
          * \return  Vector of the split substrings.
          */
-        static std::vector<std::string> splitStringsafe(const std::string& str, const std::string& delimiter, char quotes = '"', const std::string& whitespace = " \t\n\r\0\x0B");
+        static std::vector<std::string> splitStringsafe(const std::string& str, const std::string& delimiter, char quotes = '\'', const std::string& whitespace = " \t\n\r\0\x0B");
 
         /**
          * Trims the string \a str.

ext/gtest-1.7.0/src/gtest_main.cc

@@ -31,14 +31,11 @@
 
 #include "gtest/gtest.h"
 
-//int PullInMyLibrary();
-
-
 GTEST_API_ int main(int argc, char **argv) {
     printf("Running main() from gtest_main.cc\n");
     testing::InitGoogleTest(&argc, argv);
     int ret= RUN_ALL_TESTS();
     printf("gtest_main.cc returned with %d\n", ret);
-    //getchar();
     return 0;
 }
+

test/CMakeLists.txt

 PROJECT(test-campvis)
 CMAKE_MINIMUM_REQUIRED(VERSION 2.8.0 FATAL_ERROR)
-#INCLUDE(../cmake/commonconf.cmake)
-
 MESSAGE(STATUS "Configuring googletest for campvis Modules")
-#SET_DEFAULT_MODULES("vis")
-#RESOLVE_MODULE_DEPENDENCIES()
-#WRITE_PIPELINE_REGISTRATION("gen_pipelineregistration.h")
 
 FILE(GLOB TestCampvisHeaders RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
     *.h
@@ -15,8 +10,7 @@ FILE(GLOB TestCampvisSources RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
     core/datastructures/*.cpp
     core/properties/*.cpp
     core/tools/*.cpp
-    #eventhandlers/*.cpp
-    #pipeline/*.cpp
+	core/pipeline/*.cpp
 )
 
 # Summary of tuple support for Microsoft Visual Studio:
@@ -39,7 +33,7 @@ ADD_DEFINITIONS(${CampvisGlobalDefinitions} ${CampvisModulesDefinitions} ${Campv
 INCLUDE_DIRECTORIES(${CampvisGlobalIncludeDirs} ${CampvisModulesIncludeDirs} ${CampvisHome}/ext/gtest-1.7.0/ ${CampvisHome}/ext/gtest-1.7.0/include)
 TARGET_LINK_LIBRARIES(test-campvis campvis-modules campvis-core tgt gtest ${CampvisGlobalExternalLibs} ${CampvisModulesExternalLibs} ${QT_LIBRARIES})
 
-IF(CAMPVIS_GROUP_SOURCE_FILES)
+if (CAMPVIS_GROUP_SOURCE_FILES)
     DEFINE_SOURCE_GROUPS_FROM_SUBDIR(TestCampvisSources ${CampvisHome}/test "")
     DEFINE_SOURCE_GROUPS_FROM_SUBDIR(TestCampvisHeaders ${CampvisHome}/test "")
 ENDIF()

test/core/datastructures/datacontainertest.cpp

@@ -80,10 +80,6 @@ TEST_F(DataContainerTest, miscellaneousTest) {
 
     this->_dc0->removeData("data1");
     EXPECT_EQ(nullptr, this->_dc0->getData("data1").getData());
-    // TODO: if the addDataHandle is called here instead of above
-    // this->_dc0->addDataHandle("data2", DataHandle(this->_data));
-    // the program crashes. Find why!!
-
     EXPECT_EQ(false, this->_dc0->hasData("data1"));
     EXPECT_EQ(true, this->_dc0->hasData("data2"));
 }

test/core/datastructures/datahandletest.cpp

@@ -50,7 +50,6 @@ protected:
     }
 
     ~DataHandleTest() {
-        //delete _data1;    // DON'T mess with data added to the DataHandle. More at DataHanlde.h
     }
 
     virtual void SetUp() {

test/core/datastructures/imagedatatest.cpp

@@ -103,6 +103,7 @@ TEST_F(ImageDataTest, miscellaneousTest) {
 /**
  * Tests the clone() function.
  */
+// TODO: extend the test for proper ImageData cloning
 TEST_F(ImageDataTest, cloneTest) {
     ASSERT_TRUE(nullptr != _imgData1);
 

test/core/datastructures/imageseriestest.cpp

@@ -46,8 +46,7 @@ protected:
         _imgSeries3.addImage(_dh);
     }
 
-    ~ImageSeriesTest() {        
-        //delete _image;    // Don't even think about it. Check DataHandle.h
+    ~ImageSeriesTest() {
     }
 
     virtual void SetUp() {

test/core/pipeline/abstractprocessortest.cpp

@@ -38,11 +38,12 @@
 using namespace campvis;
 
 
+// FIXME: This is not what I was supposed to be. fix me please. 
 class DummyTestProcessor : public AbstractProcessor {
 public:
     DummyTestProcessor () {
         _invalidateExternally = false;
-        _level = AbstractProcessor::InvalidationLevel::VALID;
+        this->invalidate(AbstractProcessor::InvalidationLevel::VALID);
     }
     ~DummyTestProcessor () {}
 
@@ -53,31 +54,18 @@ public:
     
     virtual void updateResult(DataContainer& dataContainer) {
         dataContainer.removeData("ImageData");
-        dataContainer.addData("ImageData", new ImageData(2, tgt::svec3(1,2,3), 4));
-
-        /** 
-         * Even if I comment out the next IF clause, it still remain invalid!!
-         * shouldn't the process() set it back to ::VALID if no invalidation 
-         * occurs i.e. the queue is empty!!
-         */
-        //if(_invalidateExternally)
-        //    this->invalidate(_level);
+        dataContainer.addData("ImageData", new ImageData(2, tgt::svec3(1,2,1), 4));
     }
 
     void setExternalInvalidation(bool status, AbstractProcessor::InvalidationLevel level) {
         _invalidateExternally = status;
-        _level = level;
+        this->invalidate(level);
         
-        /**
-         * Okay, What's the problem here!! if we want to do the invalidation in the updateResult() only,
-         * things doesn't work. :@
-         */
-        this->invalidate(_level);
+        this->invalidate(this->getInvalidationLevel());
     }
 
 private:
     bool _invalidateExternally;
-    AbstractProcessor::InvalidationLevel _level;
 };
 
 
@@ -90,7 +78,7 @@ protected:
     AbstractProcessorTest() 
         : _dataContainer("testContainer")
     {
-        this->_dataContainer.addData("ImageData", new ImageData(2, tgt::svec3(1,2,3), 4));
+        this->_dataContainer.addData("ImageData", new ImageData(2, tgt::svec3(1,2,1), 4));
     }
 
     ~AbstractProcessorTest() {
@@ -117,5 +105,4 @@ TEST_F(AbstractProcessorTest, invalidationTest) {
     this->_processor1.setExternalInvalidation(true, AbstractProcessor::InvalidationLevel::INVALID_RESULT);
     this->_processor1.process(this->_dataContainer);
     EXPECT_NE(AbstractProcessor::InvalidationLevel::VALID, this->_processor1.getInvalidationLevel());
-
 }

test/core/properties/numericpropertytest.cpp

@@ -148,4 +148,5 @@ TEST_F(NumericPropertyTest, decreaseTest) {
     EXPECT_EQ(dmin, this->_doubleProp.getValue());
     this->_doubleProp.decrement();
     EXPECT_EQ(dmin, this->_doubleProp.getValue());
-}
\ No newline at end of file
+}
+

test/core/properties/stringpropertytest.cpp

@@ -87,6 +87,15 @@ TEST_F(StringPropertyTest, sharedPropertyTest) {
     EXPECT_EQ(this->_strProp1.getValue(), this->_strProp2.getValue());
     EXPECT_EQ("value1", this->_strProp2.getValue());
 
+    this->_strProp1.setValue("anotherValue");
+    EXPECT_EQ("anotherValue", this->_strProp1.getValue());
+    EXPECT_EQ("anotherValue", this->_strProp2.getValue());
+    this->_strProp2.setValue("someOtherValue");
+    EXPECT_EQ("someOtherValue", this->_strProp2.getValue());
+    EXPECT_NE("someOtherValue", this->_strProp1.getValue());
+    EXPECT_EQ("anotherValue", this->_strProp1.getValue());
+
+    this->_strProp1.setValue("value1");
     // removeSharedProperty()
     this->_strProp1.removeSharedProperty(&this->_strProp2);
     EXPECT_EQ(this->_strProp1.getValue(), this->_strProp2.getValue());

test/core/tools/concurrenthistogramtest.cpp

@@ -43,16 +43,16 @@ using namespace campvis;
  */
 
 template<size_t ND>
-class ConcurrentHistogramND {
+class ConcurrentHistogramNDUniform {
 protected:
-    ConcurrentHistogramND()
+    ConcurrentHistogramNDUniform()
     {
         max = new int[ND];
         min = new int[ND];
         numBuckets = new size_t[ND];
-        for (int i = 0; i < ND; i++) { min[i] = 0; }
-        for (int i = 0; i < ND; i++) { max[i] = 100; }
         for (int i = 0; i < ND; i++) { 
+            min[i] = 0; 
+            max[i] = 100;
             numBuckets[i] = max[i] - min[i] + 1; 
         }
 
@@ -65,7 +65,7 @@ protected:
         _cgh = new campvis::ConcurrentGenericHistogramND<int, ND>(min, max, numBuckets);
     }
 
-    ~ConcurrentHistogramND() {
+    ~ConcurrentHistogramNDUniform() {
         delete max;
         delete min;
         delete numBuckets;
@@ -80,10 +80,10 @@ protected:
 
     void initSamples() {
         int range = max[0] - min[0] + 1;
-        for(int i = 0; i < numSamples; i++) {
+        for (int i = 0; i < numSamples; i++) {
             std::vector<int> sample;
             int x = i;
-            for(int j = ND-1; j >= 0; j--) {
+            for (int j = ND-1; j >= 0; j--) {
                 sample.push_back(x / static_cast<int>(pow(range, j)));
                 x = x % static_cast<int>(pow(range, j));
             }
@@ -91,9 +91,12 @@ protected:
         }
     }
     void computeHistogram() {
+        int total = 0;
         for (int i = 0; i < numSamples; i++) {
             histogram[i] = 1;
+            total += histogram[i];
         }
+        EXPECT_EQ(numSamples, total);
     }
 
     int getND() {return ND;}
@@ -112,7 +115,7 @@ protected:
 /** 
  * Test class 1D system.
  */
-class ConcurrentHistogram1DTest : public ConcurrentHistogramND<1>, public ::testing::Test {
+class ConcurrentHistogram1DTest : public ConcurrentHistogramNDUniform<1>, public ::testing::Test {
 public:
     ConcurrentHistogram1DTest() {
     }
@@ -134,7 +137,7 @@ TEST_F(ConcurrentHistogram1DTest, concurrentAddSampleTest) {
     });
 
     for (int i = 0; i < getND(); i++) {
-        for(int j = 0; j < numBuckets[i]; j ++) {
+        for (int j = 0; j < numBuckets[i]; j ++) {
             EXPECT_EQ(histogram[i], _cgh->getNumElements(j));
         }
     }
@@ -143,7 +146,7 @@ TEST_F(ConcurrentHistogram1DTest, concurrentAddSampleTest) {
 /** 
  * Test class 2D system.
  */
-class ConcurrentHistogram2DTest : public ConcurrentHistogramND<2>, public ::testing::Test {
+class ConcurrentHistogram2DTest : public ConcurrentHistogramNDUniform<2>, public ::testing::Test {
 public:
     ConcurrentHistogram2DTest() {
     }
@@ -165,9 +168,154 @@ TEST_F(ConcurrentHistogram2DTest, concurrentAddSampleTest) {
     });
 
     for (int i = 0; i < getND(); i++) {
-        for(int j = 0; j < numBuckets[i]; j ++) {
+        for (int j = 0; j < numBuckets[i]; j ++) {
             EXPECT_EQ(histogram[i], _cgh->getNumElements(j));
         }
         break;
     }
-}
\ No newline at end of file
+}
+
+
+/**
+ * Test class for ConcurrentGenericHistogramND. Initializes a uniform distribution
+ * over ND dimensional space. Buckets are chosen in each unit, i.e. number of buckets
+ * is range+1. 
+ */
+
+template<size_t ND>
+class ConcurrentHistogramNDSpecific {
+protected:
+    ConcurrentHistogramNDSpecific()
+    {
+        max = new int[ND];
+        min = new int[ND];
+        numBuckets = new size_t[ND];
+        for (int i = 0; i < ND; i++) { 
+            min[i] = 0; 
+            max[i] = 9999;
+            numBuckets[i] = 2; 
+        }
+
+        numSamples = static_cast<int>(pow(max[0]-min[0]+1, ND));
+        initSamples();
+
+        histogram = new int[static_cast<int>(pow(2, ND))];
+        computeHistogram();
+
+        _cgh = new campvis::ConcurrentGenericHistogramND<int, ND>(min, max, numBuckets);
+    }
+
+    ~ConcurrentHistogramNDSpecific() {
+        delete max;
+        delete min;
+        delete numBuckets;
+        delete histogram;
+    }
+
+    virtual void SetUp() {
+    }
+
+    virtual void TearDown() {
+    }
+
+    void initSamples() {
+        int range = max[0] - min[0] + 1;
+        for (int i = 0; i < numSamples; i++) {
+            std::vector<int> sample;
+            int x = i;
+            for (int j = ND-1; j >= 0; j--) {
+                sample.push_back(x / static_cast<int>(pow(range, j)));
+                x = x % static_cast<int>(pow(range, j));
+            }
+            samples.push_back(sample);
+        }
+    }
+    void computeHistogram() {
+        //for (int i = 0; i < 2; i++) {
+        //    histogram[i] = numSamples / 2;
+        //}
+        int total = 0;
+        for (int i = 0; i < pow(2, ND); i++) {
+            histogram[i] = static_cast<int>(numSamples / pow(2, ND) );
+            total += histogram[i];
+        }
+        EXPECT_EQ(numSamples, total);
+
+    }
+
+    int getND() {return ND;}
+
+protected:
+    int *max, *min;
+    size_t *numBuckets;
+    campvis::ConcurrentGenericHistogramND<int, ND>* _cgh;
+
+    int numSamples;
+    std::vector<std::vector<int> > samples;
+    int *histogram;
+
+};
+
+/** 
+ * Test class 1D system.
+ */
+class ConcurrentHistogram1DTestSpecific : public ConcurrentHistogramNDSpecific<1>, public ::testing::Test {
+public:
+    ConcurrentHistogram1DTestSpecific() {
+    }
+};
+
+/** 
+ * Expected number of elements at each bucket should be 1
+ */
+TEST_F(ConcurrentHistogram1DTestSpecific, concurrentAddSampleTest) {
+    tbb::parallel_for(tbb::blocked_range<size_t>(0, samples.size()), [&] (const tbb::blocked_range<size_t>& range) {
+        for (size_t i = range.begin(); i != range.end(); ++i) {
+            std::vector<int> vec = samples[i];
+            int value[10];
+            for (int k = 0; k < getND(); k++) {
+                value[k] = vec[k];
+            }
+            _cgh->addSample(&(samples[i].front()));
+        }
+    });
+
+    for (int i = 0; i < getND(); i++) {
+        for (int j = 0; j < numBuckets[i]; j ++) {
+            EXPECT_EQ(histogram[i], _cgh->getNumElements(j));
+        }
+    }
+}
+
+/** 
+ * Test class 2D system.
+ */
+class ConcurrentHistogram2DTestSpecific : public ConcurrentHistogramNDUniform<2>, public ::testing::Test {
+public:
+    ConcurrentHistogram2DTestSpecific() {
+    }
+};
+
+/** 
+ * Expected number of elements at each bucket should be 1
+ */
+TEST_F(ConcurrentHistogram2DTestSpecific, concurrentAddSampleTest) {
+    tbb::parallel_for(tbb::blocked_range<size_t>(0, samples.size()), [&] (const tbb::blocked_range<size_t>& range) {
+        for (size_t i = range.begin(); i != range.end(); ++i) {
+            std::vector<int> vec = samples[i];
+            int value[10];
+            for (int k = 0; k < getND(); k++) {
+                value[k] = vec[k];
+            }
+            _cgh->addSample(&(samples[i].front()));
+        }
+    });
+
+    for (int i = 0; i < getND(); i++) {
+        for (int j = 0; j < numBuckets[i]; j ++) {
+            EXPECT_EQ(histogram[i], _cgh->getNumElements(j));
+        }
+        break;
+    }
+}
+

test/core/tools/glreductiontest.cpp

@@ -44,18 +44,20 @@ using namespace campvis;
  * and known data. Then compares the output of GlReduction::reduce() with 
  * expected values.
  */
-class GlReductionTest : public ::testing::Test {
+class GlReductionTest : public ::testing::TestWithParam<int> {
 protected:
     GlReductionTest() {
         name = new tgt::FramebufferObject();
 
         width = 10;
         height = 10;
-        depth = 1;
+        
+        EXPECT_TRUE(GetParam() == 2 || GetParam() == 3);
+        depth = (GetParam() == 3)? 10 : 1;
 
         rawData = new float[width * height * depth];
         initData(rawData, width * height * depth);
-        imgData = new ImageData(2, tgt::svec3(width, height, depth), 1);
+        imgData = new ImageData(depth == 1? 2 : 3, tgt::svec3(width, height, depth), 1);
         localRep = ImageRepresentationLocal::create(imgData, WeaklyTypedPointer(WeaklyTypedPointer::BaseType::FLOAT, 1, rawData));
         glReduction = nullptr;
     }
@@ -72,8 +74,15 @@ protected:
     }
     
     void initData(float* data, int size) {
-        memset(data, 0x0F, sizeof(float)*size);
-        data[0] = 1;
+        for (int i = 0; i < size; i++ ) {
+            data[i] = 1;
+        }
+        srand(static_cast<unsigned>(time(NULL)));
+        data[rand() % size] = 0;
+    }
+    
+    int getSize() {
+        return width * height * depth;
     }
 
 protected:
@@ -90,24 +99,25 @@ protected:
 /** 
  * Checks whether the OpenGL context is valid here.
  */
-TEST_F(GlReductionTest, isScopedLockWorking) {
-    if(name->getId() != 0){
+TEST_P(GlReductionTest, isScopedLockWorking) {
+    if (name->getId() != 0){
         SUCCEED();
-    } else
+    } else {
         FAIL();
+    }
 }
 
 /** 
  * Checks whether the local representation is valid.
  */
-TEST_F(GlReductionTest, localRepCreationTest) {
+TEST_P(GlReductionTest, localRepCreationTest) {
     EXPECT_NE(nullptr, localRep);
 }
 
 /** 
  * Tests for ReducationOperator::MIN.
  */
-TEST_F(GlReductionTest, minTest) {
+TEST_P(GlReductionTest, minTest) {
     glReduction = new GlReduction(GlReduction::ReductionOperator::MIN);
     std::vector<float> reduced = glReduction->reduce(imgData);
 
@@ -118,7 +128,7 @@ TEST_F(GlReductionTest, minTest) {
 /** 
  * Tests for ReducationOperator::MAX.
  */
-TEST_F(GlReductionTest, maxTest) {
+TEST_P(GlReductionTest, maxTest) {
     glReduction = new GlReduction(GlReduction::ReductionOperator::MAX);
     std::vector<float> reduced = glReduction->reduce(imgData);
 
@@ -129,21 +139,25 @@ TEST_F(GlReductionTest, maxTest) {
 /** 
  * Tests for ReducationOperator::PLUS.
  */
-TEST_F(GlReductionTest, sumTest) {
+TEST_P(GlReductionTest, sumTest) {
     glReduction = new GlReduction(GlReduction::ReductionOperator::PLUS);
     std::vector<float> reduced = glReduction->reduce(imgData);
 
-    ASSERT_NEAR(1, reduced[0], 0.0001);
+    ASSERT_NEAR(getSize() - 1, reduced[0], 0.0001);
     ASSERT_NEAR(0, reduced[1], 0.0001);
 }
 
 /** 
  * Tests for ReducationOperator::MULTIPLICATION.
  */
-TEST_F(GlReductionTest, multTest) {
+TEST_P(GlReductionTest, multTest) {
     glReduction = new GlReduction(GlReduction::ReductionOperator::MULTIPLICATION);
     std::vector<float> reduced = glReduction->reduce(imgData);
 
     ASSERT_NEAR(0, reduced[0], 0.0001);
     ASSERT_NEAR(0, reduced[1], 0.0001);
-}
\ No newline at end of file
+}
+
+INSTANTIATE_TEST_CASE_P(2DTest, GlReductionTest, ::testing::Values(2));
+INSTANTIATE_TEST_CASE_P(3DTest, GlReductionTest, ::testing::Values(3));
+

test/core/tools/stringutilstest.cpp

@@ -76,20 +76,20 @@ TEST_F(StringUtilTest, caseOperationsTest) {
  */
 TEST_F(StringUtilTest, parseFloatsTest) {
     std::vector<std::string> parsed = StringUtils::parseFloats(
-        "-0.0 xyz +.12  123.zx0 --0.14 xyz");
+        "-1m-0.0 xyz +.12  123.zx0 --0.14 xyz");