Added Metronome context menu #6119

frank-a-i · 2021-07-30T20:39:42Z

This PR adds additional settings to the Metronome, previously discussed here

This PR does not fix the timing issue #4038

frank-a-i · 2021-07-30T20:42:50Z

src/core/SamplePlayHandle.cpp

 	PlayHandle( TypeSamplePlayHandle ),
 	m_sampleBuffer( sharedObject::ref( sampleBuffer ) ),
 	m_doneMayReturnTrue( true ),
 	m_frame( 0 ),
 	m_ownAudioPort( ownAudioPort ),
-	m_defaultVolumeModel( DefaultVolume, MinVolume, MaxVolume, 1 ),
+    m_defaultVolumeModel( DefaultVolume, MinVolume, MaxVolume, 1 ),
 	m_volumeModel( &m_defaultVolumeModel ),


cannot find usage of m_volumeModel, has this become obsolete?

LmmsBot · 2021-07-30T20:59:25Z

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Linux (AppImage): lmms-1.3.0-alpha.1.134%2Bgaaee174-linux-x86_64.AppImage (build link)

Windows

Windows 32-bit: lmms-1.3.0-alpha.1.134%2Bgaaee17407-mingw-win32.exe (build link)
Windows 64-bit: lmms-1.3.0-alpha.1.134%2Bgaaee17407-mingw-win64.exe (build link)
Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

macOS

macOS : lmms-1.3.0-alpha.1.134%2Bgaaee17407-mac10.14.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://14507-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.134%2Bgaaee174-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14507?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://14508-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.134%2Bgaaee17407-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14508?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://14510-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.134%2Bgaaee17407-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14510?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/xxa3ikfa11319ptg/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/40289950"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/oa55ax4fkbqglemn/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/40289950"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://14509-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.134%2Bgaaee17407-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14509?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "1c32653db321467b0e083913fdb29075ef70f448"}

Monospace-V · 2021-08-07T03:48:16Z

*Rhythm is typoed.

Monospace-V · 2021-08-20T12:07:32Z

I found a bug in the metronome context menu behaviour. Do I report it here or on the main tracker?

zonkmachine · 2021-08-20T12:12:35Z

@Monospace-V What problem is it? Have you searched the issues? We have quite a few bugs reported for the metronome.

Monospace-V · 2021-08-20T13:24:03Z

The problem is specifically with the behaviour of metronome context menu, and I've copied out the format of a bug report and filled it in:

Metronome context menu remaining open in some cases

Bug Summary

When triggered by certain user behaviour, the metronome context menu can remain open and move to the background as a separate window. The context menu cannot be opened again until it is returned to the foreground and interacted with. Sometimes, the window can even remain open after LMMS is closed.

Steps to reproduce

Right click metronome to open context menu
Do not interact with context menu or allow it to close, instead clicking directly on the metronome or something.
Try opening metronome context menu again. (does not open)
View LMMS' open windows in taskbar. Metronome context menu is still there.

Expected behavior

Metronome context menu should close when not in foreground.

Actual behavior

Context menu remains open as another separate window. In my case, it was possible for me to close LMMS and the context menu remains open even after closing unless interacted with.

Screenshot

(Video)

RFecHtk8Z6.mp4

Affected LMMS versions

(The one downloaded from this pull request)

zonkmachine · 2021-08-20T13:41:46Z

The problem is specifically with the behaviour of metronome context menu

Ok, got it. That belongs here.

frank-a-i · 2021-08-26T20:32:30Z

@Monospace-V thanks for having an eye on it, I didn't try that out. However I 'm having difficulties reproducing it, especially step 2.

I cannot figure out, how not to interact with the context menu as it intentionally already closes once I move the cursor away from the menu-area. For me I may either interact with the controls in the menu or it closes the window. As this internally goes back to a Qt feature, could it be an outdated lib or is this really a thing specifically for Windows?
I'm running it on a ubuntu studio 20.04

Monospace-V · 2021-08-27T02:11:23Z

My VM won't even run the AppImage, so I can't tell.
I cannot tell why this is happening. Perhaps if other Windows users tried reproducing it we would be able to confirm.
What I did was very quickly get my mouse out and enable metronome without giving the menu time to close.
I also find that the menu closes only if you hover over it while it's still open, and then move your mouse away. If you do not let your mouse touch the menu, it just stays open. I believe this was how I reproduced it staying open in the first place.

frank-a-i · 2021-08-27T23:30:53Z

My VM, an Ubuntu 18.04, won't even run the AppImage, so I can't tell.
I cannot tell why this is happening. Perhaps if other Windows users tried reproducing it we would be able to confirm.
What I did was very quickly get my mouse out and enable metronome without giving the menu time to close.
I also find that the menu closes only if you hover over it while it's still open, and then move your mouse away. If you do not let your mouse touch the menu, it just stays open. I believe this was how I reproduced it staying open in the first place.

Ok I managed to reproduce it, let me see

…en requested and forced closing the menu on exiting LMMS

frank-a-i · 2021-08-28T01:59:57Z

Fixed on your side too @Monospace-V ?

Monospace-V · 2021-08-28T03:19:51Z

Where do I download it? I do not have the necessary tools or skillset to compile.

zonkmachine · 2021-08-28T12:12:31Z

@Monospace-V He meant the new binaries found here: #6119 (comment)
No compilation needed.

allejok96 · 2021-08-28T13:47:06Z

Why not do it using QMenu and QWidgetAction, instead of opening a new window? Then you wouldn't need to manually close it when focus is lost. And maybe use Knob and LcdSpinBox to make it look more LMMS themed?

PS we need 3/4.

Monospace-V · 2021-08-28T15:07:09Z

It doesn't seem to have updated today. Downloaded and checked: gives the bug, this is probably because the binary was not updated.

allejok96 · 2021-08-28T15:17:42Z

@Monospace-V you can get it by clicking "Show all checks" > "Details" below this message.
Or direct link: https://14690-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.138%2Bg3234182-linux-x86_64.AppImage

Monospace-V · 2021-08-28T15:33:19Z

Thanks, Alex!
Tested it, Frank, and I notice you've fixed the issue user-side by moving the menu to where there is no way you can not hover over it if you right click the metronome, making it likely impossible that the user ever trigger it.
I then found, after a couple of tries, that you can still reproduce it by moving your mouse fast enough.
See:
https://user-images.githubusercontent.com/76674645/131223116-78fac0b1-5db7-4df9-bf00-b175f9aca4bd.mp4

However, it does close when LMMS is closed.

frank-a-i · 2021-08-30T21:26:04Z

I updated the design to something that fits a little more into the concept, taking into account @allejok96 's QMenu hint, it's true I like it a little better (from design and coding perspective)

I generally would be happy to finalize the frontend, so I'm open for any remarks.

frank-a-i · 2021-08-30T21:26:14Z

Also it seems like we're lacking a common color definition, as the implementations I saw always come with a local numeric assignment, correct or where can I find the right color definition?

frank-a-i · 2021-08-30T21:29:43Z

Thanks, Alex!
Tested it, Frank, and I notice you've fixed the issue user-side by moving the menu to where there is no way you can not hover over it if you right click the metronome, making it likely impossible that the user ever trigger it.
I then found, after a couple of tries, that you can still reproduce it by moving your mouse fast enough.
See:
https://user-images.githubusercontent.com/76674645/131223116-78fac0b1-5db7-4df9-bf00-b175f9aca4bd.mp4

However, it does close when LMMS is closed.

Yep thanks for pointing out, my opinion is, this is acceptable. As it either gets closed on main application exit or the user is getting forced to make a decision if he doesn't want to get annoyed by an opened menu

Monospace-V · 2021-08-31T02:54:05Z

Agreed.
If possible, though, I'd suggest automatically closing the window if user doesn't interact with it for 30 seconds, or if it is not in foreground. Just to ensure that if a user triggers it, it will not render the menu unusable for the rest of the session.

frank-a-i · 2021-09-01T21:15:54Z

adapt button appearance similar to Demo: Piano-Roll chop, stamp, strum, bulldozer and mode switcher #6140

allejok96 · 2021-09-01T21:31:53Z

Although I like the design so far, it does not match LMMS' ugly style 😸

Use themed Knob / Slider for volume?

Also we must have spinboxes for a beats like 5/4. Or a checkbox that says "same as time signature" (which is the default behavior right now).

PS if you use QToolButton->setMenu you won't need to position the window and check for mouse pointer leaving it and so on, guess you've figured it out already...

frank-a-i · 2021-09-06T10:58:49Z

FYI Possible fix for the flaky metronome sounds #6149

frank-a-i · 2021-09-06T11:01:13Z

Although I like the design so far, it does not match LMMS' ugly style 😸

Use themed Knob / Slider for volume?

Good point, how to change the appearance? I was greping for the already implemented sliders but I couldn't find the design settings

allejok96 · 2021-09-06T20:37:35Z

I'm just guessing here but using Knob.h or AutomatableSlider.h (vertical only?) would give it the correct style by default. Style is defined in data/themes/default/style.css.

superpaik · 2021-09-13T10:14:31Z

I've notice some issues:

the popup window goes outside LMMS window boundaries, tested on W10 (see attached image)
it's kind of weird that when you click on a rhythm the popup closes, so you have to open it again to make sure you have selected the one you wanted. Maybe it would be better to leave it opened and close it when the cursor leaves the window (as it works now for volume slider)
it malfunctions when time signature is other than 4/4. In 1.2.2 metronome goes acordingly to time signature. For exemple, on 3/4 one expects three beats per bar -when using 1/4 as rhythm-, but you get four.

Added Metronome context menu

ca7b8b9

frank-a-i commented Jul 30, 2021

View reviewed changes

Frank AI added 8 commits August 4, 2021 19:11

Added Circle CI Fixes, Copyright text

9251a73

CodeFactor fixes

cec3041

CodeFactor fixes

9d8a567

CodeFactor fixes

bc73709

CodeFactor fixes

01e7dfc

CodeFactor fixes

c260321

CodeFactor fixes

99592dc

CodeFactor fixes

44980cf

fixed rythm typo

1c32653

frank-a-i added 2 commits August 28, 2021 03:47

fixed Metronom Settings Menu position, forced bringing it to front wh…

67f8531

…en requested and forced closing the menu on exiting LMMS

fixed Metronom Settings Menu position, forced bringing it to front wh…

80fc225

…en requested and forced closing the menu on exiting LMMS

frank-a-i force-pushed the feature/upgrade_metronome_ui branch from 67f8531 to 80fc225 Compare August 28, 2021 01:57

allejok96 mentioned this pull request Aug 28, 2021

LMMS shows odd behaviors with non-default denominator #4045

Open

4 tasks

Updated MetronomSettingsMenu's Design and inheritence --> QMenu

8f4b323

Monospace-V mentioned this pull request Sep 5, 2021

Hydrogen import: move all notes from C2 to A4 #6148

Open

frank-a-i mentioned this pull request Sep 16, 2021

Fixed partially skipping metronome tick sounds; tidied up related var… #6149

Closed

frank-a-i marked this pull request as draft September 16, 2021 19:02

allejok96 mentioned this pull request Apr 14, 2022

Add Tap Tempo Feature #6372

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Metronome context menu #6119

Added Metronome context menu #6119

frank-a-i commented Jul 30, 2021

frank-a-i Jul 30, 2021

LmmsBot commented Jul 30, 2021 •

edited

Loading

Linux

Windows

macOS

Monospace-V commented Aug 7, 2021

Monospace-V commented Aug 20, 2021 •

edited

Loading

zonkmachine commented Aug 20, 2021

Monospace-V commented Aug 20, 2021

zonkmachine commented Aug 20, 2021

frank-a-i commented Aug 26, 2021 •

edited

Loading

Monospace-V commented Aug 27, 2021 •

edited

Loading

frank-a-i commented Aug 27, 2021

frank-a-i commented Aug 28, 2021

Monospace-V commented Aug 28, 2021

zonkmachine commented Aug 28, 2021

allejok96 commented Aug 28, 2021

Monospace-V commented Aug 28, 2021

allejok96 commented Aug 28, 2021

Monospace-V commented Aug 28, 2021 •

edited

Loading

frank-a-i commented Aug 30, 2021 •

edited

Loading

frank-a-i commented Aug 30, 2021

frank-a-i commented Aug 30, 2021

Monospace-V commented Aug 31, 2021

frank-a-i commented Sep 1, 2021 •

edited

Loading

allejok96 commented Sep 1, 2021

frank-a-i commented Sep 6, 2021

frank-a-i commented Sep 6, 2021

allejok96 commented Sep 6, 2021

superpaik commented Sep 13, 2021

Added Metronome context menu #6119

Are you sure you want to change the base?

Added Metronome context menu #6119

Conversation

frank-a-i commented Jul 30, 2021

frank-a-i Jul 30, 2021

Choose a reason for hiding this comment

LmmsBot commented Jul 30, 2021 • edited Loading

Linux

Windows

macOS

Monospace-V commented Aug 7, 2021

Monospace-V commented Aug 20, 2021 • edited Loading

zonkmachine commented Aug 20, 2021

Monospace-V commented Aug 20, 2021

Metronome context menu remaining open in some cases

Bug Summary

Steps to reproduce

Expected behavior

Actual behavior

Screenshot

Affected LMMS versions

zonkmachine commented Aug 20, 2021

frank-a-i commented Aug 26, 2021 • edited Loading

Monospace-V commented Aug 27, 2021 • edited Loading

frank-a-i commented Aug 27, 2021

frank-a-i commented Aug 28, 2021

Monospace-V commented Aug 28, 2021

zonkmachine commented Aug 28, 2021

allejok96 commented Aug 28, 2021

Monospace-V commented Aug 28, 2021

allejok96 commented Aug 28, 2021

Monospace-V commented Aug 28, 2021 • edited Loading

frank-a-i commented Aug 30, 2021 • edited Loading

frank-a-i commented Aug 30, 2021

frank-a-i commented Aug 30, 2021

Monospace-V commented Aug 31, 2021

frank-a-i commented Sep 1, 2021 • edited Loading

allejok96 commented Sep 1, 2021

frank-a-i commented Sep 6, 2021

frank-a-i commented Sep 6, 2021

allejok96 commented Sep 6, 2021

superpaik commented Sep 13, 2021

LmmsBot commented Jul 30, 2021 •

edited

Loading

Monospace-V commented Aug 20, 2021 •

edited

Loading

frank-a-i commented Aug 26, 2021 •

edited

Loading

Monospace-V commented Aug 27, 2021 •

edited

Loading

Monospace-V commented Aug 28, 2021 •

edited

Loading

frank-a-i commented Aug 30, 2021 •

edited

Loading

frank-a-i commented Sep 1, 2021 •

edited

Loading