code review

Revert Hsb to using integers
Clamp drag values

Engine/source/core/color.h

@@ -46,11 +46,11 @@ const F32 gOneOver255 = 1.f / 255.f;
 struct Hsb
 {
    Hsb() :hue(0), sat(0), brightness(0) {};
-   Hsb(F64 h, F64 s, F64 b) :hue(h), sat(s), brightness(b) {};
+   Hsb(U32 h, U32 s, U32 b) :hue(h), sat(s), brightness(b) {};
 
-   F64 hue;   ///Hue
-   F64 sat;   ///Saturation
-   F64 brightness;   //Brightness/Value/Lightness
+   U32 hue;   ///Hue
+   U32 sat;   ///Saturation
+   U32 brightness;   //Brightness/Value/Lightness
 };
 
 class ColorI;

Engine/source/gui/controls/guiColorPicker.cpp

@@ -514,14 +514,15 @@ void GuiColorPickerCtrl::onMouseDragged(const GuiEvent &event)
       Point2I ext = getExtent();
       Point2I mousePoint = globalToLocalCoord(event.mousePoint);
 
+      F32 relX = mClampF(F32(mousePoint.x) / F32(ext.x), 0.0, 1.0);
+      F32 relY = mClampF(F32(mousePoint.y) / F32(ext.y), 0.0, 1.0);
       switch (mDisplayMode)
       {
       case GuiColorPickerCtrl::pPalette:
          return;
       case GuiColorPickerCtrl::pBlendRange:
       {
-         F32 relX = F32(mousePoint.x) / F32(ext.x);
-         F32 relY = 1.0f - F32(mousePoint.y) / F32(ext.y);
+         relY = 1.0f - relY;
          setSelectedSaturation(relX * 100.0);
          setSelectedBrightness(relY * 100.0);
          break;
@@ -532,13 +533,11 @@ void GuiColorPickerCtrl::onMouseDragged(const GuiEvent &event)
          {
          case GuiColorPickerCtrl::sHorizontal:
          {
-            F32 relX = F32(mousePoint.x) / F32(ext.x);
             setSelectedHue(relX * 360.0);
             break;
          }
          case GuiColorPickerCtrl::sVertical:
          {
-            F32 relY = F32(mousePoint.y) / F32(ext.y);
             setSelectedHue(relY * 360.0);
             break;
          }
@@ -553,13 +552,11 @@ void GuiColorPickerCtrl::onMouseDragged(const GuiEvent &event)
          {
          case GuiColorPickerCtrl::sHorizontal:
          {
-            F32 relX = F32(mousePoint.x) / F32(ext.x);
             setSelectedAlpha(relX * 255.0);
             break;
          }
          case GuiColorPickerCtrl::sVertical:
          {
-            F32 relY = F32(mousePoint.y) / F32(ext.y);
             setSelectedAlpha(relY * 255.0);
             break;
          }
@@ -623,7 +620,7 @@ void GuiColorPickerCtrl::onMouseUp(const GuiEvent&)
    mouseUnlock();
 }
 
-void GuiColorPickerCtrl::setSelectedHue(const F64& hueValue)
+void GuiColorPickerCtrl::setSelectedHue(const U32& hueValue)
 {
    if (hueValue < 0)
    {
@@ -641,7 +638,7 @@ void GuiColorPickerCtrl::setSelectedHue(const F64& hueValue)
    
 }
 
-void GuiColorPickerCtrl::setSelectedBrightness(const F64& brightValue)
+void GuiColorPickerCtrl::setSelectedBrightness(const U32& brightValue)
 {
    if (brightValue < 0)
    {
@@ -658,7 +655,7 @@ void GuiColorPickerCtrl::setSelectedBrightness(const F64& brightValue)
    mSelectedBrightness = brightValue;
 }
 
-void GuiColorPickerCtrl::setSelectedSaturation(const F64& satValue)
+void GuiColorPickerCtrl::setSelectedSaturation(const U32& satValue)
 {
    if (satValue < 0)
    {
@@ -675,7 +672,7 @@ void GuiColorPickerCtrl::setSelectedSaturation(const F64& satValue)
    mSelectedSaturation = satValue;
 }
 
-void GuiColorPickerCtrl::setSelectedAlpha(const F64& alphaValue)
+void GuiColorPickerCtrl::setSelectedAlpha(const U32& alphaValue)
 {
    if (alphaValue < 0)
    {
@@ -731,42 +728,42 @@ DefineEngineMethod(GuiColorPickerCtrl, activateEyeDropper, void, (), , "Activate
    object->activateEyeDropper();
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, setSelectedHue, void, (F64 hueValue), , "Sets the selected hue value should be 0-360.")
+DefineEngineMethod(GuiColorPickerCtrl, setSelectedHue, void, (S32 hueValue), , "Sets the selected hue value should be 0-360.")
 {
    object->setSelectedHue(hueValue);
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, getSelectedHue, F64, (), , "Gets the current selected hue value.")
+DefineEngineMethod(GuiColorPickerCtrl, getSelectedHue, S32, (), , "Gets the current selected hue value.")
 {
    return object->getSelectedHue();
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, setSelectedBrightness, void, (F64 brightness), , "Sets the selected brightness value should be 0-100.")
+DefineEngineMethod(GuiColorPickerCtrl, setSelectedBrightness, void, (S32 brightness), , "Sets the selected brightness value should be 0-100.")
 {
    object->setSelectedBrightness(brightness);
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, getSelectedBrightness, F64, (), , "Gets the current selected brightness.")
+DefineEngineMethod(GuiColorPickerCtrl, getSelectedBrightness, S32, (), , "Gets the current selected brightness.")
 {
    return object->getSelectedBrightness();
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, setSelectedSaturation, void, (F64 saturation), , "Sets the selected saturation value should be 0-100.")
+DefineEngineMethod(GuiColorPickerCtrl, setSelectedSaturation, void, (S32 saturation), , "Sets the selected saturation value should be 0-100.")
 {
    object->setSelectedSaturation(saturation);
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, getSelectedSaturation, F64, (), , "Gets the current selected saturation value.")
+DefineEngineMethod(GuiColorPickerCtrl, getSelectedSaturation, S32, (), , "Gets the current selected saturation value.")
 {
    return object->getSelectedSaturation();
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, setSelectedAlpha, void, (F64 alpha), , "Sets the selected alpha value should be 0-255.")
+DefineEngineMethod(GuiColorPickerCtrl, setSelectedAlpha, void, (S32 alpha), , "Sets the selected alpha value should be 0-255.")
 {
    object->setSelectedAlpha(alpha);
 }
 
-DefineEngineMethod(GuiColorPickerCtrl, getSelectedAlpha, F64, (), , "Gets the current selected alpha value.")
+DefineEngineMethod(GuiColorPickerCtrl, getSelectedAlpha, S32, (), , "Gets the current selected alpha value.")
 {
    return object->getSelectedAlpha();
 }

Engine/source/gui/controls/guiColorPicker.h

@@ -114,10 +114,10 @@ class GuiColorPickerCtrl : public GuiControl
    /// @{
    PickMode mDisplayMode;	      ///< Current color display mode of the selector
    SelectorMode mSelectorMode;	///< Current color display mode of the selector
-   F64 mSelectedHue;
-   F64 mSelectedSaturation;
-   F64 mSelectedBrightness;
-   F64 mSelectedAlpha;
+   U32 mSelectedHue;
+   U32 mSelectedSaturation;
+   U32 mSelectedBrightness;
+   U32 mSelectedAlpha;
    Point2I eyeDropperPos;
    GBitmap* eyeDropperCap;
    GFXTexHandle eyeHandle;
@@ -162,29 +162,29 @@ class GuiColorPickerCtrl : public GuiControl
    /// Set the selected hue.
    /// </summary>
    /// <param name="hueValue">Hue value, 0 - 360.</param>
-   void setSelectedHue(const F64& hueValue);
-   F64 getSelectedHue() { return mSelectedHue; }
+   void setSelectedHue(const U32& hueValue);
+   U32 getSelectedHue() { return mSelectedHue; }
 
    /// <summary>
    /// Set the selected brightness.
    /// </summary>
    /// <param name="brightValue">Brightness value, 0 - 100.</param>
-   void setSelectedBrightness(const F64& brightValue);
-   F64 getSelectedBrightness() { return mSelectedBrightness; }
+   void setSelectedBrightness(const U32& brightValue);
+   U32 getSelectedBrightness() { return mSelectedBrightness; }
 
    /// <summary>
    /// Set the selected saturation.
    /// </summary>
    /// <param name="satValue">Saturation value, 0 - 100.</param>
-   void setSelectedSaturation(const F64& satValue);
-   F64 getSelectedSaturation() { return mSelectedSaturation; }
+   void setSelectedSaturation(const U32& satValue);
+   U32 getSelectedSaturation() { return mSelectedSaturation; }
 
    /// <summary>
    /// Set the selected alpha.
    /// </summary>
    /// <param name="alphaValue">Alpha value, 0 - 255.</param>
-   void setSelectedAlpha(const F64& alphaValue);
-   F64 getSelectedAlpha() { return mSelectedAlpha; }
+   void setSelectedAlpha(const U32& alphaValue);
+   U32 getSelectedAlpha() { return mSelectedAlpha; }
 
    void activateEyeDropper();