Skip to content

Fix song-editor regressions and BB-editor bugs introduced in #3487 #4008

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.

Sign up for GitHub

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

Merged
PhysSong merged 4 commits into from
Nov 29, 2017
Merged

Fix song-editor regressions and BB-editor bugs introduced in #3487 #4008

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11eb114
Fix some (mostly)song-editor GUI regressions
PhysSong Nov 16, 2017
8434d5f
Fix TCO selection toggling
PhysSong Nov 24, 2017
9d4c377
Revert modifier logic change
PhysSong Nov 28, 2017
f8e2f53
avoids override cursor in BBEditor for sampletracks at left side
BaraMGB Nov 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 47 additions & 33 deletions src/core/Track.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,16 +720,25 @@ void TrackContentObjectView::mousePressEvent( QMouseEvent * me )
m_action = ToggleSelected;
}
Copy link
Contributor

@BaraMGB BaraMGB Nov 24, 2017

Choose a reason for hiding this comment

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

okay, but now you can't select/deselect a single TCO by [CTRL] + Click. That's why I moved this whole block into the mouseMoveEvent(). There we can test if the mouse moved 2 pixels since click(copy TCO) or not(toggleSelected). See: https://github.com/LMMS/lmms/pull/3988/files

Copy link
Member Author

@PhysSong PhysSong Nov 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@BaraMGB Thanks! I'll look into that. Anyway, what's this?
https://github.com/LMMS/lmms/pull/3649/files#diff-cb2a453c979a4777b9330e2b96644f0eL707

Copy link
Member Author

@PhysSong PhysSong Nov 24, 2017

Choose a reason for hiding this comment

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

Done via 8434d5f.

}
else if( !me->modifiers() )
else
Copy link
Member Author

@PhysSong PhysSong Nov 25, 2017

Choose a reason for hiding this comment

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

@BaraMGB Should this be else, or else if( !me->modifiers() )?

Copy link
Contributor

@BaraMGB BaraMGB Nov 25, 2017

Choose a reason for hiding this comment

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

The difference would be you can't move a tco with pressed shift and/or alt key. Perhaps else if(!me->modifier) is more discriptive.

Copy link
Member Author

@PhysSong PhysSong Nov 25, 2017

Choose a reason for hiding this comment

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

Okay. The latter is consistent with piano roll. Anyway, you changed it to the former in #3649. Was that intentional?

Copy link
Contributor

@BaraMGB BaraMGB Nov 28, 2017

Choose a reason for hiding this comment

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

No, I tried to make the code simpler. But now I think else if(! me->modifiers() is more discriptive. If someone wants to add a new modifier key, he/she can see where to add it in the code. Also I think, it's better to have a functionality only in a certain circumstance (no modifier key is pressed)

Copy link
Member Author

@PhysSong PhysSong Nov 28, 2017

Choose a reason for hiding this comment

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

Okay. Done via 9d4c377.

{
if( isSelected() )
{
m_action = MoveSelection;
}
else
{
gui->songEditor()->m_editor->selectAllTcos( false );
m_tco->addJournalCheckPoint();

// move or resize
m_tco->setJournalling( false );

setInitialMousePos( me->pos() );

SampleTCO * sTco = dynamic_cast<SampleTCO*>( m_tco );
if( me->x() < RESIZE_GRIP_WIDTH && sTco )
if( me->x() < RESIZE_GRIP_WIDTH && sTco
&& !m_tco->getAutoResize() )
{
m_action = ResizeLeft;
m_oldTime = m_tco->startPosition();
Expand All @@ -753,25 +762,24 @@ void TrackContentObjectView::mousePressEvent( QMouseEvent * me )
QApplication::setOverrideCursor( c );
s_textFloat->setTitle( tr( "Current length" ) );
}
// s_textFloat->reparent( this );
// setup text-float as if TCO was already moved/resized
mouseMoveEvent( me );
s_textFloat->show();
}

delete m_hint;
QString hint = m_action == Move || m_action == MoveSelection
? tr( "Press <%1> and drag to make a copy." )
: tr( "Press <%1> for free resizing." );
m_hint = TextFloat::displayMessage( tr( "Hint" ), hint.arg(
#ifdef LMMS_BUILD_APPLE
"⌘"),
#else
"Ctrl"),
#endif
embed::getIconPixmap( "hint" ), 0 );
}
delete m_hint;
QString hint = m_action == Move ? tr( "Press <%1> and drag to make "
"a copy." )
: tr( "Press <%1> for free "
"resizing." );
m_hint = TextFloat::displayMessage( tr( "Hint" ),
hint.arg(
#ifdef LMMS_BUILD_APPLE
"⌘"),
#else
"Ctrl"),
#endif
embed::getIconPixmap( "hint" ), 0 );
// s_textFloat->reparent( this );
// setup text-float as if TCO was already moved/resized
mouseMoveEvent( me );
s_textFloat->show();
}
else if( me->button() == Qt::RightButton )
{
Expand Down Expand Up @@ -815,28 +823,34 @@ void TrackContentObjectView::mousePressEvent( QMouseEvent * me )
*/
void TrackContentObjectView::mouseMoveEvent( QMouseEvent * me )
{
if( m_action == CopySelection )
if( m_action == CopySelection || m_action == ToggleSelected )
{
if( mouseMovedDistance( me, 2 ) == true )
{
// Clear the action here because mouseReleaseEvent will not get
// triggered once we go into drag.
m_action = NoAction;

// Collect all selected TCOs
QVector<TrackContentObjectView *> tcoViews;
QVector<selectableObject *> so =
m_trackView->trackContainerView()->selectedObjects();
for( QVector<selectableObject *>::iterator it = so.begin();
it != so.end(); ++it )
if( m_action == CopySelection )
{
TrackContentObjectView * tcov =
dynamic_cast<TrackContentObjectView *>( *it );
if( tcov != NULL )
// Collect all selected TCOs
QVector<selectableObject *> so =
m_trackView->trackContainerView()->selectedObjects();
for( auto it = so.begin(); it != so.end(); ++it )
{
tcoViews.push_back( tcov );
TrackContentObjectView * tcov =
dynamic_cast<TrackContentObjectView *>( *it );
if( tcov != NULL )
{
tcoViews.push_back( tcov );
}
}
}
else
{
gui->songEditor()->m_editor->selectAllTcos( false );
tcoViews.push_back( this );
}
// Clear the action here because mouseReleaseEvent will not get
// triggered once we go into drag.
m_action = NoAction;

// Write the TCOs to the DataFile for copying
DataFile dataFile = createTCODataFiles( tcoViews );
Expand Down