Improve color selector shaders

* Removed conversion from RGB -> HSV/HSL for each pixel (we just pass
  the HSV/HSL values directly to the shader). In this way we don't
  lost some hue information for some special cases like white/black
  colors (probably related to: https://community.aseprite.org/t/14904)
* Now Tint/Shade/Tone selector changes the aspect of the Hue slider
  depending on the current saturation/value (this was changed in the
  non-shader version too)

src/app/ui/color_selector.cpp

@@ -468,8 +468,7 @@ void ColorSelector::onPaint(ui::PaintEvent& ev)
 
       SkRuntimeShaderBuilder builder1(m_mainEffect);
       builder1.uniform("iRes") = SkV3{float(rc2.w), float(rc2.h), 0.0f};
-      builder1.uniform("iColor") = appColor_to_SkV4(m_color);
-      setShaderMainAreaParams(builder1);
+      setShaderParams(builder1, true);
       p.setShader(builder1.makeShader());
 
       if (isSRGB)
@@ -484,7 +483,7 @@ void ColorSelector::onPaint(ui::PaintEvent& ev)
 
       SkRuntimeShaderBuilder builder2(m_bottomEffect);
       builder2.uniform("iRes") = SkV3{float(rc2.w), float(rc2.h), 0.0f};
-      builder2.uniform("iColor") = appColor_to_SkV4(m_color);
+      setShaderParams(builder2, false);
       p.setShader(builder2.makeShader());
 
       canvas->drawRect(SkRect::MakeXYWH(0, 0, rc2.w, rc2.h), p);
@@ -632,9 +631,7 @@ const char* ColorSelector::getAlphaBarShader()
 {
   return R"(
 uniform half3 iRes;
-uniform half4 iColor;
-uniform half4 iBg1;
-uniform half4 iBg2;
+uniform half4 iColor, iBg1, iBg2;
 
 half4 main(vec2 fragcoord) {
  vec2 d = (fragcoord.xy / iRes.xy);

src/app/ui/color_selector.h

@@ -71,7 +71,7 @@ namespace app {
     virtual const char* getMainAreaShader() { return nullptr; }
     virtual const char* getBottomBarShader() { return nullptr; }
 #if SK_ENABLE_SKSL
-    virtual void setShaderMainAreaParams(SkRuntimeShaderBuilder& builder) { }
+    virtual void setShaderParams(SkRuntimeShaderBuilder& builder, bool main) { }
 #endif
     virtual app::Color getMainAreaColor(const int u, const int umax,
                                         const int v, const int vmax) = 0;

src/app/ui/color_spectrum.cpp

@@ -35,52 +35,50 @@ ColorSpectrum::ColorSpectrum()
 {
 }
 
+#if SK_ENABLE_SKSL
+
 const char* ColorSpectrum::getMainAreaShader()
 {
-#if SK_ENABLE_SKSL
   if (m_mainShader.empty()) {
     m_mainShader += "uniform half3 iRes;"
-                    "uniform half4 iColor;";
-    m_mainShader += kRGB_to_HSL_sksl;
+                    "uniform half4 iHsl;";
     m_mainShader += kHSL_to_RGB_sksl;
     m_mainShader += R"(
 half4 main(vec2 fragcoord) {
  vec2 d = fragcoord.xy / iRes.xy;
  half hue = d.x;
- half sat = rgb_to_hsl(iColor.rgb).y;
+ half sat = iHsl.y;
  half lit = 1.0 - d.y;
  return hsl_to_rgb(half3(hue, sat, lit)).rgb1;
 }
 )";
   }
   return m_mainShader.c_str();
-#else
-  return nullptr;
-#endif
 }
 
 const char* ColorSpectrum::getBottomBarShader()
 {
-#if SK_ENABLE_SKSL
   if (m_bottomShader.empty()) {
     m_bottomShader += "uniform half3 iRes;"
-                      "uniform half4 iColor;";
-    m_bottomShader += kRGB_to_HSL_sksl;
+                      "uniform half4 iHsl;";
     m_bottomShader += kHSL_to_RGB_sksl;
     m_bottomShader += R"(
 half4 main(vec2 fragcoord) {
  half s = (fragcoord.x / iRes.x);
- half3 hsl = rgb_to_hsl(iColor.rgb);
- return hsl_to_rgb(half3(hsl.x, s, hsl.z)).rgb1;
+ return hsl_to_rgb(half3(iHsl.x, s, iHsl.z)).rgb1;
 }
 )";
   }
   return m_bottomShader.c_str();
-#else
-  return nullptr;
-#endif
 }
 
+void ColorSpectrum::setShaderParams(SkRuntimeShaderBuilder& builder, bool main)
+{
+  builder.uniform("iHsl") = appColorHsl_to_SkV4(m_color);
+}
+
+#endif // SK_ENABLE_SKSL
+
 app::Color ColorSpectrum::getMainAreaColor(const int u, const int umax,
                                            const int v, const int vmax)
 {

src/app/ui/color_spectrum.h

@@ -18,8 +18,11 @@ namespace app {
     ColorSpectrum();
 
   protected:
+#if SK_ENABLE_SKSL
     const char* getMainAreaShader() override;
     const char* getBottomBarShader() override;
+    void setShaderParams(SkRuntimeShaderBuilder& builder, bool main) override;
+#endif
     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;

src/app/ui/color_tint_shade_tone.cpp

@@ -28,18 +28,18 @@ ColorTintShadeTone::ColorTintShadeTone()
 {
 }
 
+#if SK_ENABLE_SKSL
+
 const char* ColorTintShadeTone::getMainAreaShader()
 {
-#if SK_ENABLE_SKSL
   if (m_mainShader.empty()) {
     m_mainShader += "uniform half3 iRes;"
-                    "uniform half4 iColor;";
-    m_mainShader += kRGB_to_HSV_sksl;
+                    "uniform half4 iHsv;";
     m_mainShader += kHSV_to_RGB_sksl;
     m_mainShader += R"(
 half4 main(vec2 fragcoord) {
  vec2 d = fragcoord.xy / iRes.xy;
- half hue = rgb_to_hsv(iColor.rgb).x;
+ half hue = iHsv.x;
  half sat = d.x;
  half val = 1.0 - d.y;
  return hsv_to_rgb(vec3(hue, sat, val)).rgb1;
@@ -47,35 +47,32 @@ half4 main(vec2 fragcoord) {
 )";
   }
   return m_mainShader.c_str();
-#else
-  return nullptr;
-#endif
 }
 
 const char* ColorTintShadeTone::getBottomBarShader()
 {
-#if SK_ENABLE_SKSL
   if (m_bottomShader.empty()) {
     m_bottomShader += "uniform half3 iRes;"
-                      "uniform half4 iColor;";
-    m_bottomShader += kRGB_to_HSV_sksl;
+                      "uniform half4 iHsv;";
     m_bottomShader += kHSV_to_RGB_sksl;
     // TODO should we display the hue bar with the current sat/value?
     m_bottomShader += R"(
 half4 main(vec2 fragcoord) {
  half h = (fragcoord.x / iRes.x);
- // half3 hsv = rgb_to_hsv(iColor.rgb);
- // return hsv_to_rgb(half3(h, hsv.y, hsv.z)).rgb1;
- return hsv_to_rgb(half3(h, 1.0, 1.0)).rgb1;
+ return hsv_to_rgb(half3(h, iHsv.y, iHsv.z)).rgb1;
 }
 )";
   }
   return m_bottomShader.c_str();
-#else
-  return nullptr;
-#endif
 }
 
+void ColorTintShadeTone::setShaderParams(SkRuntimeShaderBuilder& builder, bool main)
+{
+  builder.uniform("iHsv") = appColorHsv_to_SkV4(m_color);
+}
+
+#endif // SK_ENABLE_SKSL
+
 app::Color ColorTintShadeTone::getMainAreaColor(const int u, const int umax,
                                                 const int v, const int vmax)
 {
@@ -114,9 +111,10 @@ 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();
     gfx::Point pos(rc.x + int(rc.w * hue / 360.0),
                    rc.y + rc.h/2);
-    paintColorIndicator(g, pos, false);
+    paintColorIndicator(g, pos, val < 0.5);
   }
 }
 
@@ -153,11 +151,15 @@ void ColorTintShadeTone::onPaintSurfaceInBgThread(
 
   if (m_paintFlags & BottomBarFlag) {
     os::Paint paint;
+
+    const double sat = m_color.getHsvSaturation();
+    const double val = m_color.getHsvValue();
+
     for (int x=0; x<bottom.w && !stop; ++x) {
       paint.color(
         color_utils::color_for_ui(
           app::Color::fromHsv(
-            (360.0 * x / bottom.w), 1.0, 1.0)));
+            (360.0 * x / bottom.w), sat, val)));
 
       s->drawRect(gfx::Rect(bottom.x+x, bottom.y, 1, bottom.h), paint);
     }
@@ -175,6 +177,8 @@ int ColorTintShadeTone::onNeedsSurfaceRepaint(const app::Color& newColor)
   return
     // 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);
 }
 

src/app/ui/color_tint_shade_tone.h

@@ -19,8 +19,11 @@ namespace app {
     ColorTintShadeTone();
 
   protected:
+#if SK_ENABLE_SKSL
     const char* getMainAreaShader() override;
     const char* getBottomBarShader() override;
+    void setShaderParams(SkRuntimeShaderBuilder& builder, bool main) override;
+#endif
     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;

src/app/ui/color_wheel.cpp

@@ -66,13 +66,14 @@ ColorWheel::ColorWheel()
   initTheme();
 }
 
+#if SK_ENABLE_SKSL
+
 const char* ColorWheel::getMainAreaShader()
 {
-#if SK_ENABLE_SKSL
   // TODO create one shader for each wheel mode (RGB, RYB, normal)
   if (m_mainShader.empty()) {
     m_mainShader += "uniform half3 iRes;"
-                    "uniform half4 iColor;"
+                    "uniform half4 iHsv;"
                     "uniform half4 iBack;"
                     "uniform int iDiscrete;"
                     "uniform int iMode;";
@@ -128,8 +129,7 @@ half4 main(vec2 fragcoord) {
    sat /= 100.0;
    sat = clamp(sat, 0.0, 1.0);
   }
-  vec3 hsv = rgb_to_hsv(iColor.rgb);
-  return hsv_to_rgb(vec3(hue, sat, iColor.w > 0 ? hsv.z: 1.0)).rgb1;
+  return hsv_to_rgb(vec3(hue, sat, iHsv.w > 0 ? iHsv.z: 1.0)).rgb1;
  }
  else {
   if (iMode == 2) // Normal map mode
@@ -140,42 +140,36 @@ half4 main(vec2 fragcoord) {
 )";
   }
   return m_mainShader.c_str();
-#else
-  return nullptr;
-#endif
 }
 
 const char* ColorWheel::getBottomBarShader()
 {
-#if SK_ENABLE_SKSL
   if (m_bottomShader.empty()) {
     m_bottomShader += "uniform half3 iRes;"
-                      "uniform half4 iColor;";
-    m_bottomShader += kRGB_to_HSV_sksl;
+                      "uniform half4 iHsv;";
     m_bottomShader += kHSV_to_RGB_sksl;
     // TODO should we display the hue bar with the current sat/value?
     m_bottomShader += R"(
 half4 main(vec2 fragcoord) {
  half v = (fragcoord.x / iRes.x);
- half3 hsv = rgb_to_hsv(iColor.rgb);
- return hsv_to_rgb(half3(hsv.x, hsv.y, v)).rgb1;
+ return hsv_to_rgb(half3(iHsv.x, iHsv.y, v)).rgb1;
 }
 )";
   }
   return m_bottomShader.c_str();
-#else
-  return nullptr;
-#endif
 }
 
-#if SK_ENABLE_SKSL
-void ColorWheel::setShaderMainAreaParams(SkRuntimeShaderBuilder& builder)
+void ColorWheel::setShaderParams(SkRuntimeShaderBuilder& builder, bool main)
 {
-  builder.uniform("iBack") = gfxColor_to_SkV4(m_bgColor);
-  builder.uniform("iDiscrete") = (m_discrete ? 1: 0);
-  builder.uniform("iMode") = int(m_colorModel);
+  builder.uniform("iHsv") = appColorHsv_to_SkV4(m_color);
+  if (main) {
+    builder.uniform("iBack") = gfxColor_to_SkV4(m_bgColor);
+    builder.uniform("iDiscrete") = (m_discrete ? 1: 0);
+    builder.uniform("iMode") = int(m_colorModel);
+  }
 }
-#endif
+
+#endif // SK_ENABLE_SKSL
 
 app::Color ColorWheel::getMainAreaColor(const int _u, const int umax,
                                         const int _v, const int vmax)

src/app/ui/color_wheel.h

@@ -43,14 +43,14 @@ namespace app {
     void setHarmony(Harmony harmony);
 
   protected:
+#if SK_ENABLE_SKSL
     const char* getMainAreaShader() override;
     const char* getBottomBarShader() override;
+    void setShaderParams(SkRuntimeShaderBuilder& builder, bool main) override;
+#endif
     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;
-#if SK_ENABLE_SKSL
-    void setShaderMainAreaParams(SkRuntimeShaderBuilder& builder) override;
-#endif
     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,

src/app/util/shader_helpers.h

@@ -60,6 +60,20 @@ inline SkV4 appColor_to_SkV4(const app::Color& color) {
               float(color.getAlpha() / 255.0)};
 }
 
+inline SkV4 appColorHsv_to_SkV4(const app::Color& color) {
+  return SkV4{float(color.getHsvHue() / 360.0),
+              float(color.getHsvSaturation()),
+              float(color.getHsvValue()),
+              float(color.getAlpha() / 255.0)};
+}
+
+inline SkV4 appColorHsl_to_SkV4(const app::Color& color) {
+  return SkV4{float(color.getHslHue() / 360.0),
+              float(color.getHslSaturation()),
+              float(color.getHslLightness()),
+              float(color.getAlpha() / 255.0)};
+}
+
 } // namespace app
 
 #endif