ComboBox widgets menus stop responding

[elanhickler]

After using the combo box for a while it will randomly stop responding to clicks. It is very random and I can find no pattern. Clicking furiously and fast will get it to respond again.

[elanhickler]

please debug this as soon as possible. I'd like to release a bug free version of Spiral Generator in a few days.

[elanhickler]

jura_RTreeView.cpp

void RTreeView::mouseDown(const MouseEvent &e)
{
  if( !isPointClickable(e.x, e.y) )
    return;

  RTreeViewNode *nodeUnderMouse = getNodeAtY(e.y);

  if( nodeUnderMouse != NULL )
    nodeClicked(nodeUnderMouse, e, getNodeClickPosition(nodeUnderMouse, e.x));

  dismissIfModal(); // move into ModalComponent
}

menu fails to work because of the statement nodeUnderMouse != NULL nodeUnderMouse is null.

[RobinSchmidt]

i think, this should be fixed now

[elanhickler]

you committed a fix? ill have to copy the fix over to my custom rs-met libs.

I guess soon I should try to take all your commits, or maybe post the key changes that are needed for mac/PrettyScope so you can make those changes.

[RobinSchmidt]

yes, it's a simple one line change in the constructor of : RPopUpMenu:

RPopUpMenu::RPopUpMenu(Component *componentToAttachTo) : ROwnedPopUpComponent(componentToAttachTo)
{
  // ...stuff...
  treeView->setOpenOrCloseNodesOnClick(false); // otherwise, menus with linear list of options stop
                                               // working when clicking above 1st option
  // ...stuff...
}

[RobinSchmidt]

just this call needs to be added

[RobinSchmidt]

as said in the mail - you can try before/after. the menu stopped working when you try to click on the small horizontal strip above the 1st option. btw. if you click there a 2nd time, it works again. so...this is how you can check that the fix works. ...then you can close the issue

[RobinSchmidt]

btw - thanks for tracking it down to the nodeUnderMouse == null. that really helped me finding it

[elanhickler]

major bug exists with combo box, you set it and it doesn't go to the correct value. You have to set it twice to the value you want for certain items on the list. The first few items work every time. The later items in the list do not. What........

[RobinSchmidt]

whoa, fuck! is that new since the fix? can you give me some detailed instructions to reproduce it?

[elanhickler]

I went back a few changes, didn't help. I couldn't even get the old behavior where it worked fine but stopped working randomly.

Here's a video: https://puu.sh/w8jal/e4afb62ea7.mp4

save link/file to your desktop instead of clicking the link if video doesn't play.

[elanhickler]

hint: when I run the debugger and look at the values being sent, I get strange numbers: Let's say the value should be 10, i get two values, the function gets called TWICE. Once I get 10, the second time around I get 9.99999999 which is 9 when cast to an int. I tried rounding this number, but it doesn't help, the value is actually 10 and 9 deeper in the code.

[RobinSchmidt]

damn! ok. i'll look into this tomorrow. it happens reliably (not randomly), right? so i should be able to reproduce it when i grab the latest version of your codebase and build spiralgenerator?

[RobinSchmidt]

and this bug is new right? you just added the line i gave above to the constructor of RPopUpMenu in your fork and since then this happens?

[elanhickler]

I couldn't tell you, I've tried several ways to roll back changes. I don't know the code well enough to know what's going on. I can't get the old behavior back. I coped rtreeview files form an old rs-met, that didn't work. I tried rolling back changes on the git, didn't work.

[elanhickler]

I definitely changed the code back to some older code. Still not able to get the old behavior. This new behavior definitely didn't exist till now. PrettyScope doesn't do this.

[RobinSchmidt]

but what was your change? did you just add the change i posted above (treeView->setOpenOrCloseNodesOnClick(false);) to your fork,? i guess, i need your current fork of my library together with your spiralgenerator. ...maybe i could try to reproduce in my own chainer - but i think, i have currently no menu with as much options, so i don't see it in my products. does it happen only for numbers >= 10?

or wait: straightliner (inside chainer) has 16 options for filter types and i just tried it there and there is works

[elanhickler]

I cherry picked your changes, my changes are exactly your changes.

[elanhickler]

I think we need to sync our libraries.

[RobinSchmidt]

indeed. so, you say, you, together with lorcan, have applied some changes to your fork to make it work on mac?

[elanhickler]

could you do this for now, add this to rosic_ExponentialSmoother.h

 INLINE double getCurrentValue()
 {
   return current;
 }

Edit: I'm about to sync to your RS-MET for Spiral Generator, (saving my old copy just incase of catastrophic failure)

[RobinSchmidt]

done.

[elanhickler]

i get 282 errors when using your new library. Is there a module that needs to be added?

Edit: All the errors are form my own files.

[RobinSchmidt]

what do the errors say? maybe you need to add the rapt module? but i actually don't think so. rosic does not yet depend on rapt. maybe it will in the future, but at the moment the rosic module has no dependencies whatsoever (not even on basic juce modules). ...or: you are using jura_processors, too - that now depends also on the "romos" module (that's a (skeleton of a) reaktor-like modular synth - it's availble in chainer as "Liberty", if you are curious - but there's not really much to see yet)

[elanhickler]

the errors are nonsensical. oh maybe because i don't have std:: on things

[RobinSchmidt]

i removed a lot of those "using namespace" things in my header files (as suggested by lorcan...or was it teemu?), so yeah - that may be the case.

[RobinSchmidt]

formerly, i had using namespace std somewhere deep down in the rosic headers. that's gone now

[elanhickler]

we are synced, got rid of the errors, had to get the namespace on things. and remake the project with the jucer.

Dropdown bug still exists... wow, after all that.

[RobinSchmidt]

but does it occur only in your products? can you try in straightliner? because i tried there and the menu works

[RobinSchmidt]

also, chainer itself has now 15 options - seems to work there too

[elanhickler]

how do I try in straightliner? i have to build it? really spending way too much time on this. I have to get to my normal sampling job. How could I have done anything to cause a GUI bug when I have not touched the GUI code? Maybe my call to resize is an issue.

[elanhickler]

removing the timer class didn't change anything.

[RobinSchmidt]

in my library, build the TestPluginJURA project. that builds the "Chainer" which now includes all my old plugins

[elanhickler]

ok straightliner menu works... now what.

[RobinSchmidt]

really weird. so you are now in sync with my version of my library - so if i now grab your latest version of your codebase and build spiralgenerator, i should be able to reproduce it? i will try that tomorrow.

[elanhickler]

you will need to use this code for rosic_EnvelopeGenerator.h

#ifndef rosic_EnvelopeGenerator_h
#define rosic_EnvelopeGenerator_h

// rosic-indcludes:
#include "rosic_BreakpointModulator.h"

#include <algorithm>

using std::min;
using std::max;

namespace rosic
{

/**

This class specializes the very general BreakpointModulator into a kind of ADSR envelope.

*/

class EnvelopeGenerator : public BreakpointModulator
{

public:

  //---------------------------------------------------------------------------------------------
  // construction/destruction:

  /** Constructor. */
  EnvelopeGenerator();

  //---------------------------------------------------------------------------------------------
  // parameter settings:

  /** Sets the point where the envelope starts (as raw value). */
  void setStartLevel(double newStart) { setBreakpointLevel(0, newStart); }

  /** Sets the point where the envelope starts (in dB). */
  void setStartInDecibels(double newStart) { setStartLevel(dB2amp(newStart)); }

  /** Sets the point where the envelope starts (in semitones). */
  void setStartInSemitones(double newStart) { setStartLevel(pitchOffsetToFreqFactor(newStart)); }

  /** Sets the highest point of the envelope (as raw value). */
  void setPeakLevel(double newPeak) { setBreakpointLevel(1, newPeak); }

  /** Sets the highest point of the envelope (in dB). */
  void setPeakInDecibels(double newPeak) { setPeakLevel(dB2amp(newPeak)); }

  /** Sets the highest point of the envelope (in semitones). */
  void setPeakInSemitones(double newPeak) { setPeakLevel(pitchOffsetToFreqFactor(newPeak)); }

  double getSustainLevel() { return getBreakpointLevel(2); }

  /** Sets the sustain level (as raw value). */
  void setSustainLevel(double newSustain)
  {
    sustain_level = newSustain;
    setBreakpointLevel(2, sustain_level*amp_scale);
    setBreakpointLevel(3, getBreakpointLevel(2));
  }

  /** Sets the sustain level (in dB). */
  void setSustainInDecibels(double newSustain) { setSustainLevel(dB2amp(newSustain)); }

  /** Sets the sustain level (in semitones). */
  void setSustainInSemitones(double newSustain)
  {
    setSustainLevel(pitchOffsetToFreqFactor(newSustain));
  }

  /** Sets the end point of the envelope (as raw value). */
  void setEndLevel(double newEnd) { setBreakpointLevel(4, newEnd); }

  /** Sets the end point of the envelope (in dB). */
  void setEndInDecibels(double newEnd) { setEndLevel(dB2amp(newEnd)); }

  /** Sets the end point of the envelope (in semitones). */
  void setEndInSemitones(double newEnd) { setEndLevel(pitchOffsetToFreqFactor(newEnd)); }

  /** Sets the length of attack phase (in seconds). */
  void setAttack(double x)
  {
    attack = x;
    setBreakpointTime(1, attack);
    setBreakpointLevel(1, amp_scale);
  }

  /** Sets the hold time (in seconds). */
  void setHold(double newHoldTime);

  /** Sets the length of decay phase (in seconds). */
  void setDecay(double x)
  {
    decay = x;
    setBreakpointTime(2, attack + decay);
    setBreakpointTime(3, attack + decay + decay2);
    setBreakpointLevel(2, sustain_level*amp_scale);
    setBreakpointLevel(3, sustain_level*amp_scale);
  }

  /** Sets the length of release phase (in seconds). */
  void setRelease(double x)
  {
    release = x;
    setBreakpointTime(4, attack + decay + decay2 + release);
    setBreakpointLevel(4, 0.0);
  }

  double attack = 0.1;
  double decay = 0.5;
  double decay2 = 1.0;
  double release = 1.0;
  double amp_scale = 1.0;
  double sustain_level = 1.0;

  void setAmplitudeScale(double x)
  {
    amp_scale = x;
    setBreakpointLevel(1, amp_scale);
    setBreakpointLevel(2, sustain_level*amp_scale);
    setBreakpointLevel(3, sustain_level*amp_scale);
  }

  void setShape(double x)
  {
    if (x > 0)
    {
      setBreakpointShape(1, ModBreakpoint::ANALOG);
      setBreakpointShape(2, ModBreakpoint::ANALOG);
      setBreakpointShape(4, ModBreakpoint::ANALOG);
      setBreakpointShapeAmount(1, min<double>(1.0, x));
      setBreakpointShapeAmount(2, x);
      //setBreakpointShapeAmount(4, min<int>(10, x));
      setBreakpointShapeAmount(4, x);
    }
    else
    {
      x=-x;
      setBreakpointShape(1, ModBreakpoint::ANALOG);
      setBreakpointShape(2, ModBreakpoint::GROWING);
      setBreakpointShape(4, ModBreakpoint::GROWING);
      setBreakpointShapeAmount(1, min<double>(1.0, x));
      setBreakpointShapeAmount(2, x);
      //setBreakpointShapeAmount(4, min<int>(10, x));
      setBreakpointShapeAmount(4, x);
    }
  }

  int id = -1;
  int mod_idx = -1;
  bool has_modulation_target = false;

  inline double getCurrentValue()
  {
    return previousOut;
  }

  enum class ResetMode { on_note, when_not_tied, when_silent } resetMode;
};

} // end namespace rosic

#endif // rosic_EnvelopeGenerator_h

[RobinSchmidt]

it has to do with the menu value being converted into the range 0..1 for automation, sent to the host (thereby converted from double to float) and then being sent back from the host to the plugin (or something like that). that's why we can't see it in straightliner (there are no meta-parameters attached)....ok...let's see how to fix this...

[RobinSchmidt]

by the way - if you need to use customized versions of my dsp classes, you could just add them to your library (ElanSynthLib), so we don't have deal with temporarily exchanging the files in my library.

oooh..i just see that you actually have modified files there - then why not use them from there?

[RobinSchmidt]

interesting: this occurs only with the LFO target menus, the Envelope target menus work just fine. could it be that when you create the Parameter objects, you pass "LINEAR" for the scaling instead of "STRING"?...i need to check that

[RobinSchmidt]

yep SpiralGeneratorPlugin.cpp line 103: should be: p = new Param(c->accessText, 0.0, spiralGen.ParamList.size(), 0.0, Parameter::STRING); you had: p = new Param(c->accessText, 0.0, spiralGen.ParamList.size(), 0.0); in which case the optional scaling parameter defaults to Parameter::LINEAR. with STRING, the interval is automatically set to 1.0 and there's an automatic quantization of the parameters to the given interval...and when this interval 1.0, the parameter is rounded to integers, (you could also have an interval of 0.01, say, in which case the parameter value would always be rounded to two-digits after the point)

[RobinSchmidt]

your bug :-P - but admittedly, hard to find. i checked in an update

[elanhickler]

it's not just my bug, it is my opinion that it is also a problem with your framework. Either require a scaling (do not have a default) or make it so that combo boxes automatically use parameter::String. Or produce a warning in the compiler. or a jassert, something.

also, why is it just the LFO that doesn't work? I have more than one combo box.

[RobinSchmidt]

hmm - i could make the scaling parameter non-optional - oh, but then i would also have to make min/max and default non-optional. i wanted to make the constructor call easier/shorter for more common cases but maybe that invites bugs. the combo box cannot determine the scaling of the underlying parameter. the parameter is the lower level object and the combobox just attaches itself to it and the parameter is in charge of defining these things (there can be parameters without widgets attached to them, after all)

[elanhickler]

why can't combo boxes default to string?

[elanhickler]

i didn't put Parameter::STRING on all my combo boxes yet they work???

[elanhickler]

why would combo boxes ever not be Parameter::STRING?

[RobinSchmidt]

they wouldn't, but the combobox does not get to decide what scaling the underlying parameter uses (the parameter object exists already and is set up when the box gets created - if that ever happens anyway, the user may not even open a gui). it would be rather strange, if i attach a combobox to a parameter and as side effect, that attachment changes the scaling of the parameter....BUT: i could perhaps jassert(parameter->scaling == STRING) whenever one tries to attach a combobox to a parameter. that would catch it

[RobinSchmidt]

hah! there already was such a check in place, but that checked only if the array of (string) options isn't empty and not the actual scaling setting. i refined that check. you should now hit a jassert whenever you attach a combobox to non-string parameters

[elanhickler]

thank you.