Automation point drag cancel with right click

[tecknixia]

Resolves #4671

• The issue does not seem to affect other functions.
• A "deleting while dragging" feature does not seem worth redesigning other functions
• The buggy scenario can be resolved by this "cancel drag" feature

This feature uses two variables to record the location of the point at the start of the drag

• float m_dragStartLevel
• tick_t m_dragStartTick

They are assigned the point location in mousePressEvent

	if( mouseEvent->button() == Qt::LeftButton &&
					m_editMode == DRAW )
	{
		m_pattern->addJournalCheckPoint();
		m_dragStartLeve = level;
		m_dragStartTick = pos_ticks;

When right mouse button is clicked, and m_action is MOVE_VALUE, the stored point location is sent to setDragValue to move the point back to it's original position, then apply dragValue is called to stop dragging.

	if (m_action == MOVE_VALUE)
	{
		m_pattern->setDragValue(MidiTime(m_dragStartTick), m_dragStartLevel,
			true, false);
		m_pattern->applyDragValue();
	}

Now the cursor is over another point, and m_action is still MOVE_VALUE, and both mouse buttons are down, this condition allows dragging in mouseMoveEvent, so we want to prevent dragging in this case

	if (mouseEvent->buttons() & Qt::LeftButton
		// right button held when cancelling drag
		&& !(mouseEvent->buttons() & Qt::RightButton)
		&& m_editMode == DRAW )
	{

Now that we've prevented dragging, while both buttons are down, also in mouseMoveEvent, the condition to erase is met, so we don't want to erase a bunch of stuff on accident

	// drag erase
	else if ((mouseEvent->buttons() & Qt::RightButton
		// don't want to erase when cancelling drag with left held
		&& !(mouseEvent->buttons() & Qt::LeftButton)
		&& m_editMode == DRAW)
		|| (mouseEvent->buttons() & Qt::LeftButton
		&& m_editMode == ERASE))
	{

Finally, in paintEvent, where the cursor is changed, we're not erasing, so we want to switch back to the draw tool when the drag is cancelled. That means the erase tool normally shows when the right mouse button is down, but we make an exception for when m_action is MOVE_VALUE, which is true until either button is released.

So, when we cancel a drag...

• if the right mouse button is released (left still down) cursor becomes draw tool
• if the left mouse button is released (right still down) cursor becomes erase tool

	if (m_mouseDownRight && m_action != MOVE_VALUE)
	{
		cursor = s_toolErase;
	}
	else if (m_action == MOVE_VALUE)
	{
		cursor = s_toolMove;
		if (m_mouseDownRight)
		{
			cursor = s_toolDraw;
		}
		else
		{
			cursor = s_toolMove;
		}
	}

[LmmsBot]

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

Windows

• Windows 32-bit: lmms-1.2.2.657-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.2.2.657-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.2.2-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.2.2-msvc2017-win64.exe (build link)

Linux

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

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://7253-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.657-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/7253?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://7254-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.657-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/7254?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/ur0ywlg1o1w3c8mf/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/34003045"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/nq0f7i66aj9db07b/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/34003045"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://7251-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.657-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/7251?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "c2809a369252d2de7123ce852136bb46da9f4e27"}

[PhysSong]

I think both #5315 and this works mainly by making further mouse operations no-op after certain actions. So, I'm wondering if we can somehow consolidate them partially.

[tecknixia]

@PhysSong
I may not be following your concept of consolidation. How do you mean?

I believe both pull requests do not share lines, unless I lost track when checking, I only checked once.

This PR uses a mouse button condition of the left button to prevent erasing with the right button while holding the left button, and this occurs without regard to other functions. #5315 uses a mouse button condition variable to disable the normal drag condition (by holding the left mouse button) after a line is drawn.

Both involve the left mouse button when moving the mouse, and both are disabling operations, but for different reasons. Consolidation might be tricky. One way, and I did previously like this idea, could be to use mouseDownLeft and mouseDownRight for all mouseMove conditions, which in my view makes the code easier to read. However, if all conditions are using the same variable, disconnecting the state of the mouse can cause unintended effects in other conditions while the state is disconnected. An example might be possibly accidentally entering erase mode after drawing a line (I'm not sure if that would happen, or not, I haven't checked.) My point is, any future uses of that variable, in the case of using the variable for all conditions, could potentially affect other functions, depending on where and when it is used to disconnect from the state of the mouse, so it's probably best to only use it where it is needed, and to use mouseEvent->buttons() & Qt::LeftButton in all other conditions, which is more consistent with the rest of the code.

Does that answer your question?

Aside: Hmm, thinking about it now, the shift button could be used to disable the drag state when drawing a line (#5315). However, it would have to be held until letting go of the left mouse button. If not, it would probably start dragging the last point when let go. Seems it's probably better the way I already have it.

[zonkmachine]

I think both #5315 and this works mainly by making further mouse operations no-op after certain actions. So, I'm wondering if we can somehow consolidate them partially.

@PhysSong Has this question been answered? I want at least #5315 to be merged now.

[zonkmachine]

@tecknixia This PR has conflicts that needs to be resolved since #5315 was merged.

[tecknixia]

@zonkmachine Looking at it now.

Wait... you mean to tell me this whole time, #5315 and #5316 did indeed share a line of code?!?
Did I just miss it!? My apologies to @PhysSong , I should have inspected it closer.

I've updated with a solution that I believe should work.
I gave it a quick build and played around with the associated functions.
I think it works, and I don't think there will be any unintended outcomes, but it probably should be tested anyway.

[tecknixia]

@zonkmachine @russiankumar #5592
This is not a critical fix.
If a user tries to delete points while dragging a point, the points will reappear.
#4671
This simply prevents the user from deleting points while dragging a point, due to a right click drag cancel.
I had also been working on reorganizing how automation editor handles points in another branch.
Also seems there's an issue with macos.
I'm working on other things... I don't think I have the desire to work on this anytime soon.
I would rather create a new PR later. Issue should remain open, but PR can be closed.

[ryuukumar]

If you've decided you don't want to work on this anymore, you can close it yourself, and drop a comment on the linked issue that it's open for someone else to take.

Just saying: the issue with macos is probably not related to this PR (it can't install the dependencies, I think if you rebuild it once it should work again).

[zonkmachine]

I'm working on other things... I don't think I have the desire to work on this anytime soon.
I would rather create a new PR later. Issue should remain open, but PR can be closed.

@tecknixia Fine, I'm closing this for now. Thank you for your contributions!