Music: sticky timeline sound rows

[breville]

The timeline UI now maintains the row allocated to a specific sound between renders whenever possible.

Previously, we had a simple heuristic which simply allocated sounds to rows in the order that they appeared in the array of song data events, which was sorted by the sound's start time. A side effect of this was that if new sounds were scheduled while the song was playing, existing sounds could get pushed to new rows:

musiclab_uniquesounds_before.mov

Now, we have a more sophisticated approach in which we examine the previous allocations and keep sounds in those rows. The effect of this new approach is that if a new sound is scheduled when a song is playing, even if it will be played before another sound, the existing sound will maintain its row in the timeline UI:

musiclab_uniquesounds_after.mov

It's worth noting that the new approach will remove empty rows, so it's still possible that rows will shift upward. In the future, we'll likely render each row in a way that it can still be correlated to its sound, so this won't be too severe.

Also, a useful behaviour of this implementation is that when a sound for a given play block is changed, the new sound will occupy the row taken by the old sound, so it will effectively stay in place in the timeline UI.

This PR also improves performance in the Timeline UI by caching the generated set of unique sounds at the beginning of each render so that a variety of helper functions don't need to keep regenerating it.

This PR also fixes an issue in which the Timeline's playhead can be momentarily seen at its last position when starting to play a song, before the first hit of a timer callback corrects it. Now, the playhead position is reset when playback begins.

This change was a great candidate for unit testing, so the first Music Lab unit testing is now here.

[sanchitmalhotra126]

Couple questions on the logic but not blocking. Looks good!

[sanchitmalhotra126]

Could you now remove this sound from uniqueSounds so that on the next pass below you don't have to check if the sound has already been added?

[breville]

That's a neat idea. One thing I have been attempting to do is to keep the data somewhat immutable, so I wanted to leave that prior array intact while we construct the new one, though it does mean we have to do some extra checking.

[sanchitmalhotra126]

First unit test! 😎

[sanchitmalhotra126]

Would it be better to iterate through uniqueSounds here instead and check this.previousUniqueSounds.indexOf(uniqueSounds[i])? That way you're not checking sounds that may no longer be in use?

[breville]

Great question. I might be missing a good shortcut, but given we do two passes -- one to pull over sounds into their existing slots, and another to fill in the gaps with the remaining sounds -- I think the resulting code for that first pass is not greatly different, and is in fact is a tiny bit longer:

      // For each of the current sounds...
      for (let i = 0; i < currentUniqueSounds.length; i++) {
        // ...see if it already has a slot...
        const previousSlot = this.previousUniqueSounds.indexOf(
          currentUniqueSounds[i]
        );
        // ...and if so, put it back in there.
        if (previousSlot !== -1) {
          outputSounds[previousSlot] = this.previousUniqueSounds[previousSlot];
        }
      }

[sanchitmalhotra126]

Yeah, that's fair. I suppose another slight optimization you could make here is to push any new sounds that weren't the previous list into a second array, and then iterate through that after, instead of going through the whole list again? At this scale though it's not gonna make a huge difference, so whatever is easiest.

[breville]

Oh that's a great idea, and is now implemented. As a result, I did follow your earlier suggestion and now loop through existing sounds rather than previous sounds. Thanks for the ideas here!

[sanchitmalhotra126]

Just curious, how would we end up with empty rows here if the original array was the length of uniqueSounds?

[breville]

Ah, that was a mistake. In the updated implementation we clearly handle the reality that the array before filtering is sparse and can contain entries beyond that length, particularly in the case when we have some entries that were previously in high indices and then we remove one or more entries in lower indices.