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

Select, copy, and paste in automation track #5157

Open
eland1 opened this issue Aug 28, 2019 · 14 comments
Open

Select, copy, and paste in automation track #5157

eland1 opened this issue Aug 28, 2019 · 14 comments

Comments

@eland1
Copy link

eland1 commented Aug 28, 2019

it would be amazing if you could select, copy and paste in the automation track. it is also nice if you can place two points right under eachother. i think it becomes a sidechain dream.

Below added by @Spekular

A complete implementation of group selection in song editor should also allow deletion of the selected points.

@SecondFlight SecondFlight changed the title select copy and paste in automation track Select, copy, and paste in automation track Aug 28, 2019
@Spekular
Copy link
Member

Selection is a duplicate of of #4602.

@musikBear
Copy link

place two points right under eachother.

@eland1 In respect to that, you can just set Q -quantification to highest value. You will never be able to hear the difference between 'same value' and a distance of 1/192. That is not audible

@ghost
Copy link

ghost commented Sep 5, 2019

I was looking into implementing this and found that most of the code is already there, it needs some modifications and some extra code. Is anyone working on this? Else I will give it a try :-)

@BaraMGB
Copy link
Contributor

BaraMGB commented Sep 5, 2019

@cyberdevilnl Actually I am on it. At first I want to implement the ability to have more than one control point per time key. I suppose you want to implement select, copy paste?

@ghost
Copy link

ghost commented Sep 6, 2019

@BaraMGB all right that's awesome! Yes I was talking about the select, copy and paste. Let me know if I can help :-)

@tecknixia
Copy link
Contributor

tecknixia commented Oct 19, 2019

I've been rewriting the mouse functions for clarity and I decided I might as well look into this, so in order to figure out how it works, I wrote down something like a map of the process that's involved. I figured it might help.

AutomationEditorWindow::AutomationEditorWindow

	DropToolBar *editActionsToolBar = addDropToolBarToTop(tr("Edit actions"));
	ActionGroup* editModeGroup = new ActionGroup(this);

	QAction* drawAction = editModeGroup->addAction(embed::getIconPixmap("edit_draw"),
		tr("Draw mode (Shift+D)"));
	QAction* eraseAction = editModeGroup->addAction(embed::getIconPixmap("edit_erase"),
		tr("Erase mode (Shift+E)"));

	// these next two lines are in a different format, so I changed them to match the others
	QAction* selectAction = editModeGroup->addAction(embed::getIconPixmap("edit_select"),
		tr("Select: Disabled"));
	QAction* moveAction = editModeGroup->addAction(embed::getIconPixmap("edit_move"),
		tr("Move: Disabled"));

That is just defining them, so order doesn't matter (the above)

Here is my map (below):

connect(editModeGroup, SIGNAL(triggered(int)), m_editor, SLOT(setEditMode(int)));
	-> QAction
		-> AutomationEditor::setEditMode(int mode)
			-> setEditMode((AutomationEditor::EditModes) mode);
				-> AutomationEditor::EditModes

				- (enum) DRAW, ERASE, SELECT, MOVE (in header file)
				- the order of EditModes in header file
				 determines which action is triggered
				- if you put 'erase' in front of 'draw',
				the draw button will erase, and erase button will draw

					-> m_editMode

		// there's two of them?
		-> AutomationEditor::setEditMode(AutomationEditor::EditModes mode)
			-> m_editMode = mode;
			-> AutomationEditor::removeSelection
			-> AutomationEditor::getSelectedValues

The way the code works is confusing to me, but essentially it seems to me that it just takes the signal of the buttons on the toolbar and puts m_editMode into the associated mode.

	editActionsToolBar->addAction(drawAction);
	editActionsToolBar->addAction(eraseAction);
	editActionsToolBar->addAction(selectAction);
	editActionsToolBar->addAction(moveAction);

The order of these (above) determines the order of the buttons on the toolbar.
The order does not affect the functionality

switch( m_editMode )
{
	case DRAW:
		if( m_mouseDownRight )
		{
			cursor = s_toolErase;
		}
		else if( m_action == MOVE_VALUE )
		{
			cursor = s_toolMove;
		}
		else
		{
			cursor = s_toolDraw;
		}

		break;
	case ERASE: cursor = s_toolErase; break;
	case SELECT: cursor = s_toolSelect; break;
	case MOVE: cursor = s_toolMove; break;
}
QPoint mousePosition = mapFromGlobal( QCursor::pos() );
	if( cursor != NULL && mousePosition.y() > TOP_MARGIN + SCROLLBAR_SIZE)
	{
		p.drawPixmap( mousePosition + QPoint( 8, 8 ), *cursor );
	}

And that just changes the cursor according to the mode.

From there my guess it's it's just a matter of m_editMode states within the code.

I'm thinking about combining a few Automation Editor issues, as I've talked about here:

Automation: points deleted while moving other points can be restored abnormally
#4671

Edit: I created a branch (forget this one... see below)
https:/tecknixia/lmms/tree/automationProject

Edit: New replacement for the above (see this one, not the above)
https:/tecknixia/lmms/tree/autoSelectCopyPaste

@Spekular
Copy link
Member

Personally I think this should be two separate issues, one for group selection and one for placing points at the same time. However, given the huge number of open issues I believe they can stay as one for now. Perhaps if one part is implemented and the other remains, the remaining one can be split into a new issue and this can be closed.

@tecknixia
Copy link
Contributor

In the branch I'm working on [above] I've managed to get select and move working, but I'm running into three issues that seem to be related.

  1. Issue Automation scroll limited by song editor #5254 - Select All - only selects the length of measures in Song Editor

automationSelectAll

  1. Issue Automation: points deleted while moving other points can be restored abnormally #4671 - [Maybe related:] while moving a selection, some points randomly disappear.

I'm looking into this.

  1. Issue #??? - actually I couldn't find an issue for this one... but the timeline position bar is broken.

This will be needed for pasting into the editor at the correct position. It doesn't move when play is pressed, so I imagine the code wasn't completed. I'll be looking into this too.

@tecknixia
Copy link
Contributor

tecknixia commented Oct 20, 2019

@Spekular

The more I play with this issue of moving a selection, the more it seems like the random points are disappearing as they move to a position on top of one another. When I read what you said, I agreed about the separate issues, but then I noticed this. It seems like the issues may be related after all.

Edit:

I just noticed that if you pass the selection quickly over or under the other points, they remain, but if you slowly drag the selection, it deletes all the other points.

@Spekular
Copy link
Member

Spekular commented Oct 20, 2019 via email

@tecknixia
Copy link
Contributor

@Spekular

After thinking about it some more, if points are getting deleted because they're on top of each other, than allowing them to co-exist on the same tick would probably fix that issue.

However, this might involve some strategy in how the mouse functions run. Even if the issues are related, I think you're right, it seems to deserve it's own issue because of the depth of the issue. This issue can then be mentioned by that one.

@tecknixia
Copy link
Contributor

tecknixia commented Oct 21, 2019

Just realized... if it works when dragging a single point, why can't it work for multiple points?

I'll have to take a closer look, it seems likely possible to fix this without having to worry about the issue of two points occupying the same x position.


Thoughts about moving a selection have been moved here:


So, the problem is that the points are being moved and placed with each mouse move, instead of being dragged. I tried using setDragValue, but it seems to condense all the points in the selection to a single point. So, "how to apply a drag value to multiple points?" seems to be the next question.

@tecknixia
Copy link
Contributor

tecknixia commented Nov 9, 2019

I had to redo some things, so I created a new branch for this with cleaner commits.
https:/tecknixia/lmms/commits/autoSelectCopyPaste

Improvements:

  • Select, cut, and copy all seem to be working now
  • Including "select all"
  • Move will move points, but still needs work (see below)
  • Paste will paste data, but wrong position (see below)
  • Delete will remove selected points
  • Escape will cancel selection (currently only in select mode)
  • Fixed broken toolbar buttons, including right-click mode switching
  • Made the selection box transparent
  • Enabled the ability to scroll while selecting

Also in this branch:

Some issues:

I'll probably also be looking into a few other Automation Editor issues.

#4877

@BaraMGB BaraMGB removed their assignment Nov 9, 2019
@tecknixia
Copy link
Contributor

tecknixia commented Nov 10, 2019

The main problem seems to be... each time setDragValue is run:

  • it's only being passed the coordinates of one point
  • it deletes the point at that time position
  • takes "a picture" of the other points on the map
  • sets m_dragging to true, which allows skipping of the above
  • applies the picture of the points not being dragged (each time it runs) back onto the map
  • then each time returns this->putValue(time, value, quantPos, controlKey)

For the basic idea of how to do this, it seems like the main thing that needs to be done is to take note of all the positions of the multiple points being moved, then delete those values from the map, and then copy the map of the others, so that you have a copy to place back onto the map each time a move is made.


Edit: I just realized there's already functionality like this for moving notes in piano roll.

PianoRoll::getSelectedNotes
PianoRoll::dragNotes
PianoRoll::copyToClipboard
PianoRoll::copySelectedNotes
PianoRoll::cutSelectedNotes
PianoRoll::pasteNotes
PianoRoll::deleteSelectedNotes


I think it would be a good idea for the core logic of automation points to remain consistent with the core logic of notes. Notes can already be selected, moved, copied, pasted, etc.

Notes have a class of their own, and the moving of multiple selected notes involves the use of a "NoteVector". I think the same could be done for automation points and a "PointVector". I did get started on the class files, although instead of talking about it here, this modification seems large enough to deserve it's own issue.

I'm still new to C++, and it's still a bit hard for me to read and follow the code. I'm looking into this, but I could use guidance, or assistance, from someone who is familiar with notes, Piano Roll, and the Song Editor.

tecknixia:autoSelectCopyPaste

I'm documenting my understanding of the code here: Automation Editor

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

6 participants