Skip to content
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.

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

Feature: tempo editing in the MiscWidget #19

Merged

Conversation

dgslomin
Copy link
Contributor

@dgslomin dgslomin commented Sep 2, 2019

No description provided.

@markusschwenk
Copy link
Owner

Thanks a lot! Great idea to add the tempo in this widget!

Two comments:
1.)
I tried it on linux, and it appears that, when using free-hand or line mode to add transitions in the tempo, existing events in the given time range are not deleted.
image

This screenshot was taken after using line mode once up and once down. The latest line should be permanent, and the end point set the tempo until the very next tempo change event.

2.) Could you add an update to the manual? I think there's either a description about tempo changes, but there should also be a section about the misc widget.

@dgslomin
Copy link
Contributor Author

dgslomin commented Sep 3, 2019

Oops, looks like I have some debugging to do. I didn't think to test the freehand and line modes since I'm most interested in editing existing events myself, and was very closely basing the tempo code on the existing code for editing controllers, etc. I must have missed a case.

I'll also do the manual bits.

@markusschwenk
Copy link
Owner

Thanks for fixing!
However, I still end up with max tempo sometimes (was not able to figure out a way to always reproduce) at the end of the time frame I modified using free-hand or linear tool. Could you check?

@dgslomin
Copy link
Contributor Author

dgslomin commented Sep 16, 2019 via email

Div Slomin and others added 2 commits September 16, 2019 07:20
The two pass approach didn't help, and made lines which were drawn straight
actually appear curved when you released the mouse.  The problem actually
appeared to be coming from needing to handle points a little beyond the end of
the "toAlign" array, so I had to make the interpolate function extrapolate off
the right end of that line.  Also fixed a problem with inserting lots of
"minimum possible value" events if the user released the mouse below the bottom
edge of the window.
@daslomin-microsoft
Copy link
Contributor

Nope, still no good. The extrapolation is causing some undesirable effects. I'll keep poking at it.

…es correctly

In particular, I still need to figure out what the righthand bound of the range
should be for removing the old events.  maxTick isn't correct, even if you
recompute it as xPosToTock(toAlign.last().first) each time you go around the
loop, because the end point depends on the tempo events you're going to insert
not the ones which are already there.  I may need to intersperse the remove and
add rather than having them in separate loops.
What you draw is evaluated in the context of what's already there, so that
means that a tempo line which you draw straight will end up curved once you
release the mouse.  That's unintuitive, but I'm starting to think it's not
actually a bug, just an accurate reflection of what the straight line really
means.
@markusschwenk markusschwenk merged commit 1166479 into markusschwenk:master Oct 20, 2019
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.

None yet

3 participants