Custom note labels in Pianoroll editor #5357

komodor1 · 2019-12-28T18:37:46Z

I coded function for custom note labels for Pianoroll editor. Three labels files paths are stored in lmms config file. They can be set in settings -> paths. After setting paths lmms need restart. Then in Pianoroll editor you can select which file you want to use. Function reads labels from .txt files in pattern like attached one. File can't have any empty lines between labels/comments. Empty lines can be only after labels list. Between note number and label must be ONLY ONE space.
In attached screenshots you can see how modifications looks like in Pianoroll and settings.

TODO:
Icon for labels file combo box in Pianoroll editor (currently using Quantize icon),
Only three files support, maybe add some kind of list,
May need some code clean-up and check,

labels.txt

LmmsBot · 2019-12-28T18:48:42Z

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Linux (AppImage): lmms-1.2.1.584-linux-x86_64.AppImage (build link)

Windows

Windows 32-bit: lmms-1.2.1.584-mingw-win32.exe (build link)
Windows 64-bit: lmms-1.2.1.584-mingw-win64.exe (build link)
Windows 32-bit: lmms-1.2.1-msvc2017-win32.exe (build link)
Windows 64-bit: lmms-1.2.1-msvc2017-win64.exe (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://5584-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.584-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5584?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://5582-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.584-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5582?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://5583-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.584-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5583?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/hea053kiwjaaiwo7/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/29814979"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/8j731w9enf9kro0a/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/29814979"}]}, "commit_sha": "d651fda1e55c5ddefd3ced87518a7635b17b87a4"}

Veratil

I like the idea!

But I don't like that it's new settings options rather than a specific file you read or not, and you are limited to 3 files instead of the ability to add any number to a single file.

I think this would be much more extensible if it was an XML file that can be read on load (if present in working dir or not). If like this, you can create a tree structure in XML that allows an unlimited number of labels. Granted the file is a little more difficult to write out, but it would be able to be parsed by Qt's loadSettings and not have to be parsed manually. So something like this:

<noteLabels>
  <map name="Drums">
    <label note="65" name="HI-HAT PEDAL" />
    <label note="63" name="(Hi-Hat closed)" />
    ...
  </map>
  <map name="Something something">
    <label note="53" name="whatever" />
    ...
  </map>
  ...
</noteLabels>

Additionally, there are numerous lines here which need to follow the Coding Conventions. Namely if and while blocks, a few spacing issues, and true/false usage over 1/0 are the main issues that I see.

src/gui/editors/PianoRoll.cpp

Veratil · 2019-12-29T01:25:56Z

src/gui/editors/PianoRoll.cpp

+		QTextStream stream(&labelsFile);
+		QString line = stream.readLine();
+		QRegExp num("\\d*");
+		while (!line.isNull()) {


This happens every paintEvent, which will block on file I/O which can cause freezing of the UI in worse cases. These labels need to be read on LMMS load rather than here and painted from memory.

Yes, I've noticed that. I know that is not good way, I'm thinking how to change that.

src/gui/SetupDialog.cpp

Coding conventions, removed empty If

komodor1 · 2019-12-30T16:14:37Z

I like the idea!

But I don't like that it's new settings options rather than a specific file you read or not, and you are limited to 3 files instead of the ability to add any number to a single file.

I think this would be much more extensible if it was an XML file that can be read on load (if present in working dir or not). If like this, you can create a tree structure in XML that allows an unlimited number of labels. Granted the file is a little more difficult to write out, but it would be able to be parsed by Qt's loadSettings and not have to be parsed manually.

Thanks for feedback!
XML file is really good idea. But it will be kinda difficult to write maps manually by user, so that's why I used TXT files - easy to write, and very similar to Reaper files. But I see another way. Parse .TXT file, and make tree structure like you proposed. It will be easier to maintain multiple labels maps. Then just add simple list of loaded maps in settings, where you can add or remove maps.
About parsing file in paintEvent. I don't see way how change that. Need some time to think.

qnebra · 2020-01-02T23:11:22Z

I think better this will work as another option in pianoroll bar. As this will be more user-friendly UX.

Veratil · 2020-01-03T01:01:57Z

XML file is really good idea. But it will be kinda difficult to write maps manually by user, so that's why I used TXT files - easy to write, and very similar to Reaper files. But I see another way. Parse .TXT file, and make tree structure like you proposed. It will be easier to maintain multiple labels maps. Then just add simple list of loaded maps in settings, where you can add or remove maps.

The reason I say XML is twofold. 1) You leverage Qt's XML parser, and don't need to do all this extra manual parsing. 2) Everything in LMMS is saved as an XML file, this being a plaintext file breaks from that and doesn't fit the mold.

About parsing file in paintEvent. I don't see way how change that. Need some time to think.

Read on startup from the file and save in memory.

Veratil · 2020-01-03T01:03:35Z

src/gui/SetupDialog.cpp

+		bool fileCorrect = true;
+		QFile labelsFile(new_file);
+		labelsFile.open(QIODevice::ReadOnly);
+		if (!labelsFile.isOpen()) return ;


Missing {}

Veratil · 2020-01-03T01:04:35Z

src/gui/editors/PianoRoll.cpp

@@ -128,7 +128,9 @@ static QString getNoteString( int key )
 	return s_noteStrings[key % 12] + QString::number( static_cast<int>( key / KeysPerOctave ) );
 }

-
+bool labelsFile1Correct = 0;


false instead of 0.

Veratil · 2020-01-03T01:05:46Z

src/gui/editors/PianoRoll.cpp

@@ -366,6 +369,32 @@ PianoRoll::PianoRoll() :
 	connect( &m_quantizeModel, SIGNAL( dataChanged() ),
 					this, SLOT( quantizeChanged() ) );

+	// Set up labels model


Formatting for this entire section to line 397.

Veratil · 2020-01-03T01:06:41Z

src/gui/editors/PianoRoll.cpp

@@ -3114,6 +3267,7 @@ void PianoRoll::paintEvent(QPaintEvent * pe )
 			}
 			p.drawLine( WHITE_KEY_WIDTH, y, width(), y );
 			++key;
+


Extra line added on accident?

Veratil · 2020-01-03T01:08:26Z

src/gui/editors/PianoRoll.cpp

 	{
 		p.drawPixmap( mousePosition + QPoint( 8, 8 ), *cursor );
 	}
+	if (drawCustomLabels)


Since this is the last drawn thing in paintEvent, this will draw over notes at the start of the pattern. Should it?

Veratil · 2020-01-03T01:08:43Z

src/gui/editors/PianoRoll.cpp

@@ -4421,6 +4600,50 @@ Note * PianoRoll::noteUnderMouse()
 	return NULL;
 }

+bool PianoRoll::labelsPathCorrect( QString labelsPath ){
+	bool fileCorrect = 1;


true instead of 1.

Veratil · 2020-01-03T01:09:03Z

src/gui/editors/PianoRoll.cpp

@@ -4421,6 +4600,50 @@ Note * PianoRoll::noteUnderMouse()
 	return NULL;
 }

+bool PianoRoll::labelsPathCorrect( QString labelsPath ){


Formatting for this function.

Veratil · 2020-01-03T01:09:29Z

src/gui/editors/PianoRoll.cpp

@@ -4515,6 +4738,15 @@ PianoRollWindow::PianoRollWindow() :
 	m_quantizeComboBox->setFixedSize( 64, 22 );
 	m_quantizeComboBox->setToolTip( tr( "Quantization") );

+	// setup labels-stuff


Fix whitespace in this area.

PhysSong · 2020-12-22T03:09:57Z

XML file is really good idea. But it will be kinda difficult to write maps manually by user, so that's why I used TXT files - easy to write, and very similar to Reaper files. But I see another way. Parse .TXT file, and make tree structure like you proposed. It will be easier to maintain multiple labels maps. Then just add simple list of loaded maps in settings, where you can add or remove maps.

The reason I say XML is twofold. 1) You leverage Qt's XML parser, and don't need to do all this extra manual parsing. 2) Everything in LMMS is saved as an XML file, this being a plaintext file breaks from that and doesn't fit the mold.

We can support both Reaper-compatible plaintext file and standard XML/JSON files. BTW, we need to think about #1857 first.

About parsing file in paintEvent. I don't see way how change that. Need some time to think.

Read on startup from the file and save in memory.

I can see the parsing logic is repeated a few times in many places. We can probably remove the file lists from the settings and let users load them manually(optionally with histories).

komodor1 added 5 commits December 28, 2019 19:22

Custom note labels in Pianoroll editor

ab1f4a6

Custom note labels in Pianoroll editor

88b97b9

Custom note labels in Pianoroll editor

b3583f2

Custom note labels in Pianoroll editor

7413ad7

Custom note labels in Pianoroll editor

d667692

Veratil requested changes Dec 29, 2019

View reviewed changes

komodor1 added 3 commits December 30, 2019 16:51

Changes to follow coding conventions

b33ef43

Cosmetic changes

1dec508

Coding conventions, removed empty If

Typo correction

d651fda

komodor1 requested a review from Veratil December 30, 2019 16:30

Veratil requested changes Jan 3, 2020

View reviewed changes

ryuukumar mentioned this pull request Jul 21, 2020

Reorganising LMMS, renaming editors and classes and everything in between (cont. #120) #5592

Closed

56 tasks

PhysSong added the rework required Pull requests which must be reworked to make it able to merge or review label Jan 12, 2021

PhysSong mentioned this pull request Jan 3, 2022

Custom Piano Labels #6162

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom note labels in Pianoroll editor #5357

Custom note labels in Pianoroll editor #5357

komodor1 commented Dec 28, 2019

LmmsBot commented Dec 28, 2019 •

edited

Loading

Linux

Windows

Veratil left a comment

Veratil Dec 29, 2019

komodor1 Dec 30, 2019

komodor1 commented Dec 30, 2019

qnebra commented Jan 2, 2020

Veratil commented Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

Veratil Jan 3, 2020

PhysSong commented Dec 22, 2020

Custom note labels in Pianoroll editor #5357

Are you sure you want to change the base?

Custom note labels in Pianoroll editor #5357

Conversation

komodor1 commented Dec 28, 2019

LmmsBot commented Dec 28, 2019 • edited Loading

Linux

Windows

Veratil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

komodor1 commented Dec 30, 2019

qnebra commented Jan 2, 2020

Veratil commented Jan 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhysSong commented Dec 22, 2020

LmmsBot commented Dec 28, 2019 •

edited

Loading