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 UI 60 times per second. #4570

Merged
merged 2 commits into from Sep 17, 2018
Merged

Update UI 60 times per second. #4570

merged 2 commits into from Sep 17, 2018

Conversation

@karmux
Copy link
Contributor

@karmux karmux commented Aug 29, 2018

Fixes #4562.

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Sep 6, 2018

Merge?

@SecondFlight
Copy link
Member

@SecondFlight SecondFlight commented Sep 6, 2018

I remember doing this on my own fork, and I noticed that the EQ visualizer seemed to behave somewhat differently. Has anybody else noticed this? I didn't see it as a problem, but I might have missed something.

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Sep 6, 2018

the EQ visualizer seemed to behave somewhat differently.

I think the falloff in EqFader::updateVuMeters should be adjusted to reflect the refresh rate change.

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Sep 6, 2018

Also in EqSpectrumView::paintEvent. The exact value should be about 1.06266, but I suggest slightly larger value around to bring some visual effects.

@karmux
Copy link
Contributor Author

@karmux karmux commented Sep 7, 2018

I will look into it in the weekend.

@karmux
Copy link
Contributor Author

@karmux karmux commented Sep 9, 2018

Adjusted also falloff of mixer faders (FxMixerView::updateFaders). Used value 1.07.

@SecondFlight
Copy link
Member

@SecondFlight SecondFlight commented Sep 9, 2018

I can't think of any other possible issues with this. The fact that it's not actually 60 fps still feels wrong to me (62.5 fps, assuming the timer is accurate), but I doubt this will cause any actual issues.

Merge?

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Sep 11, 2018

I'm not sure if this one is okay for stable-1.2, but the change itself looks good to me.

@SecondFlight
Copy link
Member

@SecondFlight SecondFlight commented Sep 11, 2018

I would hate to defer something so simple and beneficial to 1.3, but we are on feature freeze. It's your call, @PhysSong, I won't complain if we have to defer.

@karmux
Copy link
Contributor Author

@karmux karmux commented Sep 13, 2018

It's more like a fine-tuning than feature.

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Sep 17, 2018

Merging into stable-1.2 because we can safely revert it if it causes any issues. 🚜

@PhysSong PhysSong merged commit dd7b086 into LMMS:stable-1.2 Sep 17, 2018
2 checks passed
2 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@musikBear
Copy link

@musikBear musikBear commented Sep 18, 2018

Any win Test binaries?

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Sep 18, 2018

@musikBear This will be part of RC7. So please wait a few days. :)

@musikBear
Copy link

@musikBear musikBear commented Sep 26, 2018

@PhysSong Just want to let you know, that RC7 runs fine on my old Tube-monitor 👍

@DomClark
Copy link
Member

@DomClark DomClark commented Oct 31, 2018

Should TimeLineWidget be updated too? This looks like a 20 fps timer to me:

updateTimer->start( 50 );

@PhysSong
Copy link
Member

@PhysSong PhysSong commented Oct 31, 2018

@DomClark Nice catch!

PhysSong added a commit that referenced this pull request Nov 9, 2018
In addition to #4570.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.