Add support for vertical waveforms and overviews by ninomp · Pull Request #983 · mixxxdj/mixxx

ninomp · 2016-07-26T00:02:26Z

This PR seeks to resolve https://bugs.launchpad.net/mixxx/+bug/975203

daschuer · 2016-07-26T06:11:57Z


-    double heightGain = allGain * (double)m_waveformRenderer->getHeight()/255.0;
+    double heightGain = allGain * (double)m_waveformRenderer->getBreadth()/255.0;
    if (m_alignment == Qt::AlignTop) {


AlignTop and AlignBottom sonds wrong now. Should we use AlignRight and AlignLeft for the rotated case?

Yes, I agree.

daschuer · 2016-07-26T06:23:14Z

Thank you for all the work.
Do you have a testing skin?

ninomp · 2016-07-27T12:22:48Z

@daschuer I have just added vertical waveforms option to LateNight skin. It still has some issues, but it's good enough for demonstration.

daschuer · 2016-07-27T13:55:28Z

Thank you for the updates.

Could you separate the LateNight changes from the back-end changes?
Since I am not the LateNight-Taste-Reference, it will block the original pull request.
@ywwg is. ...

You may just branch 239a226 and issue it as a separate PR.

By the way:
I wonder if it is reasonable to squeeze the vertical waveforms as an option to LateNight.
I think vertical waveforms are great for beatmatching along with an unmistakable clear left / right deck association. If we want to make the most of it, it can be probably done the best in a new skin, a LateNightVerticalRemix or something.

ninomp · 2016-07-27T14:40:34Z

Can't I just move b12f439 into a new branch (which I will frequently rebase on top of this one)?

daschuer · 2016-07-27T16:09:03Z

That would work as well.

daschuer · 2016-07-27T21:17:34Z

I have just tested the LateNight skin and I am impressed about the skin / design opportunities this vertical waveform offers.

Here are my findings:

left channel is on the left, that is OK, but the rest looks somehow mirrored as well. I have expected that the C of the cue marker is now right. Just like a clockwise turn of 90° + left right channel swap.
What do you think?
The markers on the overview waveform are placed by width. (only in the upper part)
The white zero line is missing
The loading status texts are missing

ninomp · 2016-07-28T16:35:33Z

I am glad you like this.

About text of markers on overview, because the coordinate system originates in the top left corner I find it more logical that the text is left-aligned. But, I believe this is a decision that can be left to skin designers by adding left/right value/options for markers' Align parameter (for both waveforms and overviews).

I managed to fix most of the issues, but I wasn't able to make the white axis/zero line appear on GLSL vertical waveforms. I need help with this one.

Also, I moved skin changes to verticalwaveform_skin branch.

esbrandt · 2016-07-31T11:50:46Z

Thanks for the changes, really like that.

For horizontal waveforms, we allow to display only the top/bottom half of the waveform using the <Align> option. [1]
This option is currently intentional ignored, if <Orientation>vertical</Orientation> is set?
The reference point for the makers <Align> option differ, depending on the value of the main waveforms <Orientation>. E.g. with the waveform horizontal, <Align>top</Align> refers to the top of the waveforms box, while with the vertical waveforms, <Align>top</Align> refers to the top of the marker. I found the markers hard to read when directly drawn directly on the waveform, especially with RGB colors. Alternatively, we might just modify the markers for better visibility ( plain background = no rounded corners, no opacity, 1-2px outset dark border )

[1] http://mixxx.org/wiki/doku.php/creating_skins?s[]=align#waveform

ninomp · 2016-07-31T14:17:54Z

I am pleased that you like this. 😄

Regarding waveform alignment, if <Orientation> is set to vertical, two values are accepted for <Align>: left and right (which display only the left / right part of the waveform). All other values are ignored and the whole waveform is displayed. If you set <Orientation> to horizontal (or don't specify the <Orientation> at all), then top and bottom are accepted and display only the top/bottom part of the waveform while all the other values are ignored and the whole waveform is displayed.
You can see the code that does this here: 92c041a#diff-6d73706e001649fa0e0125989739d22bR104

Also, I noticed that (both) GLSL waveforms are currently ignoring <Align>. This issue is present in the current master as well.

Regarding the <Align> of markers, I think we should allow all combinations of left/hcenter/right with top/vcenter/bottom. This would allow skin writers to write something like <Align>top|left</Align>. Also, I would like that we support <Align>center</Align> as a shorthand for <Align>vcenter|hcenter</Align>. What do you think? We should also decide what should be default.

Also, as You noticed, <Align>top</Align> for vertical waveforms mean that the text is above the actual mark. Maybe it would make more sense if <Align>top</Align> means that the top of text is aligned with the actual mark. That would mean that the text is below the actual mark. Does this make sense to you? We can use the same logic for <Align>left</Align> in horizontal waveforms.

esbrandt · 2016-07-31T15:44:39Z

Regarding waveform alignment...

Thanks for pointing out, missed that one.
Wonder how useful that option would be for the small waveform overview in your opinion.

Also, I noticed that (both) GLSL waveforms are currently ignoring ....

This is a known bug, that has not made much fanfare yet.

Regarding the of markers

Sounds all good to me, no strong opinion about the default. Imo good readability is a requirement, it is nice to distinguish between cue points and in/out markers. Currently we solve that by positioning cues on top, and other markers on the bottom of the (horizontal) waveform. Skin designers can use custom pixmaps if still not enough options.

Maybe it would make more sense if top means that the top of text is aligned with the actual mark. That would mean that the text is below the actual mark...

Makes sense, worth a try if the readability improves for the marker text on horizontal waveforms / waveform overviews.

…overview Also, decrease transparency of marker's text on waveform.

ninomp · 2016-08-15T18:02:53Z

Wonder how useful that option would be for the small waveform overview in your opinion.

Well, I am not sure. Personally, I prefer to have both channels visible. However, this is not so difficult to implement, so we could add it as an option.

Like described in my previous comment, I implemented additional text alignment flags on waveforms and overviews, so now it is possible to align cues left and loop markers right in vertical waveforms. I have also decreased transparency of text label box, which should increase readability when textual label is centered on the waveform.

daschuer · 2016-08-18T10:35:08Z

Pleas give a hint, if this is ready for a second test.

ninomp · 2016-08-18T11:44:45Z

Oh OK This is ready for a second test.

daschuer · 2016-08-22T20:38:44Z

I have just finished my second test and it LGTM.
There is still the missing zero line, but that should us not stop from merging this.
@esbrandt: Any other pending issues that should be fixed before merge?
I consider this PR as uncritical since it does not directly effect the current skins.
I have found no regressions in my test.
We will probably find some more issue when starting to build a real vertical waveform skin, but it is OK for me.

ninomp · 2016-09-09T11:56:16Z

This is again ready for review (and merge).

esbrandt · 2016-09-16T07:51:38Z

@daschuer Sry for the delay, lgtm
I'll add documentation in the skin section of the wiki post-merge.
@ninomp Thanks, looking forward how users utilize this feature (if they find out about it :)

daschuer · 2016-09-16T14:04:58Z

Thank you for review @esbrandt

@ninomp Thanks, looking forward how users utilize this feature (if they find out about it :)

Yes, a vertical waveform skin with four decks side by side, would be a relay niice and self explaining addition to the Mixxx skin zoo.

Thank you @ninomp :-)

daschuer · 2016-09-16T14:07:15Z

@ninomp would you mind to advertise your work at http://www.mixxx.org/forums/viewforum.php?f=8

Add support for vertical waveforms and overviews

239a226

daschuer reviewed Jul 26, 2016
View reviewed changes

ninomp force-pushed the verticalwaveform branch from b12f439 to 239a226 Compare July 28, 2016 11:22

Various fixes and improvements of vertical waveform overview

a876017

Use left/right instead of top/bottom for alignment of vertical waveforms

92c041a

ninomp added 2 commits August 14, 2016 16:30

Enable left/hcenter/right alignment of marker's text on waveform and …

350748f

…overview Also, decrease transparency of marker's text on waveform.

Delete marks' images on waveform resize

1b01570

Merge branch 'master' into verticalwaveform

d3fe22b

daschuer merged commit 3bd3e5b into mixxxdj:master Sep 16, 2016

esbrandt mentioned this pull request Jun 24, 2017

WIP Manual 2.1.x updates mixxxdj/manual#59

Merged

37 tasks

mixxxbot mentioned this pull request Aug 22, 2022

Support vertical waveform #6346

Closed

Uh oh!

Conversation

ninomp commented Jul 26, 2016

Uh oh!

daschuer Jul 26, 2016

Choose a reason for hiding this comment

Uh oh!

ninomp Jul 27, 2016

Choose a reason for hiding this comment

Uh oh!

daschuer commented Jul 26, 2016

Uh oh!

ninomp commented Jul 27, 2016

Uh oh!

daschuer commented Jul 27, 2016

Uh oh!

ninomp commented Jul 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented Jul 27, 2016

Uh oh!

daschuer commented Jul 27, 2016

Uh oh!

ninomp commented Jul 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esbrandt commented Jul 31, 2016

Uh oh!

ninomp commented Jul 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esbrandt commented Jul 31, 2016

Uh oh!

ninomp commented Aug 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented Aug 18, 2016

Uh oh!

ninomp commented Aug 18, 2016

Uh oh!

daschuer commented Aug 22, 2016

Uh oh!

ninomp commented Sep 9, 2016

Uh oh!

esbrandt commented Sep 16, 2016

Uh oh!

daschuer commented Sep 16, 2016

Uh oh!

daschuer commented Sep 16, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ninomp commented Jul 27, 2016 •

edited

Loading

ninomp commented Jul 28, 2016 •

edited

Loading

ninomp commented Jul 31, 2016 •

edited

Loading

ninomp commented Aug 15, 2016 •

edited

Loading