PR changes; fix saturation calculation (scale before dividing)

cmake/ConkyBuildOptions.cmake

@@ -263,8 +263,6 @@ option(BUILD_PULSEAUDIO
 option(BUILD_INTEL_BACKLIGHT
        "Enable support for Intel backlight" false)
 
-option(BUILD_HSV_GRADIENT "Enable gradient in HSV colour space" true)
-
 message(STATUS "CMAKE_C_FLAGS: " ${CMAKE_C_FLAGS})
 message(STATUS "CMAKE_CXX_FLAGS: " ${CMAKE_CXX_FLAGS})
 

src/gradient.cc

@@ -78,7 +78,9 @@ void gradient_factory::setup_masks() {
 
 void gradient_factory::setup_colour_depth() {
 #ifdef BUILD_X11
-  if (out_to_x.get(*state)) {
+  if (state == nullptr) {
+    colour_depth = 24; // testing purposes
+  } else if (out_to_x.get(*state)) {
     colour_depth = DisplayPlanes(display, screen);
   } else
 #endif /* BUILD_X11 */
@@ -211,10 +213,10 @@ void hsv_gradient_factory::convert_from_scaled_rgb(long *const scaled,
   auto value = get_value(scaled);
   auto minimum = get_minimum(scaled);
   auto chroma = value - minimum;
-  auto saturation = (SCALE * chroma) / value;
+  auto saturation = (SCALE360 * chroma) / value;
 
   target[0] = get_hue(scaled, chroma, value);
-  target[1] = saturation * 360L;
+  target[1] = saturation;
   target[2] = value * 360L;
 }
 
@@ -254,7 +256,7 @@ namespace {
 // Using Rec.2020 color space
 // Y' = 0.2627 x R + 0.6780 x G + 0.0593 x B
 long get_luma(long *const rgb) {
-  return (2627L * rgb[0] + 6780L * rgb[1] + 593L * rgb[2]) / 10000L;
+  return 360L * (2627L * rgb[0] + 6780L * rgb[1] + 593L * rgb[2]) / 10000L;
 }
 
 // Using Rec.2020 color space
@@ -282,7 +284,7 @@ void hcl_gradient_factory::convert_from_scaled_rgb(long *const scaled,
 
   target[0] = get_hue(scaled, chroma, value);
   target[1] = chroma * 360L;
-  target[2] = luma * 360L;
+  target[2] = luma;
 }
 
 void hcl_gradient_factory::convert_to_scaled_rgb(long *const target,

src/gradient.h

@@ -56,13 +56,15 @@ class gradient_factory {
   virtual void convert_from_scaled_rgb(long *const scaled, long *target) = 0;
   virtual void convert_to_scaled_rgb(long *const target, long *scaled) = 0;
 
- protected:
   void convert_from_rgb(long original, long *array);
   int convert_to_rgb(long *const array);
-  long get_hue(long *const scaled, long chroma, long value);
-  long get_intermediate(long hue, long chroma);
+
+ protected:
   virtual void fix_diff(long *diff) {}
 
+  static long get_hue(long *const scaled, long chroma, long value);
+  static long get_intermediate(long hue, long chroma);
+
   static short colour_depth;
   static long mask[3];
   static short shift[3];

tests/CMakeLists.txt

@@ -17,6 +17,7 @@ endif()
 set(test_srcs ${test_srcs} test-core.cc)
 set(test_srcs ${test_srcs} test-diskio.cc)
 set(test_srcs ${test_srcs} test-fs.cc)
+set(test_srcs ${test_srcs} test-gradient.cc)
 
 add_executable(test-conky test-common.cc ${test_srcs})
 target_link_libraries(test-conky conky_core)

tests/test-gradient.cc

@@ -0,0 +1,137 @@
+/*
+ *
+ * Conky, a system monitor, based on torsmo
+ *
+ * Any original torsmo code is licensed under the BSD license
+ *
+ * All code written since the fork of torsmo is licensed under the GPL
+ *
+ * Please see COPYING for details
+ *
+ * Copyright (c) 2005-2021 Brenden Matthews, Philip Kovacs, et. al.
+ *	(see AUTHORS)
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "catch2/catch.hpp"
+
+#include <conky.h>
+#include <gradient.h>
+
+const int width = 4;
+const long colour = 0x996633; // brown
+const long colour_hue = 30;
+const long colour_value = 0x99;
+
+const long full_scale = conky::gradient_factory::SCALE360;
+
+TEST_CASE("gradient_factory::convert_from_rgb returns correct value") {
+  state = nullptr;
+  SECTION("rgb_gradient_factory") {
+    auto factory = new conky::rgb_gradient_factory(width, colour, colour);
+    long result[3];
+
+    factory->convert_from_rgb(colour, result);
+
+    SECTION("red") {
+      REQUIRE(result[0] == 0x99 * full_scale);
+    }
+    SECTION("green") {
+      REQUIRE(result[1] == 0x66 * full_scale);
+    }
+    SECTION("blue") {
+      REQUIRE(result[2] == 0x33 * full_scale);
+    }
+
+    delete factory;
+  }
+
+  SECTION("hsv_gradient_factory") {
+    auto factory = new conky::hsv_gradient_factory(width, colour, colour);
+    long result[3];
+
+    factory->convert_from_rgb(colour, result);
+
+    SECTION("hue") {
+      REQUIRE(result[0] == colour_hue * conky::gradient_factory::SCALE);
+    }
+    SECTION("saturation") {
+      REQUIRE(result[1] == conky::gradient_factory::SCALE * 240L);
+    }
+    SECTION("value") {
+      REQUIRE(result[2] == colour_value * full_scale);
+    }
+
+    delete factory;
+  }
+
+  SECTION("hcl_gradient_factory") {
+    auto factory = new conky::hcl_gradient_factory(width, colour, colour);
+    long result[3];
+
+    factory->convert_from_rgb(colour, result);
+
+    SECTION("hue") {
+      REQUIRE(result[0] == colour_hue * conky::gradient_factory::SCALE);
+    }
+    SECTION("chroma") {
+      REQUIRE(result[1] == 0x66 * full_scale);
+    }
+    SECTION("luma") {
+      REQUIRE(result[2] == 20712665L);
+    }
+
+    delete factory;
+  }
+}
+
+TEST_CASE(
+    "gradient_factory should convert to and from rgb "
+    "and get the initial value") {
+
+  SECTION("rgb_gradient_factory") {
+    long tmp[3];
+    auto factory = new conky::rgb_gradient_factory(width, colour, colour);
+    factory->convert_from_rgb(colour, tmp);
+    auto result = factory->convert_to_rgb(tmp);
+
+    REQUIRE(result == colour);
+    
+    delete factory;
+  }
+
+  SECTION("hsv_gradient_factory") {
+    long tmp[3];
+    auto factory = new conky::hsv_gradient_factory(width, colour, colour);
+    factory->convert_from_rgb(colour, tmp);
+    auto result = factory->convert_to_rgb(tmp);
+
+    REQUIRE(result == colour);
+    
+    delete factory;
+  }
+
+  SECTION("hcl_gradient_factory") {
+    long tmp[3];
+    auto factory = new conky::hcl_gradient_factory(width, colour, colour);
+    factory->convert_from_rgb(colour, tmp);
+    auto result = factory->convert_to_rgb(tmp);
+
+    REQUIRE(result == colour);
+    
+    delete factory;
+  }
+}