Envelope point navigation gets stuck or skips points if there is more than one point at the same position

[jcsteh]

STR (synthetic):

1. Create a new project and a new track.
2. Show the volume envelope with control+alt+v.
3. Select it with alt+l.
4. Page down to bar 2.
5. Insert a point with shift+e.
6. Run Envelope: Insert new point at current position (do not remove nearby points) twice.
7. Page down to bar 3.
8. Insert another point with shift+e.
9. Go to start with control+home.
10. Move to the next envelope point with alt+k.
    • Observe: point 0
11. Move next with alt+k.
    • Observe: point 1
12. Move next with alt+k.
    • Expected: point 2
    • Actual: point 3
13. From point 3, Move previous with alt+j.
    • Observe: point 2
14. Move previous with alt+j.
    • Expected: point 1
    • Actual: point 2

While this is probably not something someone would do in practice, it demonstrates the issue easily. In practice, this can happen if you copy items which encompass track envelope points with Preferences -> Editing Behavior -> Envelope Display -> Automatically add edge points when editing: Media items checked, which is the default.

STR:

1. Open a new project.
2. Shift+insert to insert a MIDI item. (The fact that it's MIDI isn't relevant; this is just an easy way to get a single bar item.)
3. Go to start with control+home.
4. Show the volume envelope with control+alt+v.
5. Select it with alt+l.
6. Move to beat 2 with control+page down.
7. Insert a point with shift+e.
8. Focus the item with control+left arrow.
9. Copy it with control+c.
10. Move to bar 2 with page down.
11. Paste with control+v.
12. Focus the track with up arrow.
13. Select the volume envelope with alt+l.
14. Move to the previous envelope point 4 times with alt+j.
    • Observe: point 2
15. Move previous with alt+j.
    • Expected: point 1
    • Actual: point 2

It seems that when you copy and paste items like this, REAPER inserts multiple points at the start and end of the item. This has been driving me wild for a long time, but I've never quite been able to work out wtf was going on.

I think this was one of the issues @Justinmac84 might have been encountering in #134, but I didn't have sufficient information to reproduce it back then.

[jcsteh]

The core problem here is that when I was working on OSARA's envelope point code, I made an assumption that you couldn't have multiple envelope points at precisely the same position. That assumption is very incorrect, and since the code reflects that assumption, all the things are broken.

[jcsteh]

Just to make things more interesting, when you copy items, the envelope points REAPER creates aren't necessarily quite at the same position. Sometimes, they're a few samples apart, which causes additional problems with OSARA's envelope code because we increment the time slightly to ensure we don't get the point before the time we want, which REAPER tends to do sometimes. All the things are all broken.

[day-garwood]

I have had this problem once or twice as well, but was never sure what was causing it hence never opened an issue. Definitely makes sense though - when I move items my envelope points move as well, so that explains a lot.

[jcsteh]

Yeah, I think I'm going to end up having to rewrite big chunks of that code. That ought to be fun. The problem is that I still can't quite get my head around it, which makes it difficult to get anywhere.

[jcsteh]

Current implementation thoughts (but I've gone through several iterations of contradictory thought on this, so this is subject to change):

I think we need to do two things:

1. In moveToEnvelopePoint, before calling getEnvelopePointByProjectTime, first check whether currentEnvelopePoint is at the cursor. If it is, move relative to that, rather than searching for the point at the cursor. This should simplify that case a lot, as well as preventing us from getting stuck or skipping points at exactly the same position.
2. In getEnvelopePointByProjectTime, instead of incrementing the time a bit, just use the provided time. Then, check the time of the returned point. If it's less than the specified time, try the next point. If that's equal, return that. Otherwise, return the original point. This should fix the case where there are several points that are almost (but not quite) at the same position; i.e. a few samples apart.

[jcsteh]

2. In getEnvelopePointByProjectTime, instead of incrementing the time a bit, just use the provided time. Then, check the time of the returned point.

I didn't end up doing this. It didn't seem to be necessary in the end, but maybe some other bug will show up in future...

[jcsteh]

For future reference, here's a project I used to test this. There's a track volume envelope: point 0 and 1 are in different positions, points 2 and 3 are at the same position, point 4 is almost at the same position as point 3 but just slightly after, and poin5 is further away. There's also a volume envelope on the second item with just a single point to test an envelope which doesn't begin at the start of the project.