enhancement: automation, add option 'Mirror pattern'

[musikBear]

im sure all have had/ done this task.
you make a smooth pattern that works well. Then you think - oh, it would have been nice if i could 'flip' this for the oposit transistion. I sugest to add a button 'Mirror pattern' So a user will get 2 automation blocks, one with the pattern user drawed, and one exactly oposit, or mirrored of the pattern.

[Spekular]

👍 Should be relatively simple to implement too, for someone familiar
with the codebase.

[tresf]

Related: #119 #515

👍 Should be relatively simple to implement too, for someone familiar
with the codebase.

Let's get you up and running on a dev machine first. :)

[diizy]

On 11/12/2014 07:31 PM, Tres Finocchiaro wrote:

Related: #119 #119 #515
#515

:+1: Should be relatively simple to implement too, for someone
familiar
with the codebase.

Let's get you up and running on a dev machine first. :)

AutomationPatternView is where the context menu of an Automation pattern
is implemented.

In there, you need to add an option to "reverse" (a better term than
"mirror" IMO, less ambiguous) the pattern. The context menu item needs
to be connected to a slot in AutomationPattern.

Then you add that slot to AutomationPattern, which copies m_timeMap to a
temp variable, then empties m_timeMap, and reads the values from the
temp variable in reverse and copies them back to m_timeMap. To get the
length of the AutomationPattern (in order to know the point from which
to start reversing), use the methods startPosition() and endPosition()
in AutomationPattern, substract to get length.

After that, generateTangents() has to be called once at the end of the
slot, in order to ensure functioning of cubic hermite mode.

[Spekular]

Are we talking about flipping over the x axis or y axis here? Because I can
see both being useful, so maybe both should be added? An x flip could be
based on max value - point value = new value for positive ranges (and
positive to negative ranges could be shifted first up then down) or
multiplying the points y value by 1 (and shifting positive ranges down then
up).

[Sti2nd]

Yeah, both ways would be useful. Also I suggest to copy the flipped version to the clipboard so one could easily paste it in the same pattern/clip/segment/box. Fade in and out for example. Auto-pasting the pattern every other time flipped and original could be a way of implementing the repeat function you thought were useful.

Hmm, but copying automation isn't possible yet.

[Spekular]

Hmm. I've got this working, sort of. You can only flip it vertically so far, it takes forever, and it doesn't actually mirror the pattern 👍 I can't quite describe the behavior, I'll edit this after some testing.

[Sti2nd]

If it takes forever, I reckon you make a new opposite point for every tick or every quantization?

In linear and discrete mode it would be a matter of replacing/giving the points a new value = largest value possible - point value.
In cubit hermite mode you must also change the function used for it to really be inverted. I have never learned about hermite polynomials or hermite interpolation or hermite progression, or any hermite... http://en.wikipedia.org/wiki/Hermite_interpolation it could be that changing the interpolation value to negative would do it?

[Spekular]

@Sti2nd originally it added tons of points (and only between the start and the second point), but I've fixed that now.
@tresf @diizy Closing/reopening and taking a screenshot both make it faster, so I'm assuming I need to call some sort of update/redraw. I've gotten it working for negative to positive ranges.
Before

After:

But there's still issues with positive ranges.
Before:

After:

[Spekular]

Before you ask, here's linear before & after:

And here's cubic (worth noting that cubic updates really fast, I'm guessing it calls an update in generate tangents or something):


Holy crap I'm proud :D

[Spekular]

engine::automationEditor()->update(); makes it update fast, now onto positive ranges!

[Spekular]

Turns out I hadn't updated the positive range calculations after I figured out the problem in the negative range ones. It's working now, here's before and after:

Can this be in 1.1?

[Sti2nd]

How long does it take to invert when you press the button?

[Spekular]

Not a noticeable time. I'll test with a more advanced pattern later on and
see if it ever reaches the point where I can time it.

[Sti2nd]

Oh, then it is fast... I thought you said it was slow

[Spekular]

@Sti2nd it seemed slow earlier, but it was just the visual part that wasn't
getting updated.

[tresf]

Very nice progress @Spekular!

Perhaps I read it wrong, but I thought @musikBear's request was to extend and flip (rather than simply flipping?)

i.e ...-------... becomes ..-------...--........--

-Tres

[Sti2nd]

Well, @tresf , it was, but Spekular haven't made the function musikBear wanted yet, which was flipping vertically, now it is horizontally! (oh, so you did write wrong up there @Spekular ;) )

[Spekular]

Flipping in both y and x is useful. For now it's only y, I'll work on
getting x implemented. Repeat can probably be implemented later if we need
that, but iirc the original request was to mirror (considering the title,
too)

[diizy]

On 11/21/2014 10:52 PM, Stian Jørgensrud wrote:

If it takes forever I reckon you make a new opposite point for every
tick or every quantization.
In linear and discrete mode it would be a matter of replacing/giving
the points a new value = largest value possible - point value.
In cubit hermite mode you must also change the function used for it to
really be inverted.

The function stays the same... it doesn't care which way it is.

[Sti2nd]

Oh, it don't!

[diizy]

On 11/23/2014 01:22 PM, Stian Jørgensrud wrote:

Aw yeah, that would be seriously advanced to implement at least, so I
get it.

I thought you wanted to have it perfectly mirrored

It does get perfectly mirrored. Cubic hermite is symmetric, if you have
the same nodes backwards, the curve is the exact same but backwards.

[Spekular]

Alright, here's x flip working:


I'll fork and commit my changes, then I'll make some icons and see if you guys like them.

[Sti2nd]

I get confused when you say wrong

Alright, here's x flip working:

That is y-flip 👍

@diizy Yeah, the images showed I was wrong, so I deleted my comment.

[Spekular]

@Sti2nd Those images show the pattern being flipped/mirrored OVER the y axis, but they're flipping in the direction of the x axis (the points move on the x axis). It's more clear if you consider having two points above the 0 line getting y flipped. The mirror over the x axis, but they move down on the y axis and end up under the 0 line. I agree it's not all that clear, but that was my reasoning. The icons should do a better job conveying it once they're done, and the name can be changed.

[Spekular]

Here's what I came up with for X & Y Flip Icons:

[tresf]

This is what Gimp uses for "flip". Not really the same context, but figured I'd mention it for sake if conversation.

http://docs.gimp.org/en/images/toolbox/toolbox-flip.png

[Sti2nd]

I think the icons look good, could be clearer with real arrows at the ends. As long as there is hover text which explains what the function does the user will get it.

[tresf]

We shouldn't change the dimensions of the entire toolbar for a single pixmap tho.

[diizy]

On 11/23/2014 06:22 PM, Spekular wrote:

That would be inconsistent with the y flip though, which completely
ignores length (because it's not required for the calculation).

So then it is not inconsistent. Length simply isn't a factor for
vertical flipping.

[Spekular]

@diizy I still feel like it's inconsistent, and to me it looks like a bug when the pattern gets cut off. It doesn't seem as useful either, especially considering the first part of the pattern gets kept, and not the last part as one would expect.

[diizy]

On 11/23/2014 06:44 PM, Spekular wrote:

@diizy https://github.com/diizy I still feel like it's inconsistent,
and to me it looks like a bug when the pattern gets cut off. It
doesn't seem as useful either, especially considering the first part
of the pattern gets kept, and not the last part as one would expect.

Why would anyone expect the part that is invisible to be kept?

A pattern should be considered as what is seen in the song editor. The
length is set by the user, anything that is beyond that length should be
considered superfluous leftover data. We have a mechanism for setting
the length of the pattern, so that should be respected.

[Spekular]

We have a mechanism for setting
the length of the pattern

It must be failing. When I make a three bar long automation, the track in the song editor stays at one bar.

[diizy]

On 11/23/2014 06:54 PM, Spekular wrote:

We have a mechanism for setting
the length of the pattern

It must be failing. When I make a three bar long automation, the track
in the song editor stays at one bar.

Right, because you set the length from the song editor. That's the
existing UI for determining the length of the pattern, and it should be
respected.

[Spekular]

Well if only the visible part of the pattern plays, why not reverse the entire pattern and let the user "crop" it to the part they want. That's extremely flexible.

[tresf]

let the user "crop" it to the part they want. That's extremely flexible.

Mainly because cropping from the left isn't easy in our current editor.

I don't necessarily disagree with this behavior though. When you flip a layer in Gimp for example, it flips everything, unless you've explicitly selected a smaller area. The point is, does the pattern signify a "smaller selection"? Currently, yes... and no, since a one-bar pattern usually starts as the canvas until the pattern is expanded. Piano rolls auto-expand which prevents this dilemma. Automation and bbeditors can behave a bit different in this regard.

[Sti2nd]

I think it is good now, I must admit that I don't exactly understand what diizy is trying to say. If you have a point far to the right which you aren't seeing in the Song Editor, then you flip, maybe you didn't want that to be a part of the flip, but you probably knew the point was there, and it would be strange if it was cut out or continued to be there IMO.

[Sti2nd]

Flip selection is what you are describing, diizy?

[Spekular]

@Sti2nd "length" in automation pattern gets the visible part of the pattern in song editor, so if your pattern takes more bars in the automation editor than in the song editor, the extra bars would get cut off. I've implemented it so they stay instead, but diizy wants me to revert it.

[diizy]

Only the visible part should be reversed... the rest don't have to be
cut though, think of the selection (ie. pattern length) as the area that
gets affected by the reverse, and leave the rest of the pattern untouched.

[Spekular]

@diizy ah ok. That makes more sense but it's still problematic. only reversing part would require reversing and clearing only part of the pattern while making sure to keep the rest. Also if the visible pattern doesn't start at the same point as the whole pattern there would have to be an offset implemented. I'm not sure I'd like to make this function more advanced considering my skill level.

[Sti2nd]

Both cons and pros... Diizy have a point in that the user probably only want to flip the area visible in the Song Editor, though I can easily see it being annoying. Example: You drag a knob to autotrack and then it creates one bar long pattern. I usually don't drag out how long automation I want before I have made the automation in the Automation Editor, so when a user click flip when editing the automation he will be surprised. I think that is the workflow many have, I have watched many guys on YouTube, lol.

Question: I always drag my automation tracks to the end of my song, would a flipping x (mirror vertical) cause all the points to be at the end of the automation instead of the beginning? And when per track automation is implemented, would that be like dragging them out, like I do, and flipping would cause the points to go to the end of the automation instead?

[Sti2nd]

Why would anyone expect the part that is invisible to be kept?

My previous answer explains this, because people don't adjust the length of the pattern in the Song Editor before they make the automation.

[diizy]

On 11/23/2014 07:33 PM, Spekular wrote:

@diizy https://github.com/diizy ah ok. That makes more sense but
it's still problematic. only reversing part would require reversing
and clearing only part of the pattern while making sure to keep the
rest. Also if the visible pattern doesn't start at the same point as
the whole pattern

Not possible. Everything starts at 0.

I'm not sure I'd like to make this function more advanced considering
my skill level.

Well, you can learn while you do it...

[Spekular]

Maybe I can add both. In the automation editor it flips the whole thing
(having it only flip part without an indication of which area will be
flipped would be a pain). In the song editor I can add a "reverse visible
area". Also, I know that all patterns start at 0, but what if the user has
moved the left part of the pattern so the middle is visible instead of the
start? Or is that not possible?

[diizy]

On 11/24/2014 07:32 AM, Spekular wrote:

Maybe I can add both. In the automation editor it flips the whole thing
(having it only flip part without an indication of which area will be
flipped would be a pain). In the song editor I can add a "reverse visible
area".

Even in the automation editor, I would assume that the flipping would
still happen accross the bar lines. Not so that the flipping happens at
an arbitrary point in the middle of the bar.

Also, I know that all patterns start at 0, but what if the user has
moved the left part of the pattern so the middle is visible instead of the
start? Or is that not possible?

No that is not possible currently and probably never will be. It's safe
to assume that 0 is always the left edge of the pattern.

[Spekular]

@tresf @diizy I've been trying to find time to work on this, but my branch was 103 commits behind. When I pulled all those changes to my branch it says it's 1 commit ahead, which it shouldn't be.I'd rather learn to do this right than to always delete and recreate my branches when I need to update them, so what should I do?

[diizy]

On 12/13/2014 02:26 PM, Spekular wrote:

@tresf https://github.com/tresf @diizy https://github.com/diizy
I've been trying to find time to work on this, but my branch was 103
commits behind. When I pulled all those changes to my branch it says
it's 1 commit ahead, which it shouldn't be.I'd rather learn to do this
right than to always delete and recreate my branches when I need to
update them, so what should I do?

Try git log or git diff to see if there's anything in your branch that
doesn't belong there.

[Spekular]

@diizy It wasn't ahead before the pull, which is odd. I think it's counting the pull I used to get up to date as a commit. Should I still do log/diff?

[tresf]

it says it's 1 commit ahead, which it shouldn't be.

You can usually hard reset to a specific commit, but as Vesa said, it doesn't matter much if you're ahead or behind, just that the diff against upstream only touches the files you've worked on.

I took at look at your branch and didn't find anything obvious that stood out. When writing your first feature, it's hard to know how long these branches will be out there, but as a rule of thumb, a major change should be put into its own branch so that your own personal clone of upstream version (i.e. Spekular/master or Spekular/stable-1.1, etc) doesn't get mucked up with experimental changes.

The other benefit of using pull requests is that they never need to actually be accepted, so if you royally screw something up and want to start from scratch, you can always copy off your code and delete your personal branch and start fresh. I've done this before when I accidentally rebased tresf/stable-1.1 against lmms/master. It's a learning process and it can certainly be frustrating! Just never delete your repo without backing your stuff up first! :) We'll never accept a pull request which has obvious problems! :)

[Spekular]

Alright. I've got a repo setup with this feature working on master. Two issues remain:

1. You guys work fast, so the new fork I made this morning is already 38 commits behind, and I still don't know how to fix that properly.
2. I haven't gotten horizontal flipping working the way @diizy wants. X flip accepts a boolean called "visible", which is false when it's called from the automation editor, and true when called from the song editor, but someone else will have to fix it/I'll have to fix it later.
   1 needs to be resolved before I issue yet another pull request, 2 it's up to you guys/diizy to decide.

[tresf]

1. You guys work fast, so the new fork I made this morning is already 38 commits behind, and I still don't know how to fix that properly.

How many files have you had to change? Sometimes it's easier to commit them by hand, but normally, you'd just rebase your code (or "fast-foward" it) against upstream as explained here:
https://github.com/LMMS/lmms/wiki/%5BGit%5D-Fast-forward-personal-branch-to-upstream

Before you do this, backup your code.

If you run into problems, just backup your changes and reapply them by hand.

Sometimes fast-forwarding is quick and it merges automatically, but if there have been major changes, it can require quite a bit of work and you'll have to weigh the difficulty of hand-merging each conflict over just starting fresh from master and pasting in your changes again.

-Tres

[Spekular]

@tresf that seems to work, thanks! Now onto issue number 2. I'd consider this worthy of adding even in it's current state, so it gets into LMMS already (it's taken way longer to deal with git issues and all that than making it, and it's bothering me that it was pretty much done ages ago), but what do you think?

[tresf]

Do a pull request and wait for feedback then. 👍

[diizy]

On 12/22/2014 11:42 PM, Spekular wrote:

@tresf https://github.com/tresf that seems to work, thanks! Now onto
issue number 2. I'd consider this worthy of adding even in it's
current state, so it gets into LMMS already

It'll have to go to master branch anyway, it's way too late to get in to
1.1 - we don't need any more delays to it...

So there's no rush really... you can take your time to get it right.

[tresf]

Closed per #1486