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

Faders with a dB scale (instead of linear/percentage) #7636

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

michaelgregorius
Copy link
Contributor

@michaelgregorius michaelgregorius commented Dec 27, 2024

This pull request changes the faders to use a dB scale. They have the following behavior:

  • Clicking on the knob lets the users drag the knob to change the position. The position only changes on move.
  • Clicking on the fader next to the knob immediately moves the knob to that position.

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:

  • No modifier key: adjusts in increments of 1 dB
  • Shift key: adjusts in increments of 3 dB
  • Control key: adjusts in increments of 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.

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

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.
@michaelgregorius
Copy link
Contributor Author

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:

7636-FadersWithTickMarks

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`.
@michaelgregorius
Copy link
Contributor Author

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 constexpr in commit 795c681. This allows for consistent changes and experimenting. The following images show linear scaling, quadratic scaling and cubic scaling:
7636-FaderScale17636-FaderScale27636-FaderScale3

@michaelgregorius
Copy link
Contributor Author

@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.

@regulus79
Copy link
Contributor

I did some testing of this PR.

  • Scrolling while holding alt always moves the fader down, even when scrolling up.

  • I noticed that rightclicking on the faders still shows percentages.
    image

  • When controlling the fader with +/- keys, it works fine on the keypad, but when using the +/- keys on the normal keyboard, you have to hold shift while pressing the + key (that makes sense, since it's normally the = key). But I noticed that when doing that, the fader also moves 3x as fast. I suppose isn't really important, since most people will probably use the keypad.

  • I also noticed that controlling the Mixer faders with the up/down keys only changes the last modified fader, not necessarily the currently selected channel. I guess that's fine, but since you can change the currently selected channel using the left/right arrow keys, it feels like maybe the up/down arrow keys should control the selected channel?

  • Also, just personally, I feel like it would be nice if the faders had more room for amplification. Right now 0db is quite high up on the fader, and it doesn't give much space to make the channel louder.

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.
@michaelgregorius
Copy link
Contributor Author

* I also noticed that controlling the Mixer faders with the up/down keys only changes the last modified fader, not necessarily the currently selected channel. I guess that's fine, but since you can change the currently selected channel using the left/right arrow keys, it feels like maybe the up/down arrow keys should control the selected channel?

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:

  • Left/right: select channel
  • Up/down (or plus/minus): Adjust fader of current channel
  • Space: Enter value

@michaelgregorius
Copy link
Contributor Author

Mouse wheel and the "Alt" key

* Scrolling while holding alt always moves the fader down, even when scrolling up.

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

* I noticed that rightclicking on the faders still shows percentages.

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

* When controlling the fader with +/- keys, it works fine on the keypad, but when using the +/- keys on the normal keyboard, you have to hold shift while pressing the + key (that makes sense, since it's normally the = key). But I noticed that when doing that, the fader also moves 3x as fast. I suppose isn't really important, since most people will probably use the keypad.

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

* Also, just personally, I feel like it would be nice if the faders had more room for amplification. Right now 0db is quite high up on the fader, and it doesn't give much space to make the channel louder.

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.

@Rossmaxx Rossmaxx added this to the 1.3 milestone Jan 18, 2025
@bratpeki
Copy link
Member

Volumes are now always treated as dB.

Awesome! I'll be testing this when I get the current PR tests off my plate.

@bratpeki bratpeki self-assigned this Jan 27, 2025
@bratpeki
Copy link
Member

Glad to be working on this! I get my builds off the LMMS site.

I looked at everything Regulus mentioned.

Scrolling while holding alt always moves the fader down, even when scrolling up.

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 :)

I noticed that rightclicking on the faders still shows percentages.

image

Seems to be fine now. Checked with the Delay plugin.

When controlling the fader with +/- keys...

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!

@bratpeki
Copy link
Member

it feels like maybe the up/down arrow keys should control the selected channel

Seems to have been patched (commit 4a720cb).

Also, just personally, I feel like it would be nice if the faders had more room for amplification. Right now 0db is quite high up on the fader, and it doesn't give much space to make the channel louder.

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.

@bratpeki
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants