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

Inconsistent steps on Beat/Bassline #1651

Closed
badosu opened this issue Jan 18, 2015 · 39 comments
Closed

Inconsistent steps on Beat/Bassline #1651

badosu opened this issue Jan 18, 2015 · 39 comments
Milestone

Comments

@badosu
Copy link
Contributor

badosu commented Jan 18, 2015

Suppose you are writing a beat with small steps, somethings like this:

2015-01-18-152720_574x170_scrot

And then you notice you want to add another instument to play off-beat (I don't know if this exists in english but would be written as "contratempo" on my language) to the first, so you drag and drop it do the b/b:

2015-01-18-152736_576x167_scrot

Notice that the new instrument has bigger steps than the previous, let's see what happens if you add notes every two beats:

2015-01-18-152801_572x175_scrot

If you play the pattern above you'll notice something odd, the last instrument plays beats until half the full time of the pattern, the following image depicts an equivalent pattern of what's happening (the 3rd instrument):

2015-01-18-152927_580x190_scrot

This is bad... I am not able to compose a complex beat with small steps adding new instruments when I find convenient. If I click on Add Steps all the instruments on the BB line will be refined, so the solution would be to add all instruments that I want to play previously and just after to add steps.

For me this is a bug, we should discover what would be a better behaviour:

  1. Allow instruments with different step sizes but create ways to add/reduce steps individually, this would require a fix to the current behaviour for the different step sizes
  2. Do not allow instruments to have different step sizes.

Let me describe the problem with an illustration, maybe this can help us more.

The Problem

This is what happens on LMMS today:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[XXXXX] [     ] [XXXXX] [     ] 

Is played as:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [X] [ ] [ ] [ ] [ ] [ ] 

Proposal 1

Distribute the pattern with small steps, if possible.

The equivalent now becomes:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [ ] [ ] [X] [ ] [ ] [ ] 

This would require us to deal with small steps with broken (Irrational in respect to the widest pattern) proportions.

Proposal 2

Loop the small steps pattern.

The equivalent now becomes:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [X] [ ] [X] [ ] [X] [ ] 

This is what I understood from FL Studio behaviour, let me know (@tresf) if I am mistaken.

Proposal 3

Mantain the current behaviour.

The equivalent now remains:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [X] [ ] [ ] [ ] [ ] [ ] 

This looks worse than the other proposals, but let me know if I am mistaken again and why this is better.

Proposal 4

Not allowing patterns with different step sizes, we always expand all patterns to the widest pattern length.

@tresf tresf added this to the 1.3.0 milestone Jan 18, 2015
@tresf
Copy link
Member

tresf commented Jan 18, 2015

For me this is a bug

Marking as an enhancement for now, since this has been default behavior since the beginning of time.

I'm not sure your illustrations are accurate though...

I would expect the patterns to play:

x   x   x   x   x   x   x   x   
  x   x   x   x

Which simply means we should shrink (or expand) each pattern to its appropriate width as compares to the widest (no?)

-Tres

@badosu
Copy link
Contributor Author

badosu commented Jan 18, 2015

I'm not sure your illustrations are accurate though...

You can reproduce my steps and see if this is not the case, I used master.

Which simply means we should shrink each pattern to its appropriate width as compares to the widest (no?)

You mean, adding steps and make everything consistent? If this is your proposal I agree and outline it as option number 2 on the list of proposals to fix this.

@badosu
Copy link
Contributor Author

badosu commented Jan 18, 2015

If I did not make my point clear, this is what should happen (in my opinion) if you add another instrument:

2015-01-18-163010_575x179_scrot

And that's what's not happening. This is huge for me as it makes it impossible to adjust a beat with more steps than the time signature without wiping everything out.

An alternative solution is to add/reduce steps for each instrument individually, but then we would have to understand how this would play on the whole pattern.

@tresf
Copy link
Member

tresf commented Jan 18, 2015

You can reproduce my steps and see if this is not the case, I used master.

No, we are saying the same thing... the three patterns threw me off, but we're certainly saying the same thing.

this is what should happen (in my opinion) if you add another instrument:

Sure, or show the right half as blank until they are filled in (or alternately repeat it twice on playback, rather than playing it once?)

-Tres

@Sti2nd
Copy link
Contributor

Sti2nd commented Jan 18, 2015

Auto extend with GUI like the last picture by badosu is a good approach IMO.

@diizy
Copy link
Contributor

diizy commented Jan 18, 2015

We've talked about per-pattern time signature. With that change, each
bb-pattern would also have its own time sig, and length would be an
adjustable property also.

The idea is that after this change the b/b-patterns will no longer have
"add bar" or "remove bar" buttons, instead those would be replaced with
a length widget which controls the length of the bb-pattern in bars, and
a time sig widget which controls the length of the bar in the bb-pattern.

So this issue will probably go unfixed until then.

@lukas-w
Copy link
Member

lukas-w commented Jan 19, 2015

An alternative solution is to add/reduce steps for each instrument individually, but then we would have to understand how this would play on the whole pattern.

That feature is in the pattern's context menu:
screenshot - 190115 - 10 42 09

@musikBear
Copy link

or open in piano-roll
Know its darn obvious, but just want to mention it
You have to do it anywhitch, if you need 'swing' on the events. -A dial for that would be a serious improvement

@badosu
Copy link
Contributor Author

badosu commented Jan 20, 2015

@lukas-w Never noticed that, great! So that fixes my issue with the step assignment inconsistency, at least I can move on with my projects when I need to add new tracks to BB.

However I still feel that we have a problem... It's not clear to me how tracks with different step quantities should behave, and for me the current behaviour is bad. Does anyone know how commercial DAW's solve this issue? And if even this is allowed at all?

@tresf
Copy link
Member

tresf commented Jan 20, 2015

IIRC, FLStudio relies on time signature and expands all of them. IIRC, Ableton uses a drum kit instrument stacker interface, but writes midi-style notes to a piano roll.

The step sequencer is really a simplified piano roll, so I can see why some DAWs simply use the piano roll out of the box.

@badosu
Copy link
Contributor Author

badosu commented Jan 20, 2015

IIRC, FLStudio relies on time signature and expands all of them.

@tresf What does this mean? That a pattern with 8 steps is looped twice, when the widest pattern has 16 steps, instead of once like lmms does today (playing until half the length of widest pattern)?

Let me describe the problem with an illustration, maybe this can help us more.

The Problem

This is what happens on LMMS today:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[XXXXX] [     ] [XXXXX] [     ] 

Is played as:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [X] [ ] [ ] [ ] [ ] [ ] 

Proposal 1

Distribute the pattern with small steps, if possible.

The equivalent now becomes:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [ ] [ ] [X] [ ] [ ] [ ] 

This would require us to deal with small steps with broken (Irrational in respect to the widest pattern) proportions.

Proposal 2

Loop the small steps pattern.

The equivalent now becomes:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [X] [ ] [X] [ ] [X] [ ] 

This is what I understood from FL Studio behaviour, let me know (@tresf) if I am mistaken.

Proposal 3

Mantain the current behaviour.

The equivalent now remains:

[X] [ ] [X] [ ] [X] [ ] [X] [ ] 
[X] [ ] [X] [ ] [ ] [ ] [ ] [ ] 

This looks worse than the other proposals, but let me know if I am mistaken again and why this is better.

Proposal 4

Not allowing patterns with different step sizes, we always expand all patterns to the widest pattern length.

@tresf
Copy link
Member

tresf commented Jan 21, 2015

IIRC, FLStudio relies on time signature and expands all of them.

@tresf What does this mean? That a pattern with 8 steps is looped twice, when the widest pattern has 16 steps, instead of once like lmms does today (playing until half the length of widest pattern)?

Sometime a picture is worth 1000 words.... Pretty much, option 2 in your original bug report:

"2. Do not allow instruments to have different step sizes."

image

In terms of time signature, I don't know if it's track or pattern based, but here is how it's handled...

image

Hope that helps explain.

P.S. Unrelated, but I just snagged a screenshot, I don't have this installed to actually try it out. 🍺

@badosu
Copy link
Contributor Author

badosu commented Jan 22, 2015

@tresf Thanks for the pictures...

So, is this something you think would be worth working on? Or is it only me that feels this as a problem?

Also, by using proposal 2 (not allowing different step sizes like FL) we are simplifying the usage of the BB which is a huge gain in usability and maintenance for me.

@tresf
Copy link
Member

tresf commented Jan 22, 2015

From a simplicity perspective, I'd vote we just draw the shorter patterns properly, since the playback seems to be fairly intuitive (right?)

The reason I say this is that we already have a way to add and remove steps to all patterns via:
image

So I vote we just size the smaller patterns properly for now:

image

image

Does that make sense?

-Tres

@tresf
Copy link
Member

tresf commented Jan 22, 2015

Reason being is that the auto-expansion (and shrinking) can get hairy when it's not explicitly defined in the BB pattern like how FL handles it. Without that little display widget, it can be quite confusing to users I think... and we probably don't want to add some type of pattern length widget in for 1.2 (right?)

@diizy
Copy link
Contributor

diizy commented Jan 22, 2015

On 01/22/2015 07:00 PM, Tres Finocchiaro wrote:

From a simplicity perspective, I'd vote we just draw the shorter
patterns properly, since the playback seems to be fairly intuitive
(right?)

It's not just a drawing issue. Each step in a bb-pattern represents a
note, and if those notes are absent from the pattern, the steps won't
get drawn. Empty steps are represented as 0-length notes. (This is
incidentally a quite poor and bug-prone design which we should improve
at some point.)

The problem here is not with the patterns per se, but with there not
being a proper length-property in bb-tracks. Each pattern inside the
bb-pattern has its own length, which is what is causing this issue (when
you add a track, there's no length property to check so the patterns get
added as default-length).

Adding a "length" property to bb-tracks is the most future-proof way to
solve this. Then each pattern that belongs to a bb-track can just
inherit its length from the bb-track.

@Sti2nd
Copy link
Contributor

Sti2nd commented Jan 22, 2015

So I vote we just size the smaller patterns properly for now:

But why not automatically fill them in? I can't think of empty bars at the end of a beat pattern being useful? If you want the beat to be alternate you should place a B&B pattern in the Song Editor alternately, right? Would love to get some examples of how you can use it as a feature :)

@badosu
Copy link
Contributor Author

badosu commented Jan 22, 2015

But why not automatically fill them in?

@Sti2nd Because you have to have criteria on how to expand patterns, this is important for broken tempo patterns.

I'd rather have a consistent whole BB container length property as @diizy described.

@tresf
Copy link
Member

tresf commented Jan 22, 2015

Adding a "length" property to bb-tracks is the most future-proof way to solve this. Then each pattern that belongs to a bb-track can just inherit its length from the bb-track.

Right, which I mentioned in my next comment here...

Reason being is that the auto-expansion (and shrinking) can get hairy when it's not explicitly defined in the BB pattern like how FL handles it. Without that little display widget, it can be quite confusing to users I think...

But to simply shrink the widget's display size to a respective fraction of the overall width can mitigate this for now until a proper enhancement is made. Fix the current functionality until we can later enhance the overall functionality.

@diizy
Copy link
Contributor

diizy commented Jan 22, 2015

On 01/22/2015 08:51 PM, Tres Finocchiaro wrote:

Adding a "length" property to bb-tracks is the most future-proof
way to solve this. Then each pattern that belongs to a bb-track
can just inherit its length from the bb-track.

Right, which I mentioned in my next comment here...

Reason being is that the auto-expansion (and shrinking) can get
hairy when it's not explicitly defined in the BB pattern like how
FL handles it. Without that little display widget, it can be quite
confusing to users I think...

But to simply shrink the widget's display size to a respective
fraction of the overall width can mitigate this for now until a proper
enhancement is made. Fix the current functionality until we can later
enhance the overall functionality.

Frankly, shrinking the display size of the patternview is probably more
trouble than it's worth - at the very least it adds kludge code that
we'll have to clean up later, and may not actually be any easier/faster
than the proper implementation.

@tresf
Copy link
Member

tresf commented Jan 22, 2015

Frankly, shrinking the display size of the patternview is probably more trouble than it's worth - [...] and may not actually be any easier/faster than the proper implementation.

Well.... considering the proper implementation puts the pattern size in a different part of the codebase (and a different part of the XML) and it may also add a new QWidget to the toolbar, I'm not sure it would be easier and faster.

at the very least it adds kludge code that we'll have to clean up later,

Sure, there will be a for loop that will need removal afterward. :)

-Tres

@diizy
Copy link
Contributor

diizy commented Jan 22, 2015

On 01/22/2015 09:53 PM, Tres Finocchiaro wrote:

Frankly, shrinking the display size of the patternview is probably
more trouble than it's worth - [...] and may not actually be any
easier/faster than the proper implementation.

Well.... considering the proper implementation puts the pattern size
in a different part of the codebase (and a different part of the XML)

Not really. It just adds a new property to bb-track, the patterns
themselves don't actually need to change - remember that patterns are
patterns regardless of where they are (song editor/bb), so they still
need to have a length property - just it would be ignored in bb-tracks
(actually morelike the patterns would be padded/clipped) and the
bb-track length used instead.

Backwards compat would be trivial by taking the qMax() of the pattern
lengths and using that as the bb-track length when it's not available -
this would preserve the current behaviour for old projs.

and it may also add a new QWidget to the toolbar, I'm not sure it
would be easier and faster.

Changing the +/- to a lcd widget is trivial and not really related with
this issue - it can be done (or not) regardless of this issue. Changing
the widget doesn't change the underlying logic, just the UI to access it.

@aaronfjerstad
Copy link

Most other pattern editors will simply have a grid, with each row representing an instrument. Since steps and piano-roll input are mutually exclusive for a given bassline, piano-roll inputs can take up the whole row just as they do now. Having a four step instrument take up the same amount of room as an eight-step instrument leads to a GUI that doesn't represent what the user hears. Having not looked at the source, I don't know how well the model and view are separated, but I shouldn't think making that change would be difficult or detrimental: It would be even better if horizontal and vertical zooming could be smooth, rather than discreet (100%, 200%, etc...), and if said zooming was present at all within the bassline editor.

@Spekular
Copy link
Member

Spekular commented Jul 2, 2015

I was going to add an issue for the scaling of piano roll patterns in the BB editor but as I described it I realized it's the same as this issue. If your piano roll pattern has as many bars as your BB pattern, you can eye the pattern to see where a note starts and ends:
1
2
But if your Piano roll pattern is longer or shorter, it just scales. At a glance in the BB editor
it seems like the note will play for two bars:
3
Longer patterns are a bit better as the notches show the bar splits.
4
Worst case scenario the note looks shorter than the BB pattern despite being longer:
5

I definitely think we should force all tracks in a BB pattern to be the same length, including piano roll patterns. I also think it's pretty obvious to show empty space when a piano roll pattern is shorter than a step pattern, but I'm not sure about adding blank steps when the piano roll pattern is longer. Perhaps the length of a piano roll pattern in the bb-editor shouldn't be allowed to be longer than the step patterns.

@musikBear
Copy link

(....aaaaa...nnn....dddd ...the new fancy 'bubble' notes that natively looks larger than they actually are made logically, does so absolutely not help determine the visual size of a very small note in P-roll )
sigggggghhhh

insert very sad kitten here..

@badosu
Copy link
Contributor Author

badosu commented Jul 3, 2015

@musikBear The new fancy 'bubble' notes actually occupy less area than the previous layout. Could you explain you complaints with an understandable discourse?

@musikBear
Copy link

@badosu for me it is more difficult to see the exact tick-position of a bubble-note, than a right-angled rectangular-note
-Especially on videos, where the image-quality is limited, but even on screen-shots

Here i have an old screnie
oldshape
My eyes can see that these notes are placed similar

Here the new shape
newshape

I have more difficulties seeing placement position with these notes
So it is ergonomics -more strain to -at least my eyes, with the new shape, and i cant 'see' that they appear smarter or better, just more like FL's

...and surely, the bubble take up more space inside the grid -not less ?..

@grejppi
Copy link
Contributor

grejppi commented Jul 4, 2015

@badosu, is there a reason for the rounded notes other than being rounded? Does it provide new functionality, or is it just for the looks?

@badosu
Copy link
Contributor Author

badosu commented Jul 4, 2015

@musikBear Your comparison is somewhat unfair, the last 2 notes have a different size (you probably hold alt while painting them). Using the same pattern would be better.

See: https://cloud.githubusercontent.com/assets/347552/5558524/113092d8-8d12-11e4-984e-8a77af3582e2.png

You participated of the issue #64 when I suggested this, you could have helped the discussion on that occasion.

@grejppi See: #1507

I simplified code for drawing notes and just rounded them, this can be easily reverted.

In terms of LMMS as a whole I think the square notes really degrade the experience instead of improving, if the issue is because of the smallness of the notes (this really the issue with making them difficult to see, in my opinion) I would rather enable increasing note height, which is not possible today.

This is the same issue one can have with QTractor for example, the product looks great, but when you jump on the note editor...

Anyway, what the team thinks it's better, it's better. I will digress no more.

@Umcaruje
Copy link
Member

Umcaruje commented Jul 4, 2015

Well the problem I have with the rounded notes is that they don't look very clear to me and tbh they have a lot of shades of the same color. If the rounded rectangles were borderless, they'd be more visually appealing to me.

Again this is personal preference, and I personally am a big fan of flat design, and if I was in charge of the UI, I'd probably take out a lot of gradients from it, since they look really ugly to me.

@badosu
Copy link
Contributor Author

badosu commented Jul 4, 2015

@Umcaruje I agree with you that the Piano Editor need some work to improve the readability/usability/look.

With regard to the gradients and brightness, they exist as a visual indication of panning and volume, this already existed at the time I started on the border work, I just adapted it. I can't see how they are useful today, maybe power users could help us identifying that.

I feel like unless we have a full art direction going on and a commitment to implement it, any work on this area will be overriden. Anyway, feel free to suggest reverting the border changes or improving it, it's not very hard to make it look better.

But one thing that would make wonders and would probably be reused even in the occasion of a redesign implementation is the ability to change the height of the notes.

@Umcaruje Umcaruje added the ux label Jul 4, 2015
@Wallacoloo
Copy link
Member

Wallacoloo commented Jul 4, 2015 via email

@Umcaruje
Copy link
Member

Umcaruje commented Jul 4, 2015

Yeah, that would be the best solution. Notes could have the border-radius and border-width css properties.

@badosu
Copy link
Contributor Author

badosu commented Jul 4, 2015

@Wallacoloo @Umcaruje Yes, this could be totally themeable. This would require turning the specific notes into widgets though. Today they are drawn under the paintEvent of the PianoRoll.

@badosu badosu closed this as completed Jul 4, 2015
@badosu badosu reopened this Jul 4, 2015
@Wallacoloo
Copy link
Member

@badosu I believe we could make these properties of PianoRoll instead. style.css would look like:

PianoRoll {
    note-border-radius: 5;
    note-border-width: 2;
    [...]
}

The downside is of course that "border-width" and "border-radius" are standard CSS properties whereas these new ones are not.

On the other hand, it may make sense to split the PianoRoll apart into widgets. Looking at that 505 line long paintEvent() function, every time a small portion of the piano roll is invalidated, we have to redraw the entire thing. If it was broken up into widgets, I think Qt would handle the invalidation more intelligently and only redraw what really needs to be redrawn.

@badosu
Copy link
Contributor Author

badosu commented Jul 4, 2015

@Wallacoloo This would be at your convenience, adding a QProperty would be much easier than refactoring the PianoRoll to use note widgets. But if you think you can do it, that would be great!

@musikBear
Copy link

Again this is personal preference, and I personally am a big fan of flat design,

exactly my preferences too - I will favor functionality over design always
Take a look at this picture from the visual-impaired topic #1935
Is that design really thought through (i DO NOT mean this as any bashing of anyone at all)
https://cloud.githubusercontent.com/assets/6662066/7812635/7369d644-03b4-11e5-88fe-4f2b8f36a44b.png

@softrabbit
Copy link
Member

Is that design really thought through

No, not much. :) I thought what came before it was even worse, so I intended it as an improvement...

And here's how it looked with the note style of its time:
lmms_pianoroll_markings

@Umcaruje
Copy link
Member

Umcaruje commented Aug 13, 2016

Closed via #2883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests