Sampletrack - resize from left #3487

BaraMGB · 2017-04-05T14:39:10Z

https://www.youtube.com/watch?v=BQfWqTMjxUE&feature=youtu.be

Things to do:
- save to project doesn't work, yet

This one is the halve way to split (knife tool) a sample track in two.

BaraMGB · 2017-04-05T14:40:25Z

src/core/Track.cpp

@@ -67,7 +67,7 @@
 #include "SongEditor.h"
 #include "StringPairDrag.h"
 #include "TextFloat.h"
-
+#include <QDebug>


BaraMGB · 2017-04-05T14:41:42Z

src/tracks/SampleTrack.cpp

@@ -48,7 +48,7 @@
 #include "Mixer.h"
 #include "EffectRackView.h"
 #include "TrackLabelButton.h"
-
+#include <QDebug>


Umcaruje · 2017-04-05T14:54:03Z

Omg I love you so much for doing this <3

BaraMGB · 2017-04-07T12:15:05Z

I guess, this is ready for review.

jasp00

Suggested improvements.

jasp00 · 2017-04-08T18:15:17Z

src/core/Track.cpp

+								#else
+								"Ctrl"),
+								#endif
+					embed::getIconPixmap( "hint" ), 0 );


Could you reuse the Resize case code?

I don't see how could this happen economical. Maybe you can give me a tip? All three cases are very similar. I don't know how to consolidate.

jasp00 · 2017-04-08T18:31:42Z

include/SampleTrack.h

@@ -58,6 +58,7 @@ class SampleTCO : public TrackContentObject
 		return m_sampleBuffer;
 	}

+	void setStartTimeOffset( MidiTime startTime );


Are you sure this feature will not be useful for other TCOs? You do not have to implement those cases now.

@jasp00 excellent point, yes this will be useful for other track types. Piano Roll, BBEdtior and Automation patterns can eventually benefit from this as well. 👍

I moved it into Track.h/.cpp in the TrackContentObject. 👍

jasp00 · 2017-04-08T18:33:02Z

src/core/Track.cpp

+						arg( m_tco->endPosition().getTicks() %
+								MidiTime::ticksPerTact() ) );
+				s_textFloat->moveGlobal( this, QPoint( width() + 2,
+							height() + 2) );


Could you reuse the text float code?

I don't see how could this happen economical. Maybe you can give me a tip?

jasp00 · 2017-04-08T18:37:48Z

src/core/Track.cpp

@@ -925,6 +982,24 @@ void TrackContentObjectView::mouseMoveEvent( QMouseEvent * me )
 			QCursor c( Qt::SizeHorCursor );
 			QApplication::setOverrideCursor( c );
 		}
+		else if( me->x() < RESIZE_GRIP_WIDTH && !me->buttons() )


You should merge this block with the previous one.

jasp00 · 2017-04-08T18:48:38Z

src/core/Track.cpp

@@ -954,7 +1029,7 @@ void TrackContentObjectView::mouseReleaseEvent( QMouseEvent * me )
 		setSelected( !isSelected() );
 	}

-	if( m_action == Move || m_action == Resize )
+	if( m_action == Move || m_action == Resize || m_action == ResizeLeft )
 	{
 		m_tco->setJournalling( true );


Could you add this?

// TODO: Fix m_tco->setJournalling() consistency

jasp00 · 2017-04-08T18:58:26Z

src/tracks/SampleTrack.cpp

@@ -603,10 +629,10 @@ bool SampleTrack::play( const MidiTime & _start, const fpp_t _frames,
 			float framesPerTick = Engine::framesPerTick();
 			if( _start >= sTco->startPosition() && _start < sTco->endPosition() )
 			{
-				if( sTco->isPlaying() == false )
+				if( sTco->isPlaying() == false && _start > sTco->startPosition() + sTco->startTimeOffset() )


You should refactor the offset information because playing is more important than GUI events.

Can you please explain, what is in your mind? @jasp00

Umcaruje · 2017-04-14T10:13:25Z

So, I found a bug:

When you tweak the tempo, the end point of the sample gets reset. The starting point seems fine. Other than that cutting works, duplicating the sample works and sync is rock-solid, as always 👍

BaraMGB · 2017-04-14T13:07:18Z

@Umcaruje Ah! Cool! I'll look into it. Thanks!

tresf · 2017-04-14T16:37:24Z

When you tweak the tempo, the end point of the sample gets reset.

Just an FYI, this bug had existed prior to this feature.

BaraMGB · 2017-04-17T12:43:40Z

@tresf Okay, that's another bug. I think, we should split this issue. The bug should be fixed for 1.2. I'll make a new pull request for this.

tresf · 2017-05-04T04:35:42Z

When you tweak the tempo, the end point of the sample gets reset.

Just an FYI, this bug had existed prior to this feature.

I think, we should split this issue

@BaraMGB refactor startTimeOffset(positive values)/change offset on tempochange 0849eec

@BaraMGB if I'm reading this correctly, we're fixing this in this PR, correct? Is the PR ready for testing?

BaraMGB · 2017-05-04T08:53:59Z

@tresf actually I thought this one is a feature and so it doesn't goes into 1.2. The issue by resizing from the end and changing tempo is a bug though. So my plan is to handle it in a further pull request. But this one here is complicated. It works almost but is not perfect, yet.

tresf · 2017-05-04T13:00:38Z

I thought this one is a feature and so it doesn't goes into 1.2.

Good point. We can always use some testing though, regardless of the branch. Please feel free to ping @UnityParadox when ready if you need a tester, he's been volunteering himself for testing.

BaraMGB · 2017-05-15T08:53:07Z

Okay. This seems to work. I want to clean up and optimize the code before we merge. Perhaps people wants to test this. @UnityParadox

gi0e5b06 · 2017-07-21T14:40:10Z

Actually, this feature would be great for any type of track.
click-move for adjusting at the bar, click-shift move for extra precision.

BaraMGB · 2017-08-25T21:35:44Z

Can someone please review this and merge if it's okay?

Umcaruje · 2017-08-25T22:42:57Z

@BaraMGB It'll be first on my list tommorow.

BaraMGB · 2017-10-29T23:16:13Z

Can someone please review this and merge if it's okay?

gi0e5b06 · 2017-10-30T07:56:23Z

This should be available for any type of track, not just sample tracks.

BaraMGB · 2017-10-30T08:12:38Z

This should be available for any type of track, not just sample tracks.

Step by step.

zonkmachine · 2017-11-01T08:14:58Z

1 2 3 ... testing!

save to project doesn't work, yet

Save works fine as does the rest. I can't really assess the code but others have done this already. After testing it for an hour I'm happy to include it in master but someone else should really chip in.
The only question mark I have right now is about the rebase/merge that's been done here. I usually rebase branches I check out but this wouldn't work with sampletrackcut against latest master. I pretty much assume here that if github says this is fine and mergeable it's fine and I can merge it with master all right so I guess this is more out of curiosity. I only rebase local development branches against upstream so this usually don't create a separate commit. It sometimes does and I have yet to figure out why. Maybe because a merge have rewritten upstream? The problem is first of all that if I want to check this against the latest master I have to do a merge and the commits will come interleaved with each other and will be harder to dive into if I have to do a diff.

Edit: It's been tested with the fpe debug option.

BaraMGB · 2017-11-01T18:36:02Z

Actually I merged the latest master into my branch, got the merge conflict and solved it with meld. I did this twice for this branch. But I'm a little bit tired of this. Therefore it would be nice if this could be merged.

tresf · 2017-11-01T18:43:06Z

zonkmachine approved these changes 10 hours ago

it would be nice if this could be merged.

Alrighty then. 👍

PhysSong · 2017-11-14T10:18:13Z

src/core/Track.cpp

-				new StringPairDrag( QString( "tco_%1" ).arg(
-										m_tco->getTrack()->type() ),
-									dataFile.toString(), thumbnail, this );
+				m_action = ToggleSelected;


@BaraMGB I guess something went wrong when you sync with master branch.
See https://github.com/LMMS/lmms/pull/3649/files#diff-cb2a453c979a4777b9330e2b96644f0eR701.

PhysSong · 2017-11-14T11:50:59Z

I also found some serious GUI bugs introduced here(ex. can't copy TCOs with drag & drop). I'm going to fix that if @BaraMGB is not going to do that.

BaraMGB · 2017-11-14T14:26:43Z

@PhysSong I'm appreciate for your help in this matter.

BaraMGB · 2017-11-18T18:01:16Z

@PhysSong it's a long time ago when I wrote this. For fixing I need a little more help. Can you tell me which bugs you noticed? I don't get the problems. Copy TCOs by pressing ctrl and dragging don't work anymore is what I can reproduce. This should be easy to fix. But you pointed out that there are more GUI bugs. I need to know them. Thanks for the help.

BaraMGB · 2017-11-18T23:18:20Z

Okay, I guess I see whats's going on. I'll fix that.

Fix bugs introduced in LMMS#3487

* Fix TCO copy with Ctrl + Dragging * Fix text float display in song editor * avoids override cursor in BBEditor for sampletracks at left side

Sampletrack - resize from left

) * Fix TCO copy with Ctrl + Dragging * Fix text float display in song editor * avoids override cursor in BBEditor for sampletracks at left side

BaraMGB added 3 commits April 4, 2017 13:32

resize sampletrack from start (gui)

b9c8f20

resize sampletrack from start (sample)

92099b2

code clean up

f8ae969

BaraMGB commented Apr 5, 2017

View reviewed changes

tresf mentioned this pull request Apr 5, 2017

Better Sample Track support #1471

Open

29 tasks

save/load project with left resized sampletrack works

9eb7b72

BaraMGB force-pushed the sampletrackcut branch from f342d3c to 9eb7b72 Compare April 7, 2017 12:14

jasp00 reviewed Apr 8, 2017

View reviewed changes

code improvements Part I

25fb7b2

fixes <change tempo bug> / some spaces

d6701d1

BaraMGB added the in progress label Apr 19, 2017

BaraMGB added 2 commits May 2, 2017 14:15

improve resizing

b8e9248

refactor startTimeOffset(positive values)/change offset on tempochange

0849eec

tresf added this to the 1.3.0 milestone May 4, 2017

BaraMGB added 2 commits May 7, 2017 00:34

improve rounding in sampletrack visualizing

246797a

change approach again

1c30827

cleanup the code/consolidate if blocks

2b27365

merge master

6b8353b

BaraMGB removed the in progress label Aug 25, 2017

rebase to master

a6ec9e6

zonkmachine approved these changes Nov 1, 2017

View reviewed changes

tresf merged commit c765249 into LMMS:master Nov 1, 2017

PhysSong reviewed Nov 14, 2017

View reviewed changes

BaraMGB mentioned this pull request Nov 19, 2017

fixes copy of non selected TCOs in Songeditor by Ctrl + dragging #3988

Closed

PhysSong added a commit to PhysSong/lmms that referenced this pull request Nov 24, 2017

Fix some (mostly)song-editor GUI regressions

11eb114

Fix bugs introduced in LMMS#3487

PhysSong mentioned this pull request Nov 24, 2017

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

Merged

PhysSong added a commit that referenced this pull request Nov 29, 2017

Fix song-editor regressions and BB-editor bugs from #3487 (#4008)

6de7cb6

* Fix TCO copy with Ctrl + Dragging * Fix text float display in song editor * avoids override cursor in BBEditor for sampletracks at left side

tresf mentioned this pull request Feb 3, 2018

Resizing pattern at the start #4154

Open

tresf mentioned this pull request Mar 5, 2019

Crop audio #4783

Closed

SecondFlight mentioned this pull request Apr 23, 2020

front end and back end sample editing cutting glitch? #5463

Closed

sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022

Sampletrack - resize from left (LMMS#3487)

9499fae

Sampletrack - resize from left

Sampletrack - resize from left #3487

Sampletrack - resize from left #3487

Conversation

BaraMGB commented Apr 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Umcaruje commented Apr 5, 2017

BaraMGB commented Apr 7, 2017

jasp00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Umcaruje commented Apr 14, 2017

BaraMGB commented Apr 14, 2017

tresf commented Apr 14, 2017

BaraMGB commented Apr 17, 2017

tresf commented May 4, 2017

BaraMGB commented May 4, 2017

tresf commented May 4, 2017

BaraMGB commented May 15, 2017

gi0e5b06 commented Jul 21, 2017

BaraMGB commented Aug 25, 2017

Umcaruje commented Aug 25, 2017

BaraMGB commented Oct 29, 2017

gi0e5b06 commented Oct 30, 2017

BaraMGB commented Oct 30, 2017

zonkmachine commented Nov 1, 2017 • edited Loading

BaraMGB commented Nov 1, 2017

tresf commented Nov 1, 2017

Choose a reason for hiding this comment

PhysSong commented Nov 14, 2017

BaraMGB commented Nov 14, 2017

BaraMGB commented Nov 18, 2017

BaraMGB commented Nov 18, 2017

BaraMGB commented Apr 5, 2017 •

edited

Loading

zonkmachine commented Nov 1, 2017 •

edited

Loading