-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Faders with a dB scale (instead of linear/percentage) #7636
base: master
Are you sure you want to change the base?
Conversation
Make the faders use a linear dbFS scale. They now also change position on first click and can then be dragged. The `Fader` class now has a new property called `m_faderMinDb`. It's the minimum value before the amplification drops down fully to 0. It is needed because we cannot represent a scale from -inf to maxDB. Rename `knobPosY` to `calculateKnobPosYFromModel` and move the implementation into the cpp file. The method now first converts the model's current amplification value to dbFS and then computes a ratio based on that values and the minimum and maximum dbFS values. Add the method `setVolumeByLocalPixelValue` which takes a local y coordinate of the widget and sets the amplification of the model based on a dbFS scale. Adjust `mousePressEvent` and `mouseMoveEvent` so that they mostly take the local y coordinate of the event and use that to adjust the model via `setVolumeByLocalPixelValue`. Remove `m_moveStartPoint` and `m_startValue` and they are not needed anymore.
Apply a curve, i.e. the cube function and its inverse, to the fader positions to that we have more space to work with in the "interesting" areas around 0 dB and less space in the area where we tend to "-inf dB". Set the minimum dB value of the fader to -120 dB to increase the potential headroom.
Add support for models that are in dB. There's a new member `m_modelIsLinear` which can be set via the constructor. The default value in the constructor is `true`. Add the method `modelIsLinear` which can be used to query the parameter. It is used in `calculateKnobPosYFromModel` and `setVolumeByLocalPixelValue`. Both methods got extended to deal with models in dB. They were also refactored to extract code that is common for both cases. Ensure that the constructor of `Fader` is called with `false` for `CrossoverEQControlDialog` and `EqFader` because these use models that are in dB.
Show the current value of the fader in its tool tip. Please note that this value is always shown in dB because the goal should be to get rid of the linear values for the faders. Some of the code is extracted into the new method `Fader::getModelValueAsDbString` which is shared by `Fader::modelValueChanged` (also new) and `Fader::updateTextFloat`. Please note that `getModelValueAsDbString` will use "dB" instead of "dBFS" as the unit and that it will format "-inf dB" instead of "-∞ dB". These changes will also be visible in the text float that is used to show the new values as the fader is being dragged.
Let users enter values in dB in the dialog that opens with a double click. The minimum value that can be entered is the minimum value that the fader allows to set, i.e. -120 dB. The maximum value is the maximum value of the model converted to dB. As of now this is ~6 dB. The current value is converted to dB. If it corresponds to "-inf dB", i.e. if the amplification is 0 then the minimum value is used.
Remove the option "Display volume as dBFS" from the settings dialog and the check box option "Volume as dBFS" from the "View" menu. Volumes are now always treated as dB, i.e. all evaluations of the property "displaydbfs" have been removed which results in assuming that this option is always true. The upgrade code in `ConfigManager::upgrade_1_1_91` was left as is. However, a note was added which informs the reader that the value of "displaydbfs" is not evaluated anymore.
Extend `Fader::wheelEvent` for the case where the model is not a dB model (`modelIsLinear() == true`), e.g. for the mixer faders. In that case it works as follows. Each step increments or decrements by 1 dB if no modifier is pressed. With "Shift" pressed the increment value is 3 dB. With "STRG" ("CTRL") pressed the increment value is reduced to 0.1 dB. If the value goes below the minimum positive dB value that is allowed by the fader then the fader is set to "-inf dB", i.e. zero amplification. If the fader is set to "-inf dB" and the users want to increase the value then it is first set to the minimum positive value that is allowed by the fader. If the model is a dB model then the same behavior as before is used. Although it should be considered to adjust this case as well. These models are used by the faders of the Crossover Equalizer, Equalizer, Compressor and Delay and they are not very usable with the mouse wheel.
Make the faders of the Crossover Equalizer, Equalizer, Compressor and Delay behave like the mixer faders, i.e. step in sizes of 3 dB, 1dB and 0.1 dB depending on whether a modifier key is pressed or not. Extract some common code to do so and add some `const` keywords.
Implement a more stable knob behavior. Remove the jumping behavior if the users directly click on a volume knob. By storing the offset from the knob center and taking it into account during the changes it now also feels like the users are dragging the knob. Changes to the amplification are now only applied when the mouse is moved. This makes the double click behavior much more stable, i.e. if users click on the knob when it is at 0 dB the dialog will also show 0 dB and not something like 0.3 dB because the first click is already registered as a change of volume. If the users click next to the knob the amplification will still be changed immediately to that value. ## Technical details To make the knobs more stable a variable called `m_knobCenterOffset` was introduced. It stores the offset of the click from the knob center so that this value can be taken into account for in the method `setVolumeByLocalPixelValue`.
Add an indication that a float value is assigned to a float variable.
Here's a screenshot which demonstrates/shows the cubic scaling of the dB scale by rendering some fader tick marks. The topmost mark represents 6 dB and then each tick decreases by 6 dB until -120 dB are reached. For example the master fader is at 0 dB in the screenshot while the kick fader is close to -6 dB. Some other faders even slightly amplify the signal: Please note that due to the implementation the 0 dB fader tick and the 0 dB mark of the level meters are independent of each other and therefore not aligned. If wanted I can commit these changes to this PR. |
Introduce the `constexpr c_dBScalingExponent` which describes the scaling exponent that's used to scale the dB scale in both directions. This will simplify potential adjustments by keeping the values consistent everywhere.
Draw fader ticks in case the model is a linear one. This means that for now they should only be painted for the mixer faders but not for the faders of the Compressor, Delay, etc. Extract the computation of the scaled ratio between the maximum model dB value and the minimum supported fader dB value into the new private method `computeScaledRatio`. This is necessary because it is needed to paint the fader knob at the correct position (using the knob bottom as the reference) and to paint the fader ticks at the correct position (using the knob center). Introduce the private method `paintFaderTicks` which paints the fader ticks. Note: to paint some non-evenly spaced fader ticks replace the `for` expression in `paintFaderTicks` with something like the following: ``` for (auto & i : {6.f, 0.f, -6.f, -12.f, -24.f, -36.f, -48.f, -60.f, -72.f, -84.f, -96.f, -108.f, -120.f}) ```
Allow the adjustment of the faders via the keyboard. Using the up or plus key will increment the fader value whereas the down or minus key will decrement it. The same key modifiers as for the wheel event apply: * No modifier: adjust by 1 dB * Shift: adjust by 3 dB * Control: adjust by 0.1 dB Due to the very similar behavior of the mouse wheel and key press handling some common functionality was factored out: * Determinination of the (absolute) adjustment delta value by insprecting the modifier keys of an event. Factored into `determineAdjustmentDelta`. * Adjustment of the model by a given dB delta value. Factored into `adjustModelByDBDelta`.
Fader ticks have been committed with commit 344fae2. Starting with commit aa74df2 it's now also possible to move the faders using the keyboard. The up or plus key will increment whereas the down or minus key will decrement. The same modifier keys as for the mouse wheel event apply. The scaling exponent was extracted into a |
@sakertooth, requesting a review from you because you also did some work on the mixer and are active here. If there are more suiting people to review this please feel free to add them. |
Move the fader of the selected channel instead of the fader that has focus when the up/plus or down/minus keys are pressed. Doing so also feels more natural because users can already change the selected channel via the left and right keys and now they can immediately adjust the volume of the currently selected channel while doing so. Key events are now handled in `MixerView::keyPressEvent` instead of `Fader::keyPressEvent` and the latter is removed. `MixerChannelView` now has a method called `fader` which provides the associated fader. This is needed so that the event handler of `MixerView` can act upon the currently selected fader. ## Changes in Fader The `Fader` class provides two new public methods. The `adjust` method takes the modifier key(s) and the adjustment direction and then decides internally how the modifier keys are mapped to increment values. This is done to keep the mapping between modifier keys and increment values consistent across different clients, e.g. the key event of the `MixerView` and the wheel event of the `Fader` itself. The direction is provided by the client because the means to determine the direction can differ between clients and cases, e.g. a wheel event determines the direction differently than a key event does. The method `adjustByDecibelDelta` simply adjusts the fader by the given delta amount. It currently is not really used in a public way but it still makes sense to provide this functionality in case a parent class or client wants to manipulate the faders by its very own logic. Because the `Fader` class does not react itself to key press events anymore the call to `setFocusPolicy` is removed again.
Let the users enter the fader value via dialog when the space key is pressed. Extract the dialog logic into the new method `adjustByDialog` and call it from `MixerView::keyPressEvent`. Make `Fader::mouseDoubleClickEvent` delegate to `adjustByDialog` and also fix the behavior by accepting the event.
Thanks for testing @regulus79! You are right, having a concept of focus and a selected channel made it feel disconnected. I have adjusted this with commit 4a720cb. Using the keys now adjusts the fader of the currently selected channel instead of one with focus. Commit d451531 enables users to directly enter a value via the dialog if the space key is pressed with a currently selected channel. This should give users much flexibility to adjust the mixer faders using only the keyboard:
|
Mouse wheel and the "Alt" key
I cannot reproduce the problem and it might be something related to the system. If I remember correctly Qt has some strange behavior with regards to the mouse wheel and the Alt key. However, the Alt key is not really evaluated anyway when handling the fader changes. It only supports Shift, Control and no modifier keys. So the fix here is: "Don't press the Alt key because it seems to invoke strange Qt behavior on some systems." 😉 Percentages in the model
That's because the underlying model still uses the linear scale, i.e. it is not a model of decibel values. Introducing such a model would be quite some work because you'd somehow have to support the special case of -inf dB. This is why it is outside of the scope of this PR. The goal of this PR is to introduce lots of improvements while going into the right direction. It's something that can be tackled once this PR is merged. However, we should not hold this PR until it is perfect and every "little" thing is implemented because "perfect is the enemy of the good". Keyboard layout issues
I guess this is a problem with the keyboard layout. Sometimes I also have this problem with my German keyboard layout for which I have to press modifier keys for keys that are accessible without any modifier keys on a US layout. So the solution here is to advise users to use the "free-standing" keys like "+" and "-" on the numpad/keypad and the up and down arrows. Fader headroom
Do you mean that the fader should let users amplify the channel by more than 6 dB or do you mean that the graphical ratios between 6 dB, 0 dB and -inf db should be distributed differently? In case you mean the latter: There is already quite some mapping going on and therefore I'd prefer not to add some additional mapping so that the positive dB scale gets some additional space. IMO the new implementation of this PR is already quite some improvement because the old implementation gave you half of the space to make adjustments between the finite space of 0 dB to 6 dB and the other half to make an adjustment in the infinite space of 0 dB and -inf dB. Put differently: it was much too detailed in the space between "change nothing" and "double the volume". Please also note that it is rather "normal" to pull most faders below unity anyway so that you can mix with some headroom. If you need to make large adjustments you can always insert a plugin which is called "Utility" or "Tool" in other DAWs which lets you adjust the volume with a gain larger than 6 dB. I guess in LMMS this would be the "Amplifier" plugin although even that plugin only allows to add measly 6 dB. Using a "Utility"/"Tool" plugin is also recommended if you want to automate volume levels because if you directly automate the fader itself you might for example get problems if you adjust other volumes. Last but not least users can use the modifier keys for fine adjustment or just enter the positive values via dialog. |
Awesome! I'll be testing this when I get the current PR tests off my plate. |
Glad to be working on this! I get my builds off the LMMS site. I looked at everything Regulus mentioned.
Can confirm! Happens on the Compressor, Equalizer, Delay and Compression EQ as well. I'm guessing this is implemented directly on the "Fader" class, or however it's implemented in the code! No particular idea how this should be fixed, maybe just make alt-scrolling behave like no modifier was applied? Maybe 0.01dB for some hyper-precision? If 0.01 is a genuine step applicable to the LMMS fader, that might be best, since it's another choice for the user :)
Seems to be fine now. Checked with the Delay plugin.
The +/- keystrokes needed to change the volume are keyboard layout dependent, it seems. I tried it with two layouts, it uses the appropriate layout for both. So it seems fine to me, especially with how it's localized to the user, since this would exclusively be for users who don't have either a mouse or a numpad. However, if we can get the "keypress ID", we could make it universal! |
Seems to have been patched (commit 4a720cb).
Given how the mixer is now resizable, I wouldn't think of this as a big issue. Maybe if users complain, but since you can fullscreen your mixer, it might be enough room. @michaelgregorius FWICT, the distance between each of these is half of the previous distance, so 6dB-ish? Or is it exactly 6dB? If it's something specific like that, we would need a different method of scaling. But a different method of scaling might be unconventiontional. IDK, there is discussion to be had there. |
Also, @michaelgregorius, would it be possible to add a bolder indicator of 0dB? Currently, it's in the middle of the fader, so it's easy to "gravitate" to it when mixing. Logarithmic placement moves it around, making it more difficult to find. What do you think about that, @regulus79? |
This pull request changes the faders to use a dB scale. They have the following behavior:
The faders can be adjusted between -120 dB and +6 dB. Pulling a fader down further sets it to -inf dB, i.e. to zero amplification.
The faders scale is scaled by the cube function so that users have more space to work in the "interesting" area around 0 dB and less space in the area where we tend to -inf dB anyway. Fader ticks are painted as of commit 344fae2.
Below you can find a demo video of some of the features that will be described in the following.
Adjustment via mouse wheel or keyboard
The faders can be adjusted via scrolling of the mouse wheel or by pressing the up/plus or down/minus keys. Thebehavior with regards to modifier keys and adjustments is as follows:
If the value goes below the minimum positive dB value that is allowed by the fader then the fader is set to "-inf dB", i.e. zero amplification. If the fader is set to "-inf dB" and the users want to increase the value then it is first set to the minimum positive value that is allowed by the fader.
Tool tips
A fader now show its current value as its tool tip. Please note that this value is always shown in dB because the goal is to get rid of the linear values for the faders (see below). Values are now shown with the unit "dB" instead of "dBFS" and as "-inf dB" instead of "-∞ dB". These changes will also be visible in the text float that is used to show the new values as the fader is being dragged.
Direct entry via dialog
The dialog that opens when the fader is double clicked now let users enter values in dB instead of a percentage value between 0 and 200.
The minimum value that can be entered is the minimum value that the fader allows to set, i.e. -120 dB. The maximum value is the maximum value of the model converted to dB. As of now this is ~6 dB. The current value is converted to dB. If it corresponds to "-inf dB", i.e. if the amplification is 0 then the minimum value is shown as the initial value. Although it might be considered to change this to 0 dB to be able to quickly activate a channel using the dialog.
Remove "Display volume as dBFS" and "Volume as dBFS"
Remove the option "Display volume as dBFS" from the settings dialog and the check box option "Volume as dBFS" from the "View" menu. Volumes are now always treated as dB.
Models that are already in dB
Support for models that are already in dB is preserved. This includes the following effects: Crossover Equalizer, Equalizer, Compressor and Delay. They also support the mouse wheel behavior as described above now.
Demo video
The following video demonstrates some of the aspects that have been described above:
Demo.-.Faders.with.dB.scale.webm
Proposal for integration into 1.3
I propose to integrate the changes into 1.3 as it will make LMMS more similar to other DAWs with regards to that rather fundamental feature.
Edit: adjustments with regards to the new commits