Add the FX Group name to the end of Accessible sliders

so rather than getting "channel, mix, channel, mix" you get "channel - Left, Mix - Left, Channel - Right, Mix - Right" etc The implementation of this is kinda gross, but that's part of the sturcture of the fx. Along the way add a single function go to param index to group index. Closes surge-synthesizer#7238
baconpaul · Nov 4, 2023 · 654927b · 654927b
1 parent aa95ba3
commit 654927b
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 23 deletions.
diff --git a/src/common/dsp/Effect.h b/src/common/dsp/Effect.h
@@ -67,6 +67,20 @@ class alignas(16) Effect
     {
         return -1;
     } // number of blocks it takes for the effect to 'ring out'
+    int groupIndexForParamIndex(int paramIndex)
+    {
+        int fpos = fxdata->p[paramIndex].posy / 10 + fxdata->p[paramIndex].posy_offset;
+        int res = -1;
+        for (auto j = 0; j < n_fx_params && group_label(j); ++j)
+        {
+            if (group_label(j) && group_label_ypos(j) <= fpos // constants for SurgeGUIEditor. Sigh.
+            )
+            {
+                res = j;
+            }
+        }
+        return res;
+    }
 
     virtual void process(float *dataL, float *dataR) { return; }
     virtual void process_only_control()

diff --git a/src/surge-xt/gui/AccessibleHelpers.h b/src/surge-xt/gui/AccessibleHelpers.h
@@ -211,13 +211,13 @@ template <typename T> struct OverlayAsAccessibleButton : public juce::Component
     struct RBAH : public juce::AccessibilityHandler
     {
         explicit RBAH(OverlayAsAccessibleButton<T> *b, T *s)
-            : button(b), mswitch(s), juce::AccessibilityHandler(
-                                         *b, b->role,
-                                         juce::AccessibilityActions()
-                                             .addAction(juce::AccessibilityActionType::showMenu,
-                                                        [this]() { this->showMenu(); })
-                                             .addAction(juce::AccessibilityActionType::press,
-                                                        [this]() { this->press(); }))
+            : button(b), mswitch(s),
+              juce::AccessibilityHandler(
+                  *b, b->role,
+                  juce::AccessibilityActions()
+                      .addAction(juce::AccessibilityActionType::showMenu,
+                                 [this]() { this->showMenu(); })
+                      .addAction(juce::AccessibilityActionType::press, [this]() { this->press(); }))
         {
         }
         void press() { button->onPress(mswitch); }
@@ -288,15 +288,14 @@ struct OverlayAsAccessibleButtonWithValue : public OverlayAsAccessibleButton<T>
     struct RBAHV : public juce::AccessibilityHandler
     {
         explicit RBAHV(OverlayAsAccessibleButtonWithValue<T> *b, T *s)
-            : button(b),
-              mswitch(s), juce::AccessibilityHandler(
-                              *b, b->role,
-                              juce::AccessibilityActions()
-                                  .addAction(juce::AccessibilityActionType::showMenu,
-                                             [this]() { this->showMenu(); })
-                                  .addAction(juce::AccessibilityActionType::press,
-                                             [this]() { this->press(); }),
-                              AccessibilityHandler::Interfaces{std::make_unique<BValue>(b)})
+            : button(b), mswitch(s),
+              juce::AccessibilityHandler(
+                  *b, b->role,
+                  juce::AccessibilityActions()
+                      .addAction(juce::AccessibilityActionType::showMenu,
+                                 [this]() { this->showMenu(); })
+                      .addAction(juce::AccessibilityActionType::press, [this]() { this->press(); }),
+                  AccessibilityHandler::Interfaces{std::make_unique<BValue>(b)})
         {
         }
         void press() { button->onPress(mswitch); }
@@ -364,10 +363,10 @@ template <typename T> struct OverlayAsAccessibleSlider : public juce::Component
     struct RBAH : public juce::AccessibilityHandler
     {
         explicit RBAH(OverlayAsAccessibleSlider<T> *s, T *u)
-            : slider(s),
-              under(u), juce::AccessibilityHandler(
-                            *s, s->role, juce::AccessibilityActions(),
-                            AccessibilityHandler::Interfaces{std::make_unique<SValue>(s)})
+            : slider(s), under(u),
+              juce::AccessibilityHandler(
+                  *s, s->role, juce::AccessibilityActions(),
+                  AccessibilityHandler::Interfaces{std::make_unique<SValue>(s)})
         {
         }
 
@@ -418,6 +417,17 @@ struct OverlayAsAccessibleContainer : public juce::Component
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(OverlayAsAccessibleContainer);
 };
 
+struct HasExtendedAccessibleGroupName
+{
+    virtual ~HasExtendedAccessibleGroupName() = default;
+    std::string extendedAccessibleGroupName{};
+    virtual void setExtendedAccessibleGroupName(const std::string &s)
+    {
+        extendedAccessibleGroupName = s;
+        // We should maybe fire an ally event here but this only happens at construction time
+        // so skip it for now
+    }
+};
 enum AccessibleKeyEditAction
 {
     None,

diff --git a/src/surge-xt/gui/SurgeGUIEditor.cpp b/src/surge-xt/gui/SurgeGUIEditor.cpp
@@ -5978,6 +5978,32 @@ SurgeGUIEditor::layoutComponentForSkin(std::shared_ptr<Surge::GUI::Skin::Control
 
         auto loc = juce::Point<int>(skinCtrl->x, skinCtrl->y + p->posy_offset * yofs);
 
+        // See the discussion in #7238 - for the FX this allows us to brin ghte group name
+        // into the FX, albeit in a somewhat hacky/clumsy way because of the FX group structure
+        // and general memory model errors in SGE
+        std::string extendedAccessibleGroupName{};
+        if (p->ctrlgroup == cg_FX)
+        {
+            auto p0 = &synth->storage.getPatch().fx[p->ctrlgroup_entry].p[0];
+            auto df = p - p0;
+            if (df >= 0 && df < n_fx_params)
+            {
+                auto &fxi = synth->fx[p->ctrlgroup_entry];
+                if (fxi)
+                {
+                    auto gi = fxi->groupIndexForParamIndex(df);
+                    if (gi >= 0)
+                    {
+                        auto gn = fxi->group_label(gi);
+                        if (gn)
+                        {
+                            extendedAccessibleGroupName = gn;
+                        }
+                    }
+                }
+            }
+        }
+
         if (p->is_discrete_selection())
         {
             loc = loc.translated(2, 4);
@@ -6006,6 +6032,7 @@ SurgeGUIEditor::layoutComponentForSkin(std::shared_ptr<Surge::GUI::Skin::Control
             hs->setBackgroundDrawable(dbls[0]);
             hs->setHoverBackgroundDrawable(dbls[1]);
             hs->setDeactivatedFn([p]() { return p->appears_deactivated(); });
+            hs->setExtendedAccessibleGroupName(extendedAccessibleGroupName);
 
             setAccessibilityInformationByParameter(hs.get(), p, "Adjust");
             param[p->id] = hs.get();
@@ -6058,6 +6085,7 @@ SurgeGUIEditor::layoutComponentForSkin(std::shared_ptr<Surge::GUI::Skin::Control
         {
             hs->setLabel(p->get_name());
         }
+        hs->setExtendedAccessibleGroupName(extendedAccessibleGroupName);
 
         hs->setBipolarFn([p]() { return p->is_bipolar(); });
         hs->setFontStyle(Surge::GUI::Skin::setFontStyleProperty(
@@ -7192,10 +7220,19 @@ void SurgeGUIEditor::setAccessibilityInformationByParameter(juce::Component *c,
 }
 
 void SurgeGUIEditor::setAccessibilityInformationByTitleAndAction(juce::Component *c,
-                                                                 const std::string &title,
+                                                                 const std::string &titleIn,
                                                                  const std::string &action)
 {
     auto currT = c->getTitle().toStdString();
+    std::string title = titleIn;
+    if (auto heg = dynamic_cast<Surge::Widgets::HasExtendedAccessibleGroupName *>(c))
+    {
+        if (!heg->extendedAccessibleGroupName.empty())
+        {
+            title = title + " - " + heg->extendedAccessibleGroupName;
+        }
+    }
+
 #if MAC
     c->setDescription(title);
     c->setTitle(title);

diff --git a/src/surge-xt/gui/widgets/MenuForDiscreteParams.h b/src/surge-xt/gui/widgets/MenuForDiscreteParams.h
@@ -27,6 +27,7 @@
 #include "WidgetBaseMixin.h"
 #include "ModulatableControlInterface.h"
 #include "SurgeJUCEHelpers.h"
+#include "AccessibleHelpers.h"
 
 #include "juce_gui_basics/juce_gui_basics.h"
 
@@ -47,7 +48,8 @@ namespace Widgets
 struct MenuForDiscreteParams : public juce::Component,
                                public WidgetBaseMixin<MenuForDiscreteParams>,
                                public LongHoldMixin<MenuForDiscreteParams>,
-                               public ModulatableControlInterface
+                               public ModulatableControlInterface,
+                               public HasExtendedAccessibleGroupName
 
 {
     MenuForDiscreteParams();

diff --git a/src/surge-xt/gui/widgets/ModulatableSlider.h b/src/surge-xt/gui/widgets/ModulatableSlider.h
@@ -28,6 +28,7 @@
 #include "WidgetBaseMixin.h"
 #include "ModulatableControlInterface.h"
 #include "SurgeJUCEHelpers.h"
+#include "AccessibleHelpers.h"
 
 #include "juce_gui_basics/juce_gui_basics.h"
 
@@ -40,7 +41,8 @@ namespace Widgets
 struct ModulatableSlider : public juce::Component,
                            public WidgetBaseMixin<ModulatableSlider>,
                            public LongHoldMixin<ModulatableSlider>,
-                           public ModulatableControlInterface
+                           public ModulatableControlInterface,
+                           public HasExtendedAccessibleGroupName
 {
     ModulatableSlider();
     ~ModulatableSlider();