Skip to content

fix positions of mixer controls, rearrange vinyl controls#12

Merged
Be-ing merged 21 commits intoBe-ing:deere_2.1_part_2from
ronso0:Deere-2.1-update2
Oct 21, 2017
Merged

fix positions of mixer controls, rearrange vinyl controls#12
Be-ing merged 21 commits intoBe-ing:deere_2.1_part_2from
ronso0:Deere-2.1-update2

Conversation

@ronso0
Copy link
Copy Markdown

@ronso0 ronso0 commented Oct 17, 2017

No description provided.

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 18, 2017

Recording and Shoutcast button are normal square buttons now, they have blue bg for states 1-3 and colored icons. Check if you like it that way.
Also RecordingDuration font is bold now.

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 18, 2017

Re: beatgrid buttons
In deck control grid, I'd put Eject where Curpos is and put a toggle for Vinyl Controls there, which would also be vinyl statuslight.
Curpos button I'd put in the beatgrid row and rearrange those buttons so the expand/collapse toggle stays in place. What do you think?

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 19, 2017

I notice we can save vertical space in the collapsed effect units:
I'll try to put [On/Off] [Meta] [EffectSelector] in one row per effect, like in expanded units.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

I thought about that with the effect units too, but I'm not sure it would fit in the minimum width then. Perhaps we could shrink the knobs and/or font size of the effect selector to make that work?

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

There is a bit of wasted space above the mixer knobs with split waveforms. How about shrinking the volume fader and level meters a little? Or adding a bit of vertical padding between the knobs?
image

If we could get the master level meter to align with the deck meters with 4 decks split waveforms that would be nice:
image

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

The recording and broadcasting icons feel cramped within their buttons. I think it would help to shrink the icons a tiny bit to leave more padding around the inside of the buttons.

IME the beats_translate_curpos button is by far the most frequently used for adjusting the beatgrid and it really helps to have it easily accessible, especially if it is not mapped on a controller (many controllers don't have a good way to map this). I'd rather not have it hidden behind the more extensive beatgrid editing menu.

The borders around the BPM and key do help make the buttons behind them more discoverable. I think the borders could benefit from being maybe a pixel or two thicker with rounded corners.

The changes with the vinyl control options are better than they were, but I think we can still do better. Do we really need the relative/absolute and cue mode buttons taking space in the skin? Are these actually changed while mixing? Maybe those should be moved to the Preferences? I would think the only ones that are useful to have access to while mixing would be the vinyl control enable switch and the passthrough switch. Also, I don't think it's a good use of space to have the vinyl options toggle icon showing for everyone all the time. I'd rather have all vinyl related widgets toggled by one item in the skin settings.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

I think it may be time for a larger redesign of Deere's decks as @naught101 mentioned. The tempo faders are too short and there is wasted vertical space between the overview waveforms and the bottom controls. Perhaps in rearranging those we could find a better place for the vinyl related widgets.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

@Bronxio, do you have any ideas for where to put the vinyl related widgets? Should we move the relative/absolute and cue mode switches into the Preferences?

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 19, 2017

I thought about that with the effect units too, but I'm not sure it would fit in the minimum width then. Perhaps we could shrink the knobs and/or font size of the effect selector to make that work?

Reduce padding (maybe font size) and all should fit. I'll try

There is a bit of wasted space above the mixer knobs with split waveforms.

If we could get the master level meter to align with the deck meters

I'm not done yet with the mixer level meters. I tried shifting stuff and allow level meters to grow if neither faders nor EQs are visible, but am not happy yet.

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 19, 2017

I'll shrink the Rec and Broadcasting icon even further.
beat_translate_curpos goes back to deck controls, vinyl options are toggled from skin settings, okay.

Do we really need the relative/absolute and cue mode buttons taking space in the skin?

I noticed in a videos where DJs performed with DVS and suddenly the needle started skipping due to a scratch in the disc. In such a case it's veeery convinient to quickly switch to CONSTANT mode.

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 19, 2017

The tempo faders are too short and there is wasted vertical space between the overview waveforms and the bottom controls.

please post a screenshot of that situation.
IIRC there was a previous Deere configuration where Play & Cue button would go into deck controls row and let the tempo fader expand. To re-enable that feature, we could rearrange loop and spinbox groups like this (quick sketch at minimal window size)
group loop-beatjump

In general the challenge here is to make it look good on minimal as well as on bigger screens...

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 19, 2017

The borders around the BPM and key do help make the buttons behind them more discoverable. I think the borders could benefit from being maybe a pixel or two thicker with rounded corners.

Actually those sharp corners distinguish the expand toggles from normal action buttons.
I'll test if a brighter border color works better, everything else forces the text row to grow taller.

@Bronxio
Copy link
Copy Markdown

Bronxio commented Oct 19, 2017

I think it's ok to show all of them (vinyl control on/off, passthorugh, abs/rel/const and cue mode) only if user wants to see the Vinyl Control Options. But, if so, it's great to have the four. I would miss any of them.

Although the relative mode and the cue/hot cue options seems to be popular (and always set), there are some DJ tools ready to play in absolute mode, like "Cuts & Drums Digital Version" from DJ Vajra (with the same divisions as in control vinyls, so the tracks can be found in these, without looking at the screen). So, it's nice to be able to switch the mode without going to preferences. The cue mode that I prefer depends on what I have in the deck at the very moment.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

Okay, thanks for your input @Bronxio. Let's keep all 4 buttons available in the skin then.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

please post a screenshot of that situation.

image

IIRC there was a previous Deere configuration where Play & Cue button would go into deck controls row and let the tempo fader expand. To re-enable that feature, we could rearrange loop and spinbox groups like this (quick sketch at minimal window size)

Yeah, I removed that option because it was an ugly hack around the clutter of the old looping GUI.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 19, 2017

I've thought about the possibility of abandoning the 2 x 3 button grid next to the overview waveforms and using 3 rows below the overviews for all controls.

@naught101
Copy link
Copy Markdown

+1 for vertical padding between the mixer knobs. In the 2-deck layout at least, the cross fader could be narrowed (I don't think it needs the length - I guess it only very rarely gets used in the UI), and then the mixer knobs could spread downwards. I'd also advocate for padding between the mixer knobs and the faders (maybe and extra 20% of the width of the fader?)

I don't think it's a good idea to introduce 3-row buttons, unless there is a really good visual definition between groups. I think groups should be kept visually separate, and to a max of 4-6 buttons, unless there's a really strong connection between them (e.g. 8+ auto loop length options, like in LateNight).

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 20, 2017

All mixer positions are fixed now.

As an experiment I moved Play/Cue to deck controls row and compressed loop & beatjump controls.
Now the tempo fader is easy to handle in all situations.

Please test and tell me if you like it.

@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 20, 2017

Nice! This is just about done! I tested with and without a QuickEffect loaded and it looks pretty good in all cases.

image
Master level meter is a different height compared to the deck meters with 4 decks parallel waveforms.

image

  • Play and cue buttons can expand too wide. Their maximum width should be limited.
  • reloop_toggle button is misaligned with beatloop_toggle button above it.

image
I don't know if this is a new issue, but there's a very slight misalignment on the right side of effect units between the "MASTER" button and the box below.

image
With 2 decks, I think it would help to have a tiny bit of padding above the crossfader.

If you could figure out a way to rearrange the effect units to take less vertical space when collapsed but still fit in the minimum width, that would be awesome.

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 21, 2017

I guess the mixer is fixed.

Further space improvements happen in another PR.
To relax the mixer's knob columns and to compress collapsed FX* units,we'll have to rework the big knobs' bg & notch

*at minimum size with Super knob: [focus] [On] [meta knob] [EffectSelector}

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 21, 2017

Please check the AutoDJ highlight for the crossfader

@Be-ing Be-ing merged commit 3ec4652 into Be-ing:deere_2.1_part_2 Oct 21, 2017
@Be-ing
Copy link
Copy Markdown
Owner

Be-ing commented Oct 21, 2017

I think the orange highlight of the crossfader for AutoDJ is too attention grabbing. Regardless, I think this is good enough to merge.

@ronso0
Copy link
Copy Markdown
Author

ronso0 commented Oct 21, 2017

I tried o match the AutoDJ indicators color so the connection is more obvious. It can also be styled like the hovered BPM edit trigger, with a thin border and orange background

@ronso0 ronso0 deleted the Deere-2.1-update2 branch October 21, 2017 20:51
Be-ing pushed a commit that referenced this pull request Apr 15, 2018
Add Ubuntu 16.04 Xenial as supported
Be-ing pushed a commit that referenced this pull request Apr 8, 2022
…h sync

When loading a track that is not yet present in the library (and thus
doesn't have any BPM because it hasn't been analyzed yet) while another
deck is playing and both decks have sync enabled, a debug assertion is
triggered:

    DEBUG ASSERT: "isValid()" in function double mixxx::Bpm::value() const at src/track/bpm.h:53
    Aborted (core dumped)

The backtrace looks as follows:

    #0  0x00007f175c87234c in __pthread_kill_implementation () at /usr/lib/libc.so.6
    #1  0x00007f175c8254b8 in raise () at /usr/lib/libc.so.6
    #2  0x00007f175c80f534 in abort () at /usr/lib/libc.so.6
    #3  0x00007f175cf05ee4 in qt_assert(char const*, char const*, int) () at /usr/lib/libQt5Core.so.5
    #4  0x000055deb2e67e1c in mixxx::(anonymous namespace)::handleMessage(QtMsgType, QMessageLogContext const&, QString const&) (type=<optimized out>, context=<optimized out>, input=<optimized out>) at src/util/logging.cpp:355
    #5  0x00007f175cf47128 in  () at /usr/lib/libQt5Core.so.5
    #6  0x00007f175cf3fd8a in  () at /usr/lib/libQt5Core.so.5
    #7  0x00007f175cf06526 in QMessageLogger::critical(char const*, ...) const () at /usr/lib/libQt5Core.so.5
    #8  0x000055deb2e5c720 in mixxx_debug_assert(char const*, char const*, int, char const*) (assertion=assertion@entry=0x55deb39bd0db "isValid()", file=file@entry=0x55deb39bbf30 "src/track/bpm.h", line=line@entry=53, function=function@entry=0x55deb39bbf08 "double mixxx::Bpm::value() const") at gsrc/util/assert.h:9
    #9  0x000055deb2ee7e7e in mixxx_debug_assert_return_true(char const*, char const*, int, char const*) (function=0x55deb39bbf08 "double mixxx::Bpm::value() const", line=53, file=0x55deb39bbf30 "src/track/bpm.h", assertion=0x55deb39bd0db "isValid()") at gsrc/util/assert.h:18
    #10 mixxx::Bpm::value() const (this=<synthetic pointer>) at src/track/bpm.h:53
    #11 mixxx::operator*(mixxx::Bpm, double) (multiple=1, bpm=...) at src/track/bpm.h:160
    #12 SyncControl::setLocalBpm(mixxx::Bpm) (this=<optimized out>, localBpm=...) at src/engine/sync/synccontrol.cpp:567
    #13 0x000055deb34c7ba3 in EngineBuffer::postProcess(int) (this=0x55deb56eb060, iBufferSize=2048) at src/engine/enginebuffer.cpp:1318
    #14 0x000055deb3139023 in EngineMaster::processChannels(int) (this=0x55deb5449440, iBufferSize=<optimized out>) at src/engine/enginemaster.cpp:383
    #15 0x000055deb31394f7 in EngineMaster::process(int) (this=0x55deb5449440, iBufferSize=iBufferSize@entry=2048) at src/engine/enginemaster.cpp:410
    #16 0x000055deb2f91d0b in SoundManager::onDeviceOutputCallback(long) (this=<optimized out>, iFramesPerBuffer=iFramesPerBuffer@entry=1024) at src/soundio/soundmanager.cpp:596
    #17 0x000055deb32dd794 in SoundDevicePortAudio::callbackProcessClkRef(long, float*, float const*, PaStreamCallbackTimeInfo const*, unsigned long) (this=0x55deb553e6b0, framesPerBuffer=1024, out=<optimized out>, in=<optimized out>, timeInfo=<optimized out>, statusFlags=<optimized out>) at src/soundio/sounddeviceportaudio.cpp:965

This happens because `newLocalBpm` is invalid when `localBpm` is
invalid. Trying to do sync decks while no tempo information is available
does not make sense, so we only synchronize decks if the local BPM is
available.
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