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

Update mute and solo buttons, add them to instrument windows #7708

Merged
merged 16 commits into from
Mar 11, 2025

Conversation

rubiefawn
Copy link
Contributor

@rubiefawn rubiefawn commented Feb 13, 2025

Updates mute/solo buttons with new assets (slashed speaker icon, headphone icon) to better communicate their purpose, since the current LED assets can be ambiguous without labels. Additionally, resolves #2685 by adding mute and solo controls to instrument track windows and sample track windows next to the volume knob.

  • todo: Replace icons sourced from twbs/icons with new CC0-licensed icons from @StakeoutPunch
  • todo: Rewrite the SVGs by hand to remove Inkscape metadata and move CSS styles to attributes (LMMS won't render the inline CSS)
  • todo (non-blocking): Add assets for classic theme
  • todo (non-blocking): Adjust SVG icons to be pixel perfect

image

Colors:

red stroke  : oklch(40% 0.16 20)
red fill    : oklch(55% 0.22 20)
blue stroke : oklch(46% 0.10 238)
blue fill   : oklch(64% 0.13 238)

@sakertooth
Copy link
Contributor

From the screenshot you provided, it looks like the mute and solo buttons are being merged into the same space as that of the volume knob. I'm thinking having a separate section for the mute and solo buttons (with a label like M/S) might be better.

@rubiefawn
Copy link
Contributor Author

Here's what that looks like:
image

@sakertooth
Copy link
Contributor

Here's what that looks like: image

That looks a lot better IMO!

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good to me. Will test and wait for CI before approving.

Co-authored-by: Sotonye Atemie <[email protected]>
@regulus79
Copy link
Contributor

To me the knobs feel a bit crowded, almost like the pitch range lcd should be moved closer to the channel lcd, idk

@sakertooth
Copy link
Contributor

sakertooth commented Feb 14, 2025

To me the knobs feel a bit crowded, almost like the pitch range lcd should be moved closer to the channel lcd, idk

I think there was some separation already added before this PR, but since we need that space now it's probably okay to remove.

@rubiefawn
Copy link
Contributor Author

rubiefawn commented Feb 14, 2025

If we're going to address the lack of real estate in this PR, I would like to move the oversized preset save button to the left of the preset name, and make it the same size as the previous/next instrument buttons.

Wait, do we really need the previous/next instrument buttons in the first place? I don't think I've ever used them, since I just click on the specific instrument I want to edit from the track view.

Here's what it could look like if the previous/next buttons were replaced by the save button (this one is a hasty photoshop job)
413423609-1df92737-76d4-4f8f-ab23-e67839dbadb7(1)

- Change label from "M/S" to "M&S", as to not be confused with mid/side
- Adjust layout to preserve space between panning and pitch bend controls
@rubiefawn
Copy link
Contributor Author

rubiefawn commented Feb 19, 2025

After some discussion in the Discord, the save button size and other such layout issues not directly related to the addition of these buttons will be addressed elsewhere.

After some more discussion in the Discord, there may be some other changes after all.

@rubiefawn rubiefawn marked this pull request as draft February 24, 2025 21:27
@rubiefawn rubiefawn changed the title Add mute and solo buttons to instrument windows Update mute and solo buttons, add them to instrument windows Feb 27, 2025
Since SVGs can be used directly
- Updates the track mute/solo buttons
- Makes the compact track view slightly larger to fit new assets (sorry)
- Update mixer mute/solo buttons
- Add mute/solo buttons to sample track window to match instruments
@rubiefawn rubiefawn marked this pull request as ready for review February 27, 2025 21:29
@AW1534
Copy link
Contributor

AW1534 commented Mar 1, 2025

The icons are missing on my windows system (msvc)
image
And in compact mode for some reason the track labels are missing
image

@rubiefawn
Copy link
Contributor Author

rubiefawn commented Mar 2, 2025

Good catch! I am able to replicate it as well using the CI build artifacts on my Windows system. Running it from the console reveals the following output:

Error loading icon pixmap "mute_active": "File not found"
Error loading icon pixmap "mute_inactive": "File not found"
Error loading icon pixmap "solo_active": "File not found"
Error loading icon pixmap "solo_inactive": "File not found"

Spectrum Analyzer's SVG controls show up fine, so this is likely a problem with my code. I'll reference Spectrum Analyzer to see how it should be done.

Edit: It looks like the new assets were just not being included in the installer at all.

Also, track labels disappear in compact mode, that's part of how it becomes compact. That is intended behavior. You can still open the track window by clicking on the icon instead.

@rubiefawn
Copy link
Contributor Author

@AW1534 That should be fixed, it was an issue with SVG theme resources not getting packaged into the installer. Can you test it again?

@bratpeki
Copy link
Member

bratpeki commented Mar 2, 2025

And in compact mode for some reason the track labels are missing

That's how compact mode works

@rubiefawn rubiefawn requested a review from sakertooth March 2, 2025 09:12
@AW1534
Copy link
Contributor

AW1534 commented Mar 2, 2025

image
image
Looks great!

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if its just my system acting odd, but can you check the Greppi demo for me real quick? I had an issue where I couldn't mute the mixer channels in the project ranging from [1, 7]. Both regular mode and compacted mode had this issue. The solo button works fine however.

@sakertooth
Copy link
Contributor

sakertooth commented Mar 2, 2025

I don't know if its just my system acting odd, but can you check the Greppi demo for me real quick? I had an issue where I couldn't mute the mixer channels in the project ranging from [1, 7]. Both regular mode and compacted mode had this issue. The solo button works fine however.

Okay, so it might be an issue elsewhere because master has the same problem.

Edit: Nevermind, the mute buttons were connected to MIDI 🤯

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it in regular mode and compacted mode. The PR works fine AFAICT.

Copy link
Member

@tresf tresf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested:

  • PASS: default theme
  • PASS: classic theme
  • PASS: default + compact
  • PASS: classic + compact

Due to the difference in colors between some mixer channels, the instrument window and the song editor, it's a near impossible feat to get a "button" that looks OK in all of these.

By switching the assets to transparent in the latest commit (screenshot), I think this is the closet this theme can get to having a single shared svg to work this well on all of the various background colors.

Moving forward, I have a feeling that as @Gabrielxd195 has pointed out that the color and brightness of the "ON" buttons is likely to be continually scrutinized. Although I personally prefer them to be dimmer like @RoxasKH's mockup, I feel this minor change will sort itself out over time, so I'm approving this PR as it stands.

@Gabrielxd195
Copy link

I was looking at how Ardour and Qtractor have their "Solo" and "Volume" buttons. And I thought that if we apply this to LMMS, it would be to leave all the tracks in "ON" with the buttons in white with their respective icons (the speaker and the headphone), if we leave a track in "Solo" its "headphone" icon will be painted green; while the other buttons of the tracks in "Mute" will have the speaker icon in red, but that green and red would be as I showed them in my edition of the previous mockup.

The only difference would be that we will no longer have all the buttons in green as we currently have in LMMS, but with the implementation of this PR, it would not be so confusing for users because there will be a change in the design of those buttons and it will be more intuitive and similar to other DAWs. Personally I don't see so many complications in this, I think (if I'm not mistaken) that we are in time to correct it. (Google Translate).

@tresf
Copy link
Member

tresf commented Mar 7, 2025

we are in time to correct it.

Correct what? The color? This point was already made in your previous post. Green is a primary color in LMMS. Solo should stand out as an outlier. This is how GarageBand does it too, they use yellow. If you compare 100 DAWs, you'll get 100 different color combos.💚

(Not to mention that green looks a bit too much like the app icon)

@Gabrielxd195
Copy link

In each DAW the buttons are given a color according to their color palette, in this case in LMMS it is green, white, gray and dark gray, but in this change I see that the chosen colors (pink for the "OFF" button and a turquoise blue for the "Solo" button) do not match at all with the rest of the buttons in the LMMS interface, nor is it that the colors Green and Red are seen everywhere in the interface, therefore, the green that I showed in my mockup is only in that single "Solo" button, it is not that it is painted everywhere. And I do not see any problem with it resembling the application button, that is irrelevant. Greetings

@Monospace-V
Copy link
Contributor

but in this change I see that the chosen colors (pink for the "OFF" button and a turquoise blue for the "Solo" button) do not match at all with the rest of the buttons in the LMMS interface, nor is it that the colors Green and Red are seen everywhere in the interface, therefore, the green that I showed in my mockup is only in that single "Solo" button, it is not that it is painted everywhere.

If the mute and solo matched with the colour palette, it is possible that a person using mute or solo may not realise it since everything is coloured the same way.
Mute and solo can change the way your entire project sounds by making instrument tracks silent. So, they have a very big effect.
I am trying to say that by having very unusual colours for mute and solo, we make sure that people see that they are enabled, and they don't blend into the rest of the colour palette.

Also, the lmms application icon is not based off the solo, so it will look strange for the solo icon to be like the application icon but slightly different shaped. It is not a good idea to have buttons that look similar to the lmms icon unless they have functions that are by themselves about LMMS.

@Gabrielxd195
Copy link

@Monospace-V No bro, that's why I highlighted Red and Green in my mockup. Also, if you look closely, in many apps and devices, red is used as something that is disabled and green as enabled, precisely because these two colors are closely related to turning things on and off, they are the most suitable colors and well combined with the LMMS palette to let users know when a track is on, off or soloed.

Also, you yourself say that the LMMS button is not based on the newly created solo button, because both designs look very different: the one in the app looks like isometric square headphones; while the solo button looks like more rounded headphones. I don't think this is a big problem.

@Gabrielxd195
Copy link

I made another edition of the mockup, but this time with the "Solo" button in amber yellow, and leaving the same scarlet red as in my previous mockup for the "OFF" button. In this edition, green will not be used to avoid the resemblance of the solo button to the LMMS application icon.
Botones de Volumen y Solo mal coloreados - Botones amarillos
This yellow visually does not break the LMMS color palette and is better than the pink and blue chosen, and in Ardour the solo button is yellow.

@bratpeki bratpeki self-assigned this Mar 7, 2025
@tresf
Copy link
Member

tresf commented Mar 7, 2025

I think that the chosen colors do not correspond with the color palette of the classic LMMS theme [...]

Please provide a mockup specific to the classic theme if you have a better highlight color proposal.

[...] break the LMMS color palette

We're actively discussing this on Discord and I think that the existing theme colors are being broken. Here's a side-by-side.

image image

(although these are deliberately intended to grab attention, so they may need to break the theme regardless of the palette)

This yellow visually does not break the LMMS color palette and is better than the pink and blue chosen, and in Ardour the solo button is yellow.

I think calling things "yellow" and "pink" are not specific enough, so let's work directly from the palette we have please. These icons are saved as Optimized SVG in inkscape and the downloads for this PR are here: https://lmms.io/download/pull-request/7708. You can edit them yourself.

I want to be clear though, only constructive feedback will be tolerated. Repeating the same thing may cause posts to be hidden. Brief and specific please.

@tresf
Copy link
Member

tresf commented Mar 7, 2025

I think that the existing theme colors are being broken. Here's a side-by-side.

Correction, @rubiefawn has informed me that the existing colors take directly from the palette (although at second glance this may have in reference only to the red? Clarification may be needed here.)

@rubiefawn
Copy link
Contributor Author

The red is taken from the color palette, and lightened as needed. The blue is taken from #5166.

@bratpeki
Copy link
Member

bratpeki commented Mar 7, 2025

2025-03-07-215210_726x421_scrot

The buttons following the mixer colors is a nice touch!

@bratpeki
Copy link
Member

bratpeki commented Mar 7, 2025

image

I was playing around with the PR and managed to make this. No idea how to replicate it. Odd!

@tresf
Copy link
Member

tresf commented Mar 11, 2025

image

I was playing around with the PR and managed to make this. No idea how to replicate it. Odd!

Please file a new bug report once this can be reproduced!

Merging this now. Any theming tweaks can come afterwards.

@rubiefawn rubiefawn force-pushed the mute-solo-on-instruments branch from bd86e59 to 8d4eb45 Compare March 11, 2025 16:23
@rubiefawn rubiefawn force-pushed the mute-solo-on-instruments branch from 8d4eb45 to e74fed2 Compare March 11, 2025 16:29
@rubiefawn
Copy link
Contributor Author

Amended history to properly credit @StakeoutPunch, nothing else was changed

@tresf tresf merged commit 6233c5b into LMMS:master Mar 11, 2025
10 checks passed
@rubiefawn rubiefawn deleted the mute-solo-on-instruments branch March 11, 2025 18:56
@bratpeki
Copy link
Member

Before PR:

image

After PR (screenshot by @SpomJ):

image

Was this changed intentionally? Why?

@rubiefawn
Copy link
Contributor Author

Yes, I did change the alignment of those, but I was not aware you could resize tracks vertically. That's a mistake on my part. I'll fix that.

@tresf tresf mentioned this pull request Mar 13, 2025
5 tasks
rubiefawn added a commit that referenced this pull request Mar 13, 2025
In #7708, the track operations widget was given a centered vertical alignment in order to fix an aesthetic layout issue. This is being reverted, since it is possible to resize tracks, and all the other track interface elements are top-aligned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On/Off toggle button on instruments
9 participants