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

render duration of intro & outro ranges on overview waveforms #2089

Merged
merged 8 commits into from
May 5, 2019

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Apr 24, 2019

MarkRange elements in skins only have their durations rendered if they specify a DurationTextColor. This avoids rendering the durations of loops on the overview waveform.

image

durations scale with rate slider:
image

I played a 3 hour set with this last night and it is extremely helpful for me. Having Mixxx calculate these times for me allows me to be more intentional about how I align tracks. I used to do this in headphones continually. With this I don't have to keep the times in my head, I don't have to do any math, and I don't have to repeat that the next time I play a track.

I understand this could be distracting or clutter for others, so I'll add an option to disable this unless there are objections to adding an option.

MarkRange elements in skins only have their durations rendered if they
specify a DurationTextColor. This avoids rendering the durations of
loops on the overview waveform.
src/widget/woverview.cpp Outdated Show resolved Hide resolved
@Be-ing Be-ing added this to the 2.3.0 milestone Apr 24, 2019
src/widget/woverview.cpp Outdated Show resolved Hide resolved
@uklotzde
Copy link
Contributor

uklotzde commented Apr 24, 2019

Very helpful feature 👍

Instead of only enabling/disabling the display we could think about specifying a minimum duration, e.g. with a configurable range between 0 to 30 seconds and a default value of 3 or 5 seconds -> Checkox + Slider. This value may vary depending on your music and mixing style. Not sure if this is more confusing than helpful?

Now I also noticed that I need minute markers on the overview waveforms for a quick peek how long a phrase might be 🙈

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 24, 2019

Not sure if this is more confusing than helpful?

I think that would be more confusing than helpful.

@daschuer
Copy link
Member

daschuer commented May 1, 2019

How about this appearance:
grafik

We can move the time label conditionally into the region if enough space is available.

I don't think we need a preferences option for it.

@Be-ing
Copy link
Contributor Author

Be-ing commented May 1, 2019

Why do you want the text moved outside the range? And why move it to the top?

If we can find a solution that everyone is satisfied with that does not require a preference option that would be ideal.

@daschuer
Copy link
Member

daschuer commented May 1, 2019

It look IMHO ugly if the time labele escapes the boundaries of the highlighted region.
Like proposed it labels the triangle "00:15 ◣" and "◢ 00:23" which keeps all the eye candy in one place.

@Be-ing
Copy link
Contributor Author

Be-ing commented May 1, 2019

To clarify, are you proposing putting the text outside the range on the side closer to the middle of the track? So for the intro it would be drawn after the range and for the outro it would be drawn before the range?

@uklotzde
Copy link
Contributor

uklotzde commented May 1, 2019

This is exactly how I understood Daniel's proposal. The labels would be displayed outside of the (probably short region) within the main track range, next to their nearest marker.

For short intro/outro ranges the labels might not fit entirely into their sections and would otherwise be displayed across the boundaries with varying backgrounds.

There is no ideal solution. With Daniel's proposal the appearance is slightly more consistent.

@uklotzde
Copy link
Contributor

uklotzde commented May 1, 2019

A typical example of a short intro:
Screenshot from 2019-05-01 20-41-35

Reusing the translucent background from highlighted region for the label might be an option to improve readability. The waveform will still shine through. Might be confused with the actual region, not a good idea.

@daschuer
Copy link
Member

daschuer commented May 1, 2019

To clarify, are you proposing putting the text outside the range on the side closer to the middle of the track? So for the intro it would be drawn after the range and for the outro it would be drawn before the range?

Yes, as an idea for discussion. The part I like most is the idea to have the texts and the triangle close to each other and not draw across the region borders.

Be-ing added 3 commits May 5, 2019 11:31
This avoids the text overlapping the borders of the range, which
looks ugly.
This saves a little space, which is helpful for the duration
text for intro/outro sections on the overview waveforms.
@Be-ing
Copy link
Contributor Author

Be-ing commented May 5, 2019

How is this? Do we still need to add a preference option?
image

@daschuer
Copy link
Member

daschuer commented May 5, 2019

Ok, thank you.
Is the ◣ under the blue overlay? It should have the same color than the time text.

This was changed in 5161968 so the cue markers did not get drawn
over range duration text, but that concern was obviated by
0ab6a46
@Be-ing
Copy link
Contributor Author

Be-ing commented May 5, 2019

Good catch, fixed:
image

@daschuer
Copy link
Member

daschuer commented May 5, 2019

LGTM Thanks, waiting for CI.

@Be-ing
Copy link
Contributor Author

Be-ing commented May 5, 2019

CI passed 👍

@Be-ing Be-ing merged commit 73e74af into mixxxdj:master May 5, 2019
@Be-ing
Copy link
Contributor Author

Be-ing commented May 5, 2019

If there is still a demand for adding an option to disable this, that can still be added during the 2.3 beta period. Hopefully this looks good enough that it isn't bothersome enough to anyone that an option is required.

@Be-ing Be-ing deleted the intro_outro_time branch May 5, 2019 21:57
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.

3 participants