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

Show units of effect parameter knobs #11033

Closed
daschuer opened this issue Nov 2, 2022 · 5 comments · Fixed by #11041
Closed

Show units of effect parameter knobs #11033

daschuer opened this issue Nov 2, 2022 · 5 comments · Fixed by #11041

Comments

@daschuer
Copy link
Member

daschuer commented Nov 2, 2022

Feature Description

This may be an addition to #9022
Currently only the value is shown. It would be very useful to see the unit is addition like
"-6 dB"
or
"0.2 s"

We already have an unused stub for this:

enum class UnitsHint : int {

But I think we should adopt the LV2 system:
http://lv2plug.in/ns/extensions/units#unit

@ronso0
Copy link
Member

ronso0 commented Nov 3, 2022

@daschuer Could you give some hints re "adopt the LV2 system"?

@daschuer
Copy link
Member Author

daschuer commented Nov 3, 2022

When you follow the link you find a list of units including their rendering. We currently maintain a different incomplete list in UnitsHint enum.
My proposal is to replace our enum with the LV2 one.

@ronso0
Copy link
Member

ronso0 commented Nov 3, 2022

Yes, understood. My question was rather whether you propose to a) adopt the LV2 map (replacing existing Mixxx enum, change all built-in effects' manifests) or b) somehow map LV2 units to the extended Mixxx list.
The latter sounds complicated and means we'd have two lists / standards. So I'd go with a

Second question that arises: how to translate the enum to dispay strings?
I never did that, and it seems there are a few ways to accomplish this. However, for sake of consistency, I'd use a switch-case function like

static QString LinkTypeToString(LinkType type) {
switch (type) {
case LinkType::Linked:
return QLatin1String("LINKED");
case LinkType::LinkedLeft:
return QLatin1String("LINKED_LEFT");

Edit LV2 units are all lowercase, enum style is PascalCase, soo:
break the style rule or have another map LV2 unit > UnitHint enum?

@ronso0
Copy link
Member

ronso0 commented Nov 3, 2022

Though, a QMap looks nicer, but requires a translator:

QMap<UnitsHint, QString> unitStringsMap {
    {UnitsHint::Unknown, ""},
    {UnitsHint::Beat, "Beat"},
    {UnitsHint::Beats, "Beats"},
    {UnitsHint::Bar, "Bar"},
    {UnitsHint::BPM, "BPM"},
    {UnitsHint::Cent, "Cent"},
    {UnitsHint::Hz, "Hz"}
};

const QString UnitsHintToString(const UnitsHint unitHint) {
    return unitStringsMap.find(unitHint);
}

@ronso0
Copy link
Member

ronso0 commented Nov 3, 2022

I'll do a quick test with:

  • existing enum class UnitsHint : int extended by LV2 units
  • QMap<QString, EffectManifestParameter::UnitsHint> lv2ToUnitsHintEnum
  • QMap<EffectManifestParameter::UnitsHint, QString> unitsHintToDisplayString
  • const QString UnitsHintToString(const UnitsHint unitHint) { return unitStringsMap.find(unitHint); }

@ronso0 ronso0 linked a pull request Nov 9, 2022 that will close this issue
8 tasks
@fwcd fwcd added the effects label Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants