Skip to content

Knife Tool for Sample Clips (Again) #5524

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
Spekular merged 35 commits into from
Mar 12, 2021
Merged

Knife Tool for Sample Clips (Again) #5524

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
c8c710b
Rebase BaraMGB's Knife
Spekular May 29, 2019
c14efd4
Draw marker
Spekular May 29, 2019
6672bfc
Refactoring and shift mode
Spekular Jul 27, 2019
b52fbdb
Allow resizing
Spekular Jul 27, 2019
79fda6f
Add Icon
Spekular Jul 28, 2019
05a2275
Fix stuck marker on RMB, remove unnecessary cast
Spekular Jul 29, 2019
06bccf6
Remove redundant line, more const
Spekular Jul 29, 2019
d1e5de4
Fix
Spekular Jul 29, 2019
18bd074
Review fixes
Spekular Sep 7, 2019
c2c0c3c
Only perform split logic for SampleTCO
Spekular Sep 7, 2019
a519f7c
Add unquantizedModHeld function
Spekular Sep 25, 2019
cfa5389
missed one
Spekular Sep 25, 2019
f05ada0
Merge remote-tracking branch 'upstream/master' into Knife
PhysSong May 3, 2020
aac9c07
Don't use copy/paste
Spekular May 5, 2020
bdde426
Don't use copy/paste
Spekular May 5, 2020
54b2002
Committed too early
Spekular May 5, 2020
8e8cf2c
More git troubles
Spekular May 5, 2020
3f39abf
Fix undo
Spekular May 5, 2020
5b8bd6d
Merge
Spekular May 10, 2020
31b938d
git dammit
Spekular May 10, 2020
902f775
Cleaner solution
Spekular May 22, 2020
e5a818d
Merge branch 'ROT' of https://github.com/Spekular/lmms
Spekular Jun 2, 2020
12d818d
Set cursor, add copy assignment to SampleBuffer
Spekular Jun 2, 2020
e9906bc
Add TODO comment
Spekular Oct 3, 2020
84ff7e7
Merge commit
Spekular Jan 1, 2021
19c31f3
Make it build
Spekular Jan 1, 2021
3a532b3
Fixes from review
Spekular Jan 4, 2021
08aaea3
Make splitTCO virtual
Spekular Jan 5, 2021
22a8d47
Make splitTCO more generic
Spekular Jan 14, 2021
4782994
Prevent resizing of MIDI clips in knife mode
Spekular Jan 14, 2021
245160d
Fix move/resize and rework box select via ctrl
Spekular Jan 16, 2021
d498802
Apply suggestions from code review.
Spekular Jan 26, 2021
81fc127
Don't show inaccurate/useless/empty text float in knife mode
Spekular Jan 26, 2021
4c604d8
Addresses Github review
IanCaio Feb 12, 2021
6fb38c4
Revert format changes in Track
Spekular Mar 12, 2021
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
Prev Previous commit
Next Next commit
Make splitTCO virtual
  • Loading branch information
@Spekular
Spekular committed Jan 5, 2021
commit 08aaea3b6891b14c8555b3bca02db1b6237c3ab6
1 change: 1 addition & 0 deletions include/SampleTrack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ public slots:
private:
SampleTCO * m_tco;
QPixmap m_paintPixmap;
bool splitTCO( QMouseEvent * me ) override;
} ;


Expand Down
16 changes: 9 additions & 7 deletions include/TrackContentObjectView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ public slots:
Merge
};

TrackView * m_trackView;
TimePos m_initialTCOPos;
TimePos m_initialTCOEnd;

virtual void constructContextMenu( QMenu * )
{
}
Expand All @@ -157,6 +161,9 @@ public slots:
selectableObject::resizeEvent( re );
}

bool unquantizedModHeld( QMouseEvent * me );
TimePos quantizeMarkerPos( TimePos, bool shiftMode );

float pixelsPerBar();


Expand Down Expand Up @@ -186,12 +193,9 @@ protected slots:
static TextFloat * s_textFloat;

TrackContentObject * m_tco;
TrackView * m_trackView;
Actions m_action;
QPoint m_initialMousePos;
QPoint m_initialMouseGlobalPos;
TimePos m_initialTCOPos;
TimePos m_initialTCOEnd;
QVector<TimePos> m_initialOffsets;

TextFloat * m_hint;
Expand Down Expand Up @@ -222,12 +226,10 @@ protected slots:
void setInitialOffsets();

bool mouseMovedDistance( QMouseEvent * me, int distance );
bool unquantizedModHeld( QMouseEvent * me );
TimePos draggedTCOPos( QMouseEvent * me );
int knifeMarkerPos( QMouseEvent * me );
TimePos quantizeMarkerPos( TimePos, bool shiftMode );
//! Return true iff TCO could be split
bool splitTCO( QMouseEvent * me );
//! Return true iff TCO could be split. Currently only implemented for samples
virtual bool splitTCO( QMouseEvent * me ){ return false; };
} ;


Expand Down
52 changes: 3 additions & 49 deletions src/gui/TrackContentObjectView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ TrackContentObjectView::TrackContentObjectView( TrackContentObject * tco,
TrackView * tv ) :
selectableObject( tv->getTrackContentWidget() ),
ModelView( NULL, this ),
m_tco( tco ),
m_trackView( tv ),
m_initialTCOPos( TimePos(0) ),
m_initialTCOEnd( TimePos(0) ),
m_tco( tco ),
m_action( NoAction ),
m_initialMousePos( QPoint( 0, 0 ) ),
m_initialMouseGlobalPos( QPoint( 0, 0 ) ),
m_initialTCOPos( TimePos(0) ),
m_initialTCOEnd( TimePos(0) ),
m_initialOffsets( QVector<TimePos>() ),
m_hint( NULL ),
m_mutedColor( 0, 0, 0 ),
Expand Down Expand Up @@ -1424,52 +1424,6 @@ TimePos TrackContentObjectView::quantizeMarkerPos( TimePos midiPos, bool shiftMo



// This should eventually be available for all clip types, hence why it's not
// located inside SampleTCOView
bool TrackContentObjectView::splitTCO( QMouseEvent * me )
{
SampleTCO * leftTCO = dynamic_cast<SampleTCO*>( m_tco );

if (!leftTCO) { return false; }

leftTCO->setMarkerEnabled( false );

const int relativePixelPos = me->pos().x();
const float ppb = m_trackView->trackContainerView()->pixelsPerBar();
TimePos splitPos = relativePixelPos * TimePos::ticksPerBar() / ppb;

if ( !unquantizedModHeld(me) )
{
splitPos = quantizeMarkerPos( splitPos, me->modifiers() & Qt::ShiftModifier );
}

splitPos += m_initialTCOPos;

//Don't split if we slid off the TCO or if we're on the clip's start/end
//Cutting at exactly the start/end position would create a zero length
//clip (bad), and a clip the same length as the original one (pointless).
if ( splitPos > m_initialTCOPos && splitPos < m_initialTCOEnd )
{
leftTCO->getTrack()->addJournalCheckPoint();
leftTCO->getTrack()->saveJournallingState( false );

SampleTCO * rightTCO = new SampleTCO ( *leftTCO );

leftTCO->changeLength( splitPos - m_initialTCOPos );

rightTCO->movePosition( splitPos );
rightTCO->changeLength( m_initialTCOEnd - splitPos );
rightTCO->setStartTimeOffset( leftTCO->startTimeOffset() - leftTCO->length() );

leftTCO->getTrack()->restoreJournallingState();
return true;
}
else { return false; }
}




// Return the color that the TCO's background should be
QColor TrackContentObjectView::getColorForDisplay( QColor defaultColor )
{
Expand Down
41 changes: 41 additions & 0 deletions src/tracks/SampleTrack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,47 @@ void SampleTCOView::reverseSample()



//! Split this TCO
bool SampleTCOView::splitTCO( QMouseEvent * me )
{
m_tco->setMarkerEnabled( false );

const int relativePixelPos = me->pos().x();
const float ppb = m_trackView->trackContainerView()->pixelsPerBar();
TimePos splitPos = relativePixelPos * TimePos::ticksPerBar() / ppb;

if ( !unquantizedModHeld(me) )
{
splitPos = quantizeMarkerPos( splitPos, me->modifiers() & Qt::ShiftModifier );
}
Copy link
Contributor

@IanCaio IanCaio Jan 6, 2021

Choose a reason for hiding this comment

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

I was thinking here, since we have the position of the marker stored on m_marker, couldn't we use it on this method to avoid calling the unquantizedModHeld and quantizeMarkerPos methods? Since the marker position visually represents the place where the splitting should happen, it should be accurate on the moment this method is called.

That would save a couple method calls and maybe even make it easier to move away from using the mouse event, since that conditional was the trickier part to get around without using the event.

Copy link
Member Author

@Spekular Spekular Jan 7, 2021

Choose a reason for hiding this comment

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

See comments about this same idea on previous version:

You'd think that the result from knifeMarkerPos( me ) could be converted back to a MidiTime and used for the actual cut itself, but for some reason the shift/default behavior ends up inverted if you do. Adding a boolean switch to flip the two still left some weirdness, so I moved the code that decides the position back into mouseReleaseEvent.

#4998 (comment)

#4998 (comment)

Copy link
Contributor

@IanCaio IanCaio Jan 7, 2021

Choose a reason for hiding this comment

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

That's odd, I wonder why that didn't behave properly. I know we are in a bit of a schedule, but you mind if I investigate this a little? If I don't figure out soon enough why using m_marker didn't work we can go with the current method.


splitPos += m_initialTCOPos;

//Don't split if we slid off the TCO or if we're on the clip's start/end
//Cutting at exactly the start/end position would create a zero length
//clip (bad), and a clip the same length as the original one (pointless).
if ( splitPos > m_initialTCOPos && splitPos < m_initialTCOEnd )
{
m_tco->getTrack()->addJournalCheckPoint();
m_tco->getTrack()->saveJournallingState( false );

SampleTCO * rightTCO = new SampleTCO ( *m_tco );

m_tco->changeLength( splitPos - m_initialTCOPos );

rightTCO->movePosition( splitPos );
rightTCO->changeLength( m_initialTCOEnd - splitPos );
rightTCO->setStartTimeOffset( m_tco->startTimeOffset() - m_tco->length() );

m_tco->getTrack()->restoreJournallingState();
return true;
}
else { return false; }
}






SampleTrack::SampleTrack(TrackContainer* tc) :
Expand Down