Update render.js to reduce CPU usage

[OpenSourceAnarchist]

I am once again asking to merge 🙏 😄

Should address your comments, let me know if I can fix anything before closing... I really want to reduce the CPU usage!!

Oh and run minify :)

[OpenSourceAnarchist]

Waveforms truly go to 0 after a small period of time, but 250 ms is more than plenty. Also noticed notes weren't falling down when there weren't any voices, that's fixed now too.

[spessasus]

It still doesn't work it seems. The waveform remains after voices drop to 0. And also checking time is a really inneficient way of checking for that. Why not just create a bool hasRendered and when voices reach 0, render once, flag it as true and on the next renders check that it's true and avoid rendering?

Also, if possible avoid calling document.getElementById, store the value in a variable.

PS: A tip for javascript:

[OpenSourceAnarchist]

I tried to address everything. I still can't reproduce your waveform behavior... it goes to 0 under every condition and file I've tested, along with real-time input. I don't have access to MIDI in, so do test if that bug is happening. And if you still get frozen waveforms, personally I think it's a cool look but if you don't want it please try changing if (!this.renderBool || this.hasRendered > 1) to a higher value, e.g. if (!this.renderBool || this.hasRendered > 20) and see if it helps. I can't fix it if I don't know why it's happening.

Edit: Everything I can reasonably figure out how to fix, is addressed. I couldn't find a less hacky way than retrieving the voice meter text string directly from the document element... yes I know it's not great but I spent hours trying to debug why I could never match your calculation in the voice meter locally, even adding up all the channel voices in addition to the synth voicesActive count. It was simpler to grab the pre-computed string, else the waveforms were never matching entirely. If it's unacceptable I would love to learn how to match the calculation in sync unless it's still more efficient the way I did it.

[OpenSourceAnarchist]

Still having issues rendering falling notes, however I think it's because you're computing noteFallingTimeMs in compute_note_positions.js incorrectly. It shouldn't be static all the time, but it's never updated once a Renderer is constructed. Theoretically, it should be dynamically updated whenever a note is first on the user's screen, at the top of the canvas. Instead, it's only at note on. So this has the effect that notesOnScreen is always wrong, because it's not the actual number of notes on screen! If you could update compute_note_positions.js to compute this based on the canvas height, playback rate, note time, current time, etc. the math should be pretty simple unless there's an even easier way to do it.

I've been banging my head wondering why notesOnScreen is wrong, and this is the culprit, which messes up my hasRendered check!

[spessasus]

Still having issues rendering falling notes, however I think it's because you're computing noteFallingTimeMs in compute_note_positions.js incorrectly. It shouldn't be static all the time, but it's never updated once a Renderer is constructed. Theoretically, it should be dynamically updated whenever a note is first on the user's screen, at the top of the canvas. Instead, it's only at note on. So this has the effect that notesOnScreen is always wrong, because it's not the actual number of notes on screen! If you could update compute_note_positions.js to compute this based on the canvas height, playback rate, note time, current time, etc. the math should be pretty simple unless there's an even easier way to do it.

I've been banging my head wondering why notesOnScreen is wrong, and this is the culprit, which messes up my hasRendered check!

I honestly have no idea what you're talking about. computeNotePositions runs every frame and recomputes everything every frame too. If setting it as a prototype for the Renderer class is confusing to you, it's just a way to split a JavaScript class into multiple files. You can think of it like C#'s partial class keyword.

Though I've noticed that I've left a console log in the render method. Whoops!

Edit: please run minify_website.sh when you commit stuff to the PR.

[OpenSourceAnarchist]

Well, it doesn't matter anymore. I finally figured out a solution. Just checking the length of notesToDraw fixed the timing issue. Sorry I wasn't clear on what was happening.

In essence, you aren't computing the actual time a note shows up on screen. notesOnScreen is only the number of active notes. That confused me for a long time.

I am very sorry, but I am manually editing the files on GitHub myself. My "dev environment" is a local download of the repo, so I can manually upload all the changed files from running minify_website.sh but I think it's easier for you to do it I imagine. Plus, it only matters if you accept the changes :/

I can safely say the last commit 38r3a08 "I figured out a solution" fixes every single issue I've had and passes all my stress tests with live input, notes falling, pausing, resuming, and the waveforms always rendering correctly. If it passes your tests too, I'm sorry to make you run minify but I do hope it saves some CPU cycles for all the demo sites.

Edit: Feel free to make any style edits, and instead of merging all my commits you can just do squash and merge for a cleaner history.

[spessasus]

Hi, I've looked at your implementation and there a few two things that look strangr:

1. Why use other html elements when the renderer has access to both synth and sequencer?
2. The notes keep being rendered when the sequencer is paused.
3. Generally the idea is bad. Using the select element to check if the notes are playing to a MIDI out is an okay way to handle this, but what about playing to a Web MIDI Link? The simplest solution (that I've used) is to just check if the sequencer is paused or undefined (then there are no notes so if no voices we can skip)

I've addressed these issues with my own soluton (that's now live). So since i've fixed this myself, I'm closing this PR. No hard feelings, though! Thanks for the idea and a proposed solution :-)

I hope this will improve the performance like you wanted.

[OpenSourceAnarchist]

I'm sitting here laughing at how simple the solution was... I don't know why I thought all the extra conditional checks were necessary, or that I had to wait an extra render cycle (which was the whole reason for hasRendered). At least I learned a lot about JavaScript. If only I realized all my conditionals reduced to when the seq was undefined or paused. So obvious! Thank you for all your help and it absolutely improved the performance, even better than mine so I'm glad you didn't use my version.

The reason I used html elements though is because I didn't think to just check the sequencer for being undefined or paused. I thought that wouldn't allow the waveforms to completely render all the voices (if there was reverb, e.g. voices playing a little after the sequencer was paused). Getting the values from the sequencer and synth was unreliable, as in the values weren't matching up with the html elements. For notesOnScreen, this is because of the reason I stated in a previous comment. For the voices, I tried very hard to match your code to compute the number of voices in the voice meter but I couldn't so I just grabbed the value directly from the html element. In hindsight this was completely unnecessary because like you said, we just need to check if it's undefined or paused apparently.

Thanks again, and I'm almost done with "DynaTune", stay tuned :))