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

issue 2340: Mixer volume percentage labels are off by a factor of 100 #2614

Closed
wants to merge 1 commit into from
Closed

issue 2340: Mixer volume percentage labels are off by a factor of 100 #2614

wants to merge 1 commit into from

Conversation

StCyr
Copy link

@StCyr StCyr commented Feb 26, 2016

Simple fix for issue 2340

It seems to do the trick

@StCyr
Copy link
Author

StCyr commented Feb 26, 2016

Corresponding issue: #2340

@@ -1044,7 +1044,7 @@ void AutomationEditor::paintEvent(QPaintEvent * pe )
for( int i = 0; i < 2; ++i )
{
const QString & label = m_pattern->firstObject()
->displayValue( level[i] );
->displayValue( level[i] /100 );
Copy link
Member

Choose a reason for hiding this comment

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

This should also use a space after the slash, like the other lines, for consistency: / 100

@Umcaruje
Copy link
Member

@StCyr Nice PR! it fixes the values in the context menus:
screenshot from 2016-02-26 17 55 26

Though I noticed some things in the tooltips that could also get fixed:

The volume tooltip looks ok, but it has a space before the % sign:
screenshot from 2016-02-26 17 50 41

It should look like Volume: 109.4% (without the space).

The send knob tooltip displays it as a decimal:
screenshot from 2016-02-26 17 50 15

Also it should have some descriptive text. Ideally it should look like Send amount: 85.3%

@Umcaruje
Copy link
Member

Oh and in the automation editor the values are still decimal:
screenshot from 2016-02-26 18 01 28

@grejppi
Copy link
Contributor

grejppi commented Feb 26, 2016

@StCyr @Umcaruje Meanwhile everywhere else...

100scale

@Umcaruje
Copy link
Member

@grejppi ouch. Forgot to check that.

@StCyr
Copy link
Author

StCyr commented Feb 29, 2016

you're right @grejppi; I didn't check that, sorry.

So, the problem is more difficult to handle than what I previously thought.

After some debugging, it appears that Faders store their value in a range from 0 to 2, while Knobs use a range from 0 to 200.

I guess, solving the problem will require unifying these behaviors, which will probably be quite invasive.

For the record, here are some debugging outputs I could get:

Knob::getValue() value=-0,116667
AutomatableModel::setValue value=158,133347
Knob::getValue() value=-0,116667
AutomatableModel::setValue value=158,333344
Knob::getValue() value=-0,116667
AutomatableModel::setValue value=158,533340
Knob::getValue() value=-0,116667
AutomatableModel::setValue value=158,733337
Knob::getValue() value=0,000000
Knob::getValue() value=-0,116667
AutomatableModel::setValue value=158,933334
...
Fader::mouseMoveEvent value=1,442256
AutomatableModel::setValue value=1,442256
Fader::mouseMoveEvent value=1,465512
AutomatableModel::setValue value=1,465512
Fader::mouseMoveEvent value=1,488768
AutomatableModel::setValue value=1,488768
Fader::mouseMoveEvent value=1,512023
AutomatableModel::setValue value=1,512023
Fader::mouseMoveEvent value=1,535279
AutomatableModel::setValue value=1,535279

@tresf
Copy link
Member

tresf commented Feb 29, 2016

@diizy did some work with this back in the day.... https://github.com/LMMS/lmms/compare/models. Not sure how much of it is relevant here.

@Umcaruje
Copy link
Member

I'm closing this PR out to declutter the tracker, because this issue requires a lot more work. @StCyr if/when you come up with a solution to the problem, you can either open up a new PR or just push again to this one, and we'll reopen it.

@Umcaruje Umcaruje closed this Apr 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants