Add 5/4 and 4/5 BPM scaling controls - #16026
Conversation
There was a problem hiding this comment.
Looks and works already good. Thank you.
With all these options showing the resulting BPM like in the context menu is very useful.
Do you have interest to add this also to the BPM window?
I have also noticed that we have different tooltips at the GUI buttons and the control picker. Fractional vs. Percent. Not sure what is the most convenient version, but we may use the same at both places. What wotks for you the best?
Testing this I have also came across some ideas for improvement.
The workflow of the deck preferrences menu is a big weak, because in almost all cases you want to verify the result in a deck. Especially with the factional multipliers an offset adjustment is probably also required. So a load button comes handy.
Interestingly the beat detector already knows the solution. Is is currently tunes for 120 BPM. We may rerun it with a different tuning. ... Idea for a different PR/feature request.
I also consider a online lookup interesting for example at tunebat. Maybe we can move the fetch a suggestion?
Also a future PR / feature request.
6daf7ea to
64d9486
Compare
|
Thanks for the thorough review and all the great ideas, @daschuer! Showing resulting BPM in the Track Properties BPM tab — Great idea! I'll add this in a separate PR so the labels dynamically show the resulting BPM (e.g., "3/4 | 90 BPM"), just like the context menu already does. Fractional vs. Percent tooltips — I think the fractional style (×5/4, ×4/5, etc.) is more intuitive for BPM scaling since DJs naturally think in terms of "double", "halve", "three-fourths". Percentages like "+25%" feel more abstract. Happy to align both the GUI buttons and the controller picker to use the fractional style — what do you think? Load button in deck preferences workflow — That's a really useful idea for verifying BPM changes quickly. I'll keep that in mind for a future PR. Beat detector re-tuning & online BPM lookup (tunebat) — Both great feature ideas! Would you like me to open feature request issues for these so they don't get lost? For now, I'll address the // NEW comment removal in this PR and open the two follow-up PRs for the BPM preview and tooltip unification separately. Quick note: the two follow-up PRs I mentioned will branch off this once it's merged. Happy to address any remaining review feedback here in the meantime. |
Add updateBpmScaleButtonLabels() to update the BPM scale button labels in the Track Properties dialog with the resulting BPM value, matching the format already used in the track right-click context menu (e.g. "3/4 BPM | 90 BPM"). Labels update in all BPM change code paths: - Initial track load into the dialog (reloadTrackBeats) - After a scale button is clicked (slotBpmScale) - After the BPM is cleared (slotBpmClear) - After manual edits to the BPM spin box (slotSpinBpmValueChanged) Addresses feedback from @daschuer in mixxxdj#16026.
Add updateBpmScaleButtonLabels() to update the BPM scale button labels in the Track Properties dialog with the resulting BPM value, matching the format already used in the track right-click context menu (e.g. "3/4 BPM | 90 BPM"). Labels update in all BPM change code paths: - Initial track load into the dialog (reloadTrackBeats) - After a scale button is clicked (slotBpmScale) - After the BPM is cleared (slotBpmClear) - After manual edits to the BPM spin box (slotSpinBpmValueChanged) Addresses feedback from @daschuer in mixxxdj#16026.
9d2d7bd to
64d9486
Compare
Complement the FiveFourths (5/4) scaling from mxmilkiib's commit with its inverse FourFifths (4/5 = 0.8x) scaling. Added across all layers: enum, beats math, engine control, controller picker menu, right-click track menu, track properties dialog, and unit tests. Closes mixxxdj#14686
Rebased onto main after BPM lock toggle merge (mixxxdj#16025). Ensures 5/4 and 4/5 scaling buttons respect the BPM lock state, consistent with existing scaling controls.
64d9486 to
786e31a
Compare
Add updateBpmScaleButtonLabels() to update the BPM scale button labels in the Track Properties dialog with the resulting BPM value, matching the format already used in the track right-click context menu (e.g. "3/4 BPM | 90 BPM"). Labels update in all BPM change code paths: - Initial track load into the dialog (reloadTrackBeats) - After a scale button is clicked (slotBpmScale) - After the BPM is cleared (slotBpmClear) - After manual edits to the BPM spin box (slotSpinBpmValueChanged) Addresses feedback from @daschuer in mixxxdj#16026.
|
We should stretch the TAP button to four rows. |
Since this is already merged, I'll open a separate PR for this. Unless you'd prefer I bundle it into #16141 which touches the same area? |
|
Also wondering if the button order makes sense to revisit — ideally inverse pairs like ×2/÷2, ×5/4/×4/5 should sit next to each other so it's easy to undo a misclick. Could bundle this along with the TAP button fix into one PR if that works better. |
|
Yes, please open a PR with for the stretched TAP button and the pairing 👍 |
Add updateBpmScaleButtonLabels() to update the BPM scale button labels in the Track Properties dialog with the resulting BPM value, matching the format already used in the track right-click context menu (e.g. "3/4 BPM | 90 BPM"). Labels update in all BPM change code paths: - Initial track load into the dialog (reloadTrackBeats) - After a scale button is clicked (slotBpmScale) - After the BPM is cleared (slotBpmClear) - After manual edits to the BPM spin box (slotSpinBpmValueChanged) Addresses feedback from @daschuer in mixxxdj#16026.
Add updateBpmScaleButtonLabels() to update the BPM scale button labels in the Track Properties dialog with the resulting BPM value, matching the format already used in the track right-click context menu (e.g. "3/4 BPM | 90 BPM"). Labels update in all BPM change code paths: - Initial track load into the dialog (reloadTrackBeats) - After a scale button is clicked (slotBpmScale) - After the BPM is cleared (slotBpmClear) - After manual edits to the BPM spin box (slotSpinBpmValueChanged) Addresses feedback from @daschuer in mixxxdj#16026.
3 participants
Implements both the ×5/4 and ×4/5 BPM scaling options requested in #14686.
Changes
BpmScaleenum (src/track/beats.h)src/track/beats.cpp)bpmcontrol.cpp/.h) with keybeats_set_fourfifthscontrolpickermenu.cpp)wtrackmenu.cpp/.h)dlgtrackinfo.cpp/.ui)beatgridtest.cpp,beatmaptest.cpp)Closes #14686
Based on work from #14780 by @mxmilkiib
Screenshots
Before
After