Fix VolumeKnob and tooltip bugs#8359
Conversation
LOMM, Compressor, FreeBoy and other plugins have Knobs with both normal static tooltips (set with setToolTip()) which provide a description of the Knob's function as well as dynamic floating text showing the model's current value. There was a regression causing both of these types of tooltips to be displayed at the same time. This commit fixes it.
bratpeki
left a comment
bratpeki
left a comment
There was a problem hiding this comment.
Approved for code. Still needs testing.
|
I tested this PR and it fixes the 3 regressions listed above, but it introduces a new bug in volume knobs where the context menu shows the value as a percentage but with the dbFS unit: For comparison, this is what it looks like on master: |
|
@extern-stat In trying to fix that issue, I realized that parameter value-to-text conversions are reimplemented in multiple places and in inconsistent ways, so a proper fix for that issue (and others) will require both an extensive refactoring and improvements to our VST implementation. I wrote about it here: #8379. For this PR, I'll try to fix the issue you spotted in a simple yet unsatisfactory way which will get us back to the master branch status quo where volume knobs display their value in dBFS for the tooltip and in a percentage for the context menu. What we really want is for it to display the value in dBFS everywhere, but that's what will require a refactor, so it's best to leave it for a future PR. |
- Fix context menu unit regression - Fix dBFS value displayed by volume knobs in Delay and Flanger plugins - Fix incorrect dBFS values in the "Set value" dialog box for volume knobs in the Delay, Dynamics Processor, Flanger, and Wave Shaper plugins - Fix units in Vibed's volume knob context menus - Support negative amplitude volume knobs like the one in the Flanger plugin - Add sanity checks for issues with VolumeKnob's model - Add
|
@extern-stat The regression you found has been fixed in d4f12fd. While fixing it, I also discovered and fixed several other issues with volume knobs - see the commit message for more information. |
|
@messmerd I tested the changes in your latest commit and they work well; the only issue I found was that Vibed's volume knob doesn't show a unit in its context menu: |
Thanks for testing! You're right, that was an oversight on my part. It should be fixed now. |
3 participants
Bugs fixed by this PR:
Fader(regression from #8253)For (1), I reworked how static tooltips work in
FloatModelEditorBase. Rather than useQWidget's tooltips, it shadows the tooltip methods fromQWidgetand redirects them to the existingSimpleTextFloat-based system.Supporting both "static" and "dynamic" tooltips required some improvements to keep better track of user interactions with the Knobs. There was an existing
m_buttonPressedboolean, but this was insufficient, so I converted it into a newInteractionTypeenum for keeping track of the user interaction state.See the "Expected Behaviour" section of the bug report for an explanation of how it works now from the user's perspective.
Fixes #8358