Basic ghost notes feature. #4575

ghost · 2018-09-02T15:22:45Z

This adds a option to the pattern context-menu to add the pattern as ghost notes to the Piano-Roll. Also added a button in the Piano-Roll to clear the ghost notes.

Edited by @tresf: Closes #4449, #4303, #520, Related: #1438, #696

* Changed `int m_noteOpacity` to `unsigned char m_noteOpacity` (aslo converted the relative functions (`noteOpacity` and `setNoteOpacity`) to `unsigned char`) since the maximum value will be 255 and minimum 0.

LmmsBot · 2018-09-02T15:37:40Z

Downloads for this pull request

Linux: download.
Windows 32 Bit: download.
Windows 64 Bit: download.
Mac: download.

Generated by the LMMS pull requests bot.

…n` in the PianoRoll header since we defined it as `pattern` in the cpp file as well.

src/tracks/Pattern.cpp

PhysSong · 2018-09-03T11:19:00Z

You should handle the case where the ghost pattern is deleted. If not, you may get a crash after deleting the pattern.
You can use destroyedPattern signal for this purpose. Here's an example:

lmms/src/gui/editors/PianoRoll.cpp

Lines 656 to 657 in a1415a3

    
           // make sure to always get informed about the pattern being destroyed 
        
           connect( m_pattern, SIGNAL( destroyedPattern( Pattern* ) ), this, SLOT( hidePattern( Pattern* ) ) );

include/PianoRoll.h

…n` signal. * changed `and` to `&&` for MSVC support. - Thanks to PhysSong for the review.

ghost · 2018-09-03T19:58:42Z

@PhysSong yes it crashed indeed on deletion of the ghost pattern, so I connected to the destroyedPattern signal as you suggested. It doesn't crash anymore on ghost pattern deletion.

Also changed the and to && for MSVC support.

Thanks.

…n the pattern context-menu.

enp2s0 · 2018-09-18T03:51:33Z

This is definitely a much-needed feature!

dev-sr0 · 2018-09-19T23:00:55Z

This is so good. I can't overstate how much this would help me.

tresf · 2018-09-24T18:43:04Z

@Umcaruje please chime in on artwork or open a task for yourself and/or @RebeccaDeField to review it after merge.

ghost · 2018-09-24T19:40:00Z

I’m also working on a feature to set ghost notes in the Automation Editor with the option to scale the ghost notes. I have something working but I need to clean it first before putting anything online, but should I open a new PR or just merge the code with this?

The slider normally scaled the ghost notes vertically, on double click of the slider handle it controls the position offset. Styling is just there for example.

tresf · 2018-09-24T20:00:49Z

should I open a new PR or just merge the code with this?

I recommend a new PR. This one's near-ready for merge, so you can just rebase/fast forward the other PR off of master once this is merged.

ghost · 2018-09-24T20:06:30Z

Ok great I'll do that, thanks.

FeralBytes · 2019-01-06T01:56:13Z

@cyberdevilnl I can't wait for this to get merged, Thank you for your work on this much needed feature!

zonkmachine · 2019-01-06T02:20:04Z

@Umcaruje please chime in on artwork or open a task for yourself and/or @RebeccaDeField to review it after merge.

The artwork folks isn't that active right now so I don't think we can wait for them. What else is left to do here?

PhysSong · 2019-01-06T02:24:54Z

Testing? If this one works well, why not merge this? The artwork stuff can be handled later.

zonkmachine · 2019-01-06T02:28:51Z

OK. I'll take it for a spin. :)

FeralBytes · 2019-01-06T11:20:07Z

Why even delay for the artwork folks, the art work is included in the commit? ~~If it is not good enough then the artist can come back and fix it when they comeback from hiatus, there commit activity is practically nill last year~~. (Correction/Addition: I do see that they have been actively communicating though in other PRs so that is good.) I will be downloading this to use for sure and I will report back if I encounted any issues.

zonkmachine · 2019-01-06T16:52:57Z

Tested works. Basically it works like this. When you have an open Piano Roll you can right click a pattern in the Song Editor and set it as ghost pattern in the open Piano Roll. If you have no Piano Roll open it's the last opened Pattern that is effected (and the pattern is opened in a Piano Roll window when the ghost pattern is set). You can clear the ghost pattern with a button but the ghost pattern is also not persistent so when you close the Piano Roll it will be forgotten. The artwork is adorable.

It's good enough for master in my opinion but I want a second opinion.
I've never used a function like this in any other software so I don't know what people expect from this.
Should a ghost pattern be persistent?
Are there better ways to set a ghost Pattern? Right click/copy, right click/"paste as ghost pattern"?

@cyberdevilnl Is the artwork your original?

musikBear · 2019-01-06T17:31:49Z

Should a ghost pattern be persistent?

imo yes, until the user decides to remove the ghosts. Not only does persistence open more options, toggle-able features are always more user-friendly, than a rule based behaviour
Besides that ; HUGE firework for this feature ! @cyberdevilnl +1

ghost · 2019-01-06T18:08:17Z

@zonkmachine : Yes I created the icons with Inkscape from scratch, feel free to use/replace them.

@FeralBytes @musikBear : glad it’s been helpful :-)

If there are behaviour changes needed please let me know. One thing I could think of is disabling/hiding the clear ghost icon in PianoRoll when there is no ghost set.

zonkmachine · 2019-01-06T18:17:04Z

One thing I could think of is disabling/hiding the clear ghost icon in PianoRoll when there is no ghost set.

Yes. This is a good idea. Also, if you right click an empty pattern, maybe the context menu ghost icon should be disabled?

RebeccaDeField · 2019-01-06T21:01:13Z

@FeralBytes Actually hoping to have some more activity this year, but I agree that it would be best to commit now, update art later. I have some ideas on how the icons could be finished up, but I'm not available to work on it yet.

tresf · 2019-01-06T22:30:51Z

Why even delay for the artwork folks

Agreed. This shouldn't be a factor unless it's actively be worked on. Most features come with default artwork and is re-themed as time goes on. This should be the standard as to encourage the coding of features and discourage lengthy merge times. If we can get some more volunteer project maintainers we can shorten the merge time as well. <3

* Small change to comply more with the rest of the coding style.

ghost · 2019-01-06T23:57:58Z

@zonkmachine Good, I’ve made some changes. The context menu entry won’t be added if the pattern is empty and the clear ghost notes button in the piano-roll is only enabled when a ghost pattern is set.

…host set.

FeralBytes · 2019-01-07T02:14:02Z

@cyberdevilnl It definitely would be better with some persistence. But even so this is a huge productivity boost. Also several checks have failed but looks like they just took too long. How do we get the LMMS Bot to build a new PR app Image once all checks are passing?
@RebeccaDeField Sorry that the message may have come off as a personal attack, it was not meant like that. I understand we all get busy and these are open source volunteer based programs. I just more so wanted to make sure that the PR does not get held up if the current artwork is satisfactory.

tresf · 2019-01-07T02:25:55Z

How do we get the LMMS Bot to build a new PR app Image once all checks are passing?

Don't believe there's a way yet and I'm not sure if the bot's still running for new PRs. @Reflexe?

PhysSong · 2019-01-07T08:20:32Z

I'm not sure if the bot's still running for new PRs.

It seems like there was a problem with the bot and @Reflexe have fixed it. For some reason, however, there are no transfer.sh links in build logs. The upload works fine locally though. I suspect they may have blocked uploads from Travis-CI, but not really sure...

zonkmachine · 2019-01-08T22:07:02Z

@zonkmachine Good, I’ve made some changes. The context menu entry won’t be added if the pattern is empty and the clear ghost notes button in the piano-roll is only enabled when a ghost pattern is set.

Better. 👍

src/tracks/Pattern.cpp

zonkmachine · 2019-01-08T22:42:09Z

I haven't looked deep into the code of this but @PhysSong had a go at it earlier in the thread.

@cyberdevilnl Did you look into the ghost notes being remembered? It is a bit volatile at the moment as it's lost as soon as you exit the Piano Roll.

ghost · 2019-01-10T09:28:37Z

@zonkmachine Ah I get what you mean, should be fixed now.

* Don’t paint ghost if it’s the same pattern.

zonkmachine · 2019-01-12T14:23:44Z

@zonkmachine Ah I get what you mean, should be fixed now.

Great! Now the pattern is remembered for the complete session. The PR is well defined and works.

You set a ghost pattern and it's set globally and is there until you either clear it, set another pattern or close the project.

zonkmachine · 2019-01-16T17:44:00Z

Merge?

Lets you set a melody pattern as visible in the background of the Piano Roll as support when building a new pattern. The pattern is visible throughout the session or until cleared via the provided button.

* Added basic ghost notes feature.

f19e640

* Changed `int m_noteOpacity` to `unsigned char m_noteOpacity` (aslo converted the relative functions (`noteOpacity` and `setNoteOpacity`) to `unsigned char`) since the maximum value will be 255 and minimum 0.

* changed setGhostPattern parameter name from newPattern to `patter…

43459e5

…n` in the PianoRoll header since we defined it as `pattern` in the cpp file as well.

PhysSong reviewed Sep 3, 2018

View reviewed changes

src/tracks/Pattern.cpp Outdated Show resolved Hide resolved

PhysSong reviewed Sep 3, 2018

View reviewed changes

include/PianoRoll.h Outdated Show resolved Hide resolved

* set ghost pattern m_ghostPattern to nullptr on `destroyedPatter…

e103e76

…n` signal. * changed `and` to `&&` for MSVC support. - Thanks to PhysSong for the review.

CYBERDEViLNL added 2 commits September 3, 2018 22:16

resized ghost note icon so it is aligned with the rest of the icons i…

a05223e

…n the pattern context-menu.

Changed opacity variables data types back to int from unsigned char.

2946905

tresf added the enhancement label Sep 24, 2018

tresf added this to the 1.3.0 milestone Sep 24, 2018

ghost mentioned this pull request Sep 27, 2018

Phantom automation #4628

Closed

Gabrielxd195 mentioned this pull request Oct 13, 2018

Improvement: Pitch Correction with piano roll #4661

Open

* Don’t show Set as ghost in piano-roll on empty pattern.

4e5493f

* Small change to comply more with the rest of the coding style.

Disable clear ghost notes button in Piano-Roll when there is no g…

0da2a15

…host set.

zonkmachine reviewed Jan 8, 2019

View reviewed changes

src/tracks/Pattern.cpp Outdated Show resolved Hide resolved

CYBERDEViLNL added 2 commits January 10, 2019 10:37

* Don’t clear ghost automaticly.

e455253

* Don’t paint ghost if it’s the same pattern.

remove empty line.

8681269

zonkmachine approved these changes Jan 12, 2019

View reviewed changes

zonkmachine merged commit 5126070 into LMMS:master Jan 17, 2019

zonkmachine mentioned this pull request Jan 17, 2019

Overlay notes - Enhancement #4303

Closed

zonkmachine mentioned this pull request Feb 2, 2019

Step Recording feature #4544

Merged

4 tasks

DomClark mentioned this pull request Feb 20, 2019

I wanna implement a ghost channel notes on the piano roll. #4832

Closed

LostRobotMusic mentioned this pull request Dec 26, 2019

Add chord progressions highlights. #5354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic ghost notes feature. #4575

Basic ghost notes feature. #4575

ghost commented Sep 2, 2018 •

edited by tresf

LmmsBot commented Sep 2, 2018 •

edited

PhysSong commented Sep 3, 2018

ghost commented Sep 3, 2018

enp2s0 commented Sep 18, 2018

dev-sr0 commented Sep 19, 2018

tresf commented Sep 24, 2018

ghost commented Sep 24, 2018

tresf commented Sep 24, 2018

ghost commented Sep 24, 2018

FeralBytes commented Jan 6, 2019

zonkmachine commented Jan 6, 2019

PhysSong commented Jan 6, 2019

zonkmachine commented Jan 6, 2019

FeralBytes commented Jan 6, 2019 •

edited

zonkmachine commented Jan 6, 2019

musikBear commented Jan 6, 2019

ghost commented Jan 6, 2019

zonkmachine commented Jan 6, 2019

RebeccaDeField commented Jan 6, 2019

tresf commented Jan 6, 2019

ghost commented Jan 6, 2019

FeralBytes commented Jan 7, 2019

tresf commented Jan 7, 2019

PhysSong commented Jan 7, 2019

zonkmachine commented Jan 8, 2019

zonkmachine commented Jan 8, 2019

ghost commented Jan 10, 2019

zonkmachine commented Jan 12, 2019

zonkmachine commented Jan 16, 2019

Basic ghost notes feature. #4575

Basic ghost notes feature. #4575

Conversation

ghost commented Sep 2, 2018 • edited by tresf

LmmsBot commented Sep 2, 2018 • edited

Downloads for this pull request

PhysSong commented Sep 3, 2018

ghost commented Sep 3, 2018

enp2s0 commented Sep 18, 2018

dev-sr0 commented Sep 19, 2018

tresf commented Sep 24, 2018

ghost commented Sep 24, 2018

tresf commented Sep 24, 2018

ghost commented Sep 24, 2018

FeralBytes commented Jan 6, 2019

zonkmachine commented Jan 6, 2019

PhysSong commented Jan 6, 2019

zonkmachine commented Jan 6, 2019

FeralBytes commented Jan 6, 2019 • edited

zonkmachine commented Jan 6, 2019

musikBear commented Jan 6, 2019

ghost commented Jan 6, 2019

zonkmachine commented Jan 6, 2019

RebeccaDeField commented Jan 6, 2019

tresf commented Jan 6, 2019

ghost commented Jan 6, 2019

FeralBytes commented Jan 7, 2019

tresf commented Jan 7, 2019

PhysSong commented Jan 7, 2019

zonkmachine commented Jan 8, 2019

zonkmachine commented Jan 8, 2019

ghost commented Jan 10, 2019

zonkmachine commented Jan 12, 2019

zonkmachine commented Jan 16, 2019

ghost commented Sep 2, 2018 •

edited by tresf

LmmsBot commented Sep 2, 2018 •

edited

FeralBytes commented Jan 6, 2019 •

edited