Audiomixerboard: Auto-reduce size when switching between meter styles

[hoffie]

Short description of changes

There are two different base meter styles (LED vs. bars) with multiple variants each (regular, slim, round). The base meter styles are implmeneted as widgets and are always part of a QStackedLayout. The selected meter style is shown via setCurrentWidget(). This hides the inactive widget, but the inactive widget is still there and is part of QStackedLayout's size calculations which always uses the maximum size over all widgets, not just the currently visible one.

This leads to problems when the currently visible meter widget is narrower than the hidden counterpart. In that case, the QStackedLayout is wider than expected.

Technically, the following transitions show this issue:

• Select Bar, then Small LEDs (or start with Small LEDs), or
• Select LEDs, then Narrow Bar (or start with Narrow Bar)

However, the latter case already had a workaround (choosing Narrow Bar silently adjusted details of the LED meter as well).

The proper fix is using a subclass of QStackedLayout which calculates its sizeHint solely based on the currently visible widget.
This fixes the Bar->Small LEDs transition and makes the LEDs->Narrow Bar workaround redundant, which has therefore been removed.

Context: Fixes an issue?

Fixes #2351
Initially reported here: #1688 (comment)

Does this change need documentation? What needs to be documented and how?

No ChangeLog entry as the bug was not part of a release. The PR adds itself to the existing ChangeLog entry.
CHANGELOG:SKIP

Status of this Pull Request

Ready for testing/review

What is missing until this pull request can be merged?

• Confirm that it fixes the bug (@pgScorpio)
• Confirm correctness with someone familiar with the initial PR (@henkdegroot?)

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

[hoffie]

Something like this explicitly sets a border similar to what it looks like for me for the Bar style, but I'm not sure it would be a good idea as it probabl overrides default operating styles:

diff --git a/src/levelmeter.cpp b/src/levelmeter.cpp
index 1244daf3..4658d1b3 100644
--- a/src/levelmeter.cpp
+++ b/src/levelmeter.cpp
@@ -173,6 +173,7 @@ void CLevelMeter::SetBarMeterStyleAndClipStatus ( const ELevelMeterType eNType,
         if ( bIsClip )
         {
             pBarMeter->setStyleSheet ( "QProgressBar        { border:        2px solid red;"
+                                       "                      border-radius: 4px;"
                                        "                      margin:        1px;"
                                        "                      padding:       1px;"
                                        "                      width:         15px;"
@@ -181,7 +182,9 @@ void CLevelMeter::SetBarMeterStyleAndClipStatus ( const ELevelMeterType eNType,
         }
         else
         {
-            pBarMeter->setStyleSheet ( "QProgressBar        { margin:     1px;"
+            pBarMeter->setStyleSheet ( "QProgressBar        { border:        2px solid #ccc;"
+                                       "                      border-radius: 4px;"
+                                       "                      margin:        1px;"
                                        "                      padding:       1px;"
                                        "                      width:         15px; }"
                                        "QProgressBar::chunk { background:    green; }" );

Also, based on my CSS understanding, I don't know why it would be necessary at all. But maybe Qt is specific or I'm missing some detail.

[henkdegroot]

I believe that the stylesheet is not reset to a default or that it remembers what the previous setting was, as if it remove all previous set values. Think it needs explicit to be set again when changing.

Round LEDs and Small LEDs should probably use the narrow meter style.

Round LEDs can be selected in any skin mode. So don't think they should always use the "narrow" style, but think in "Compact" mode it could.

[henkdegroot]

Small LEDs now seem to work as how I originally would have like to implement it. Nice work @hoffie and thanks for fixing (tested with the build generate from this PR).

So at least we have an option to keep the smallest space for the fader when switching between narrow bar and small leds.

Other styles still changes the width, but to be honest I would not call that a bug. It is done by "design".

[hoffie]

I believe that the stylesheet is not reset to a default or that it remembers what the previous setting was, as if it remove all previous set values. Think it needs explicit to be set again when changing.

I suspected something like this, but found contrary information here: https://www.qtcentre.org/threads/38841-How-to-reset-a-stylesheet-to-default?p=178450#post178450 (just a forum thread though, so might be inaccurate).

Do you have an idea what to set border to (see my patch from the comment above)?

Round LEDs and Small LEDs should probably use the narrow meter style.

Round LEDs can be selected in any skin mode. So don't think they should always use the "narrow" style, but think in "Compact" mode it could.

Ok, I'll look into it.

[henkdegroot]

Do you have an idea what to set border

I have seen really odd behaviours sometimes it can be fixed by explicit setting the border option to no. But then I have also seen that, when doing it, is suddenly inherits some style/spacing from other item it is placed in.

[pljones]

No ChangeLog entry as the bug was not part of a release:
CHANGELOG:SKIP

Pardon? Do you mean it's a fix to a bug introduced within the release cycle? That means tagging the new feature with this PR as well, then.

[hoffie]

Round LEDs can be selected in any skin mode. So don't think they should always use the "narrow" style, but think in "Compact" mode it could.

Hmm, ok. I've moved Round LEDs back to the regular code path.

I think I've found the real reason for the size changes: The stacked layout seems to take the width of the widest member. I'm currently looking for ways around that.

[henkdegroot]

I remember now I that I tried to only "modify" the meterstyle when changing the appearance and not also updating the "skin" style settings. It seems almost not possible to not have a link between them to keep a consistent/view layout.

[hoffie]

Short update: I still think that the QStackedLayout behavior is the issue. I'm currently experimenting with the following:

• Subclassing QStackedLayout to only consider the width of the visible widget
• Restricting the maximum size of the hidden meter
• Trying different SizeConstraints
• Removing the hidden meter from the layout while not in use

I think the following is code which only exists due to this exact bug and I don't think it is a nice solution:
https://github.com/jamulussoftware/jamulus/blob/master/src/levelmeter.cpp#L132-L136 (introduced in cf7a01d)

Edit: Oh and BTW, what I did yesterday was the same category of change. I think it's wrong to do it that way, now that I understand it more deeply.

[hoffie]

I think I now understand the issue and have a fix which looks proper and seems to work. I've updated the PR along with its description.
@henkdegroot @pgScorpio: Can you please re-check?

@pljones

Do you mean it's a fix to a bug introduced within the release cycle? That means tagging the new feature with this PR as well, then.

Yep, I've added the PR to the existing ChangeLog entry.

[softins]

I think this is OK. On my Pi, I've compiled and compared this commit with its parent commit, and the sizing is more consistent with this PR. When changing meter modes, I only observed any changes in the overall width of a channel when in Compact mode. In other modes, the channel width was determined by the name box, and was not affected by changes to the LED/Bar.

[pljones]

That looks like a really nice fix! (Not tested yet.)

[pgScorpio]

@hoffie

@pgScorpio: Can you please re-check?

How to?? (assets?)
I cloned your repo, but there seems nothing changed,

[softins]

How to?? (assets?) I cloned your repo, but there seems nothing changed,

You likely still need to do git checkout levelmeter-small-led-width-fix before building. Or try the assets here

[pgScorpio]

Or try the assets here

This one look OK to me!

[henkdegroot]

Seem working fine to me as well. Nice work!