From d4caa65cae106f9ccb9f93dc402e374209a8aa7e Mon Sep 17 00:00:00 2001
From: David Capello <davidcapello@gmail.com>
Date: Wed, 8 Dec 2010 11:35:08 -0300
Subject: [PATCH] Use gfx::Rgb and gfx::Hsv classes for HSV <-> RGB
 conversions. + Removed rgb_to_hsv_int() and hsv_to_rgb_int() functions. +
 Changed HSV ranges from [0,255] to H=[0,360], S=[0,100], V=[0,100]. +
 Simplified Color class (src/app/color.h) using a m_type and m_value union.

---
 data/widgets/palette_editor.xml     |   6 +-
 src/app/color.cpp                   | 332 +++++++++++++++-------------
 src/app/color.h                     |  52 ++---
 src/app/color_unittest.cpp          |   6 +-
 src/app/color_utils.cpp             | 124 ++---------
 src/commands/cmd_palette_editor.cpp | 100 +++++----
 src/raster/blend.cpp                | 122 ----------
 src/raster/blend.h                  |   3 -
 src/raster/palette.cpp              |  32 +--
 src/raster/quant.cpp                |  20 +-
 src/widgets/color_selector.cpp      |   8 +-
 src/widgets/colview.cpp             |   2 +-
 12 files changed, 315 insertions(+), 492 deletions(-)

diff --git a/data/widgets/palette_editor.xml b/data/widgets/palette_editor.xml
index 5d929b6b0..f64044d7c 100644
--- a/data/widgets/palette_editor.xml
+++ b/data/widgets/palette_editor.xml
@@ -30,15 +30,15 @@
 	  <entry  name="B_entry"  maxsize="3" />
 
 	  <label  name="H_label"  text="H" />
-	  <slider name="H_slider" min="0" max="255" cell_align="horizontal" />
+	  <slider name="H_slider" min="0" max="360" cell_align="horizontal" />
 	  <entry  name="H_entry"  maxsize="3" />
 
 	  <label  name="S_label"  text="S" />
-	  <slider name="S_slider" min="0" max="255" cell_align="horizontal" />
+	  <slider name="S_slider" min="0" max="100" cell_align="horizontal" />
 	  <entry  name="S_entry"  maxsize="3" />
 
 	  <label  name="V_label"  text="V" />
-	  <slider name="V_slider" min="0" max="255" cell_align="horizontal" />
+	  <slider name="V_slider" min="0" max="100" cell_align="horizontal" />
 	  <entry  name="V_entry"  maxsize="3" />
 
 	  <label name="hex_label" text="#" />
diff --git a/src/app/color.cpp b/src/app/color.cpp
index be0cabf1e..b379e7a62 100644
--- a/src/app/color.cpp
+++ b/src/app/color.cpp
@@ -25,43 +25,54 @@
 
 #include "app/color.h"
 #include "app/color_utils.h"
+#include "gfx/hsv.h"
+#include "gfx/rgb.h"
+#include "modules/palettes.h"
 #include "raster/image.h"
 #include "raster/palette.h"
-#include "modules/palettes.h"
 
-#define MAKE_DATA(c1,c2,c3)	((c3) << 16) | ((c2) << 8) | (c1)
-#define GET_DATA_C1(c)		(((c) >> 0) & 0xff)
-#define GET_DATA_C2(c)		(((c) >> 8) & 0xff)
-#define GET_DATA_C3(c)		(((c) >> 16) & 0xff)
+using namespace gfx;
 
 // static
 Color Color::fromMask()
 {
-  return Color(Color::MaskType, 0);
+  return Color(Color::MaskType);
 }
 
 // static
 Color Color::fromRgb(int r, int g, int b)
 {
-  return Color(Color::RgbType, MAKE_DATA(r & 0xff, g & 0xff, b & 0xff));
+  Color color(Color::RgbType);
+  color.m_value.rgb.r = r;
+  color.m_value.rgb.g = g;
+  color.m_value.rgb.b = b;
+  return color;
 }
 
 // static
 Color Color::fromHsv(int h, int s, int v)
 {
-  return Color(Color::HsvType, MAKE_DATA(h & 0xff, s & 0xff, v & 0xff));
+  Color color(Color::HsvType);
+  color.m_value.hsv.h = h;
+  color.m_value.hsv.s = s;
+  color.m_value.hsv.v = v;
+  return color;
 }
 
 // static
 Color Color::fromGray(int g)
 {
-  return Color(Color::GrayType, g & 0xff);
+  Color color(Color::GrayType);
+  color.m_value.gray = g;
+  return color;
 }
 
 // static
 Color Color::fromIndex(int index)
 {
-  return Color(Color::IndexType, index & 0xff);
+  Color color(Color::IndexType);
+  color.m_value.index = index;
+  return color;
 }
 
 // static
@@ -141,7 +152,6 @@ Color Color::fromString(const std::string& str)
 std::string Color::toString() const
 {
   std::stringstream result;
-  int data;
 
   switch (getType()) {
 
@@ -150,33 +160,25 @@ std::string Color::toString() const
       break;
 
     case Color::RgbType:
-      data = getRgbData();
-
       result << "rgb{"
-	     << GET_DATA_C1(data) << ","
-	     << GET_DATA_C2(data) << ","
-	     << GET_DATA_C3(data) << "}";
+	     << m_value.rgb.r << ","
+	     << m_value.rgb.g << ","
+	     << m_value.rgb.b << "}";
       break;
 
     case Color::HsvType:
-      data = getHsvData();
-
       result << "hsv{"
-	     << GET_DATA_C1(data) << ","
-	     << GET_DATA_C2(data) << ","
-	     << GET_DATA_C3(data) << "}";
+	     << m_value.hsv.h << ","
+	     << m_value.hsv.s << ","
+	     << m_value.hsv.v << "}";
       break;
 
     case Color::GrayType:
-      data = getGrayData();
-
-      result << "gray{" << data << "}";
+      result << "gray{" << m_value.gray << "}";
       break;
 
     case Color::IndexType:
-      data = getIndexData();
-
-      result << "index{" << data << "}";
+      result << "index{" << m_value.index << "}";
       break;
 
   }
@@ -187,7 +189,6 @@ std::string Color::toString() const
 std::string Color::toFormalString(int imgtype, bool long_format) const
 {
   std::stringstream result;
-  int data;
 
   // Long format
   if (long_format) {
@@ -198,16 +199,14 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
 	break;
 
       case Color::RgbType:
-	data = getRgbData();
 	if (imgtype == IMAGE_GRAYSCALE) {
-	  result << "Gray "
-		 << _graya_getv(color_utils::color_for_image(*this, imgtype));
+	  result << "Gray " << getGray();
 	}
 	else {
 	  result << "RGB "
-		 << GET_DATA_C1(data) << " "
-		 << GET_DATA_C2(data) << " "
-		 << GET_DATA_C3(data);
+		 << m_value.rgb.r << " "
+		 << m_value.rgb.g << " "
+		 << m_value.rgb.b;
 
 	  if (imgtype == IMAGE_INDEXED)
 	    result << " Index "
@@ -216,15 +215,14 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
 	break;
 
       case Color::HsvType:
-	data = getHsvData();
 	if (imgtype == IMAGE_GRAYSCALE) {
-	  result << "Gray " << GET_DATA_C3(data);
+	  result << "Gray " << getGray();
 	}
 	else {
 	  result << "HSV "
-		 << GET_DATA_C1(data) << " "
-		 << GET_DATA_C2(data) << " "
-		 << GET_DATA_C3(data);
+		 << m_value.hsv.h << " "
+		 << m_value.hsv.s << " "
+		 << m_value.hsv.v;
   
 	  if (imgtype == IMAGE_INDEXED)
 	    result << " Index " << color_utils::color_for_image(*this, imgtype);
@@ -232,15 +230,14 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
 	break;
 	
       case Color::GrayType:
-	data = getGrayData();
-	result << "Gray " << data;
+	result << "Gray " << m_value.gray;
 	break;
 
-      case Color::IndexType:
-	data = getIndexData();
-	if (data >= 0 && data < (int)get_current_palette()->size()) {
-	  ase_uint32 _c = get_current_palette()->getEntry(data);
-	  result << "Index " << data
+      case Color::IndexType: {
+	int i = m_value.index;
+	if (i >= 0 && i < (int)get_current_palette()->size()) {
+	  ase_uint32 _c = get_current_palette()->getEntry(i);
+	  result << "Index " << i
 		 << " (RGB "
 		 << (int)_rgba_getr(_c) << " "
 		 << (int)_rgba_getg(_c) << " "
@@ -248,10 +245,11 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
 	}
 	else {
 	  result << "Index "
-		 << data
+		 << i
 		 << " (out of range)";
 	}
 	break;
+      }
 
       default:
 	ASSERT(false);
@@ -267,15 +265,14 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
 	break;
 
       case Color::RgbType:
-	data = getRgbData();
 	if (imgtype == IMAGE_GRAYSCALE) {
-	  result << "V " << _graya_getv(color_utils::color_for_image(*this, imgtype));
+	  result << "V " << getGray();
 	}
 	else {
-	  result << "RGB " << std::hex << std::setfill('0')
-		 << std::setw(2) << GET_DATA_C1(data)
-		 << std::setw(2) << GET_DATA_C2(data)
-		 << std::setw(2) << GET_DATA_C3(data);
+	  result << "RGB #" << std::hex << std::setfill('0')
+		 << std::setw(2) << m_value.rgb.r
+		 << std::setw(2) << m_value.rgb.g
+		 << std::setw(2) << m_value.rgb.b;
 
 	  if (imgtype == IMAGE_INDEXED) {
 	    result << " (" << std::dec
@@ -285,31 +282,28 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
 	break;
 
       case Color::HsvType:
-	data = getHsvData();
 	if (imgtype == IMAGE_GRAYSCALE) {
-	  result << "V " << GET_DATA_C3(data);
+	  result << "V " << getGray();
 	}
 	else {
-	  result << "HSV " << std::hex << std::setfill('0')
-		 << std::setw(2) << GET_DATA_C1(data)
-		 << std::setw(2) << GET_DATA_C2(data)
-		 << std::setw(2) << GET_DATA_C3(data);
+	  result << "HSV "
+		 << m_value.hsv.h << ","
+		 << m_value.hsv.s << ","
+		 << m_value.hsv.v;
 
 	  if (imgtype == IMAGE_INDEXED) {
-	    result << " (" << std::dec
+	    result << " ("
 		   << color_utils::color_for_image(*this, imgtype) << ")";
 	  }
 	}
 	break;
 
       case Color::GrayType:
-	data = getGrayData();
-	result << "V " << data;
+	result << "V " << m_value.gray;
 	break;
 
       case Color::IndexType:
-	data = getIndexData();
-	result << "I " << data;
+	result << "I " << m_value.index;
 	break;
 
       default:
@@ -321,6 +315,40 @@ std::string Color::toFormalString(int imgtype, bool long_format) const
   return result.str();
 }
 
+bool Color::operator==(const Color& other) const
+{
+  if (getType() != other.getType())
+    return false;
+
+  switch (getType()) {
+
+    case Color::MaskType:
+      return true;
+
+    case Color::RgbType:
+      return
+	m_value.rgb.r == other.m_value.rgb.r &&
+	m_value.rgb.g == other.m_value.rgb.g &&
+	m_value.rgb.b == other.m_value.rgb.b;
+
+    case Color::HsvType:
+      return
+	m_value.hsv.h == other.m_value.hsv.h &&
+	m_value.hsv.s == other.m_value.hsv.s &&
+	m_value.hsv.v == other.m_value.hsv.v;
+
+    case Color::GrayType:
+      return m_value.gray == other.m_value.gray;
+
+    case Color::IndexType:
+      return m_value.index == other.m_value.index;
+
+    default:
+      ASSERT(false);
+      return false;
+  }
+}
+
 // Returns false only if the color is a index and it is outside the
 // valid range (outside the maximum number of colors in the current
 // palette)
@@ -329,7 +357,7 @@ bool Color::isValid() const
   switch (getType()) {
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       return (i >= 0 && i < get_current_palette()->size());
     }
 
@@ -345,22 +373,18 @@ int Color::getRed() const
       return 0;
 
     case Color::RgbType:
-      return GET_DATA_C1(getRgbData());
+      return m_value.rgb.r;
 
-    case Color::HsvType: {
-      int c = getHsvData();
-      int h = GET_DATA_C1(c);
-      int s = GET_DATA_C2(c);
-      int v = GET_DATA_C3(c);
-      hsv_to_rgb_int(&h, &s, &v);
-      return h;
-    }
+    case Color::HsvType:
+      return Rgb(Hsv(m_value.hsv.h,
+		     double(m_value.hsv.s) / 100.0,
+		     double(m_value.hsv.v) / 100.0)).red();
       
     case Color::GrayType:
-      return getGrayData();
+      return m_value.gray;
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       ASSERT(i >= 0 && i < get_current_palette()->size());
 
       return _rgba_getr(get_current_palette()->getEntry(i));
@@ -380,22 +404,18 @@ int Color::getGreen() const
       return 0;
 
     case Color::RgbType:
-      return GET_DATA_C2(getRgbData());
+      return m_value.rgb.g;
+
+    case Color::HsvType:
+      return Rgb(Hsv(m_value.hsv.h,
+		     double(m_value.hsv.s) / 100.0,
+		     double(m_value.hsv.v) / 100.0)).green();
 
-    case Color::HsvType: {
-      int c = getHsvData();
-      int h = GET_DATA_C1(c);
-      int s = GET_DATA_C2(c);
-      int v = GET_DATA_C3(c);
-      hsv_to_rgb_int(&h, &s, &v);
-      return s;
-    }
-      
     case Color::GrayType:
-      return getGrayData();
+      return m_value.gray;
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       ASSERT(i >= 0 && i < get_current_palette()->size());
 
       return _rgba_getg(get_current_palette()->getEntry(i));
@@ -415,22 +435,18 @@ int Color::getBlue() const
       return 0;
 
     case Color::RgbType:
-      return GET_DATA_C3(getRgbData());
+      return m_value.rgb.b;
+
+    case Color::HsvType:
+      return Rgb(Hsv(m_value.hsv.h,
+		     double(m_value.hsv.s) / 100.0,
+		     double(m_value.hsv.v) / 100.0)).blue();
 
-    case Color::HsvType: {
-      int c = getHsvData();
-      int h = GET_DATA_C1(c);
-      int s = GET_DATA_C2(c);
-      int v = GET_DATA_C3(c);
-      hsv_to_rgb_int(&h, &s, &v);
-      return v;
-    }
-      
     case Color::GrayType:
-      return getGrayData();
+      return m_value.gray;
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       ASSERT(i >= 0 && i < get_current_palette()->size());
 
       return _rgba_getb(get_current_palette()->getEntry(i));
@@ -449,31 +465,26 @@ int Color::getHue() const
     case Color::MaskType:
       return 0;
 
-    case Color::RgbType: {
-      int c = getRgbData();
-      int r = GET_DATA_C1(c);
-      int g = GET_DATA_C2(c);
-      int b = GET_DATA_C3(c);
-      rgb_to_hsv_int(&r, &g, &b);
-      return r;
-    }
+    case Color::RgbType:
+      return Hsv(Rgb(m_value.rgb.r,
+		     m_value.rgb.g,
+		     m_value.rgb.b)).hueInt();
 
     case Color::HsvType:
-      return GET_DATA_C1(getHsvData());
+      return m_value.hsv.h;
 
     case Color::GrayType:
       return 0;
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       ASSERT(i >= 0 && i < get_current_palette()->size());
 
       ase_uint32 c = get_current_palette()->getEntry(i);
-      int r = _rgba_getr(c);
-      int g = _rgba_getg(c);
-      int b = _rgba_getb(c);
-      rgb_to_hsv_int(&r, &g, &b);
-      return r;
+
+      return Hsv(Rgb(_rgba_getr(c),
+		     _rgba_getg(c),
+		     _rgba_getb(c))).hueInt();
     }
 
   }
@@ -489,31 +500,26 @@ int Color::getSaturation() const
     case Color::MaskType:
       return 0;
 
-    case Color::RgbType: {
-      int c = getRgbData();
-      int r = GET_DATA_C1(c);
-      int g = GET_DATA_C2(c);
-      int b = GET_DATA_C3(c);
-      rgb_to_hsv_int(&r, &g, &b);
-      return g;
-    }
+    case Color::RgbType:
+      return Hsv(Rgb(m_value.rgb.r,
+		     m_value.rgb.g,
+		     m_value.rgb.b)).saturationInt();
 
     case Color::HsvType:
-      return GET_DATA_C2(getHsvData());
+      return m_value.hsv.s;
 
     case Color::GrayType:
       return 0;
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       ASSERT(i >= 0 && i < get_current_palette()->size());
 
       ase_uint32 c = get_current_palette()->getEntry(i);
-      int r = _rgba_getr(c);
-      int g = _rgba_getg(c);
-      int b = _rgba_getb(c);
-      rgb_to_hsv_int(&r, &g, &b);
-      return g;
+
+      return Hsv(Rgb(_rgba_getr(c),
+		     _rgba_getg(c),
+		     _rgba_getb(c))).saturationInt();
     }
 
   }
@@ -529,31 +535,61 @@ int Color::getValue() const
     case Color::MaskType:
       return 0;
 
-    case Color::RgbType: {
-      int c = getRgbData();
-      int r = GET_DATA_C1(c);
-      int g = GET_DATA_C2(c);
-      int b = GET_DATA_C3(c);
-      rgb_to_hsv_int(&r, &g, &b);
-      return b;
-    }
+    case Color::RgbType:
+      return Hsv(Rgb(m_value.rgb.r,
+		     m_value.rgb.g,
+		     m_value.rgb.b)).valueInt();
 
     case Color::HsvType:
-      return GET_DATA_C3(getHsvData());
+      return m_value.hsv.v;
       
     case Color::GrayType:
-      return getGrayData();
+      return 100 * m_value.gray / 255;
 
     case Color::IndexType: {
-      int i = getIndexData();
+      int i = m_value.index;
       ASSERT(i >= 0 && i < get_current_palette()->size());
 
       ase_uint32 c = get_current_palette()->getEntry(i);
-      int r = _rgba_getr(c);
-      int g = _rgba_getg(c);
-      int b = _rgba_getb(c);
-      rgb_to_hsv_int(&r, &g, &b);
-      return b;
+
+      return Hsv(Rgb(_rgba_getr(c),
+		     _rgba_getg(c),
+		     _rgba_getb(c))).valueInt();
+    }
+
+  }
+
+  ASSERT(false);
+  return -1;
+}
+
+int Color::getGray() const
+{
+  switch (getType()) {
+
+    case Color::MaskType:
+      return 0;
+
+    case Color::RgbType:
+      return 255 * Hsv(Rgb(m_value.rgb.r,
+			   m_value.rgb.g,
+			   m_value.rgb.b)).valueInt() / 100;
+
+    case Color::HsvType:
+      return 255 * m_value.hsv.v / 100;
+      
+    case Color::GrayType:
+      return m_value.gray;
+
+    case Color::IndexType: {
+      int i = m_value.index;
+      ASSERT(i >= 0 && i < get_current_palette()->size());
+
+      ase_uint32 c = get_current_palette()->getEntry(i);
+
+      return 255 * Hsv(Rgb(_rgba_getr(c),
+			   _rgba_getg(c),
+			   _rgba_getb(c))).valueInt() / 100;
     }
 
   }
@@ -580,10 +616,10 @@ int Color::getIndex() const
       break;
 
     case Color::GrayType:
-      return getGrayData();
+      return m_value.gray;
 
     case Color::IndexType:
-      return getIndexData();
+      return m_value.index;
 
   }
 
diff --git a/src/app/color.h b/src/app/color.h
index 4dfa0af1e..1424b796b 100644
--- a/src/app/color.h
+++ b/src/app/color.h
@@ -35,13 +35,11 @@ public:
   };
 
   // Default ctor is mask color
-  Color()
-    : m_value(fromMask().m_value) {
-  }
+  Color() : m_type(MaskType) { }
 
   static Color fromMask();
   static Color fromRgb(int r, int g, int b);
-  static Color fromHsv(int h, int s, int v);
+  static Color fromHsv(int h, int s, int v); // h=[0,360], s=[0,100], v=[0,100]
   static Color fromGray(int g);
   static Color fromIndex(int index);
 
@@ -52,52 +50,44 @@ public:
   std::string toString() const;
   std::string toFormalString(int imgtype, bool long_format) const;
 
-  bool operator==(const Color& other) const {
-    return m_value == other.m_value;
-  }
-
+  bool operator==(const Color& other) const;
   bool operator!=(const Color& other) const {
-    return m_value != other.m_value;
+    return !operator==(other);
   }
 
   Type getType() const {
-    return static_cast<Type>(m_value >> 24);
+    return m_type;
   }
 
   bool isValid() const;
 
+  // Getters
   int getRed() const;
   int getGreen() const;
   int getBlue() const;
   int getHue() const;
   int getSaturation() const;
   int getValue() const;
+  int getGray() const;
   int getIndex() const;
 
 private:
-  Color(Type type, uint32_t data)
-    : m_value((static_cast<int>(type) << 24) |
-	      (data & 0xffffff)) {
-    ASSERT((data & 0xff000000) == 0);
-  }
+  Color(Type type) : m_type(type) { }
 
-  uint32_t getRgbData() const {
-    return m_value & 0xffffff;
-  }
+  // Color type
+  Type m_type;
 
-  uint32_t getHsvData() const {
-    return m_value & 0xffffff;
-  }
-
-  uint8_t getGrayData() const {
-    return m_value & 0xff;
-  }
-
-  uint8_t getIndexData() const {
-    return m_value & 0xff;
-  }
-
-  uint32_t m_value;
+  // Color value
+  union {
+    struct {
+      int r, g, b;
+    } rgb;
+    struct {
+      int h, s, v;
+    } hsv;
+    int gray;
+    int index;
+  } m_value;
 };
 
 #endif
diff --git a/src/app/color_unittest.cpp b/src/app/color_unittest.cpp
index 90f50c855..4c0e577c8 100644
--- a/src/app/color_unittest.cpp
+++ b/src/app/color_unittest.cpp
@@ -32,9 +32,9 @@ TEST(Color, fromRgb)
 
 TEST(Color, fromHsv)
 {
-  EXPECT_EQ(32, Color::fromHsv(32, 16, 255).getHue());
-  EXPECT_EQ(16, Color::fromHsv(32, 16, 255).getSaturation());
-  EXPECT_EQ(255, Color::fromHsv(32, 16, 255).getValue());
+  EXPECT_EQ(60, Color::fromHsv(60, 5, 100).getHue());
+  EXPECT_EQ(5, Color::fromHsv(60, 5, 100).getSaturation());
+  EXPECT_EQ(100, Color::fromHsv(60, 5, 100).getValue());
 }
 
 TEST(Color, fromString)
diff --git a/src/app/color_utils.cpp b/src/app/color_utils.cpp
index a556ce741..823626162 100644
--- a/src/app/color_utils.cpp
+++ b/src/app/color_utils.cpp
@@ -22,12 +22,16 @@
 
 #include "app/color_utils.h"
 #include "app/color.h"
+#include "gfx/hsv.h"
+#include "gfx/rgb.h"
 #include "modules/palettes.h"
 #include "raster/image.h"
 #include "raster/layer.h"
 #include "raster/sprite.h"
 #include "raster/palette.h"
 
+using namespace gfx;
+
 // Internal functions
 namespace {
 
@@ -73,26 +77,15 @@ int color_utils::color_for_allegro(const Color& color, int depth)
       break;
 
     case Color::RgbType:
+    case Color::HsvType:
       c = makeacol_depth(depth,
 			 color.getRed(),
 			 color.getGreen(),
 			 color.getBlue(), 255);
       break;
 
-    case Color::HsvType: {
-      int h, s, v;
-
-      h = color.getHue();
-      s = color.getSaturation();
-      v = color.getValue();
-      hsv_to_rgb_int(&h, &s, &v);
-
-      c = makeacol_depth(depth, h, s, v, 255);
-      break;
-    }
-      
     case Color::GrayType:
-      c = color.getValue();
+      c = color.getGray();
       if (depth != 8)
 	c = makeacol_depth(depth, c, c, c, 255);
       break;
@@ -117,106 +110,21 @@ int color_utils::color_for_allegro(const Color& color, int depth)
 
 int color_utils::color_for_image(const Color& color, int imgtype)
 {
+  if (color.getType() == Color::MaskType)
+    return 0;
+
   int c = -1;
 
-  switch (color.getType()) {
-
-    case Color::MaskType:
-      switch (imgtype) {
-	case IMAGE_RGB:
-	  c = _rgba(0, 0, 0, 0);
-	  break;
-	case IMAGE_GRAYSCALE:
-	  c = _graya(0, 0);
-	  break;
-	case IMAGE_INDEXED:
-	  c = 0;
-	  break;
-      }
+  switch (imgtype) {
+    case IMAGE_RGB:
+      c = _rgba(color.getRed(), color.getGreen(), color.getBlue(), 255);
       break;
-
-    case Color::RgbType: {
-      int r, g, b;
-
-      r = color.getRed();
-      g = color.getGreen();
-      b = color.getBlue();
-
-      switch (imgtype) {
-	case IMAGE_RGB: {
-	  c = _rgba(r, g, b, 255);
-	  break;
-	}
-	case IMAGE_GRAYSCALE: {
-	  rgb_to_hsv_int(&r, &g, &b);
-	  c = _graya(b, 255);
-	  break;
-	}
-	case IMAGE_INDEXED:
-	  c = get_current_palette()->findBestfit(r, g, b);
-	  break;
-      }
+    case IMAGE_GRAYSCALE:
+      c = _graya(color.getGray(), 255);
       break;
-    }
-
-    case Color::HsvType: {
-      int h, s, v;
-
-      h = color.getHue();
-      s = color.getSaturation();
-      v = color.getValue();
-      
-      switch (imgtype) {
-	case IMAGE_RGB:
-	  hsv_to_rgb_int(&h, &s, &v);
-	  c = _rgba(h, s, v, 255);
-	  break;
-	case IMAGE_GRAYSCALE: {
-	  c = _graya(v, 255);
-	  break;
-	}
-	case IMAGE_INDEXED:
-	  hsv_to_rgb_int(&h, &s, &v);
-	  c = get_current_palette()->findBestfit(h, s, v);
-	  break;
-      }
+    case IMAGE_INDEXED:
+      c = get_current_palette()->findBestfit(color.getRed(), color.getGreen(), color.getBlue());
       break;
-    }
-      
-    case Color::GrayType:
-      switch (imgtype) {
-	case IMAGE_RGB:
-	  c = color.getValue();
-	  c = _rgba(c, c, c, 255);
-	  break;
-	case IMAGE_GRAYSCALE:
-	  c = color.getValue();
-	  break;
-	case IMAGE_INDEXED:
-	  c = color.getValue();
-	  c = get_current_palette()->findBestfit(c, c, c);
-	  break;
-      }
-      break;
-
-    case Color::IndexType:
-      switch (imgtype) {
-	case IMAGE_RGB: {
-	  ase_uint32 _c = get_current_palette()->getEntry(color.getIndex());
-	  c = _rgba(_rgba_getr(_c),
-		    _rgba_getg(_c),
-		    _rgba_getb(_c), 255);
-	  break;
-	}
-	case IMAGE_GRAYSCALE:
-	  c = _graya(color.getIndex(), 255);
-	  break;
-	case IMAGE_INDEXED:
-	  c = MID(0, color.getIndex(), get_current_palette()->size()-1);
-	  break;
-      }
-      break;
-
   }
 
   return c;
diff --git a/src/commands/cmd_palette_editor.cpp b/src/commands/cmd_palette_editor.cpp
index cfec5cab4..0da57ba88 100644
--- a/src/commands/cmd_palette_editor.cpp
+++ b/src/commands/cmd_palette_editor.cpp
@@ -32,6 +32,8 @@
 #include "commands/params.h"
 #include "core/cfg.h"
 #include "dialogs/filesel.h"
+#include "gfx/hsv.h"
+#include "gfx/rgb.h"
 #include "gfx/size.h"
 #include "modules/editors.h"
 #include "modules/gui.h"
@@ -122,8 +124,8 @@ static bool palette_editor_change_hook(JWidget widget, void *data);
 static bool select_rgb_hook(JWidget widget, void *data);
 static bool select_hsv_hook(JWidget widget, void *data);
 static bool expand_button_select_hook(JWidget widget, void *data);
-static void modify_rgb_of_selected_entries(int dst_r, int dst_g, int dst_b, bool set_r, bool set_g, bool set_b);
-static void modify_hsv_of_selected_entries(int dst_h, int dst_s, int dst_v, bool set_h, bool set_s, bool set_v);
+static void modify_rgb_of_selected_entries(const Rgb& dst_rgb, bool set_r, bool set_g, bool set_b);
+static void modify_hsv_of_selected_entries(const Hsv& dst_hsv, bool set_h, bool set_s, bool set_v);
 static void on_color_changed(const Color& color);
 
 static void set_new_palette(Palette *palette, const char* operationName);
@@ -730,13 +732,14 @@ static void sliderRGB_change_hook(Slider* widget)
   int r = R_slider->getValue();
   int g = G_slider->getValue();
   int b = B_slider->getValue();
-  Color color = Color::fromRgb(r, g, b);
+  Rgb rgb(r, g, b);
+  Hsv hsv(rgb);
 
-  H_slider->setValue(color.getHue());
-  V_slider->setValue(color.getValue());
-  S_slider->setValue(color.getSaturation());
+  H_slider->setValue(hsv.hueInt());
+  S_slider->setValue(hsv.saturationInt());
+  V_slider->setValue(hsv.valueInt());
 
-  modify_rgb_of_selected_entries(r, g, b,
+  modify_rgb_of_selected_entries(rgb,
 				 widget == R_slider,
 				 widget == G_slider,
 				 widget == B_slider);
@@ -752,14 +755,15 @@ static void sliderHSV_change_hook(Slider* widget)
   int h = H_slider->getValue();
   int s = S_slider->getValue();
   int v = V_slider->getValue();
-  Color color = Color::fromHsv(h, s, v);
-  int r, g, b;
 
-  R_slider->setValue(r = color.getRed());
-  G_slider->setValue(g = color.getGreen());
-  B_slider->setValue(b = color.getBlue());
+  Hsv hsv(double(h), double(s) / 100.0, double(v) / 100.0);
+  Rgb rgb(hsv);
 
-  modify_hsv_of_selected_entries(h, s, v,
+  R_slider->setValue(rgb.red());
+  G_slider->setValue(rgb.green());
+  B_slider->setValue(rgb.blue());
+
+  modify_hsv_of_selected_entries(hsv,
 				 widget == H_slider,
 				 widget == S_slider,
 				 widget == V_slider);
@@ -778,13 +782,15 @@ static bool entryRGB_change_hook(JWidget widget, void *data)
   r = MID(0, r, 255);
   g = MID(0, g, 255);
   b = MID(0, b, 255);
-  Color color = Color::fromRgb(r, g, b);
 
-  H_entry->setTextf("%d", color.getHue());
-  V_entry->setTextf("%d", color.getValue());
-  S_entry->setTextf("%d", color.getSaturation());
+  Rgb rgb(r, g, b);
+  Hsv hsv(rgb);
 
-  modify_rgb_of_selected_entries(r, g, b,
+  H_entry->setTextf("%d", hsv.hueInt());
+  S_entry->setTextf("%d", hsv.saturationInt());
+  V_entry->setTextf("%d", hsv.valueInt());
+
+  modify_rgb_of_selected_entries(rgb,
 				 widget == R_slider,
 				 widget == G_slider,
 				 widget == B_slider);
@@ -801,14 +807,18 @@ static bool entryHSV_change_hook(JWidget widget, void *data)
   int h = H_entry->getTextInt();
   int s = S_entry->getTextInt();
   int v = V_entry->getTextInt();
-  Color color = Color::fromHsv(h, s, v);
-  int r, g, b;
+  h = MID(0, h, 360);
+  s = MID(0, s, 100);
+  v = MID(0, v, 100);
 
-  R_entry->setTextf("%d", r = color.getRed());
-  G_entry->setTextf("%d", g = color.getGreen());
-  B_entry->setTextf("%d", b = color.getBlue());
+  Hsv hsv(double(h), double(s) / 100.0, double(v) / 100.0);
+  Rgb rgb(hsv);
 
-  modify_hsv_of_selected_entries(h, s, v,
+  R_entry->setTextf("%d", rgb.red());
+  G_entry->setTextf("%d", rgb.green());
+  B_entry->setTextf("%d", rgb.blue());
+
+  modify_hsv_of_selected_entries(hsv,
 				 widget == H_entry,
 				 widget == S_entry,
 				 widget == V_entry);
@@ -825,7 +835,6 @@ static bool hex_entry_change_hook(JWidget widget, void *data)
   Palette* palette = get_current_palette();
   std::string text = hex_entry->getText();
   int r, g, b;
-  float h, s, v;
   bool array[256];
   int c;
 
@@ -840,7 +849,7 @@ static bool hex_entry_change_hook(JWidget widget, void *data)
   G_slider->setValue(g = ((hex & 0xff00) >> 8));
   B_slider->setValue(b = ((hex & 0xff)));
 
-  rgb_to_hsv(r, g, b, &h, &s, &v);
+  Hsv hsv(Rgb(r, g, b));
 
   palette_editor->getSelectedEntries(array);
   for (c=0; c<256; c++) {
@@ -849,9 +858,9 @@ static bool hex_entry_change_hook(JWidget widget, void *data)
     }
   }
 
-  H_slider->setValue(255.0 * h / 360.0);
-  V_slider->setValue(255.0 * v);
-  S_slider->setValue(255.0 * s);
+  H_slider->setValue(hsv.hueInt());
+  V_slider->setValue(hsv.saturationInt());
+  S_slider->setValue(hsv.valueInt());
 
   update_entries_from_sliders();
   update_current_sprite_palette("Color Change");
@@ -1067,7 +1076,7 @@ static bool expand_button_select_hook(JWidget widget, void *data)
   return true;
 }
 
-static void modify_rgb_of_selected_entries(int dst_r, int dst_g, int dst_b, bool set_r, bool set_g, bool set_b)
+static void modify_rgb_of_selected_entries(const Rgb& dst_rgb, bool set_r, bool set_g, bool set_b)
 {
   bool array[256];
   palette_editor->getSelectedEntries(array);
@@ -1081,21 +1090,20 @@ static void modify_rgb_of_selected_entries(int dst_r, int dst_g, int dst_b, bool
       src_color = palette->getEntry(c);
 
       // Setup the new RGB values depending the desired values in set_X component.
-      r = (set_r ? dst_r: _rgba_getr(src_color));
-      g = (set_g ? dst_g: _rgba_getg(src_color));
-      b = (set_b ? dst_b: _rgba_getb(src_color));
+      r = (set_r ? dst_rgb.red():   _rgba_getr(src_color));
+      g = (set_g ? dst_rgb.green(): _rgba_getg(src_color));
+      b = (set_b ? dst_rgb.blue():  _rgba_getb(src_color));
 
       palette->setEntry(c, _rgba(r, g, b, 255));
     }
   }
 }
 
-static void modify_hsv_of_selected_entries(int dst_h, int dst_s, int dst_v, bool set_h, bool set_s, bool set_v)
+static void modify_hsv_of_selected_entries(const Hsv& dst_hsv, bool set_h, bool set_s, bool set_v)
 {
   bool array[256];
   palette_editor->getSelectedEntries(array);
   ase_uint32 src_color;
-  int r, g, b;
 
   Palette* palette = get_current_palette();
   for (int c=0; c<256; c++) {
@@ -1103,23 +1111,23 @@ static void modify_hsv_of_selected_entries(int dst_h, int dst_s, int dst_v, bool
       src_color = palette->getEntry(c);
 
       // Get the current RGB values of the palette entry
-      r = _rgba_getr(src_color);
-      g = _rgba_getg(src_color);
-      b = _rgba_getb(src_color);
+      Rgb rgb(_rgba_getr(src_color),
+	      _rgba_getg(src_color),
+	      _rgba_getb(src_color));
 
-      // RGB -> HSV
-      rgb_to_hsv_int(&r, &g, &b);
+      // Convert RGB to HSV
+      Hsv hsv(rgb);
 
       // Only modify the desired HSV components
-      if (set_h) r = dst_h;
-      if (set_s) g = dst_s;
-      if (set_v) b = dst_v;
+      if (set_h) hsv.hue(dst_hsv.hue());
+      if (set_s) hsv.saturation(dst_hsv.saturation());
+      if (set_v) hsv.value(dst_hsv.value());
 
-      // HSV -> RGB
-      hsv_to_rgb_int(&r, &g, &b);
+      // Convert HSV to RGB
+      rgb = Rgb(hsv);
 
       // Update the palette entry
-      palette->setEntry(c, _rgba(r, g, b, 255));
+      palette->setEntry(c, _rgba(rgb.red(), rgb.green(), rgb.blue(), 255));
     }
   }
 }
diff --git a/src/raster/blend.cpp b/src/raster/blend.cpp
index 1edf8d202..9a643e5cf 100644
--- a/src/raster/blend.cpp
+++ b/src/raster/blend.cpp
@@ -206,125 +206,3 @@ int _graya_blend_merge(int back, int front, int opacity)
   
   return _graya(D_k, D_a);
 }
-
-/**********************************************************************/
-/* Routines from The GIMP                                             */
-/**********************************************************************/
-
-void rgb_to_hsv_int(int *red, int *green, int *blue)
-{
-  int    r, g, b;
-  double h, s, v;
-  int    min, max;
-  int    delta;
-
-  h = 0.0;
-
-  r = *red;
-  g = *green;
-  b = *blue;
-
-  if (r > g)
-    {
-      max = MAX (r, b);
-      min = MIN (g, b);
-    }
-  else
-    {
-      max = MAX (g, b);
-      min = MIN (r, b);
-    }
-
-  v = max;
-
-  if (max != 0)
-    s = ((max - min) * 255) / (double) max;
-  else
-    s = 0;
-
-  if (s == 0)
-    h = 0;
-  else
-    {
-      delta = max - min;
-      if (r == max)
-	h = (g - b) / (double) delta;
-      else if (g == max)
-	h = 2 + (b - r) / (double) delta;
-      else if (b == max)
-	h = 4 + (r - g) / (double) delta;
-      h *= 42.5;
-
-      if (h < 0)
-	h += 255;
-      if (h > 255)
-	h -= 255;
-    }
-
-  *red   = (int)h;
-  *green = (int)s;
-  *blue  = (int)v;
-}
-
-void hsv_to_rgb_int(int *hue, int *saturation, int *value)
-{
-  double h, s, v;
-  double f, p, q, t;
-
-  if (*saturation == 0)
-    {
-      *hue        = *value;
-      *saturation = *value;
-      *value      = *value;
-    }
-  else
-    {
-      h = *hue * 6.0  / 255.0;
-      s = *saturation / 255.0;
-      v = *value      / 255.0;
-
-      f = h - (int) h;
-      p = v * (1.0 - s);
-      q = v * (1.0 - (s * f));
-      t = v * (1.0 - (s * (1.0 - f)));
-
-      switch ((int) h)
-	{
-	case 0:
-	  *hue        = (int)(v * 255);
-	  *saturation = (int)(t * 255);
-	  *value      = (int)(p * 255);
-	  break;
-
-	case 1:
-	  *hue        = (int)(q * 255);
-	  *saturation = (int)(v * 255);
-	  *value      = (int)(p * 255);
-	  break;
-
-	case 2:
-	  *hue        = (int)(p * 255);
-	  *saturation = (int)(v * 255);
-	  *value      = (int)(t * 255);
-	  break;
-
-	case 3:
-	  *hue        = (int)(p * 255);
-	  *saturation = (int)(q * 255);
-	  *value      = (int)(v * 255);
-	  break;
-
-	case 4:
-	  *hue        = (int)(t * 255);
-	  *saturation = (int)(p * 255);
-	  *value      = (int)(v * 255);
-	  break;
-
-	case 5:
-	  *hue        = (int)(v * 255);
-	  *saturation = (int)(p * 255);
-	  *value      = (int)(q * 255);
-	  break;
-	}
-    }
-}
diff --git a/src/raster/blend.h b/src/raster/blend.h
index 2e96d362f..b58291377 100644
--- a/src/raster/blend.h
+++ b/src/raster/blend.h
@@ -43,8 +43,5 @@ int _graya_blend_copy(int back, int front, int opacity);
 int _graya_blend_forpath(int back, int front, int opacity);
 int _graya_blend_merge(int back, int front, int opacity);
 
-void rgb_to_hsv_int(int *red, int *green, int *blue);
-void hsv_to_rgb_int(int *hue, int *saturation, int *value);
-
 #endif
 
diff --git a/src/raster/palette.cpp b/src/raster/palette.cpp
index a18f804ef..4716f949c 100644
--- a/src/raster/palette.cpp
+++ b/src/raster/palette.cpp
@@ -21,10 +21,14 @@
 #include <allegro.h>
 #include <algorithm>
 
+#include "gfx/hsv.h"
+#include "gfx/rgb.h"
 #include "raster/image.h"
 #include "raster/palette.h"
 #include "util/col_file.h"
 
+using namespace gfx;
+
 //////////////////////////////////////////////////////////////////////
 
 Palette::Palette(int frame, int ncolors)
@@ -278,29 +282,25 @@ bool SortPalette::operator()(ase_uint32 c1, ase_uint32 c2)
     case SortPalette::HSV_Hue:
     case SortPalette::HSV_Saturation:
     case SortPalette::HSV_Value: {
-      int h1, s1, v1;
-      int h2, s2, v2;
-      h1 = _rgba_getr(c1);
-      s1 = _rgba_getg(c1);
-      v1 = _rgba_getb(c1);
-      h2 = _rgba_getr(c2);
-      s2 = _rgba_getg(c2);
-      v2 = _rgba_getb(c2);
-      rgb_to_hsv_int(&h1, &s1, &v1);
-      rgb_to_hsv_int(&h2, &s2, &v2);
+      Hsv hsv1(Rgb(_rgba_getr(c1),
+		   _rgba_getg(c1),
+		   _rgba_getb(c1)));
+      Hsv hsv2(Rgb(_rgba_getr(c2),
+		   _rgba_getg(c2),
+		   _rgba_getb(c2)));
 
       switch (m_channel) {
 	case SortPalette::HSV_Hue:
-	  value1 = h1;
-	  value2 = h2;
+	  value1 = hsv1.hueInt();
+	  value2 = hsv2.hueInt();
 	  break;
 	case SortPalette::HSV_Saturation:
-	  value1 = s1;
-	  value2 = s2;
+	  value1 = hsv1.saturationInt();
+	  value2 = hsv2.saturationInt();
 	  break;
 	case SortPalette::HSV_Value:
-	  value1 = v1;
-	  value2 = v2;
+	  value1 = hsv1.valueInt();
+	  value2 = hsv2.valueInt();
 	  break;
 	default:
 	  ASSERT(false);
diff --git a/src/raster/quant.cpp b/src/raster/quant.cpp
index 474419e91..08b3822be 100644
--- a/src/raster/quant.cpp
+++ b/src/raster/quant.cpp
@@ -18,12 +18,16 @@
 
 #include "config.h"
 
+#include "gfx/hsv.h"
+#include "gfx/rgb.h"
 #include "raster/blend.h"
 #include "raster/image.h"
 #include "raster/quant.h"
 #include "raster/palette.h"
 #include "raster/rgbmap.h"
 
+using namespace gfx;
+
 Image *image_set_imgtype(const Image* image, int imgtype,
 			 int dithering_method,
 			 const RgbMap* rgbmap,
@@ -63,11 +67,12 @@ Image *image_set_imgtype(const Image* image, int imgtype,
 	  gray_address = (ase_uint16*)new_image->dat;
 	  for (i=0; i<size; i++) {
 	    c = *rgb_address;
-	    r = _rgba_getr(c);
-	    g = _rgba_getg(c);
-	    b = _rgba_getb(c);
-	    rgb_to_hsv_int(&r, &g, &b);
-	    *gray_address = _graya(b, _rgba_geta(c));
+
+	    g = 255 * Hsv(Rgb(_rgba_getr(c),
+	    		      _rgba_getg(c),
+	    		      _rgba_getb(c))).valueInt() / 100;
+	    *gray_address = _graya(g, _rgba_geta(c));
+
 	    rgb_address++;
 	    gray_address++;
 	  }
@@ -152,8 +157,9 @@ Image *image_set_imgtype(const Image* image, int imgtype,
 	      r = _rgba_getr(palette->getEntry(c));
 	      g = _rgba_getg(palette->getEntry(c));
 	      b = _rgba_getb(palette->getEntry(c));
-	      rgb_to_hsv_int(&r, &g, &b);
-	      *gray_address = _graya(b, 255);
+
+	      g = 255 * Hsv(Rgb(r, g, b)).valueInt() / 100;
+	      *gray_address = _graya(g, 255);
 	    }
 	    idx_address++;
 	    gray_address++;
diff --git a/src/widgets/color_selector.cpp b/src/widgets/color_selector.cpp
index 2d061ac8f..484a5b245 100644
--- a/src/widgets/color_selector.cpp
+++ b/src/widgets/color_selector.cpp
@@ -194,9 +194,9 @@ static Widget* create_hsv_container()
   Label* hlabel = new Label("H");
   Label* slabel = new Label("S");
   Label* vlabel = new Label("V");
-  Slider* hslider = new Slider(0, 255, 0);
-  Slider* sslider = new Slider(0, 255, 0);
-  Slider* vslider = new Slider(0, 255, 0);
+  Slider* hslider = new Slider(0, 360, 0);
+  Slider* sslider = new Slider(0, 100, 0);
+  Slider* vslider = new Slider(0, 100, 0);
   jgrid_add_child(grid, hlabel, 1, 1, JI_RIGHT);
   jgrid_add_child(grid, hslider, 1, 1, JI_HORIZONTAL);
   jgrid_add_child(grid, slabel, 1, 1, JI_RIGHT);
@@ -305,7 +305,7 @@ static void colorselector_set_color2(JWidget widget, const Color& color,
     hsv_vslider->setValue(color.getValue());
   }
   if (exclude_this_model != models+MODEL_GRAY) {
-    gray_vslider->setValue(color.getValue());
+    gray_vslider->setValue(color.getGray());
   }
   
   switch (color.getType()) {
diff --git a/src/widgets/colview.cpp b/src/widgets/colview.cpp
index a1a902955..98b977b3c 100644
--- a/src/widgets/colview.cpp
+++ b/src/widgets/colview.cpp
@@ -107,7 +107,7 @@ static bool colorviewer_msg_proc(JWidget widget, JMessage msg)
       break;
 
     case JM_REQSIZE: {
-      msg->reqsize.w = ji_font_text_len(widget->getFont(), "255,255,255,255");
+      msg->reqsize.w = ji_font_text_len(widget->getFont(), "XXX 255,255,255 (255)");
       msg->reqsize.h = jwidget_get_text_height(widget);
 
       msg->reqsize.w += widget->border_width.l + widget->border_width.r;