Skip to content

Conversation

@zonkmachine
Copy link
Contributor

Fix regression from b68dc57 where deleting a point by right clicking without dragging is defunct.

I misunderstood the algorithm. The selection for add/delete was dual.

   Left mouse button and above a point, move it.
   Right mouse button and below a point, delete it.

The latter appeared to me to be an obvious glitch so I pushed a fix to stable-1.2, messed up and was set for hours of fun with git and submodules. ☠️ The final bug fixed in this PR removes the dual action altogether.

@zonkmachine
Copy link
Contributor Author

zonkmachine commented Nov 19, 2017

Here is the essence of the changes from merged commits b68dc57, 3de3ea6 and this PR.

$ git diff b68dc572a3a58ad2187efe3ef5b4848ef7254abd~1

diff --git a/src/gui/editors/AutomationEditor.cpp b/src/gui/editors/AutomationEditor.cpp
index be3fb52..733cf28 100644
--- a/src/gui/editors/AutomationEditor.cpp
+++ b/src/gui/editors/AutomationEditor.cpp
@@ -517,16 +517,13 @@ void AutomationEditor::mousePressEvent( QMouseEvent* mouseEvent )
                                        ( it+1==time_map.end() ||
                                                pos_ticks <= (it+1).key() ) &&
                ( pos_ticks<= it.key() + MidiTime::ticksPerTact() *4 / m_ppt ) &&
-                                       level <= it.value() )
+               ( level == it.value() || mouseEvent->button() == Qt::RightButton ) )
                                {
                                        break;
                                }

I just rolled along with the original code. It feels a bit quirky but it works quite well actually. Trying to straighten it out a bit now but I doubt that I can do it.

( pos_ticks<= it.key() + MidiTime::ticksPerTact() *4 / m_ppt ) &&
level == it.value() )
( level == it.value() || ( level != it.value() &&
mouseEvent->button() == Qt::RightButton ) ) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is equivalent to this:

( level == it.value() || mouseEvent->button() == Qt::RightButton ) )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@zonkmachine zonkmachine force-pushed the automationpointdelete branch from 2e91976 to 394912a Compare November 20, 2017 14:26
Fix regression from b68dc57
Let the right mouse button delete the automation point like before
and add the space above it too.
@zonkmachine zonkmachine force-pushed the automationpointdelete branch from 394912a to 8bc7b01 Compare November 20, 2017 16:28
@zonkmachine
Copy link
Contributor Author

The action on delete is not as spot on as for left click to move. It's the same as before though. Fixing this is going to be a lot messier. I think this is good for now.

@zonkmachine
Copy link
Contributor Author

Merge?

@zonkmachine zonkmachine merged commit c6ae1dc into LMMS:stable-1.2 Nov 24, 2017
@zonkmachine zonkmachine deleted the automationpointdelete branch November 24, 2017 07:44
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Fix regression from 7772edc
Let the right mouse button delete the automation point like before
and add the space above it too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants