Add option to apply Saturation/Value to Hue slider in Tint/Shade/Tone optionally

This was changed in df33744c85, but now
we're adding an option to switch between both behaviors. We prefer to
keep the old behavior as the default one and the new one as an
optional preference.

Related to: https://community.aseprite.org/t/15065

data/pref.xml

@@ -214,6 +214,7 @@
       <option id="use_native_clipboard" type="bool" default="true" />
       <option id="use_native_file_dialog" type="bool" default="false" />
       <option id="use_shaders_for_color_selectors" type="bool" default="true" />
+      <option id="hue_with_sat_value_for_color_selector" type="bool" default="false" />
       <option id="one_finger_as_mouse_movement" type="bool" default="true" />
       <option id="load_wintab_driver" type="bool" default="false" />
       <option id="flash_layer" type="bool" default="false" />

data/strings/en.ini

@@ -1303,6 +1303,7 @@ new_render_engine = New render engine for sprite editor
 native_clipboard = Use native clipboard
 native_file_dialog = Use native file dialog
 shaders_for_color_selectors = Use shaders for color selectors
+hue_with_sat_value = Apply Saturation/Value to Hue slider on Tint/Shade/Tone selector
 one_finger_as_mouse_movement = Interpret one finger as mouse movement
 one_finger_as_mouse_movement_tooltip = <<<END
 Only for Windows 8/10 Pointer API: Interprets one finger as mouse movement

data/widgets/options.xml

@@ -517,7 +517,10 @@
                    pref="experimental.use_shaders_for_color_selectors" />
             <link text="(#960)" url="https://github.com/aseprite/aseprite/issues/960" />
           </hbox>
-	  <hbox id="load_wintab_driver_box">
+          <check id="tint_shade_tone_hue_with_sat_value"
+                 text="@.hue_with_sat_value"
+                 pref="experimental.hue_with_sat_value_for_color_selector" />
+          <hbox id="load_wintab_driver_box">
             <check id="load_wintab_driver2"
                    text="@.load_wintab_driver"
                    tooltip="@.load_wintab_driver_tooltip" />

src/app/ui/color_selector.cpp

@@ -626,6 +626,7 @@ void ColorSelector::updateColorSpace()
 }
 
 #if SK_ENABLE_SKSL
+
 // static
 const char* ColorSelector::getAlphaBarShader()
 {
@@ -678,6 +679,12 @@ sk_sp<SkRuntimeEffect> ColorSelector::buildEffect(const char* code)
     return result.effect;
   }
 }
+
+void ColorSelector::resetBottomEffect()
+{
+  m_bottomEffect.reset();
+}
+
 #endif  // SK_ENABLE_SKSL
 
 } // namespace app

src/app/ui/color_selector.h

@@ -104,6 +104,11 @@ namespace app {
     // atomic because we need atomic bitwise operations.
     std::atomic<int> m_paintFlags;
 
+  protected:
+#if SK_ENABLE_SKSL
+    void resetBottomEffect();
+#endif
+
   private:
     app::Color getAlphaBarColor(const int u, const int umax);
     void onPaintAlphaBar(ui::Graphics* g, const gfx::Rect& rc);

src/app/ui/color_tint_shade_tone.cpp

@@ -12,6 +12,7 @@
 #include "app/ui/color_tint_shade_tone.h"
 
 #include "app/color_utils.h"
+#include "app/pref/preferences.h"
 #include "app/ui/skin/skin_theme.h"
 #include "app/util/shader_helpers.h"
 #include "ui/graphics.h"
@@ -26,6 +27,18 @@ using namespace ui;
 
 ColorTintShadeTone::ColorTintShadeTone()
 {
+  m_conn = Preferences::instance()
+    .experimental.hueWithSatValueForColorSelector.AfterChange.connect(
+      [this](){
+        m_paintFlags |= AllAreasFlag;
+
+#if SK_ENABLE_SKSL
+        m_bottomShader.clear();
+        resetBottomEffect();
+#endif
+
+        invalidate();
+      });
 }
 
 #if SK_ENABLE_SKSL
@@ -55,12 +68,20 @@ const char* ColorTintShadeTone::getBottomBarShader()
     m_bottomShader += "uniform half3 iRes;"
                       "uniform half4 iHsv;";
     m_bottomShader += kHSV_to_RGB_sksl;
-    // TODO should we display the hue bar with the current sat/value?
-    m_bottomShader += R"(
+
+    if (m_hueWithSatValue)
+      m_bottomShader += R"(
 half4 main(vec2 fragcoord) {
  half h = (fragcoord.x / iRes.x);
  return hsv_to_rgb(half3(h, iHsv.y, iHsv.z)).rgb1;
 }
+)";
+    else
+      m_bottomShader += R"(
+half4 main(vec2 fragcoord) {
+ half h = (fragcoord.x / iRes.x);
+ return hsv_to_rgb(half3(h, 1.0, 1.0)).rgb1;
+}
 )";
   }
   return m_bottomShader.c_str();
@@ -107,11 +128,22 @@ void ColorTintShadeTone::onPaintMainArea(ui::Graphics* g, const gfx::Rect& rc)
   }
 }
 
+void ColorTintShadeTone::onPaint(ui::PaintEvent& ev)
+{
+  m_hueWithSatValue = Preferences::instance().experimental.hueWithSatValueForColorSelector();
+  ColorSelector::onPaint(ev);
+}
+
 void ColorTintShadeTone::onPaintBottomBar(ui::Graphics* g, const gfx::Rect& rc)
 {
   if (m_color.getType() != app::Color::MaskType) {
     double hue = m_color.getHsvHue();
-    double val = m_color.getHsvValue();
+    double val;
+    if (m_hueWithSatValue)
+      val = m_color.getHsvValue();
+    else
+      val = 1.0;
+
     gfx::Point pos(rc.x + int(rc.w * hue / 360.0),
                    rc.y + rc.h/2);
     paintColorIndicator(g, pos, val < 0.5);
@@ -151,9 +183,16 @@ void ColorTintShadeTone::onPaintSurfaceInBgThread(
 
   if (m_paintFlags & BottomBarFlag) {
     os::Paint paint;
+    double sat, val;
 
-    const double sat = m_color.getHsvSaturation();
-    const double val = m_color.getHsvValue();
+    if (m_hueWithSatValue) {
+      sat = m_color.getHsvSaturation();
+      val = m_color.getHsvValue();
+    }
+    else {
+      sat = 1.0;
+      val = 1.0;
+    }
 
     for (int x=0; x<bottom.w && !stop; ++x) {
       paint.color(
@@ -174,12 +213,17 @@ void ColorTintShadeTone::onPaintSurfaceInBgThread(
 
 int ColorTintShadeTone::onNeedsSurfaceRepaint(const app::Color& newColor)
 {
-  return
+  int flags =
     // Only if the hue changes we have to redraw the main surface.
     (cs_double_diff(m_color.getHsvHue(), newColor.getHsvHue()) ? MainAreaFlag: 0) |
-    (cs_double_diff(m_color.getHsvSaturation(), newColor.getHsvSaturation()) ||
-     cs_double_diff(m_color.getHsvValue(), newColor.getHsvValue()) ? BottomBarFlag: 0) |
     ColorSelector::onNeedsSurfaceRepaint(newColor);
+
+  if (m_hueWithSatValue) {
+    flags |=
+      (cs_double_diff(m_color.getHsvSaturation(), newColor.getHsvSaturation()) ||
+       cs_double_diff(m_color.getHsvValue(), newColor.getHsvValue()) ? BottomBarFlag: 0);
+  }
+  return flags;
 }
 
 } // namespace app

src/app/ui/color_tint_shade_tone.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "app/ui/color_selector.h"
+#include "obs/connection.h"
 
 namespace app {
   class Color;
@@ -27,6 +28,8 @@ namespace app {
     app::Color getMainAreaColor(const int u, const int umax,
                                 const int v, const int vmax) override;
     app::Color getBottomBarColor(const int u, const int umax) override;
+
+    void onPaint(ui::PaintEvent& ev) override;
     void onPaintMainArea(ui::Graphics* g, const gfx::Rect& rc) override;
     void onPaintBottomBar(ui::Graphics* g, const gfx::Rect& rc) override;
     void onPaintSurfaceInBgThread(os::Surface* s,
@@ -37,8 +40,12 @@ namespace app {
     int onNeedsSurfaceRepaint(const app::Color& newColor) override;
 
   private:
+#if SK_ENABLE_SKSL
     std::string m_mainShader;
     std::string m_bottomShader;
+#endif
+    bool m_hueWithSatValue = false;
+    obs::scoped_connection m_conn;
   };
 
 } // namespace app