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

Scalable consistent faders with themeable gradients, marker at unity, dbFS by default #7045

Merged

Conversation

michaelgregorius
Copy link
Contributor

@michaelgregorius michaelgregorius commented Jan 2, 2024

The changes mostly affect the mixer and the LMMS plugins Compressor, EQ and Delay.

How does it look/feel?

A video is worth a thousand words:
https://github.com/LMMS/lmms/assets/9293269/d74e2070-2e54-4a4d-8351-34bce4a215b1

FadersWithGradientsAndUnityLines.mp4

Detailed overview of the changes

The main changes are:

  • Fader levels are rendered in code using a gradient instead of pixmaps. This ensures a consistent rendering. Code that used the pixmaps has been removed completely. The pixmaps have also been deleted.
  • A marker at unity position is rendered, i.e. at position 1.0 in linear levels and 0 dbFS in dbFS scale. Rendering of the unity marker can be turned on and off via a style sheet property.
  • It's now possible to specify the pixmap that's used to render the knob (again). So the "Crossover EQ" now renders with its own knob images again.
  • Faders can be scaled/resized. The knob always renders along the mid line and the available space is used for the level indicators.
  • The peak indicators do not use the three distinct colors anymore but instead use the color of the gradient at that position of the peak. This makes everything look "smooth".
  • All faders now render in dbFS by default. This also improves the faders of the LMMS plugins Compressor, EQ and Delay.
  • The default min and max values for the faders has been adjusted to -42 dbFS and 9 dbFS. This also improves Compressor, EQ and Delay.
  • The style sheet names for the colors have been renamed to something more neutral: peakOk, peakClip, peakWarn. The colors are used for the stops of the gradient.

The problem with the pixmap approach was that the pixmaps don't "know" how a fader instance is configured with regards to the minimum and maximum value. This means that the display can give quite a wrong impression. Artists that create pixmaps also don't know the technical details how to design them in a way that the code would render them correctly.

The minimum size of the faders is set to the previous size of the background pixmap.

Levels can technically still be rendered in linear but it should be considered to remove that feature.

Some code simplifications have also been applied: constructor delegation, remove constructors that are not needed anymore, remove Fader::init.

Some more technical details can be found in the commit messages of the individual commits.

Here's how larger faders render/look:
FaderAdjustmentsLargerFadersExample2

A clipping fader that shows the full scale of the gradient:
FaderAdjustmentsClippingFaderFullScale

Look of the LMMS Compressor:
FaderAdjustmentsCompressor

Look of the LMMS Equalizer:
FaderAdjustmentsEqualizer

Look of the LMMS Delay:
FaderAdjustmentsDelay

Look of the LMMS Crossover EQ:
FaderAdjustmentsCrossoverEQ

Render the fader level in code using a gradient instead of using pixmaps. The problem with the pixmaps is that they don't "know" how a fader instance is configured with regards to the minimum and maximum value. This means that the display can give quite a wrong impression.

The rendering of levels has been unified in the method `paintLevels`. It can render using dbFS and linear scale. The method `paintLinearLevels` has been removed completely, i.e. there's no more code that renders using pixmaps.

Much of the previous code relied on the size of the background image `fader_background.png`, e.g. the initialization of the size. For now the `Fader` widget is initially resized to the size of that background image as it is present in the default and classic theme (see `Fader::init`). All rendering uses the size of the widget itself to determine where to draw what. This means that the widget is prepared to be resizable.

The method `paintLevels` first renders the background of the level indicators and uses these as clipping paths for all other rendering operations, e.g. for the rendering of the levels themselves. Levels are rendered using a gradient which is defined with the following stops:
* Two stops for the ok levels.
* One stop for warning levels.
* One stop for clipping levels.

Peak indicators do not use the three distinct colors anymore but instead use the color of the gradient at that position of the peak. This makes everything look "smooth".

The code now also renders a marker at unity position, i.e. at position 1.0 in linear levels and 0 dbFS in dbFS scale.

The painting code makes lots of use of the class `PaintHelper`. This class is configured with a minimum and maximum value and can then return linear factors for given values. There are two supported modes:
* Map min to 0 and max to 1
* Map min to 1 and max to 0

It can also compute rectangles that correspond to a given value. These methods can be given rectangles that are supposed to represent the span from min to max. The returned result is then a rectangle that fills the lower part of the source rectangle according to the given value with regards to min and max (`getMeterRect`). Another method returns a rectangle of height 1 which lies inside the given source rectangle at the corresponding level (`getPersistentPeakRect`).

The method `paintLevels` uses a mapping function to map the amplitude values (current peak value, persistent peak, etc.) to the display values. There's one mapper that keeps the original value and it is used to display everything in a linear scale. Another mapper maps everything to dbFS and uses these values as display everything in a dbFS scale. The following values must be mapped for the left and right channel to make this work:
* Min and max display values (min and max peak values)
* The current peak value
* The persistent peak value
* The value for unity, i.e. 1.0 in linear levels and 0 dbFS in dbFS scale.

Remove the method `calculateDisplayPeak` which was used in the old method to render linear levels.

`Fader::setPeak` now uses `std::clamp` instead of doing "manual" comparisons.

The LMMS plugins Compressor, EQ and Delay are still configured to use linear displays. It should be considered to switch them to dbFS/logarithmic displays as well and to remove the code that renders linearly.
Remove the now unused pixmaps for the background and the LEDs from the `Fader` class and remove the files from the default and classic theme directories.
Rename the peak properties as follows:
* peakGreen -> peakOk
* peakRed -> peakClip
* peakYellow -> peakWarn

The reasoning is that a style might for example use a different color than green to indicate levels that are ok.

Use the properties to initialize the gradient that is used to render the levels.

Initialize the properties to the colors of the current default theme so that it's not mandatory to set them in a style sheet. Up until now they have all been initialized as black.
Render the knob in the middle of the fader regardless of the width. The previous implementation was dependent on the fader pixmap having a matching width because it always rendered at x=0.
Set the size policy of the fader to minimum expanding in both directions. This will make the fader grow in layouts if there is space.
Default to dbFS levels for all faders and set some better minimum and maximum peak values.
Fix the faders of the Crossover EQ which were initialized and rendered much too wide and with a line at unity. The large width also resulted in the knobs being rendered outside of view.

Resize the fader to the minimum size so that it is constructed at a sane default.

Introduce a property that allows to control if the unity line is rendered. The property is available in style sheets and defaults to the unity lines being rendered. Adjust the paint code to evaluate the property.

Initialize the faders of the Crossover EQ such that the unity line is not drawn.
Remove the constructor of `EqFader` that takes the pixmaps to the fader background, leds and knob. The background and leds pixmaps are not used by the base class `Fader` for rendering anymore to make the `Fader` resizable. A pixmap is still used to render the knob but the constructor that takes the knob as an argument does not do anything meaningful with it, i.e. all faders are rendered with the default knob anyway.

Remove the resources for the fader background, leds and knob as they are not used and the knob was the same image as the default knob anyway.

Remove the static pixmaps from the constructor of `EqControlsDialog`. Switch the instantiations of the EQ's faders to use the remaining constructor of `EqFader`. This constructor sets a different fixed size of (23, 116) compared to the removed constructor which set a size of (23, 80). Therefore all faders that used the removed constructor are now set explicitly to a fixed size of (23, 80).

The constructor that's now used also calls a different base constructor than the removed one. The difference between the two base constructors of `Fader` is that one of them sets the member `m_conversionFactor` to 100.0 whereas the other one keeps the default of 1.0. The adjusted faders in `EqControlsDialog` are thus now constructed with the conversion factor set to 100. However, all of them already call `setDisplayConversion` with `false` after construction which results in the conversion factor being reset to 1.0. So the result should be the same as before the changes.
Remove the parameters for the background and LEDs pixmap from the second `Fader` constructor. Make the knob pixmap parameter in the constructor a const reference. Assign the reference to the knob pixmap of the `Fader` itself. This enables clients to use their own fader knobs as is the case with the Crossover EQ. The EQ now renders using it's own knobs again.

Make the second constructor delegate to the first one. This will additionally set the conversion factor to 100 but this is not a problem with the current code because the only user of the second constructor, the Crossover EQ, already calls `setDisplayConversion` with the parameter set to `false`, hence reinstating a conversion factor of 1.

Remove the resources for the background and LEDs from the Crossover EQ as they are not used anymore. Remove the three QPixmap members from `CrossoverEQControlDialog` as they are not needed. The background and LEDs are not used anyway and the knob is passed in as a constant reference which is copied. Hence we can use a local variable in the constructor of `CrossoverEQControlDialog`.
Remove the `init` method from `Fader` as it is not needed anymore due to the constructor delegation. Tidy up the parameter lists and use of spaces in the constructor.
…adients

Conflicts:
* include/Fader.h
* src/gui/widgets/Fader.cpp
@he29-net
Copy link
Contributor

he29-net commented Jan 2, 2024

Some small issues at UHD resolution (2x scale):

  • the Mixer is a bit oversized: each fader is a bit wider (possibly related to the increased padding in LCDs?) and taller (probably because of increased padding between SEND/knob/arrows);
  • some of the longer labels are cropped at the beginning;
  • LED toggles seem to be drawn at 2x size but within 1x boundary.
    screen

I'm not sure about the smooth gradients: the sharp contrast between green and yellow was very easy to notice at a glance, drawing attention to channels that may be approaching clipping. Not so much with the gradient. Maybe the gradient stops could be doubled, for example using thresholds like peakWarnLow = 0.75, peakWarnHigh = 0.8, resulting in a sharper change and areas that are fully green, yellow, and red? (The number is completely random, no idea what thresholds are currently used). But it may be just a personal preference.

Great work overall; a nice step towards fully scalable UI.

@he29-net
Copy link
Contributor

he29-net commented Jan 2, 2024

One even smaller issue: the unity line overlaps with the borders, resulting in bright pixels where they cross:
screen

@michaelgregorius
Copy link
Contributor Author

Thanks for checking @he29-net!

Some small issues at UHD resolution (2x scale):

* the Mixer is a bit oversized: each fader is a bit wider (possibly related to the increased padding in LCDs?) and taller (probably because of increased padding between SEND/knob/arrows);

* some of the longer labels are cropped at the beginning;

* LED toggles seem to be drawn at 2x size but within 1x boundary.

I think all these issues are related to the layout changes that have been made with pull request #6591. So I propose to create a new issue for these so that we do not mix topics (mixer channels with layouts vs. fader changes).

I'm not sure about the smooth gradients: the sharp contrast between green and yellow was very easy to notice at a glance, drawing attention to channels that may be approaching clipping. Not so much with the gradient. Maybe the gradient stops could be doubled, for example using thresholds like peakWarnLow = 0.75, peakWarnHigh = 0.8, resulting in a sharper change and areas that are fully green, yellow, and red? (The number is completely random, no idea what thresholds are currently used). But it may be just a personal preference.

We can also double the yellow area , similar to how it's done for the green area (see here in the code).

With the current code everything below -12 dbFS is fully green. Then it becomes yellow as it approaches unity and from there on it goes into red. So as part of the fix we could also increase the dbFS value of the last fully green signal, e.g. -6 dbFS or -3 dbFS.

Any preferences with regards to the transitions? What should be the last fully green dbFS value? At how many dbFS should we approach the full on warning area? How far should it exceed? At what dbFS value are we at full clipping?

Great work overall; a nice step towards fully scalable UI.

Thanks! 😃

@michaelgregorius
Copy link
Contributor Author

One even smaller issue: the unity line overlaps with the borders, resulting in bright pixels where they cross:
[snip]

I think this might be caused by the order of rendering. Currently I render as follows:

  • Draw the rounded background (and use it to clip the remaining draw calls)
  • Draw the levels
  • Draw the unity lines
  • Finally do some white-ish overdraw of the background's outline to approach the look of the original pixmap.

I think it might help if I draw the unity lines as the very last step.

Draw the unity lines last so that they are not affected by the white-ish overdraw in line 423 (after the changes).
@michaelgregorius
Copy link
Contributor Author

With commit 045a2cd the unity lines are now drawn last.

Introduce a second point in the gradient for the warn colors so that we get a certain range with the full/solid warn color.

The colors are distributed as follows now. The solid ok range goes from -inf dbFS to -12 dbFS. The warn range goes from -6 dbFS to 0 dbFS. In between the colors are interpolated. Values above 0 dbFS interpolate from the warn color to the clip color.

This is now quite similar to the previous implementation.

# Analysis of the previous pixmap implementation
The pixmap implementation used pixmaps with a height of 116 pixels to map 51 dbFS (-42 dbFS to 9 dbFS) across the whole height. The pixels of the LED pixmap were distributed as follows along the Y-axis:
* Margin: 4
* Red: 18
* Yellow: 14
* Green: 76
* Margin: 4

Due to the margins the actual red, yellow and green areas only represent a range of (1 - (4+4) / 116) * 51 ~ 47,48 dbFS. This range is distributed as follows across the colors:
Red: 7.91 dbFS
Yellow: 6.16 dbFS
Green: 33.41 dbFS

The borders between the colors are located along the following dbFS values:
* Red/yellow: 9 - (4 + 18) / 116 * 51 dbFS ~ -0.67 dbFS
* Yellow/green: 9 - (4 + 18 + 14) / 116 * 51 dbFS ~ -6.83 dbFS
* The green marker is rendered for values above -40.24 dbFS.
@michaelgregorius
Copy link
Contributor Author

@he29-net, with commit a644bdc there is now a warn range with a solid yellow color. It goes from -6 dbFS to 0 dfBS. I have analyzed the previous implementation (see below) and have chosen the values such that they look very similar.

Here's how it looks:
Minus12ToMinus6

Analysis of the previous pixmap implementation

The pixmap implementation used pixmaps with a height of 116 pixels to map 51 dbFS (-42 dbFS to 9 dbFS) across the whole height. The pixels of the LED pixmap were distributed as follows along the Y-axis:

  • Margin: 4
  • Red: 18
  • Yellow: 14
  • Green: 76
  • Margin: 4

Due to the margins the actual red, yellow and green areas only represent a range of (1 - (4+4) / 116) * 51 ~ 47,48 dbFS. This range is distributed as follows across the colors:
Red: 7.91 dbFS
Yellow: 6.16 dbFS
Green: 33.41 dbFS

The borders between the colors are located along the following dbFS values:

  • Red/yellow: 9 - (4 + 18) / 116 * 51 dbFS ~ -0.67 dbFS
  • Yellow/green: 9 - (4 + 18 + 14) / 116 * 51 dbFS ~ -6.83 dbFS
  • The green marker is rendered for values above -40.24 dbFS.

Adjust the `Fader` so that it can correctly render arbitrary ranges of min and max peak values, e.g. that it would render a display range of [-12 dbFS, -42 dbFS] correctly.

Until now the gradient was defined to start at the top of the levels rectangle and end at the bottom. As a result the top was always rendered in the "clip" color and the bottom in the "ok" color. However, this is wrong, e.g. if we configure the `Fader` with a max value of -12 dbFS and a min value of -42 dbFS. In that case the whole range of the fader should be rendered with the "ok" color.

The fix is to compute the correct window coordinates of the start and end point of gradient using from the "window" of values that the `Fader` displays and then to map the in-between colors accordingly. See the added comments in the code for more details.

Add the templated helper class `LinearMap` to `lmms_math.h`. The class defines a linear function/map which is initialized using two points. With the `map` function it is then possible to evaluate arbitrary X-coordinates.
Remove the now unused mapping methods from `PaintHelper`. Their functionality has been replaced with the usage of `LinearMap` in the code.
Include `cassert` for some  builds that otherwise fail.
@michaelgregorius
Copy link
Contributor Author

Hi @LMMS/developers,

I consider this pull request done now and would like to request a review so that the changes can be merged. Please note that this PR is not just a cosmetic change but that it fixes fundamental functionality of the Fader class. Therefore I propose to include this PR in the next 1.3 release.

The fix can be demonstrated by configuring the Fader with a minimum peak value of -42 dbFS and a maximum peak value of -12 dbFS. In this case the correct rendering is an all green line if the fader is maxed out. This in fact now happens with the PR:

FadersRangeCorrected.mp4

And here's how the setup with a range of [-42, -12] dbFS renders in the current master:

FadersRangeWrong.mp4

The master just renders the whole pixmap which makes it look as it there's some heavy clipping going on when in fact it isn't.

Please refer to the original PR message for all other additional improvements.

@he29-net
Copy link
Contributor

he29-net commented Jan 6, 2024

with commit a644bdc there is now a warn range with a solid yellow color.

Great, subjectively I think it looks better now. Maybe the orange just above the unity line could more aggressively inform the user that "hey, danger, we are clipping now", but I think the addition of the unity line itself takes care about that. Combined with the colored peak markers I think it is sufficient.

Speaking of the unity line, 045a2cd did not really change anything for me. This is how it looks in the mixer:
screen

It's not as pronounced on the lighter background compared to EQ, but the bright dots are still there. I think the issue is simply the alpha of 127 in unityMarkerColor, which allows it to mix with the dark (but not black) border under it. So maybe the line should be made completely opaque, or drawn one pixel shorter on each side so there is no overlap?

As a side note, I almost complained about uneven alignment on the red faders; turns out it is real, but it's caused by the physical distance of red and green subpixels on my LCD... :) So not a code issue.
IMG_5748_crop
(8 white pixels were added in editor)

Make the unity marker opaque by default and enable to style it with the style sheets. None of the two style sheets uses this option though.
@michaelgregorius
Copy link
Contributor Author

Hi @he29-net,

with commit 99113c3 the unity marker now defaults to an opaque color. It is now also possible to style this color via the style sheet.

With regards to warning the users about clipping: I think in some situations, especially with regards to mixing in digital, it might be okay for one or more channels to "clip", for example if they get routed to another channel which attenuates the mix again. On the master channel on the other hand it's interesting to get warned about clipping though. So it might all be relative anyway.

@he29-net
Copy link
Contributor

he29-net commented Jan 6, 2024

After 99113c3 the unity line is now pixel-perfect. 👍

Maybe (63, 63, 63, 255) would be a better choice for the default though, if we are not updating the styles? It would more or less match the previous appearance, i.e. it would blend better with the dark background.

(Sorry for constantly pointing out tiny graphical issues.. ^^; This is why I prefer working on embedded systems: there is usually no graphical interface the user can complain about, haha..)

The peak values for the shelves and peaks of the Equalizer plugin are computed in `EqEffect::peakBand`. The previous implementation evaluated the bins of the corresponding frequency spectrum and determined the "loudest" one. The value of this bin was then converted to dbFS and mapped to the interval [0, inf[ where all values less or equal to -60 dbFS were mapped to 0 and a value of 40 dbFS was mapped to 1. So effectively everything was mapped somewhere into [0, 1] yet in a quite "distorted" way because a signal of 40 dbFS resulted in being displayed as unity in the fader.

This commit directly returns the value of the maximum bin, i.e. it does not map first to dbFS and then linearize the result anymore. This should work because the `Fader` class assumes a "linear" input signal and if the value of the bin was previously mapped to dbFS it should have some "linear" character. Please note that this is still somewhat of a "proxy" value because ideally the summed amplitude of all relevant bins in the frequency range would be shown and not just the "loudest" one.

## Other changes
Rename `peakBand` to `linearPeakBand` to make more clear that a linear value is returned.

Handle a potential division by zero by checking the value of `fft->getEnergy()` before using it.

Index into `fft->m_bands` so that no parallel incrementing of the pointer is needed. This also enables the removal of the local variable `b`.
@michaelgregorius
Copy link
Contributor Author

I have solved this differently now. Instead of extending the Fader for this single case I decided to remove the mapping to [0, 1] (or technically rather [0, inf[) that's done at the end and to convert the dbFS values to linear values because that's what the Fader expects with regards to the peaks. Then I realized that the code would effectively do linear -> dbFS -> linear if I did this and that I can simply use the maximum bin as calculated in the for loop.

The corresponding changes are introduced via commit 449ab02 which also introduces the handling of a potential division by zero. It also removes the clamping to -60 dbFS so that the fader can decide how to present the raw data.

Thanks for introducing the Equalizer in the first place, @curlymorphic! 🙂

@michaelgregorius
Copy link
Contributor Author

I consider this pull request ready for review and merge now. There are several other things that could be done in separate PRs once this one is merged:

  • Render and evaluate the fader knob on a dbFS scale instead of a linear one. The models should be kept linear though because they can be automated and switching them to dbFS values would likely introduce too much data upgrade efforts for save files. A linear model with the interval [0, 2] also enables real silence whereas a dbFS model would have to map -∞ somehow.
  • Make the mixer vertically resizable so that additional available space could be use by the faders.
  • Unification of Fader and EqFader (if it is really wanted)
  • More accurate computation of the Equalizer peak levels (if it is efficiently possible in FFT space)

@RoxasKH
Copy link
Contributor

RoxasKH commented Mar 5, 2024

@RoxasKH, I have added some commits which render the clipping area as a flat/solid region. The full scale currently looks as follows:

The change also showed that there was a problem with the unity marker. This was fixed by replacing the helper class PaintHelper with another mechanism.

Here's a test file that can be used to test the fader. It consists of a sine that's played at unity: MixerDevelopmentSineAtZero.zip

I know i'm late but I'm in favour of this cause LMMS needs everything it can have to be dinamically rendered, so i'll add a bit from a design perspective.

Here's a 1:1 view against the current faders

immagine

Now, aside from the sizes of the mixer buses itself

  1. We're losing both the slight outline and the curvature at the ends of the faders, which made it more polished than the abrupt cut which is now present to m

immagine

  1. The volume levels dimensions changed? Now bars have a width of 6px, while the old one were of 7. This is inherently bad, but while there are other daws with thin levels, they do not have the volume fader overlayed on the levels themselves, so i believe having them larger is actually better.

immagine

  1. The bars is slightly larger than the fader so it's 7 px. This creates a weird effect where the bar seems to be getting thinner when it peaks, as the light color of the bar kind of merges with the yellow part of the volume. Now there's more than 1 single possible fix; FL Studio which is my reference cause it's the only one having bars on the volume levels, keep them thinner than the level, so that the bar is actually visible only when it's not hovered: at the end of the day once the bar gets covered the red color itself will tell the producer about the clipping.

immagine

On active faders the color of the bar makes it merge with the background as well, so it might be worth changing color? Not sure actually, once made them thinner there should be a better separation.
Actually, by bringing back the volume levels to 7px wide they should cover the line by themselves with no need to make them thinner i guess.

immagine

Obviously this are probably all minor design things, but i believe they should be addressed when this will substitute the current faders.

The levels rendering now more explicitly distinguished between the rendering of the level outline/border and the level meters. The level rectangles are "inset" with regards to the borders so that there is a margin between the level borders and the meter readings. This margin is now also applied to the top and bottom of the levels. Levels are now also rendered as rounded rectangles similar to the level borders.

Only render the levels and peaks if their values are greater than the minimum level.

Make the radius of the rounded rectangles more pronounced by increasing its value from 1 to 2.

Decrease the margins so that the level readings become wider, i.e. so that they are rendered with more pixels.

Add the lambda `computeLevelMarkerRect` so that the rendering of the level markers is more decoupled from the rendering of the peak markers.
@michaelgregorius
Copy link
Contributor Author

Thanks for your feedback, @RoxasKH!

Most of your points should have been addressed by commit b4bfc71:

  • It introduces a margin in all directions between the level borders and the level meters.
  • Levels are now also rendered as rounded rectangles similar to the level borders.
  • The radius of the rounded rectangles was increased to make it more pronounced.
  • The margins have been decreased so that the level readings become wider.
  • The unity indicator is now rendered with the same width as the level meters so that they become fully covered by them.

Other improvements:

  • Only render the levels and peaks if their values are greater than the minimum level. Previously there was always a slight green line at the bottom which I did not see due to fractional scaling.

Here's how it looks:

FadersBranchRoundLevelsInset

@michaelgregorius
Copy link
Contributor Author

The failing MSVC builds seem to be a problem with Cloudflare, see here.

…adients

Conflicts:
* plugins/Eq/EqEffect.cpp
@michaelgregorius
Copy link
Contributor Author

I have merged the current master to fix the self-inflicted merge conflict that was introduced with my pull request #7141. 😅

Ready for review. 😉

@RoxasKH
Copy link
Contributor

RoxasKH commented Mar 22, 2024

Thanks for your feedback, @RoxasKH!

Most of your points should have been addressed by commit b4bfc71:

* It introduces a margin in all directions between the level borders and the level meters.

* Levels are now also rendered as rounded rectangles similar to the level borders.

* The radius of the rounded rectangles was increased to make it more pronounced.

* The margins have been decreased so that the level readings become wider.

* The unity indicator is now rendered with the same width as the level meters so that they become fully covered by them.

Other improvements:

* Only render the levels and peaks if their values are greater than the minimum level. Previously there was always a slight green line at the bottom which I did not see due to fractional scaling.

Here's how it looks:

Looks good to me.

We're just losing the small border highlight around them, but i guess it's not a big deal, you choose if you feel like it's worth being perfectionist and working on that or not.

I have some other concerns about the mixer stuff placement, positioning and width (and cursor not changing to pointer when hovering the send button), but i'm sure those were merged with another PR as i saw the same things happening in the current nightly build.

It's stuff that can be fixed later somewhere else and doesn't need to be addressed here tho.

@michaelgregorius michaelgregorius requested a review from Veratil April 2, 2024 19:01
include/Fader.h Outdated Show resolved Hide resolved
plugins/Eq/EqEffect.cpp Outdated Show resolved Hide resolved
include/lmms_math.h Show resolved Hide resolved
src/gui/widgets/Fader.cpp Show resolved Hide resolved
src/gui/widgets/Fader.cpp Outdated Show resolved Hide resolved
src/gui/widgets/Fader.cpp Show resolved Hide resolved
src/gui/widgets/Fader.cpp Outdated Show resolved Hide resolved
src/gui/widgets/Fader.cpp Show resolved Hide resolved
src/gui/widgets/Fader.cpp Outdated Show resolved Hide resolved
src/gui/widgets/Fader.cpp Outdated Show resolved Hide resolved
Reduce code repetition in `EqEffect::setBandPeaks` by introducing a lambda. Adjust code formatting.
Code review changes in `Fader.cpp`. Mostly whitespace adjustments.

Split up the calculation of the meter width to make it more understandable. This also reduces the number of parentheses.
Use `MEMBER` instead of `READ`/`WRITE` for some properties that are not called explicitly from outside of the class.
Use default member initializers for the members in `Fader` that have previously been initialized in the constructor list.

Mark `ampToDbfs` and `dbfsToAmp` as `constexpr`. The function `dbfsToAmp` is used in the initialzation and this way nothing has to be computed at runtime.
Remove `constexpr` from `ampToDbfs` and ` dbfsToAmp` again because the MSVC compiler does not like it. Seems that for some reason `log10f` and `std::pow` are not `constexpr` there (at least in the version used on the build server).
include/Fader.h Outdated Show resolved Hide resolved
plugins/Eq/EqEffect.cpp Outdated Show resolved Hide resolved
plugins/Eq/EqEffect.cpp Outdated Show resolved Hide resolved
plugins/Eq/EqEffect.cpp Outdated Show resolved Hide resolved
@michaelgregorius michaelgregorius merged commit ba4fda7 into LMMS:master Apr 5, 2024
9 checks passed
@michaelgregorius michaelgregorius deleted the FaderInCodeWithGradients branch April 5, 2024 10:50
enp2s0 pushed a commit to enp2s0/lmms that referenced this pull request Apr 12, 2024
… dbFS by default (LMMS#7045)

* Render fader levels in code with a gradient

Render the fader level in code using a gradient instead of using pixmaps. The problem with the pixmaps is that they don't "know" how a fader instance is configured with regards to the minimum and maximum value. This means that the display can give quite a wrong impression.

The rendering of levels has been unified in the method `paintLevels`. It can render using dbFS and linear scale. The method `paintLinearLevels` has been removed completely, i.e. there's no more code that renders using pixmaps.

Much of the previous code relied on the size of the background image `fader_background.png`, e.g. the initialization of the size. For now the `Fader` widget is initially resized to the size of that background image as it is present in the default and classic theme (see `Fader::init`). All rendering uses the size of the widget itself to determine where to draw what. This means that the widget is prepared to be resizable.

The method `paintLevels` first renders the background of the level indicators and uses these as clipping paths for all other rendering operations, e.g. for the rendering of the levels themselves. Levels are rendered using a gradient which is defined with the following stops:
* Two stops for the ok levels.
* One stop for warning levels.
* One stop for clipping levels.

Peak indicators do not use the three distinct colors anymore but instead use the color of the gradient at that position of the peak. This makes everything look "smooth".

The code now also renders a marker at unity position, i.e. at position 1.0 in linear levels and 0 dbFS in dbFS scale.

The painting code makes lots of use of the class `PaintHelper`. This class is configured with a minimum and maximum value and can then return linear factors for given values. There are two supported modes:
* Map min to 0 and max to 1
* Map min to 1 and max to 0

It can also compute rectangles that correspond to a given value. These methods can be given rectangles that are supposed to represent the span from min to max. The returned result is then a rectangle that fills the lower part of the source rectangle according to the given value with regards to min and max (`getMeterRect`). Another method returns a rectangle of height 1 which lies inside the given source rectangle at the corresponding level (`getPersistentPeakRect`).

The method `paintLevels` uses a mapping function to map the amplitude values (current peak value, persistent peak, etc.) to the display values. There's one mapper that keeps the original value and it is used to display everything in a linear scale. Another mapper maps everything to dbFS and uses these values as display everything in a dbFS scale. The following values must be mapped for the left and right channel to make this work:
* Min and max display values (min and max peak values)
* The current peak value
* The persistent peak value
* The value for unity, i.e. 1.0 in linear levels and 0 dbFS in dbFS scale.

Remove the method `calculateDisplayPeak` which was used in the old method to render linear levels.

`Fader::setPeak` now uses `std::clamp` instead of doing "manual" comparisons.

The LMMS plugins Compressor, EQ and Delay are still configured to use linear displays. It should be considered to switch them to dbFS/logarithmic displays as well and to remove the code that renders linearly.

* Remove unused pixmaps from `Fader`

Remove the now unused pixmaps for the background and the LEDs from the `Fader` class and remove the files from the default and classic theme directories.

* Rename peak properties and use them to render levels

Rename the peak properties as follows:
* peakGreen -> peakOk
* peakRed -> peakClip
* peakYellow -> peakWarn

The reasoning is that a style might for example use a different color than green to indicate levels that are ok.

Use the properties to initialize the gradient that is used to render the levels.

Initialize the properties to the colors of the current default theme so that it's not mandatory to set them in a style sheet. Up until now they have all been initialized as black.

* Always render the knob in the middle of the fader

Render the knob in the middle of the fader regardless of the width. The previous implementation was dependent on the fader pixmap having a matching width because it always rendered at x=0.

* Set size policy of fader to minimum expanding

Set the size policy of the fader to minimum expanding in both directions. This will make the fader grow in layouts if there is space.

* Default dbFS levels and better peak values

Default to dbFS levels for all faders and set some better minimum and maximum peak values.

* Fix faders of Crossover EQ

Fix the faders of the Crossover EQ which were initialized and rendered much too wide and with a line at unity. The large width also resulted in the knobs being rendered outside of view.

Resize the fader to the minimum size so that it is constructed at a sane default.

Introduce a property that allows to control if the unity line is rendered. The property is available in style sheets and defaults to the unity lines being rendered. Adjust the paint code to evaluate the property.

Initialize the faders of the Crossover EQ such that the unity line is not drawn.

* Remove EqFader constructor with pixmaps

Remove the constructor of `EqFader` that takes the pixmaps to the fader background, leds and knob. The background and leds pixmaps are not used by the base class `Fader` for rendering anymore to make the `Fader` resizable. A pixmap is still used to render the knob but the constructor that takes the knob as an argument does not do anything meaningful with it, i.e. all faders are rendered with the default knob anyway.

Remove the resources for the fader background, leds and knob as they are not used and the knob was the same image as the default knob anyway.

Remove the static pixmaps from the constructor of `EqControlsDialog`. Switch the instantiations of the EQ's faders to use the remaining constructor of `EqFader`. This constructor sets a different fixed size of (23, 116) compared to the removed constructor which set a size of (23, 80). Therefore all faders that used the removed constructor are now set explicitly to a fixed size of (23, 80).

The constructor that's now used also calls a different base constructor than the removed one. The difference between the two base constructors of `Fader` is that one of them sets the member `m_conversionFactor` to 100.0 whereas the other one keeps the default of 1.0. The adjusted faders in `EqControlsDialog` are thus now constructed with the conversion factor set to 100. However, all of them already call `setDisplayConversion` with `false` after construction which results in the conversion factor being reset to 1.0. So the result should be the same as before the changes.

* Remove background and LEDs pixmap from Fader constructor

Remove the parameters for the background and LEDs pixmap from the second `Fader` constructor. Make the knob pixmap parameter in the constructor a const reference. Assign the reference to the knob pixmap of the `Fader` itself. This enables clients to use their own fader knobs as is the case with the Crossover EQ. The EQ now renders using it's own knobs again.

Make the second constructor delegate to the first one. This will additionally set the conversion factor to 100 but this is not a problem with the current code because the only user of the second constructor, the Crossover EQ, already calls `setDisplayConversion` with the parameter set to `false`, hence reinstating a conversion factor of 1.

Remove the resources for the background and LEDs from the Crossover EQ as they are not used anymore. Remove the three QPixmap members from `CrossoverEQControlDialog` as they are not needed. The background and LEDs are not used anyway and the knob is passed in as a constant reference which is copied. Hence we can use a local variable in the constructor of `CrossoverEQControlDialog`.

* Remove the init method from Fader

Remove the `init` method from `Fader` as it is not needed anymore due to the constructor delegation. Tidy up the parameter lists and use of spaces in the constructor.

* Introduce range with solid warn color

Introduce a second point in the gradient for the warn colors so that we get a certain range with the full/solid warn color.

The colors are distributed as follows now. The solid ok range goes from -inf dbFS to -12 dbFS. The warn range goes from -6 dbFS to 0 dbFS. In between the colors are interpolated. Values above 0 dbFS interpolate from the warn color to the clip color.

This is now quite similar to the previous implementation.

# Analysis of the previous pixmap implementation
The pixmap implementation used pixmaps with a height of 116 pixels to map 51 dbFS (-42 dbFS to 9 dbFS) across the whole height. The pixels of the LED pixmap were distributed as follows along the Y-axis:
* Margin: 4
* Red: 18
* Yellow: 14
* Green: 76
* Margin: 4

Due to the margins the actual red, yellow and green areas only represent a range of (1 - (4+4) / 116) * 51 ~ 47,48 dbFS. This range is distributed as follows across the colors:
Red: 7.91 dbFS
Yellow: 6.16 dbFS
Green: 33.41 dbFS

The borders between the colors are located along the following dbFS values:
* Red/yellow: 9 - (4 + 18) / 116 * 51 dbFS ~ -0.67 dbFS
* Yellow/green: 9 - (4 + 18 + 14) / 116 * 51 dbFS ~ -6.83 dbFS
* The green marker is rendered for values above -40.24 dbFS.

* Remove unused method Fader::clips

* Fader: Correctly render arbitrary ranges

Adjust the `Fader` so that it can correctly render arbitrary ranges of min and max peak values, e.g. that it would render a display range of [-12 dbFS, -42 dbFS] correctly.

Until now the gradient was defined to start at the top of the levels rectangle and end at the bottom. As a result the top was always rendered in the "clip" color and the bottom in the "ok" color. However, this is wrong, e.g. if we configure the `Fader` with a max value of -12 dbFS and a min value of -42 dbFS. In that case the whole range of the fader should be rendered with the "ok" color.

The fix is to compute the correct window coordinates of the start and end point of gradient using from the "window" of values that the `Fader` displays and then to map the in-between colors accordingly. See the added comments in the code for more details.

Add the templated helper class `LinearMap` to `lmms_math.h`. The class defines a linear function/map which is initialized using two points. With the `map` function it is then possible to evaluate arbitrary X-coordinates.

* Remove unused methods in PaintHelper

Remove the now unused mapping methods from `PaintHelper`. Their functionality has been replaced with the usage of `LinearMap` in the code.

* Fix some builds

Include `cassert` for some  builds that otherwise fail.

* Opaque unity marker with styling option

Make the unity marker opaque by default and enable to style it with the style sheets. None of the two style sheets uses this option though.

* Darker default color for the unity line

* Move code

Move the computation of most mapped values at the top right after the definition of the mapper so that it is readily available in all phases of the painting code.

* Render unity lines in background

Render the unity lines before rendering the levels so that they get overdrawn and do not stick out when they are crossed.

* Don't draw transparent white lines anymore

Don't draw the transparent white lines anymore as they were mostly clipped anyway and only create "smudge".

* Full on clip color at unity

Adjust the gradient so that the full on clip color shows starting at unity. There is only a very short transition from the end of warning to clipping making it look like a solid color "standing" on top of a gradient.

* Fix discrepancy between levels and unity markers

This commit removes the helper class `PaintHelper` and now uses two lambdas to compute the rectangles for the peak indicators and levels. It uses the linear map which maps the peak values (in dbFS or linear) to window coordinates of the widget.

The change fixes a discrepancy in the following implementation for which the full on clip rectangle rendered slightly below the unity marker.

* Fix fader display for Equalizer shelves and peaks

The peak values for the shelves and peaks of the Equalizer plugin are computed in `EqEffect::peakBand`. The previous implementation evaluated the bins of the corresponding frequency spectrum and determined the "loudest" one. The value of this bin was then converted to dbFS and mapped to the interval [0, inf[ where all values less or equal to -60 dbFS were mapped to 0 and a value of 40 dbFS was mapped to 1. So effectively everything was mapped somewhere into [0, 1] yet in a quite "distorted" way because a signal of 40 dbFS resulted in being displayed as unity in the fader.

This commit directly returns the value of the maximum bin, i.e. it does not map first to dbFS and then linearize the result anymore. This should work because the `Fader` class assumes a "linear" input signal and if the value of the bin was previously mapped to dbFS it should have some "linear" character. Please note that this is still somewhat of a "proxy" value because ideally the summed amplitude of all relevant bins in the frequency range would be shown and not just the "loudest" one.

## Other changes
Rename `peakBand` to `linearPeakBand` to make more clear that a linear value is returned.

Handle a potential division by zero by checking the value of `fft->getEnergy()` before using it.

Index into `fft->m_bands` so that no parallel incrementing of the pointer is needed. This also enables the removal of the local variable `b`.

* Improve the rendering of the levels

The levels rendering now more explicitly distinguished between the rendering of the level outline/border and the level meters. The level rectangles are "inset" with regards to the borders so that there is a margin between the level borders and the meter readings. This margin is now also applied to the top and bottom of the levels. Levels are now also rendered as rounded rectangles similar to the level borders.

Only render the levels and peaks if their values are greater than the minimum level.

Make the radius of the rounded rectangles more pronounced by increasing its value from 1 to 2.

Decrease the margins so that the level readings become wider, i.e. so that they are rendered with more pixels.

Add the lambda `computeLevelMarkerRect` so that the rendering of the level markers is more decoupled from the rendering of the peak markers.

* Reduce code repetition

Reduce code repetition in `EqEffect::setBandPeaks` by introducing a lambda. Adjust code formatting.

* Code review changes

Code review changes in `Fader.cpp`. Mostly whitespace adjustments.

Split up the calculation of the meter width to make it more understandable. This also reduces the number of parentheses.

* Use MEMBER instead of READ/WRITE

Use `MEMBER` instead of `READ`/`WRITE` for some properties that are not called explicitly from outside of the class.

* Use default member initializers for Fader

Use default member initializers for the members in `Fader` that have previously been initialized in the constructor list.

* Make code clearer

Make code clearer in `Fader::FadermouseDoubleClickEvent`. Only divide if the dialog was accepted with OK.
@Rossmaxx Rossmaxx mentioned this pull request May 9, 2024
messmerd added a commit that referenced this pull request Nov 17, 2024
* Analyze and improve search in the file browser (again) (#6985)

Improves performance when searching in the file browser (confirmed with profiling using KCacheGrind), adds a search indicator at the bottom to let the user know a search is in progress, blacklists unnecessary system directories (speeding up both the search speed and potentially load times as well by reducing the number of filesystem entries to consider), and fixes an issue that causes not all of the search results to appear.

* Fix memory leaks (#6879)

* Replace knobFModel with std::vector

* Create QPixmap's on the stack

* Assign parent for QGraphicsScene
A call to QGraphicsView::setScene does not make
the view take ownership of the scene.

* Do not allocate QList on the heap

* Use static QPixmap's
The QPixmap's need to be created within the constructor, and not outside
where they are defined, since it can't find them otherwise.
I'm not too sure why.

* Clear m_vi2->knobFModel in destructor

* Use local static QPixmap's

* Do not allocate QPixmap with new in AudioFileProcessor

* Do not allocate QPixmap with new in Nes

* Do not allocate QPixmap with new in Organic

* Do not allocate QPixmap with new in SaControlsDialog

* Do not allocate QPixmap with new in Vestige

* Do not allocate QPixmap with new for FileBrowser

* Do not allocate QPixmap with new in MixerLine

* Do not allocate QPixmap with new in SendButtonIndicator

* Do not allocate QPixmap with new in AutomationClipView

* Do not allocate QPixmap with new in MidiClipView

* Do not allocate QPixmap with new in AutomationEditor

* Do not allocate QPixmap with new in PianoRoll

* Do not allocate QPixmap with new in TimeLineWidget

* Do not allocate QPixmap with new in EnvelopeAndLfoView

* Do not allocate QPixmap with new in PianoView

* Do not allocate QPixmap with new in ComboBox

* Do not allocate QPixmap with new in Fader

* Do not allocate QPixmap with new for LcdWidget

* Do not allocate QPixmap with new for LedCheckbox

* Use m_ as prefix for members

* Use uniform initialization
I already started using uniform initialization for the QPixmap changes
for some reason, so I'm finishing that up.

* Uniform initiaization

* And then he realized he was making copies...

* Do not call QPixmap copy constructor

* Do not call QPixmap copy constructor in SaControlsDialog

* Do not make pixmap's static for Lcd's and Led's

* Initialize pixmaps in-class

* Fix few mistakes and formatting

* Use `std::optional` for colour interfaces and storage (#6991)

* Rewrite Amplifier plugin code (#6989)

* rewrite amplifier plugin style

* pomelo

* oroblanco

* grapefruit

* calamlamondin

* Enforce lazy loading for `FileBrowser` (#6996)

* Ghost notes for the automation editor (#6940)

Show ghost notes or sample track as a visual aid in the Automation Editor.

---------

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Fix AudioFileProcessor reverse bug (#6958)

* Fix floating point exception in Spectrum Analyzer (#7018)

Fix a floating point exception in the Spectrum Analyzer which occurs if the amplitude is 0. The amplitude is used in the calculation of a logarithm which is not defined for values smaller or equal to 0.

The fix is to replace a value of 0 with the minimum positive float value. Negative values are not handled because it seems that only values greater or equal to 0 are fed into the method. This assumption is asserted in case this ever changes.

* Split `TimeLineWidget` into core and GUI parts (#7004)

* Fix LOMM crossovers (#7026)

* Refactor ``SampleBuffer`` (#6610)

* Add refactored SampleBuffer

* Add Sample

* Add SampleLoader

* Integrate changes into AudioSampleRecorder

* Integrate changes into Oscillator

* Integrate changes into SampleClip/SamplePlayHandle

* Integrate changes into Graph

* Remove SampleBuffer include from SampleClipView

* Integrate changes into Patman

* Reduce indirection to sample buffer from Sample

* Integrate changes into AudioFileProcessor

* Remove old SampleBuffer

* Include memory header in TripleOscillator

* Include memory header in Oscillator

* Use atomic_load within SampleClip::sample

* Include memory header in EnvelopeAndLfoParameters

* Use std::atomic_load for most calls to Oscillator::userWaveSample

* Revert accidental change on SamplePlayHandle L.111

* Check if audio file is empty before loading

* Add asserts to Sample

* Add cassert include within Sample

* Adjust assert expressions in Sample

* Remove use of shared ownership for Sample
Sample does not need to be wrapped around a std::shared_ptr.
This was to work with the audio thread, but the audio thread
can instead have their own Sample separate from the UI's Sample,
so changes to the UI's Sample would not leave the audio worker thread
using freed data if it had pointed to it.

* Use ArrayVector in Sample

* Enforce std::atomic_load for users of std::shared_ptr<const SampleBuffer>

* Use requestChangesGuard in ClipView::remove
Fixes data race when deleting SampleClip

* Revert only formatting changes

* Update ClipView::remove comment

* Revert "Remove use of shared ownership for Sample"

This reverts commit 1d452331d16626ac2f3c42bbd25f1267a61cc116.
In some cases, you can infact do away with shared ownership
on Sample if there are no writes being made to either of them,
but to make sure changes are reflected to the object in cases
where writes do happen, they should work with the same one.

* Fix heap-use-after-free in Track::loadSettings

* Remove m_buffer asserts

* Refactor play functionality (again)
The responsibility of resampling the buffer
and moving the frame index is now in Sample::play, allowing the removal
of both playSampleRangeLoop and playSampleRangePingPong.

* Change copyright

* Cast processingSampleRate to float
Fixes division by zero error

* Update include/SampleLoader.h

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Update include/SampleLoader.h

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Format SampleLoader.h

* Remove SampleBuffer.h include in SampleRecordHandle.h

* Update src/core/Oscillator.cpp

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Use typeInfo<float> for float equality comparison

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Use std::min in Sample::visualize

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Move in result to m_data

* Use if block in playSampleRange

* Pass in unique_ptr to SampleClip::setSampleBuffer

* Return const QString& from SampleBuffer::audioFile

* Do not pass in unique_ptr by r-value reference

* Use isEmpty() within SampleClipView::updateSample

* Remove use of atomic_store and atomic_load

* Remove ArrayVector comment

* Use array specialization for unique_ptr when managing DrumSynth data
Also made it so that we don't create result
before checking if we failed to decode the file,
potentially saving us an allocation.

* Don't manually delete Clip if it has a Track

* Clean up generateAntiAliasUserWaveTable function
Also, make it so that we actually call this function
when necessary in TripleOscillator.

* Set user wave, even when value is empty
If the value or file is empty, I think showing a
error popup here is ideal.

* Remove whitespace in EnvelopeAndLfoParameters.cpp L#121

* Fix error in c5f7ccba492dd867524156afa652c4eff99f9b40
We still have to delete the Clip's, or else we would just be eating
up memory. But we should first make sure that the Track's no longer
see this Clip in their m_clips vector. This has to happen
as it's own operation because we have to wait for the audio thread(s)
first. This would ensure that Track's do not create
PlayHandle's that would refer to a Clip that is currently
being destroyed. After that, then we call deleteLater on the Clip.

* Convert std::shared_ptr<Sample> to Sample
This conversion does not apply to Patman as there seems to be issues
with it causing heap-use-after-free issues, such as with
PatmanInstrument::unloadCurrentPatch

* Fix segfault when closing LMMS
Song should be deleted before AudioEngine.

* Construct buffer through SampleLoader in FileBrowser's previewFileItem function
+ Remove const qualification in SamplePlayHandle(const QString&)
constructor for m_sample

* Move guard out of removeClip and deleteClips
+ Revert commit 1769ed517da389997b40568f26dd54b800a5d62c since
this would fix it anyway
(we don't try to lock the engine to
delete the global automation track when closing LMMS now)

* Simplify the switch in play function for loopMode

* Add SampleDecoder

* Add LMMS_HAVE_OGGVORBIS comment

* Fix unused variable error

* Include unordered_map

* Simplify SampleDecoder
Instead of using the extension (which could be wrong) for the file,
we simply loop through all the decoders available. First sndfile because
it covers a lot of formats, then the ogg decoder for the few cases where sndfile
would not work for certain audio codecs, and then the DrumSynth decoder.

* Attempt to fix Mac builds

* Attempt to fix Mac builds take 2

* Add vector include to SampleDecoder

* Add TODO comment about shared ownership with clips

Calls to ClipView::remove may occur at any point, which can cause
a problem when the Track is using the clip about to be removed.
A suitable solution would be to use shared ownership between the Track
and ClipView for the clip. Track's can then simply remove the shared
pointer in their m_clips vector, and ClipView can call reset on the
shared pointer on calls to ClipView::remove.

* Adjust TODO comment
Disregard the shared ownership idea. Since we would be modifying
the collection of Clip's in Track when removing the Clip, the Track
could be iterating said collection while this happens,
causing a bug. In this case, we do actually
want a synchronization mechanism.

However, I didn't mention another separate issue in the TODO comment
that should've been addressed: ~Clip should not be responsible for
actually removing the itself from it's Track. With calls to removeClip,
one would expect that to already occur.

* Remove Sample::playbackSize
Inside SampleClip::sampleLength, we should be using Sample::sampleSize
instead.

* Fix issues involving length of Sample's
SampleClip::sampleLength should be passing the Sample's sample rate to
Engine::framesPerTick.

I also changed sampleDuration to return a std::chrono::milliseconds
instead of an int so that the callers know what time interval
is being used.

* Simplify if condition in src/gui/FileBrowser.cpp

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Simplify if condition in src/core/SampleBuffer.cpp

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Update style in include/Oscillator.h

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Format src/core/SampleDecoder.cpp

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Set the sample rate to be that of the AudioEngine by default

I also removed some checks involving the state of the SampleBuffer.
These functions should expect a valid SampleBuffer each time.
This helps to simplify things since we don't have to validate it
in each function.

* Set single-argument constructors in Sample and SampleBuffer to be explicit

* Do not make a copy when reading result from the decoder

* Add constructor to pass in vector of sampleFrame's directly

* Do a pass by value and move in SampleBuffer.cpp

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Pass vector by value in SampleBuffer.h

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Make Sample(std::shared_ptr) constructor explicit

* Properly draw sample waveform when reversed

* Collect sample not found errors when loading project

Also return empty buffers when trying to load
either an empty file or empty Base64 string

* Use std::make_unique<SampleBuffer> in SampleLoader

* Fix loop modes

* Limit sample duration to [start, end] and not the entire buffer

* Use structured binding to access buffer

* Check if GUI exists before displaying error

* Make Base64 constructor pass in the string instead

* Remove use of QByteArray::fromBase64Encoding

* Inline simple functions in SampleBuffer

* Dynamically include supported audio file types

* Remove redundant inline specifier

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Translate file types

* Cache calls to SampleDecoder::supportedAudioTypes

* Fix translations in SampleLoader (again)
Also ensure that all the file types are listed first.
Also simplified the generation of the list a bit.

* Store static local variable for supported audio types instead of in the header

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* Clamp frame index depending on loop mode

* Inline member functions of PlaybackState

* Do not collect errors in SampleLoader when loading projects

Also fix conflicts with surrounding codebase

* Default construct shared pointers to SampleBuffer

* Simplify and optimize Sample::visulaize()

* Remove redundant gui:: prefix

* Rearrange Sample::visualize after optimizations by DanielKauss

* Apply amplification when visualizing sample waveforms

* Set default min and max values to 1 and -1

* Treat waveform as mono signal when visualizing

* Ensure visualization works when framesPerPixel < 1

* Simplify Sample::visualize a bit more

* Fix CPU lag in Sample by using atomics (with relaxed ordering)

Changing any of the frame markers originally took a writer
lock on a mutex.

The problem is that Sample::play took a reader lock first before
executing. Because Sample::play has to wait on the writer, this
created a lot of lag and raised the CPU meter. The solution
would to be to use atomics instead.

* Fix errors from merge

* Fix broken LFO controller functionality

The shared_ptr should have been taken by reference.

* Remove TODO

* Update EnvelopeAndLfoView.cpp

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Update src/gui/clips/SampleClipView.cpp

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Update plugins/SlicerT/SlicerT.cpp

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Update plugins/SlicerT/SlicerT.cpp

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Store shortest relative path in SampleBuffer

* Tie up a few loose ends

* Use sample_rate_t when storing sample rate in SampleBuffer

* Add missing named requirement functions and aliases

* Use sampledata attribute when loading from Base64 in AFP

* Remove initializer for m_userWave in the constructor

* Do not use trailing return syntax when return is void

* Move decoder functionality into unnamed namespace

* Remove redundant gui:: prefix

* Use PathUtil::toAbsolute to simplify code in SampleLoader::openAudioFile

* Fix translations in SampleLoader::openAudioFile

Co-authored-by: DomClark <mrdomclark@gmail.com>

* Fix formatting for ternary operator

* Remove redundant inlines

* Resolve UB when decoding from Base64 data in SampleBuffer

* Fix up SampleClip constructors

* Add AudioResampler, a wrapper class around libsamplerate

The wrapper has only been applied to Sample::PlaybackState for now.
AudioResampler should be used by other classes in the future that do
resampling with libsamplerate.

* Move buffer when moving and simplify assignment functions in Sample

* Move Sample::visualize out of Sample and into the GUI namespace

* Initialize supportedAudioTypes in static lambda

* Return shared pointer from SampleLoader

* Create and use static empty SampleBuffer by default

* Fix header guard in SampleWaveform.h

* Remove use of src_clone
CI seems to have an old version of libsamplerate and does not have this method.

* Include memory header in SampleBuffer.h

* Remove mutex and shared_mutex includes in Sample.h

* Attempt to fix string operand error within AudioResampler

* Include string header in AudioResampler.cpp

* Add LMMS_EXPORT for SampleWaveform class declaration

* Add LMMS_EXPORT for AudioResampler class declaration

* Enforce returning std::shared_ptr<const SampleBuffer>

* Restrict the size of the memcpy to the destination size, not the source size

* Do not make resample const

AudioResampler::resample, while seemingly not changing the data of the resampler, still alters its internal state and therefore should not be const.
This is because libsamplerate manages state when
resampling.

* Initialize data.end_of_input

* Add trailing new lines

* Simplify AudioResampler interface

* Fix header guard prefix to LMMS_GUI instead of LMMS

* Remove Sample::resampleSampleRange

---------

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>
Co-authored-by: Daniel Kauss <daniel.kauss.serna@gmail.com>
Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>
Co-authored-by: DomClark <mrdomclark@gmail.com>

* New loop marker shortcuts, attempt 2 (#6382)

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>

* Add missing copyright in SampleDecoder.cpp (#7030)

* Use Qt layouts for mixer channels (#6591)

Use Qt layouts for the mixer channels. These changes will enable several other improvements, like for example making the mixer and faders resizable, adding peak indicators, etc.

This is a squash commit which consists of the following individual commits:

* Remove extra transparency in send/receive arrows
The extra transparency was conflicting with the positioning of
the arrows in the layout

* Begin reimplementing MixerChannelView

MixerChannelView is now a combination of the
MixerLine with the previous MixerChannelView

* Adjust SendButtonIndicator to use MixerChannelView

* Remove MixerLine
- Move MixerChannelView into src/gui

* Remove MixerView::MixerChannelView

* Remove header of MixerLine

* Change MixerView.h to use MixerChannelView
Change MixerView.h to use MixerChannelView rather than MixerLine
Also do some cleanup, such as removing an unused forward declaration
of QButtonGroup

* Create EffectRackView
+ Set height of sizeHint() using MIXER_CHANNEL_HEIGHT (287)

* Remove include of MixerLine
- Include MixerChannelView

* Phase 1: Adjust MixerView to use new MixerChannelView

* Move children wigets into header file

* Phase 2: Adjust MixerView to use new MixerChannelView

* Phase 3: Adjust MixerView to use new MixerChannelView

* Phase 4: Adjust MixerView to use new MixerChannelView

* Phase 5: Adjust MixerView to use new MixerChannelView

* Phase 5: Adjust MixerView to use new MixerChannelView

* Remove places where MixerChannelView is being deleted

Before, MixerChannelView was not inherited by QWidget,
meaning it could not have a parent and had to be deleted
when necessary. Since the MixerView owns the
new MixerChannelView, this is no longer necessary.

* Replace MixerLine with MixerChannelView
- Include MixerChannelView in MixerView

* Replace setCurrentMixerLine calls with setCurrentMixerChannel around codebase

* Add event handlers in MixerChannelView

* Implement MixerChannelView::eventFilter

* Update theme styles to use MixerChannelView

* Add QColor properties from style
- Set the Qt::WA_StyledBackground attribute on

* Add effect rack to rack layout when adding channel

* Set size for MixerChannelView
- Change nullptr to this for certain widgets
   - Some custom widgets may expect there to be a parent
- Add spacing in channel layout
- Increase size of mixer channel

* Retain size when widgets are hidden

* Implement paintEvent
- Rename states in SendReceiveState

* Implement send/receive arrow toggling
- Make maxTextHeight constexpr in elideName
- Remove background changing on mouse press
(is now handled in paintEvent)

* Implement renaming mixer channels

* Implement color functions

* Implement channel moving/removing functions

* Do some cleanup
Not sure if that connection with the mute model was needed, but removing
it did not seem to introduce any issues.

* Include cassert

* Replace references to MixerLine with MixerChannelView

* Reduce height
+ Make m_renameLineEdit transparent
+ Retain size when LCD is hidden
+ Remove stretch after renameLineEdit in layout

* Remove trailing whitespace

* Make m_renameLineEdit read only
+ Transpose m_renameLineEditView rectangle (with 5px offset)

* Set spacing in channel layout back to 0

* Remove sizeHint override and constant size

* Use sizeHint for mixerChannelSize
+ Leave auto fill background to false in MixerChannelView
+ Only set width for EffectRackView

* Set margins to 4 on all sides in MixerChannelView

* Move solo and mute closer to each other

Move the solo and mute buttons closer to each other in the mixer channels.

Technically this is accomplished by putting them into their own layout with minimal margins and spacing.

* Fixes for CodeFactor

* Code review changes

Mostly whitespace and formatting changes: remove tabs, remove spaces in parameter lists, remove underscores from parameter names.

Some lines have been shortened by introducing intermediate variables, e.g. in `MixerChannelView`.

`MixerView` has many changes but only related to whitespace. Spaces have been introduced for if and for statements. Whitespace at round braces has been removed everywhere in the implementation file even if a line was not touched by the intial changes.

Remove duplicate forward declaration of `MixerChannelView`.

* Adjust parameter order in MixerChannelView's constructor

Make the parent `QWidget` the first parameter as it is a Qt convention. The default parameter had to be removed due to this.

* Move styling of rename line edit into style sheets

Move the style of the `QGraphicsView` for the rename line edit from the code into the style sheets of the default and classic theme.

* More code review changes

Fix spaces between types and references/pointers, e.g. use `const QBrush& c` instead of `const QBrush & c`.
Remove underscores from parameter names.
Remove spaces near parentheses.
Replace tabs with spaces.
Introduce intermediate variable to resolve "hanging" + operator.
Replace the connection for the periodic fader updates with one that uses function pointers instead of `SIGNAL` and `SLOT`.

---------

Co-authored-by: Michael Gregorius <michael.gregorius.git@arcor.de>

* Fix minimum size of LADSPA dialogs (#6982) (#7019)

Remove the code which computes a minimum height for the LADSPA dialogs. It was intended to make sure that no scrollbar is shown in most cases. However, doing so came at the cost that the computed height was the minimum height as well. Therefore the dialogs took a lot of space on low-res displays and could not be made smaller.

After the removal the behavior is still sane. Small dialogs are shown in full and dialogs which are larger, e.g. "Calf Equalizer 12 Band LADSPA", seem to be sized around half the height of the workspace and show scrollbars.

* Extend TabWidget's style sheet options

# Extend TabWidget's style sheet options

Extend the `TabWidget` class so that the text color of the selected tab
can be set in the style sheet. Adjust the paint method to make use of
the new property.

Adjust the default style sheet as follows:
* Background color of the selected tab is the green of the knobs
* Text color of the selected tab is full on white

Adjust the classic style sheet in such a way that nothing changes, i.e.
the text colors of the selected tab and the other ones are the same.

# Code review style changes

Completely adjust the code style of TabWidget:
* Pointer/reference close to type
* Remove underscores from parameter names
* Remove spaces from parentheses
* Add space after if and for statements

# Remove repeated iterator dereferences

Remove repeated iterator dereferences by introducing variables with speaking names.

Fixes #6730.

* Fix equalizer peak updates (#7038)

Fix the peak update of the LMMS equalizer which accidentally used the same sample twice.

Simplify the `gain` method by removing repeated deferences which made the code hard to read.

Use `std::max` to update the peak values to the maximum of the buffer.

* Fix regressions from #6610 (#7040)

Fixes crashes when using decodeSampleOggVorbis, as well as missing supported audio formats

* Fix `Fader` pixmaps (#7041)

* Accept Editor close event (#7035)

* Handle divisions by 0 in Lb302 and Monstro (#7021)

* Handle divisions by 0 in Lb302

Handle potential division by 0 which in turn lead to floating point exceptions in Lb302. The division can occur in the `process` method if it is called with the member `vco_inc` still initialized to 0. This seems to happen then the Lb302 processes with no notes actually being played.

The offending call is `BandLimitedWave::pdToLen(vco_inc)` which is now prevented when the increment is 0. In the latter case a sample of 0 is produced. This works because in all other cases the Lb302 instance should process a note which in turn sets the increments to something different from 0.

* Fix FPEs in Monstro

Fix some floating point exceptions in `MonstroSynth::renderOutput` that occurred due to calling `BandLimitedWave::pdToLen` with a value of 0. This happened when one of the phase deltas was set to 0, i.e. `pd_l` or `pd_r`. These cases are now checked and produce silence in the corresponding components if the phase delta is set to 0.

* Fix uninitialized variables

Initialize the local variables `len_l` and `len_r` to 0. This fixes compiler warnings a la "error: 'len_r' may be used uninitialized in this function" which are treated as errors on the build server.

* Adjust whitespace of touched code

* Add setSampleRate to BasicFilters and update LOMM allpass sample rate (#7048)

* Fix crash on subsequent song loads (#7051)

Fix a crash that occurs when songs are loaded after each other. The crash only occurs if the "participating" songs have at least one other channel besides the master channel.

The crash occurred because the `MixerChannelViews` were not really deleted in `MixerView::refreshDisplay`. As a consequence the parent child relationship was still given between the `MixerView` and the `MixerChannelView`. When a song was loaded the event queue was evaluated which in turn resulted in the method `Fader::knobPosY` being called on a `Fader` which did not have a model anymore.

Fixes #7046.

* Run without terminal for non-debug Windows builds (#7022)

* set win32 flag

* added mingw too to win32 flag + dom's suggestions

* revert unnecessary changes i made.

* simplify msys linker flag condition

* removed extra whitespace

* whitespace change 2

* Fix scaling of PixmapButton in layouts (#7053)

* Abstraction in MixerChannelView (#7057)

Reduce code repetition in `MixerChannelView` by introducing methods that:
* Retrieve the `MixerChannel` that's associated with the view
* Check if the associated channel is the master channel

This abstracts some functionality and also reduces direct usage of the variable `m_channelIndex`.

* Fix sliding of waveform when drawing sample in reverse (#7063)

* Some minor UI fixes (#7044)

* fix ladspa browser description (unwanted line breaks), add missing labels to instruments' controls

* some more ui fixes

* translation context + style fixup

* Fix AFP playback indicator not shown when dragging and dropping samples (#7064)

The issue occurred because `AudioFileProcessorWaveView::updateSampleRange` was not being called in `AudioFileProcessorWaveView::dropEvent`.

* Resizable mixer window (#7037)

Make the mixer window resizable and allow the effects rack to grow. This enables the users a better overview of the effects used.

* Fix memory leaks within tests (#7001)

Caused by not calling `Engine::destroy`, as well as creating tracks/clips on the heap and not freeing the memory at the end of the test.

Instead of creating tracks/clips on the heap, they are now made on the stack, similar to how the surrounding tests create its objects.

* Fix mixer channel updates on solo/mute (#7055)

* Fix mixer channel updates on solo/mute

Fix the update of the mixer channels whenever a channel is soloed or muted.

The solution is rather "brutal" as it updates all mixer channel views when one of the models changes.

Introduce private helper method `MixerView::updateAllMixerChannels`. It calls the `update` method on each `MixerChannelView` so that they can get repainted.

Call `updateAllMixerChannels` at the end of the existing method `toggledSolo`.

Introduce a new method `MixerView::toggledMute` which is called whenever the mute model of a channel changes. It also updates all mixer channels.

Fixes #7054.

* Improve mixer channel update for mute events

Improve the mixer channel update for mute events by not delegating to `MixerView` like for the solo case. Instead the `MixerChannelView` now has its own `toggledMute` slot which simply updates the widget on changes to the mute model.

Also fix `MixerChannelView::setChannelIndex` by disconnecting from the signals of the previous mixer channel and connecting to the signals for the new one.

Remove `toggledMute` from `MixerView`.

The solo implementation is kept as is because it also triggers changes to the core model. So the chain seems to be:
* Solo button clicked in mixer channel view
* Button changes solo model
* Solo model signals to mixer view which was connected via the mixer channel view
* Mixer view performs changes in the other core models
* All mixer channels are updated.

Are better chain would first update the core models and then update the GUI from the changes:
* Solo button clicked in mixer channel view
* Button changes solo model
* Solo model signals to core mixer, i.e. not a view!
* Mixer view performs changes in the other core models
* Changed models emit signal to GUI elements

* Revert "Improve mixer channel update for mute events"

This reverts commit ede65963ea1d6131944679a131641c18b97bce0b.

* Add comment

After the revert done with commit the code is more consistent again but not in a good way. Hence a comment is added which indicates that an inprovement is needed.

* Abstract mixer retrieval

Abstract the retrieval of the mixer behind the new method `getMixer`. This is done in preparation for some dependency injection so that the `MixerView` does not have to ask the `Engine` for the mixer but gets it injected, a.k.a. the "Hollywood principle": "Don't call us, we'll call you."

It's called `getMixer` and not just mixer because it allows for locale variables to be called `mixer` without confusing it with the method.

* Let MixerView connect directly to models

Let the `MixerView` connect directly to the solo and mute models it is connected with.

Remove the connections that are made in `MixerChannelView` which acted as a proxy which only complicated things.

Add `connectToSoloAndMute` which connects the `MixerView` to the solo and mute models of a given channel. Call it whenever a new channel is created.

Add `disconnectFromSoloAndMute` which disconnects the `MixerView` from the solo and mute models of a given channel. Call it when a channel is deleted.

* Code cleanup

Cleanup code related to the creation of the master channel view.

* Inject the Mixer into the MixerView

Inject the `Mixer` into the `MixerView` via the constructor. This makes it more explicit that the `Mixer` is the model of the view. It also implements the "Dependency Inversion Principle" in that the `MIxerView` does not have to ask the `Engine` for the `Mixer` anymore.

The current changes should be safe in that the `Mixer` instance is static and does not change.

* Fix connections on song load

Disconnect and reconnect in `MixerView::refreshDisplay` which is called when a song is loaded.

* Migrate to CTest (#7062)

* Compressor plugin hideable controls (#7008)

* Fix most of the track cloning for MIDI ports (#7066)

Fixes most of the problem that the MIDI port information is not cloned when a track is cloned.

The bug was caused because cloning is implemented via serialization and deserialization of the Track. The previous code only serialized the MIDI port if `Engine::getSong()->isSavingProject()` is true. However, when we are cloning the statement will be `false` because we are not saving the song. Therefore the MIDI port's state was not saved and the clone was initialized with the default values.

The fix is to serialize the MIDI port in the following cases:
* We are not in song saving mode, i.e. we are in the state that this issue is about, e.g. cloning.
* We save a song and the MIDI connections are not discarded.

Using boolean algebra these conditions can be simplified as seen in the changed statement.

* Disentangle and clean the classes of the Audio File Processor (#7069)

## Extract views
Extract the classes `AudioFileProcessorWaveView` and `AudioFileProcessorView` into their own files.

Reformat the new classes by removing unnecessary whitespace and underscores. Add spaces to if-statements.

Cleanup the includes.

## Remove friend relationship
Remove the friend relationship between `AudioFileProcessor` and `AudioFileProcessorView`.

Introduce getters for entities that the view is interested in.

* Add private setters for "from" and "to" (#7071)

* Add private setters for "from" and "to"

Add private setters for the "from" and "to" values in `AudioFileProcessorWaveView`. When being used the setters will ensure that the bounds are respected.

Also add a `range` method because this computation was done repeatedly throughout the code.

Fixes #7068 but masks some of the original problems with the code that computes out-of-bounds values for "from" and "to" like the `slide` method. Problematic code can still be found by temporarily adding the following assertions to the setters:
* `assert (to <= m_sample->sampleSize());` in `setTo`
* `assert (from >= 0);` in `setFrom`

* Remove superfluous calls to qMax and qMin

* Sf2Player missing nullptr initialization (#7084)

* Update URL of tap-plugins submodule (#7087)

* Fix vcpkg dependencies (#7088)

* Center effects W/D knob at 0 (#7077)

Set the center value of the W/D amount knob as zero specifically in Effect.cpp

* Hide the LED button for "Custom Base Velocity" (#7067)

Hide the LED button for "Custom Base Velocity" as it did not have any other effect besides disabling the spinbox in the GUI. It did not affect any model which could be evaluated elsewhere.

## Technical details
Add functionality to show/hide the LED button to `GroupBox`. There's also a corresponding getter called `ledButtonShown`. The latter is evaluated in the following situations:
* Mouse clicks: if the LED button is hidden then the model is not toggled.
* Paining: The X position of the caption changes depending on whether the LED button is shown or not.

At a certain point the class `GroupBox` should be replaced by something that's implemented with layouts and which is used wherever it's possible.

* Fix deletion of mixer channels when calling MixerView::clear (#7081)

* Fix deletion of mixer channels when calling MixerView::clear

* clear channel 0 + delete from back to front

* Add `WANT_OSS` as a compile time option (#7099)

* Use GitHub mirror of tap-plugins (#7110)

* Fix playback within `Sample` (#7100)

This revisits two main aspects of playback from `Sample`: copying frames into a temporary playback buffer before resampling, and advancing the state's frame index.  Both operations were improperly done, causing distorted audio to be heard when resampling from within the `Sample::play` function.

To fix this, playback into the temporary playback buffer is now done using the `playRaw` function, which copies the frame one by one in a loop, moving through the buffer correctly as determined by the loop mode. In addition, advancement of the playback index is done using the `advance` function, which advances the index by the given amount and handles an out of bounds index for the loop modes as necessary using the modulo operator.

* Prevent out of bound read in `Sample::playRaw` (#7113)

* Fixes #6626: Throw if Lv2 object CTORs fail (#6951)

On plugin instantiation failure, `Lv2Proc::m_valid` was being set to false. However, `Lv2Proc::run` did not evaluate `m_valid` and still called `lilv_instance_run`, which caused undefined behavior, including crashes.

This bug fixes this by not even create such zombie classes, and instead `throw`s right away. The throws are caught in `lmms_plugin_main`, as suggested in the PR discussion and as the VST3 approach.

* Track resizing behavior (#7114)

* Simplify TrackLabelButton

Remove the dependency to `Instrument` and `InstrumentTrack` from `TrackLabelButton`. The icon of an `InstrumentTrackView` is now determined in the class `InstrumentTrackView` itself using the new static method `determinePixmap`. This enables the removal of the overridden `paintEvent` method from `TrackLabelButton`.

It was also attempted to keep a non-static member function version and to use the `InstrumentTrackView`'s model with a cast. However, this did not work because 'setModel' is executed as the very last step in the constructor, i.e. the model is not already set when `determinePixmap` is called. Pulling it to the top is too risky right now.

* Add helper method isInCompactMode

Add the helper method `isInCompactMode` which knows how to use the `ConfigManager` to determine if the option for compact track buttons is enabled. This removes duplicate code with intricate knowledge of the keys under which compact mode is stored.

* Set song as modified when track name changes

Extend `Track::setName` to set the song as modified whenever the track name changes. This moves the update into a core class and enables the removal of the dependencies to `Engine` and `Song` from the GUI class `TrackLabelButton`.

Also add a check if the name is really changed and only perform the actions if that's the case.

To make this work the implementation of `setName` had to be moved from the header into the implementation file.

* Keep instrument and sample content at top on resize

Keep the content of the instrument and sample track at the top of the widget if the widget is resized. This is also fixes a bug where the `TrackLabelButton` does not react anymore when the size is increased.

Technically the layout with the actual widgets is put into another vertical layout with a spacer that consumes all remaining space at the bottom.

* Vertical track resizing via mouse wheel

Enable to vertically resize tracks using the mouse wheel. Scrolling the mouse wheel over the track view with the control key pressed will increase/decrease the height by one pixel per wheel event. Pressing the shift key will increase/decrease in steps of five pixels.

Extract code that can be shared between the existing and the new way to resize the track into the private helper method `resizeToHeight`.

* Render beat pattern step buttons at the top

Render the step buttons of beat patterns at the top instead of at the bottom so that they stay aligned with the other elements to the left of them (buttons, knobs, etc).

Set the y offset to 4 so that the step buttons are vertically aligned with the other elements. The previous calculation lead to a minimum offset of 6 which always made the step buttons look misaligned.

Introduce the new static variable `BeatStepButtonOffset` which ensures that the rendering and the evaluation of mouse clicks do not go out of sync.

* Remove message location check from check-strings test (#7112)

* Ignore message location in check-strings test

* Support running test verification script on Windows

* Rewrite EffectSelectDialog to add effect groups and delete Qt Designer UI file (#7024)

* Update src/common/ to coding conventions (#7082)

* basic whitespace removal on RemoteBasePlugin.cpp (src/common/RemoteBasePlugin.cpp)

* Additional RPB. (src/common/RemotePluginBase.cpp).

* Additional RemotePluginBase.cpp cleaning (src/common/SharedMemory.cpp)

* something more

* Fix invalidated iterator when removing notes in Piano Roll (#7080)

* Fix invalidated iterator when removing notes in Piano Roll

* fixup - typo

* Add MidiClip::removeNote(iterator) function

* Use iterator version of remoteNote

* Fix parameter name

* Change variable name again

* Reenable song editor zooming (#7118)

Reenable zooming in the song editor by
* only resizing the track if the alt modifier is pressed and
* only accepting the wheel even if we in fact resize the track.

By doing so all other wheel events bubble up so that zooming and scrolling of the song editor can be handled by parent components.

The code that handles the retrieval of the delta value had to be adjusted as well. The reason is that pressing the alt key messes with the way that deltas are reported in the wheel event. The wheel event is interpreted as a horizontal scroll when alt is pressed.

* Fix crash in Audio File Processor (#7124)

* Fix crash in Audio File Processor

Fix a crash in the Audio File Processor that occurs when an Audio File Processor with a reversed sample is loaded from a save file and then the plugin window is opened.

The problem was caused by a call to `AudioFileProcessorWaveView::slideSampleByFrames` during the execution of constructor of `AudioFileProcessorWaveView`. In that situation the three `knob` members were all `nullptr` because for some reason there was an explicit setter for them which was only called after construction. This is fixed by passing and setting the knobs in the constructor.

Another question is if it's not a problem in the first place that the knobs are given to the `AudioFileProcessorWaveView` instead of their underlying models.

* Remove `MemoryManager` (#7128)

Removes `MemoryManager` and the use of rpmalloc in favor of the `new` and `delete` operators found in C++.

---------

Co-authored-by: Veratil <veratil@gmail.com>

* Apply master gain outside audio devices (#7135)

* Fix a NaN in EqSpectrumView (#7140)

Fix a `NaN` in `EqSpectrumView` that's caused by `m_peakSum` not being initialzed.

* Fix NaNs in basic filters and Equalizer (#7141)

Fix several NaNs in the context of the basic filters and the Equalizer.

The coefficients of the `BiQuad` were not initialized which seems to have led to NaNs down the line.

Fix a division by zero in `EqEffect::peakBand` and `EqSpectrumView::paintEvent` if the energy is zero.

The condition `m_peakSum <= 0` was removed from `EqSpectrumView::paintEvent` because the value is initialized to 0 down the line and later only potentially positive values are added.

* Update game-music-emu submodule

* Update to latest commit

* Fix missing icons for some instruments (#7132)

Revert some of the changes made in commit 88e0e94dcdb. The underlying idea was that the `InstrumentTrackView` should be responsible for assigning the icon that's shown by its `TrackLabelButton`. However, this does not work because in some cases the `InstrumentTrack` that's passed into `InstrumentTrackView::determinePixmap` does not have an `Instrument` assigned. This in turn seems to be caused due to initalizations that are running in parallel in different threads.

Here are the steps to reproduce the threading problem (line numbers refer to commit 360254f):
1. Set a break point in line 1054 of `InstrumentTrack`, i.e. the line in `InstrumentTrack::loadInstrument` where `m_instrument` is being assigned to.
2. Set a break point in `InstrumentTrackView::determinePixmap`, e.g. inside of the first if statement.
3. Drop an instance of "Sf2 Player" onto the Song Editor.
4. The first break point in `InstrumentTrack` is hit in a thread called "lmms::Instrumen" (shown like that in my debugger). Try to step over it.
5. The second break point in `InstrumentTrackView` now gets hit before the code is stepped over. This time we are in the thread called "lmms". I guess this is the GUI main thread.
6. Continue execution.

If you now switch to the application then the icon is shown. I guess the debugger is halted long enough in the main thread so that the InstrumentTrack gets an instrument assigned in another thread.

If you delete/disable the break point in `InstrumentTrack::determinePixmap` and follow the coarse steps above then the icon is not shown because the track has no instrument.

The current fix still delegates to the `InstrumentTrackView` to determine the pixmap in hopes that one day there will be a better solution where the parent component can be fully responsible for its child component.

Fixes #7116.

* Check if `m_peakSum` is less than or equal to 0 again (#7146)

* Replace QRegExp with QRegularExpression (#7133)

* replace QRegExp with QRegularExpression (find n replace)

* follow up to fix errors

* removed rpmalloc

* Fix compilation for qt5, to be fixed when qt6 fully supported.

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Added QtGlobal header for version finding fix

* Use the other syntax to try fix compilation.

* Check for 5.12 instead.

* Fix the header

* Attempt at fixing it further.

* Use version checks properly in header file

* Use QT_VERSION_CHECK macro in sources

* Apply suggestions from messmerd's review

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

---------

Co-authored-by: Kevin Zander <veratil@gmail.com>
Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Add GitHub issue forms for bug reports and feature requests (#7102)

* Update jack2 (#7147)

* Update carla (#7149)

* Set mixer channel LCD when its index changes (#7160)

* Move confirmation to remove mixer channels outside `MixerView::deleteChannel` #7154)

Moves the confirmation to remove mixer channels outside of `MixerView::deleteChannel` and into `MixerChannelView::removeChannel`. This is so that the confirmation only applies when the user uses the context menu to remove a channel, which is important since no confirmation should apply when, for example, the mixer view is cleared with calls to `MixerView::clear`.

* Bump project year (#7167)

* CMake: Replace EXEC_PROGRAM with execute_process (#7166)

* CMake: Replace EXEC_PROGRAM with execute_process

exec_program is Deprecated since version 3.0: Use the execute_process() command instead.

* Make Compressor background themeable (#7157)

* Enable different colors for oscilloscope channels (#7155)

Enable to set different colors for the oscilloscope channels:
* Left channel color
* Right channel color
* Color of all other channels

The clipping color is now used per channel, i.e. if the left channel clips but the right does not then only the signal of the left channel is painted in the clipping color.

Enable setting the colors in the style sheets and adjust the style sheets of the default and classic theme accordingly.

* Refactor `gui_templates.h` (#7159)

* remove the gui_templates header where functions not used

* refactor and replace template with function argument

* refactor: merge pointSizeF function with pointSize

* removed template per michael's review

* use std::max for more readability

* remove the QDesktopWidget header

* cleanup arguments and remove parentheses from return

* Fix DynamicsProcessor bugs (#7168)

* Fix glitch in SlicerT (#7174)

* Revisit the initialization for local variables (#7143)

* clang-tidy: Apply cppcoreguidelines-init-variables everywhere (treating NaNs as zeros)

* Initialize msec and tick outside switch

* Update plugins/Vestige/Vestige.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/Vestige/Vestige.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/Vestige/Vestige.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/VstEffect/VstEffectControls.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/VstEffect/VstEffectControls.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/VstEffect/VstEffectControls.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Use initialization with =

* Use tabs

* Use static_cast

* Update DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Do not use tabs for alignment in src/core/DrumSynth.cpp

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Move x variable inside loop

* Use ternary operator for b variable

* Revert "Use tabs"

This reverts commit 07afd8a83f58b539c3673310b2aad4b63c9198a0.

* Remove unnecessary variables in XpressiveView

* Simplify initialization in Plugin

* Combine declaration and initialization in EqCurve

* Combine declaration and initialization in Song

* Combine declaration and initialization in AudioAlsa

* Combine declaration and initialization in EqCurve (again)

* Missed some

* Undo changes made to non-LMMS files

* Undo indentation changes in SidInstrument.cpp

* Combine declaration with assignment in IoHelper

* Combine declaration with assignment using auto in Carla

* Combine declaration with assignment

* Combine declaration with assignment in BasicFilters

* Simplify assignments in AudioFileProcessorWaveView::zoom

* Simplify out sample variable in BitInvader

* Remove sampleLength variable in DelayEffect

* Move gain variable in DynamicsProcessor

* Combine peak variable declaration with assignment in EqSpectrumView

* Move left/right lfo variables in for loop in FlangerEffect

* Use ternary operator for group variable in LadspaControlDialog

* Combine declaration with assignment in Lb302

* Combine declaration with assignment in MidiExport

* Combine declaration with assignment in MidiFile

* Combine declaration with assignment in MidiImport

* Use ternary operator for vel_adjusted variable in OpulenZ

* Move tmpL and dcblkL variables in for loop in ReverbSC

* Combine declaration with initialization in SlicerT

* Combine declaration with assignment in SaSpectrumView

* Combine declaration with assignment in SaWaterfallView

* Combine declaration with assignment in StereoEnhancerEffect

* Combine declaration with assignment in VibratingString

* Combine declaration with assignment in VstEffectControls

* Combine declaration with assignment in Xpressive

* Combine declaration with assignment in AutomatableModel

* Combine declaration with assignment in AutomationClip

* Move sample variable in for loop in BandLimitedWave

* Combine declaration with assignment in DataFile

* Combine declaration with assignment in DrumSynth

* Combine declaration with assignment in Effect

* Remove redundant assignment to nphsLeft in InstrumentPlayHandle

* Combine declaration with assignment in LadspaManager

* Combine declaration with assignment in LinkedModelGroups

* Combine declaration with assignment in MemoryHelper

* Combine declaration with assignment in AudioAlsa

* Combine declaration with assignment in AudioFileOgg

* Combine declaration with assignment in AudioPortAudio

* Combine declaration with assignment in AudioSoundIo

* Combine declaration with assignment in Lv2Evbuf

* Combine declaration with assignment in Lv2Proc

* Combine declaration with assignment in main

* Combine declaration with assignment in MidiAlsaRaw

* Combine declaration with assignment in MidiAlsaSeq

* Combine declaration with assignment in MidiController

* Combine declaration with assignment in MidiJack

* Combine declaration with assignment in MidiSndio

* Combine declaration with assignment in ControlLayout

* Combine declaration with assignment in MainWindow

* Combine declaration with assignment in ProjectNotes

* Use ternary operator for nextValue variable in AutomationClipView

* Combine declaration with assignment in AutomationEditor

* Move length variable in for-loop in PianoRoll

* Combine declaration with assignment in ControllerConnectionDialog

* Combine declaration with assignment in Graph

* Combine declaration with assignment in LcdFloatSpinBox

* Combine declaration with assignment in TimeDisplayWidget

* Remove currentNote variable in InstrumentTrack

* Combine declaration with assignment in DrumSynth (again)

* Use ternary operator for factor variable in BitInvader

* Use ternary operator for highestBandwich variable in EqCurve

Bandwich?

* Move sum variable into for loop in Graph

* Fix format in MidiSndio

* Fixup a few more

* Cleanup error variables

* Use ternary operators and combine declaration with initialization

* Combine declaration with initialization

* Update plugins/LadspaEffect/LadspaControlDialog.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/OpulenZ/OpulenZ.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update plugins/SpectrumAnalyzer/SaProcessor.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/midi/MidiAlsaRaw.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/gui/MainWindow.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/gui/clips/AutomationClipView.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/gui/editors/AutomationEditor.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/gui/widgets/Fader.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Move static_cast conversion into separate variable

* Use real index when interpolating

* Remove empty line

* Make helpBtn a private member

* Move controller type into separate variable

* Fix format of DrumSynth::waveform function

* Use tabs and static_cast

* Remove redundant if branch

* Refactor using static_cast/reinterpret_cast

* Add std namespace prefix

* Store repeated conditional into boolean variable

* Cast to int before assigning to m_currentLength

* Rename note_frames to noteFrames

* Update src/core/Controller.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/core/DrumSynth.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Update src/gui/widgets/Graph.cpp

Co-authored-by: Kevin Zander <veratil@gmail.com>

* Revert changes that initialized variables redudantly

For situations where the initialization is
more complex or passed into a function
by a pointer, we dont need to do
initialization ourselves since it is
already done for us, just in a different way.

* Remove redundant err variable

* Remove explicit check of err variable

* Clean up changes and address review

* Do not initialize to 0/nullptr when not needed

* Wrap condition in parentheses for readability

---------

Co-authored-by: Kevin Zander <veratil@gmail.com>
Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

* Fix infinite loop in `InstrumentPlayHandle::play` (#7176)

* Fix B/B Track Label Button (#7177)

* Fix Pattern Track Label Button

* PianoRoll::paintEvent: don't shadow member variable (#7181)

* Optimize map accesses in LadspaManager (#7173)

Some methods in `LadspaManager` performed repeated searches through the map by first calling `contains` and then by actually fetching the entry. This is fixed by using `find` on the map. It returns an iterator which can directly provide the result or indicate that nothing was found. That way the map is only searched once.

* Conditionally remove use of QApplication::desktop in ComboBox.cpp (#7179)

Prepare the application for Qt6 by conditionally removing the use of `QApplication::desktop` in `ComboBox.cpp`. The method was already deprecated in Qt5 and is removed in Qt6.

Instead the method `QWidget::screen` is used now if the Qt version is equal to or newer than 5.14 (because the method was only introduced with that version). Fall back to `QApplication::desktop` if an older version is detected during the build. This is for example the case for the CI builds which are based on Ubuntu 18.04.

Generalize the check if the menu can be shown in the screen. The code now also does handles the case where the menu would go out of screen along the X axis.

Also remove the unused include `embed.h`.

* Ensure that build and target are directories in .gitignore (#6884)

Ensure that `build` and `target` are directories in `.gitignore`.

* Only repaint LcdWidget if necessary (#7187)

Some analysis done with Callgrind showed that the `LcdWidget` spends quite some time repainting itself even when nothing has changed. Some widget instances are used in song update contexts where `LcdWidget::setValue` is called 60 times per second.

This commit fixes the problem by only updating the `LcdWidget` if its value really has changed.

Adjust the condition in the if-clause so that it becomes clearer what's the interval of interest.

* Remove "GENERAL SETTINGS" tab (#7188)

Remove the "GENERAL SETTINGS" tab from the instrument track and sample track window.

This removes clutter as well as dependencies to the non-scalable `TabWidget`.

Remove superfluous calls to `setSpacing` which have set the default value of 6.

Use the default content margins of (9, 9, 9, 9) instead of (8, 8, 8, 8).

* Refactor ArpDirection::Down and ArpDirection::DownAndUp (#7007)

Fixes an issue with the arpeggiation when it is set to go downwards and the cycle is over 0.

* Font size adjustments (#7185)

Adjust and rename the function `pointSize` so that it sets the font size in pixels. Rename `pointSize` to `adjustedToPixelSize` because that's what it does now. It returns a font adjusted to a given pixel size. Rename `fontPointer` to `font` because it's not a pointer but a copy. Rename `fontSize` to simply `size`.

This works if the intended model is that users use global fractional scaling. In that case pixel sized fonts are also scaled so that they should stay legible for different screen sizes and pixel densities.

## Adjust plugins with regards to adjustedToPixelSize

Adjust the plugins with regards to the use of `adjustedToPixelSize`.

Remove the explicit setting of the font size of combo boxes in the following places to make the combo boxes consistent:
* `AudioFileProcessorView.cpp`
* `DualFilterControlDialog.cpp`
* `Monstro.cpp` (does not even seem to use text)
* `Mallets.cpp`

Remove calls to `adjustedToPixelSize` in the following places because they can deal with different font sizes:
* `LadspaBrowser.cpp`

Set an explicit point sized font size for the "Show GUI" button in `ZynAddSubFx.cpp`

Increase the font size of the buttons in the Vestige plugin and reduce code repetition by introducing a single variable for the font size.

I was not able to find out where the font in `VstEffectControlDialog.cpp` is shown. So it is left as is for now.

## Adjust the font sizes in the area of GUI editors and instruments.

Increase the font size to 10 pixels in the following places:
* Effect view: "Controls" button and the display of the effect name at the bottom
* Automation editor: Min and max value display to the left of the editor
* InstrumentFunctionViews: Labels "Chord:", "Direction:" and "Mode:"
* InstrumentMidiIOView: Message display "Specify the velocity normalization base for MIDI-based instruments at 100% note velocity."
* InstrumentSoundShapingView: Message display "Envelopes, LFOs and filters are not supported by the current instrument."
* InstrumentTuningView: Message display "Enables the use of global transposition"

Increase the font size to 12 pixels in the mixer channel view, i.e. the display of the channel name.

Render messages in system font size in the following areas because there should be enough space for almost all sizes:
* Automation editor: Message display "Please open an automation clip by double-clicking on it!"
* Piano roll: Message display "Please open a clip by double-clicking on it!"

Use the application font for the line edit that can be used to change the instrument name.

Remove overrides which explicitly set the font size for LED check boxes in:
* EnvelopeAndLfoView: Labels "FREQ x 100" and "MODULATE ENV AMOUNT"

Remove overrides which explicitly set the font size for combo boxes in:
* InstrumentSoundShapingView: Filter combo box

## Adjust font sizes in widgets

Adjust the font sizes in the area of the custom GUI widgets.

Increase and unify the pixel font size to 10 pixels in the following classes:
* `ComboBox`
* `GroupBox`
* `Knob`
* `LcdFloatSpinBox`
* `LcdWidget`
* `LedCheckBox`
* `Oscilloscope`: Display of "Click to enable"
* `TabWidget`

Shorten the text in `EnvelopeAndLfoView` from "MODULATE ENV AMOUNT" to "MOD ENV AMOUNT" to make it fit with the new font size of `LedCheckBox`.

Remove the setting of the font size in pixels from `MeterDialog` because it's displayed in a layout and can accommodate all font sizes. Note: the dialog can be triggered from a LADSPA plugin with tempo sync, e.g. "Allpass delay line". Right click on the time parameter and select "Tempo Sync > Custom..." from the context menu.

Remove the setting of the font size in `TabBar` as none of the added `TabButton` instances displays text in the first place.

Remove the setting of the font size in `TabWidget::addTab` because the font size is already set in the constructor. It would be an unexpected size effect of setting a tab anyway. Remove a duplicate call to setting the font size in `TabWidget::paintEvent`.

Remove unnecessary includes of `gui_templates.h` wherever this is possible now.

## Direct use of setPixelSize

Directly use `setPixelSize` when drawing the "Note Velocity" and "Note Panning" strings as they will likely never be drawn using point sizes.

* Use layouts for the instrument sound shaping tab / Extract classes for Envelope and LFO graphs (#7193)

Move the envelope and LFO graphs into their own classes.

Besides the improved code organization this step had to be done to be able to use layouts in `EnvelopeAndLfoView`. The class previously had fixed layouts mixed with custom rendering in the paint event. Mouse events are now also handled in both new classes instead of in `EnvelopeAndLfoView`.

## Layouts in EnvelopeAndLfoView
Use layouts to align the elements of the `EnvelopeAndLfoView`. This removes lots of hard-coded values. Add helper lambdas for the repeated creation of `Knob` and `PixmapButton` instances.

The spacing that is explicitly introduced between the envelope and LFO should be removed once there is a more open layout.

## Layouts for InstrumentSoundShapingView
Use layouts to align the elements of the `InstrumentSoundShapingView`.

## Info text improvements in LFO graph
Draw the info text at around 20% of the LFO graph's height. This prepares the dialog to be scaled later.

Write "1000 ms/LFO" instead of "ms/LFO: 1000" with a larger gap.

## Accessors for EnvelopeAndLfoParameters
Make the enum `LfoShape` in `EnvelopeAndLfoParameters` public so that it can be used without friend declarations. Add accessor methods for the model of the LFO.

## Other improvements
* Adjust include orders
* Variable initialization in headers
* Prevention of most vexing parses

* Scalable consistent faders with themeable gradients, marker at unity, dbFS by default (#7045)

* Render fader levels in code with a gradient

Render the fader level in code using a gradient instead of using pixmaps. The problem with the pixmaps is that they don't "know" how a fader instance is configured with regards to the minimum and maximum value. This means that the display can give quite a wrong impression.

The rendering of levels has been unified in the method `paintLevels`. It can render using dbFS and linear scale. The me…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review A functional code review is currently required for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants