Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended PB range tweaks #5132

Closed
2 tasks
mkruselj opened this issue Sep 21, 2021 · 4 comments
Closed
2 tasks

Extended PB range tweaks #5132

mkruselj opened this issue Sep 21, 2021 · 4 comments
Labels
Feature Request New feature request UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc.
Milestone

Comments

@mkruselj
Copy link
Collaborator

  • It is nigh impossible to get any useful mouse behavior with PB range numfield set to extended mode. I suggest normal drag should still retain the normal non-extended behavior and jump in steps of 100 steps, and holding Shift gives us current cent precision
  • Typein should be in semicents (xx.yy) not cents (yyyy)
@mkruselj mkruselj added Feature Request New feature request UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc. labels Sep 21, 2021
@mkruselj mkruselj added this to the Surge XT 1.0 milestone Sep 21, 2021
@baconpaul
Copy link
Collaborator

OK so here's some notes

The basic problem is we still store an int just now it goes 0 -> 2400. So it's mostly formatting stuff.

On formatting and typein:
So in Parameter.cpp around line 3146 is where we convert it to cents or keys. In that if instead of %i cents, i do a %0.2f keys, i * 1.f / 100.
And in same file around line 3990 you can see I do an if(extend_range && contains /). You can see we set ni (which is the output) to the cents. So this i where we put the *100 to do the 1.23 -> 123.

if( extend_range )
{
   if (contains / )
      // current tuning code
   else
     ni = std::round(std::atof(s.c_str()) * 100)
}

As to the UI that's src/surge-xt/gui/widgets/NumberField. You can see there's a method called changeBy(int) and it is called by both mouseWheelMove and mouseDrag. There's also a member called isExtended and a controlMode. So the way I would do ti is

  1. Add a method int changeByMultiplier(const juce::MouseEvent &event)
  2. Implement it as something like
{
   if (controlType == PB_DEPTH && isExtended && ! event.mods.isShiftPressed() ) return 100;
   return 1;
}
  1. Go find the places that call changeBy(1) or changeBy(-1) and replace them with changeBy(changeByMultiplier(event)) or changeBy(-changeByMultilier(event))

@cannero
Copy link
Contributor

cannero commented Sep 29, 2021

OK so here's some notes

Thanks a lot, that makes the implementation a lot easier.

Just one thing, there is the DEBUG_WRITABLE_EXTEND_RANGE define to check the writes on the extend_range member in the Parameter class. In SurgeFXProcessor.h the member is set directly, not per set_extend_range() call.
Should I change that to set extended_range per function call or leave it like it is?

@mkruselj
Copy link
Collaborator Author

@cannero This FR pertains to Surge the synthesizer, not the separate Surge FX plugin, so you can disregard what is in SurgeFXProcessor IMO.

@baconpaul
Copy link
Collaborator

Although that would be a nice fix if you happen to get it - changing from the direct to the accessor method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature request UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc.
Projects
None yet
Development

No branches or pull requests

3 participants