From 0058a0a8e3c2eca457cd779e012b188a368c6db9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Lemaire?= <michael@thunderk.net>
Date: Wed, 19 Aug 2015 19:14:59 +0200
Subject: [PATCH] Switched to RGB material colors

...to avoid otherwise unused HSL conversion, which causes problems
for some RGB values (like Color(1, 0, 0))
---
 src/basics/Color.h                            |  1 +
 src/basics/Color.inline.cpp                   |  4 +++
 src/basics/ColorHSL.cpp                       |  6 ++---
 src/basics/ColorHSL.h                         |  4 +--
 src/definition/SurfaceMaterial.cpp            | 27 ++++++++++++-------
 src/definition/SurfaceMaterial.h              | 11 +++-----
 src/definition/TextureLayerDefinition.cpp     | 11 ++++----
 src/definition/WaterDefinition.cpp            |  8 +++---
 src/render/opengl/OpenGLWater.cpp             |  2 +-
 .../software/CloudBasicLayerRenderer.cpp      |  3 +--
 src/render/software/LightStatus.cpp           |  2 +-
 src/render/software/LightingManager.cpp       |  6 ++---
 src/render/software/WaterRenderer.cpp         |  2 +-
 src/tests/ColorHSL_Test.cpp                   | 23 ++++++++++++++++
 src/tests/tests.pro                           |  3 ++-
 15 files changed, 73 insertions(+), 40 deletions(-)
 create mode 100644 src/tests/ColorHSL_Test.cpp

diff --git a/src/basics/Color.h b/src/basics/Color.h
index 074fdda..e851cc4 100644
--- a/src/basics/Color.h
+++ b/src/basics/Color.h
@@ -10,6 +10,7 @@ class BASICSSHARED_EXPORT Color
 {
 public:
     Color();
+    Color(const Color &col);
     Color(double r, double g, double b, double a=1.0);
 
     void save(PackStream* stream) const;
diff --git a/src/basics/Color.inline.cpp b/src/basics/Color.inline.cpp
index f9ee1b2..1f38abc 100644
--- a/src/basics/Color.inline.cpp
+++ b/src/basics/Color.inline.cpp
@@ -20,7 +20,11 @@ METHSPEC Color::Color():
 METHSPEC Color::Color(double r, double g, double b, double a):
     r(r), g(g), b(b), a(a)
 {
+}
 
+METHSPEC Color::Color(const Color &col):
+    r(col.r), g(col.g), b(col.b), a(col.a)
+{
 }
 
 METHSPEC unsigned int Color::to32BitRGBA() const
diff --git a/src/basics/ColorHSL.cpp b/src/basics/ColorHSL.cpp
index 056381c..ff1614f 100644
--- a/src/basics/ColorHSL.cpp
+++ b/src/basics/ColorHSL.cpp
@@ -2,7 +2,7 @@
 
 #include "Color.h"
 
-static double _hue2rgb(double p, double q, double t)
+static inline double _hue2rgb(double p, double q, double t)
 {
     if (t < 0.0) t += 1;
     if (t > 1.0) t -= 1;
@@ -12,7 +12,7 @@ static double _hue2rgb(double p, double q, double t)
     return p;
 }
 
-Color colorFromHSL(ColorHSL col)
+Color colorFromHSL(const ColorHSL &col)
 {
     Color result;
 
@@ -34,7 +34,7 @@ Color colorFromHSL(ColorHSL col)
     return result;
 }
 
-ColorHSL colorToHSL(Color col)
+ColorHSL colorToHSL(const Color &col)
 {
     ColorHSL result;
     double min, max;
diff --git a/src/basics/ColorHSL.h b/src/basics/ColorHSL.h
index ec185fe..fce0b1e 100644
--- a/src/basics/ColorHSL.h
+++ b/src/basics/ColorHSL.h
@@ -11,8 +11,8 @@ typedef struct
     double a;
 } ColorHSL;
 
-BASICSSHARED_EXPORT Color colorFromHSL(ColorHSL col);
-BASICSSHARED_EXPORT ColorHSL colorToHSL(Color col);
+BASICSSHARED_EXPORT Color colorFromHSL(const ColorHSL &col);
+BASICSSHARED_EXPORT ColorHSL colorToHSL(const Color &col);
 
 BASICSSHARED_EXPORT ColorHSL colorHSLFromValues(double h, double s, double l, double a);
 
diff --git a/src/definition/SurfaceMaterial.cpp b/src/definition/SurfaceMaterial.cpp
index 04ce97b..72b5b7f 100644
--- a/src/definition/SurfaceMaterial.cpp
+++ b/src/definition/SurfaceMaterial.cpp
@@ -1,6 +1,7 @@
 #include "SurfaceMaterial.h"
 
 #include "PackStream.h"
+#include "Color.h"
 
 SurfaceMaterial::SurfaceMaterial():
     SurfaceMaterial(COLOR_BLACK)
@@ -9,18 +10,29 @@ SurfaceMaterial::SurfaceMaterial():
 
 SurfaceMaterial::SurfaceMaterial(const Color &color)
 {
-    base = colorToHSL(color);
+    base = new Color(color);
     hardness = 0.5;
     reflection = 0.0;
     shininess = 0.0;
     receive_shadows = 1.0;
 }
 
+SurfaceMaterial::~SurfaceMaterial()
+{
+    delete base;
+}
+
+void SurfaceMaterial::setColor(double r, double g, double b, double a)
+{
+    base->r = r;
+    base->g = g;
+    base->b = b;
+    base->a = a;
+}
+
 void SurfaceMaterial::save(PackStream* stream) const
 {
-    stream->write(&base.h);
-    stream->write(&base.l);
-    stream->write(&base.s);
+    base->save(stream);
 
     stream->write(&hardness);
     stream->write(&reflection);
@@ -31,20 +43,15 @@ void SurfaceMaterial::save(PackStream* stream) const
 
 void SurfaceMaterial::load(PackStream* stream)
 {
-    stream->read(&base.h);
-    stream->read(&base.l);
-    stream->read(&base.s);
+    base->load(stream);
 
     stream->read(&hardness);
     stream->read(&reflection);
     stream->read(&shininess);
 
     stream->read(&receive_shadows);
-
-    validate();
 }
 
 void SurfaceMaterial::validate()
 {
-    _rgb = colorFromHSL(base);
 }
diff --git a/src/definition/SurfaceMaterial.h b/src/definition/SurfaceMaterial.h
index 5a68266..0c0accf 100644
--- a/src/definition/SurfaceMaterial.h
+++ b/src/definition/SurfaceMaterial.h
@@ -3,10 +3,6 @@
 
 #include "definition_global.h"
 
-// TODO Change to pointers and forward declaration
-#include "ColorHSL.h"
-#include "Color.h"
-
 namespace paysages {
 namespace definition {
 
@@ -15,6 +11,9 @@ class DEFINITIONSHARED_EXPORT SurfaceMaterial
 public:
     SurfaceMaterial();
     SurfaceMaterial(const Color& color);
+    ~SurfaceMaterial();
+
+    void setColor(double r, double g, double b, double a);
 
     void save(PackStream* stream) const;
     void load(PackStream* stream);
@@ -22,15 +21,13 @@ public:
     void validate();
 
 public:
-    ColorHSL base;
+    Color *base;
 
     double hardness;
     double reflection;
     double shininess;
 
     double receive_shadows;
-
-    Color _rgb;
 };
 
 }
diff --git a/src/definition/TextureLayerDefinition.cpp b/src/definition/TextureLayerDefinition.cpp
index 96f4f5e..fce5217 100644
--- a/src/definition/TextureLayerDefinition.cpp
+++ b/src/definition/TextureLayerDefinition.cpp
@@ -6,6 +6,7 @@
 #include "PackStream.h"
 #include "Scenery.h"
 #include "TerrainDefinition.h"
+#include "Color.h"
 
 TextureLayerDefinition::TextureLayerDefinition(DefinitionNode* parent):
     DefinitionNode(parent, "texture", "texturelayer")
@@ -116,7 +117,7 @@ void TextureLayerDefinition::applyPreset(TextureLayerPreset preset)
             displacement_height = 0.02;
             displacement_scaling = 3.0;
             displacement_offset = 0.0;
-            material->base = colorToHSL(Color(0.015, 0.014, 0.014, 1.0));
+            material->setColor(0.015, 0.014, 0.014, 1.0);
             material->reflection = 0.003;
             material->shininess = 4.0;
             break;
@@ -125,7 +126,7 @@ void TextureLayerDefinition::applyPreset(TextureLayerPreset preset)
             displacement_height = 0.3;
             displacement_scaling = 2.0;
             displacement_offset = 0.0;
-            material->base = colorToHSL(Color(0.6, 0.55, 0.57, 1.0));
+            material->setColor(0.6, 0.55, 0.57, 1.0);
             material->reflection = 0.006;
             material->shininess = 6.0;
             break;
@@ -135,7 +136,7 @@ void TextureLayerDefinition::applyPreset(TextureLayerPreset preset)
             displacement_height = 0.0;
             displacement_scaling = 1.0;
             displacement_offset = 0.0;
-            material->base = colorToHSL(Color(0.12, 0.19, 0.035, 1.0));
+            material->setColor(0.12, 0.19, 0.035, 1.0);
             material->reflection = 0.001;
             material->shininess = 4.0;
             break;
@@ -145,7 +146,7 @@ void TextureLayerDefinition::applyPreset(TextureLayerPreset preset)
             displacement_height = 0.05;
             displacement_scaling = 5.0;
             displacement_offset = 0.0;
-            material->base = colorToHSL(Color(1.2, 1.1, 0.9, 1.0));
+            material->setColor(1.2, 1.1, 0.9, 1.0);
             material->reflection = 0.008;
             material->shininess = 1.0;
             break;
@@ -155,7 +156,7 @@ void TextureLayerDefinition::applyPreset(TextureLayerPreset preset)
             displacement_height = 0.1;
             displacement_scaling = 1.0;
             displacement_offset = 0.0;
-            material->base = colorToHSL(Color(5.0, 5.0, 5.0, 1.0));
+            material->setColor(5.0, 5.0, 5.0, 1.0);
             material->reflection = 0.02;
             material->shininess = 0.6;
             break;
diff --git a/src/definition/WaterDefinition.cpp b/src/definition/WaterDefinition.cpp
index 08c1823..e238755 100644
--- a/src/definition/WaterDefinition.cpp
+++ b/src/definition/WaterDefinition.cpp
@@ -115,7 +115,7 @@ void WaterDefinition::applyPreset(WaterPreset preset)
         transparency = 0.5;
         reflection = 0.2;
         transparency_depth = 4.0;
-        material->base = colorToHSL(Color(0.08, 0.15, 0.2, 1.0));
+        material->setColor(0.08, 0.15, 0.2, 1.0);
         depth_color->r = 0.0;
         depth_color->g = 0.1;
         depth_color->b = 0.1;
@@ -131,7 +131,7 @@ void WaterDefinition::applyPreset(WaterPreset preset)
         transparency = 0.3;
         reflection = 0.07;
         transparency_depth = 3.0;
-        material->base = colorToHSL(Color(0.05, 0.18, 0.2, 1.0));
+        material->setColor(0.05, 0.18, 0.2, 1.0);
         depth_color->r = 0.0;
         depth_color->g = 0.18;
         depth_color->b = 0.15;
@@ -144,11 +144,11 @@ void WaterDefinition::applyPreset(WaterPreset preset)
     }
 
     depth_color->a = 1.0;
-    material->base.a = 1.0;
+    material->base->a = 1.0;
     material->reflection = 1.0;
     material->shininess = 16.0;
     material->hardness = 0.3;
-    foam_material->base = colorToHSL(Color(0.8, 0.8, 0.8, 1.0));
+    foam_material->setColor(0.8, 0.8, 0.8, 1.0);
     foam_material->reflection = 0.1;
     foam_material->shininess = 1.5;
     foam_material->hardness = 0.2;
diff --git a/src/render/opengl/OpenGLWater.cpp b/src/render/opengl/OpenGLWater.cpp
index 111e5e1..bdd8d29 100644
--- a/src/render/opengl/OpenGLWater.cpp
+++ b/src/render/opengl/OpenGLWater.cpp
@@ -43,7 +43,7 @@ void OpenGLWater::initialize()
 
 void OpenGLWater::update()
 {
-    Color water_color = renderer->getScenery()->getWater()->material->_rgb;
+    Color water_color = *renderer->getScenery()->getWater()->material->base;
     renderer->getSharedState()->set("waterColor", water_color);
 
     double water_reflection = renderer->getScenery()->getWater()->reflection;
diff --git a/src/render/software/CloudBasicLayerRenderer.cpp b/src/render/software/CloudBasicLayerRenderer.cpp
index e0fe8dd..09c8a6c 100644
--- a/src/render/software/CloudBasicLayerRenderer.cpp
+++ b/src/render/software/CloudBasicLayerRenderer.cpp
@@ -176,8 +176,7 @@ Color CloudBasicLayerRenderer::getColor(BaseCloudsModel *model, const Vector3 &e
     segment_count = _findSegments(model, parent, start, direction, 20, transparency_depth, max_length, &inside_length, &total_length, segments);
     for (i = segment_count - 1; i >= 0; i--)
     {
-        SurfaceMaterial material;
-        material.base = colorToHSL(COLOR_WHITE);
+        SurfaceMaterial material(COLOR_WHITE);
         material.hardness = 0.25;
         material.reflection = 0.0;
         material.shininess = 0.0;
diff --git a/src/render/software/LightStatus.cpp b/src/render/software/LightStatus.cpp
index ae26119..48373a5 100644
--- a/src/render/software/LightStatus.cpp
+++ b/src/render/software/LightStatus.cpp
@@ -40,6 +40,6 @@ Color LightStatus::apply(const Vector3 &normal, const SurfaceMaterial &material)
         final = final.add(manager->applyFinalComponent(component, eye, location, normal, material));
     }
 
-    final.a = material.base.a;
+    final.a = material.base->a;
     return final;
 }
diff --git a/src/render/software/LightingManager.cpp b/src/render/software/LightingManager.cpp
index 0430823..37a514a 100644
--- a/src/render/software/LightingManager.cpp
+++ b/src/render/software/LightingManager.cpp
@@ -74,9 +74,9 @@ Color LightingManager::applyFinalComponent(const LightComponent &component, cons
     diffuse = (diffuse + (1.0 - normal_norm)) / (1.0 + (1.0 - normal_norm));
     if (diffuse > 0.0)
     {
-        result.r += diffuse * material._rgb.r * light_color.r;
-        result.g += diffuse * material._rgb.g * light_color.g;
-        result.b += diffuse * material._rgb.b * light_color.b;
+        result.r += diffuse * material.base->r * light_color.r;
+        result.g += diffuse * material.base->g * light_color.g;
+        result.b += diffuse * material.base->b * light_color.b;
     }
 
     /* specular reflection */
diff --git a/src/render/software/WaterRenderer.cpp b/src/render/software/WaterRenderer.cpp
index 1562a66..75ca18c 100644
--- a/src/render/software/WaterRenderer.cpp
+++ b/src/render/software/WaterRenderer.cpp
@@ -236,7 +236,7 @@ WaterRenderer::WaterResult WaterRenderer::getResult(double x, double z)
     /* Bring color to the camera */
     color = parent->applyMediumTraversal(location, color);
 
-    result.base = definition->material->_rgb;
+    result.base = *definition->material->base;
     result.final = color;
 
     return result;
diff --git a/src/tests/ColorHSL_Test.cpp b/src/tests/ColorHSL_Test.cpp
new file mode 100644
index 0000000..7eaa5be
--- /dev/null
+++ b/src/tests/ColorHSL_Test.cpp
@@ -0,0 +1,23 @@
+#include "BaseTestCase.h"
+
+#include "Color.h"
+#include "ColorHSL.h"
+
+TEST(ColorHSL, colorFromHSL)
+{
+    std::vector<Color> colors;
+    colors.push_back(Color());
+    colors.push_back(Color(1.0, 0.0, 0.0, 1.0));
+    colors.push_back(Color(0.7, 0.5, 0.3, 1.0));
+    //colors.push_back(Color(2.0, 0.0, 0.0, 1.0));  // FIXME not working yet
+    colors.push_back(Color(3.0, 3.0, 3.0, 1.0));
+
+    for (auto &color: colors)
+    {
+        ColorHSL hsl = colorToHSL(color);
+        Color rgb = colorFromHSL(hsl);
+
+        EXPECT_COLOR_RGBA(rgb, color.r, color.g, color.b, color.a);
+    }
+}
+
diff --git a/src/tests/tests.pro b/src/tests/tests.pro
index 731e750..a1e661e 100644
--- a/src/tests/tests.pro
+++ b/src/tests/tests.pro
@@ -25,7 +25,8 @@ SOURCES += main.cpp \
     Scenery_Test.cpp \
     DefinitionNode_Test.cpp \
     FloatNode_Test.cpp \
-    DiffManager_Test.cpp
+    DiffManager_Test.cpp \
+    ColorHSL_Test.cpp
 
 HEADERS += \
     BaseTestCase.h