LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8.12k stars 1.01k forks source link

enhancement: automation, add option 'Mirror pattern' #1289

Closed musikBear closed 9 years ago

musikBear commented 9 years ago

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 commented 9 years ago

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

tresf commented 9 years ago

Related: #119 #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. :)

diizy commented 9 years ago

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

Related: #119 https://github.com/LMMS/lmms/issues/119 #515 https://github.com/LMMS/lmms/issues/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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 :+1: I can't quite describe the behavior, I'll edit this after some testing.

Sti2nd commented 9 years ago

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 commented 9 years ago

@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 start flip After: mid flip But there's still issues with positive ranges. Before: volume flip After: volume end

Spekular commented 9 years ago

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

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

Spekular commented 9 years ago

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

Spekular commented 9 years ago

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: positive before positive after

Can this be in 1.1?

Sti2nd commented 9 years ago

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

Spekular commented 9 years ago

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 commented 9 years ago

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

Spekular commented 9 years ago

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

tresf commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

Oh, it don't!

diizy commented 9 years ago

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 commented 9 years ago

Alright, here's x flip working: before after I'll fork and commit my changes, then I'll make some icons and see if you guys like them.

Sti2nd commented 9 years ago

I get confused when you say wrong

Alright, here's x flip working:

That is y-flip :+1:

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

Spekular commented 9 years ago

@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 commented 9 years ago

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

tresf commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

Spekular commented 9 years ago

I exported it as 20x20 for the actual pull request/commit, tho. EDIT: These are also 20x20... Just like the interpolation modes icons. What's wrong with the size?

diizy commented 9 years ago

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

Alright, here's x flip working:

There seems to be something a bit off here... I'd assume a pattern that is 3 bars long would be flipped so that the end of the 3rd bar would become the new beginning.

How are you determining the area to be flipped?

Spekular commented 9 years ago

@diizy I've set it to be the position of the last note. I tried getting the length, but that was based on the length of the pattern visible in the song editor, so that didn't work right.

diizy commented 9 years ago

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

@diizy https://github.com/diizy I've set it to be the position of the last note. I tried getting the length, but that was based on the length of the pattern visible in the song editor, so that didn't work right.

I think that would actually be the best length to use for flipping.

That way the pattern that is visible to the user gets flipped. Makes sense...

Spekular commented 9 years ago

@diizy When it was that way, if the user flipped a two bar long automation from the song editor with only one bar visible, the first bar would flip, and the second part would be cut off. The way I'd want to use this feature is how it is now, but I guess other people might want to use it in other ways.

diizy commented 9 years ago

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

@diizy https://github.com/diizy When it was that way, if the user flipped a two bar long automation from the song editor with only one bar visible, the first bar would flip, and the second part would be cut off. The way I'd want to use this feature is how it is now, but I guess other people might want to use it in other ways.

Yeah, I think it logically makes more sense to have it use the visible area for flipping, even if that cuts off some data from the pattern. It's also more flexible, since that way you can control the area you want to flip by adjusting the size of the pattern beforehand.

Spekular commented 9 years ago

That would be inconsistent with the y flip though, which completely ignores length (because it's not required for the calculation). @tresf @Sti2nd what do you guys think?

diizy commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

Flip selection is what you are describing, diizy?

Spekular commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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...