[logging] refactor logging (#247)

* refactor logging

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

* address review comments

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

* fix formatting

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

* fix unused

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

---------

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

Author: antond-weta

include/rawtoaces/rawtoaces_core.h

@@ -169,6 +169,9 @@ class SpectralSolver
     /// @pre white balance calculation must have been performed successfully
     const std::vector<double> &get_WB_multipliers() const;
 
+    /// Error message from the most recent method call that returned false.
+    std::string last_error_message;
+
     int verbosity = 0;
 
 private:

include/rawtoaces/spectral_data.h

@@ -128,7 +128,17 @@ struct SpectralData
     /// The spectral data storage.
     std::map<std::string, SpectralSet> data;
 
-    bool load( const std::string &path, bool reshape = true );
+    /// Loads spectral data from a given file path.
+    /// @param path a path to the file to load data from
+    /// @param reshape if set to `true`, the data will be reshaped to the
+    /// reference shape (`rta::core::Spectrum::ReferenceShape`).
+    /// @param error_message an optional destination for any error
+    /// message occured during loading.
+    /// @result `true` if loaded successfully.
+    bool load(
+        const std::string &path,
+        bool               reshape       = true,
+        std::string       *error_message = nullptr );
 
     /// A convenience operator returning the `Spectrum` of a given channel name
     /// in the "main" data set.

src/rawtoaces_core/rawtoaces_core.cpp

@@ -54,11 +54,7 @@ void calculate_daylight_SPD( const int &cct_input, Spectrum &spectrum )
     else if ( cct_input >= 4000 && cct_input <= 25000 )
         cct = cct_input * 1.0;
     else
-    {
-        std::cerr << "The range of Correlated Color Temperature for "
-                  << "Day Light should be from 4000 to 25000." << std::endl;
-        exit( 1 );
-    }
+        assert( false );
 
     spectrum.values.clear();
 
@@ -100,12 +96,7 @@ void calculate_daylight_SPD( const int &cct_input, Spectrum &spectrum )
 
 void calculate_blackbody_SPD( const int &cct, Spectrum &spectrum )
 {
-    if ( cct < 1500 || cct >= 4000 )
-    {
-        std::cerr << "The range of Color Temperature for BlackBody "
-                  << "should be from 1500 to 3999." << std::endl;
-        exit( 1 );
-    }
+    assert( cct >= 1500 && cct < 4000 );
 
     spectrum.values.clear();
 
@@ -129,12 +120,15 @@ void calculate_blackbody_SPD( const int &cct, Spectrum &spectrum )
 /// @param type Type of light source (e.g. "d50", "d65", "d75", "A", "B", "C", "D50", "D65", "D75")
 /// @param is_daylight True if the light source is a daylight source, false if it is a blackbody source
 /// @param illuminant Reference to SpectralData object to fill with generated illuminant data
+/// @param error_message a destination for any potential error message.
+/// @result `true` if generated successfully.
 /// @pre cct is in valid range for the specified illuminant type
-void generate_illuminant(
+bool generate_illuminant(
     int                cct,
     const std::string &type,
     bool               is_daylight,
-    SpectralData      &illuminant )
+    SpectralData      &illuminant,
+    std::string       &error_message )
 {
     illuminant.data.clear();
 
@@ -150,12 +144,30 @@ void generate_illuminant(
     illuminant.type = type;
     if ( is_daylight )
     {
+        if ( cct < 40 || ( cct > 250 && cct < 4000 ) || cct > 25000 )
+        {
+            error_message =
+                "The range of Correlated Color Temperature for "
+                "Day Light should be from 4000 to 25000.\n";
+            return false;
+        }
+
         calculate_daylight_SPD( cct, power_spectrum );
     }
     else
     {
+        if ( cct < 1500 || cct >= 4000 )
+        {
+            error_message =
+                "The range of Color Temperature for BlackBody "
+                "should be from 1500 to 3999.\n";
+            return false;
+        }
+
         calculate_blackbody_SPD( cct, power_spectrum );
     }
+
+    return true;
 }
 
 SpectralSolver::SpectralSolver(
@@ -328,15 +340,15 @@ bool SpectralSolver::find_illuminant( const std::string &type )
     {
         int               cct             = atoi( type.substr( 1 ).c_str() );
         const std::string illuminant_type = "d" + std::to_string( cct );
-        generate_illuminant( cct, illuminant_type, true, illuminant );
-        return true;
+        return generate_illuminant(
+            cct, illuminant_type, true, illuminant, last_error_message );
     }
     else if ( is_blackbody )
     {
         int cct = atoi( type.substr( 0, type.length() - 1 ).c_str() );
         const std::string illuminant_type = std::to_string( cct ) + "k";
-        generate_illuminant( cct, illuminant_type, false, illuminant );
-        return true;
+        return generate_illuminant(
+            cct, illuminant_type, false, illuminant, last_error_message );
     }
     else
     {
@@ -360,8 +372,9 @@ bool SpectralSolver::find_illuminant( const vector<double> &wb )
     if ( camera.data.count( "main" ) == 0 ||
          camera.data.at( "main" ).size() != 3 )
     {
-        std::cerr << "ERROR: camera needs to be initialised prior to calling "
-                  << "SpectralSolver::find_illuminant()" << std::endl;
+        last_error_message =
+            "Camera needs to be initialised prior to calling "
+            "SpectralSolver::find_illuminant().";
         return false;
     }
 
@@ -372,15 +385,21 @@ bool SpectralSolver::find_illuminant( const vector<double> &wb )
         {
             SpectralData     &illuminant_data = _all_illuminants.emplace_back();
             const std::string type = "d" + std::to_string( cct / 100 );
-            generate_illuminant( cct, type, true, illuminant_data );
+
+            [[maybe_unused]] bool success = generate_illuminant(
+                cct, type, true, illuminant_data, last_error_message );
+            assert( success );
         }
 
         // Blackbody - pre-calculate
         for ( int cct = 1500; cct < 4000; cct += 500 )
         {
             SpectralData     &illuminant_data = _all_illuminants.emplace_back();
             const std::string type            = std::to_string( cct ) + "k";
-            generate_illuminant( cct, type, false, illuminant_data );
+
+            [[maybe_unused]] bool success = generate_illuminant(
+                cct, type, false, illuminant_data, last_error_message );
+            assert( success );
         }
 
         auto illuminant_files = collect_data_files( "illuminant" );
@@ -425,16 +444,18 @@ bool SpectralSolver::calculate_WB()
     if ( camera.data.count( "main" ) == 0 ||
          camera.data.at( "main" ).size() != 3 )
     {
-        std::cerr << "ERROR: camera needs to be initialised prior to calling "
-                  << "SpectralSolver::calculate_WB()" << std::endl;
+        last_error_message =
+            "Camera needs to be initialised prior to calling "
+            "SpectralSolver::calculate_WB().";
         return false;
     }
 
     if ( illuminant.data.count( "main" ) == 0 ||
          illuminant.data.at( "main" ).size() != 1 )
     {
-        std::cerr << "ERROR: illuminant needs to be initialised prior to "
-                  << "calling SpectralSolver::calculate_WB()" << std::endl;
+        last_error_message =
+            "Illuminant needs to be initialised prior to "
+            "calling SpectralSolver::calculate_WB().";
         return false;
     }
 
@@ -717,34 +738,36 @@ bool SpectralSolver::calculate_IDT_matrix()
     if ( camera.data.count( "main" ) == 0 ||
          camera.data.at( "main" ).size() != 3 )
     {
-        std::cerr << "ERROR: camera needs to be initialised prior to calling "
-                  << "SpectralSolver::calculate_IDT_matrix()" << std::endl;
+        last_error_message =
+            "Camera needs to be initialised prior to calling "
+            "SpectralSolver::calculate_IDT_matrix().";
         return false;
     }
 
     if ( illuminant.data.count( "main" ) == 0 ||
          illuminant.data.at( "main" ).size() != 1 )
     {
-        std::cerr << "ERROR: illuminant needs to be initialised prior to "
-                  << "calling SpectralSolver::calculate_IDT_matrix()"
-                  << std::endl;
+        last_error_message =
+            "Illuminant needs to be initialised prior to "
+            "calling SpectralSolver::calculate_IDT_matrix().";
         return false;
     }
 
     if ( observer.data.count( "main" ) == 0 ||
          observer.data.at( "main" ).size() != 3 )
     {
-        std::cerr << "ERROR: observer needs to be initialised prior to calling "
-                  << "SpectralSolver::calculate_IDT_matrix()" << std::endl;
+        last_error_message =
+            "Observer needs to be initialised prior to "
+            "calling SpectralSolver::calculate_IDT_matrix().";
         return false;
     }
 
     if ( training_data.data.count( "main" ) == 0 ||
          training_data.data.at( "main" ).empty() )
     {
-        std::cerr << "ERROR: training data needs to be initialised prior to "
-                  << "calling SpectralSolver::calculate_IDT_matrix()"
-                  << std::endl;
+        last_error_message =
+            "Training data needs to be initialised prior to "
+            "calling SpectralSolver::calculate_IDT_matrix().";
         return false;
     }
 

src/rawtoaces_core/spectral_data.cpp

@@ -6,6 +6,7 @@
 #include <assert.h>
 #include <fstream>
 #include <iostream>
+#include <sstream>
 #include <nlohmann/json.hpp>
 
 namespace rta
@@ -178,7 +179,8 @@ parse_string( nlohmann::json &j, std::string &dst, const std::string &key )
         dst = v;
 }
 
-bool SpectralData::load( const std::string &path, bool reshape )
+bool SpectralData::load(
+    const std::string &path, bool reshape, std::string *error_message )
 {
     // Reset all in case the object has been initialised before.
     manufacturer.erase();
@@ -206,8 +208,10 @@ bool SpectralData::load( const std::string &path, bool reshape )
         std::ifstream i( path );
         if ( !i.is_open() )
         {
-            std::cerr << "Error: Failed to open file " << path << "."
-                      << std::endl;
+            if ( error_message != nullptr )
+            {
+                *error_message = "Failed to open file " + path + ".";
+            }
             return false;
         }
         nlohmann::json file_data = nlohmann::json::parse( i );
@@ -282,10 +286,15 @@ bool SpectralData::load( const std::string &path, bool reshape )
 
                     if ( shape.step != 0 && new_step != shape.step )
                     {
-                        std::cerr << "Error: Inconsistent wavelength step "
-                                  << "detected in " << path
-                                  << ". Expected: " << shape.step
-                                  << ", got: " << new_step << "." << std::endl;
+                        if ( error_message != nullptr )
+                        {
+                            std::stringstream stream;
+                            stream << "Inconsistent wavelength "
+                                   << "step detected in " << path
+                                   << ". Expected: " << shape.step
+                                   << ", got: " << new_step << ".";
+                            *error_message = stream.str();
+                        }
                         return false;
                     }
 
@@ -321,14 +330,20 @@ bool SpectralData::load( const std::string &path, bool reshape )
     }
     catch ( nlohmann::detail::parse_error &error )
     {
-        std::cerr << "Error: JSON parsing of " << path
-                  << " failed with error: " << error.what() << std::endl;
+        if ( error_message != nullptr )
+        {
+            *error_message = "Error: JSON parsing of " + path +
+                             " failed with error: " + error.what() + ".";
+        }
         return false;
     }
     catch ( const std::exception &error )
     {
-        std::cerr << "Error: JSON parsing of " << path
-                  << " failed with error: " << error.what() << std::endl;
+        if ( error_message != nullptr )
+        {
+            *error_message = "Error: JSON parsing of " + path +
+                             " failed with error: " + error.what() + ".";
+        }
         return false;
     }
 

src/rawtoaces_util/colour_transforms.cpp

@@ -29,7 +29,7 @@ void print_data_error(
     }
     error_message +=
         ". Please check the database search path in "
-        "RAWTOACES_DATABASE_PATH";
+        "RAWTOACES_DATABASE_PATH.";
 }
 
 bool configure_spectral_solver(
@@ -116,7 +116,7 @@ bool solve_illuminant_from_multipliers(
     if ( !solver.find_illuminant( wb_multipliers ) )
     {
         error_message =
-            "Failed to find illuminant from white balance multipliers";
+            "Failed to find illuminant from white balance multipliers.";
         return false;
     }
 
@@ -126,6 +126,7 @@ bool solve_illuminant_from_multipliers(
     cache_data.second[1]    = multipliers[1];
     cache_data.second[2]    = multipliers[2];
 
+    error_message = "";
     return true;
 }
 
@@ -204,7 +205,8 @@ bool solve_multipliers_from_illuminant(
 
     if ( !solver.calculate_WB() )
     {
-        error_message = "Failed to calculate white balance multipliers";
+        error_message = "Failed to calculate white balance multipliers. " +
+                        solver.last_error_message;
         return false;
     }
 
@@ -213,6 +215,7 @@ bool solve_multipliers_from_illuminant(
     cache_data[1]           = multipliers[1];
     cache_data[2]           = multipliers[2];
 
+    error_message = "";
     return true;
 }
 
@@ -299,11 +302,13 @@ bool solve_matrix_from_illuminant(
 
     if ( !solver.calculate_WB() )
     {
+        error_message = solver.last_error_message;
         return false;
     }
 
     if ( !solver.calculate_IDT_matrix() )
     {
+        error_message = solver.last_error_message;
         return false;
     }
 
@@ -348,11 +353,8 @@ bool fetch_matrix_from_illuminant(
     bool success = entry.first;
     if ( !success )
     {
-        // Set fallback error message if none was set by solve_matrix_from_illuminant
-        if ( error_message.empty() )
-        {
-            error_message = "Failed to calculate IDT matrix from illuminant";
-        }
+        error_message =
+            "Failed to calculate IDT matrix from illuminant. " + error_message;
         return false;
     }