Only render when stuff on-screen actually changes!

This means we can basically guarantee 0% CPU usage when nothing on the
screen is changing! There may still be some kinks to work out, but I've
tested most features and fixed any apparent bugs (including an unrelated
bug in the suspend feature which made it crash when resuming the process).

6 files changed, 76 insertions, 20 deletions

diff --git a/client.js b/client.js
index a899c2d..c591a00 100644
--- a/client.js
+++ b/client.js
@@ -16,7 +16,7 @@ const {
   }
 } = require('./tui-lib')
 
-const setupClient = async ({backend, writable, interfacer, appConfig, frameRate = 50}) => {
+const setupClient = async ({backend, writable, interfacer, appConfig}) => {
   const cleanTerminal = () => {
     writable.write(ansi.cleanCursor())
     writable.write(ansi.disableAlternateScreen())
@@ -29,14 +29,15 @@ const setupClient = async ({backend, writable, interfacer, appConfig, frameRate
 
   dirtyTerminal()
 
-  const root = new Root(interfacer)
+  const flushable = new Flushable(writable, true)
+  const root = new Root(interfacer, flushable)
+  root.on('rendered', () => flushable.flush())
 
   const size = await interfacer.getScreenSize()
   root.w = size.width
   root.h = size.height
   root.fixAllLayout()
 
-  const flushable = new Flushable(writable, true)
   flushable.resizeScreen(size)
   flushable.write(ansi.clearScreen())
   flushable.flush()
@@ -78,12 +79,7 @@ const setupClient = async ({backend, writable, interfacer, appConfig, frameRate
   appElement.queueListingElement.buildItems()
   appElement.playbackInfoElement.updateTrack(backend.playingTrack)
 
-  const renderInterval = setInterval(() => {
-    root.renderTo(flushable)
-    flushable.flush()
-  }, frameRate)
-
-  return {appElement, cleanTerminal, flushable, renderInterval}
+  return {appElement, cleanTerminal, dirtyTerminal, flushable, root}
 }
 
 module.exports = setupClient
diff --git a/index.js b/index.js
index 2b6978b..26f0fbb 100755
--- a/index.js
+++ b/index.js
@@ -67,9 +67,8 @@ async function main() {
     }
   })
 
-  const { appElement, renderInterval } = await setupClient({
+  const { appElement, dirtyTerminal, flushable, root } = await setupClient({
     backend,
-    frameRate: 50,
     interfacer: new CommandLineInterfacer(),
     writable: process.stdout
   })
@@ -78,7 +77,6 @@ async function main() {
     if (telnetServer) {
       telnetServer.disconnectAllSockets('User closed mtui - see you!')
     }
-    clearInterval(renderInterval)
     process.exit(0)
   })
 
@@ -91,6 +89,7 @@ async function main() {
     process.stdin.setRawMode(false)
     process.stdin.setRawMode(true)
     dirtyTerminal()
+    root.renderNow()
   })
 
   const loadPlaylists = async () => {
diff --git a/telnet-server.js b/telnet-server.js
index 72869a2..64304c6 100644
--- a/telnet-server.js
+++ b/telnet-server.js
@@ -25,11 +25,10 @@ class TelnetServer extends EventEmitter {
 
   async handleConnection(socket) {
     const interfacer = new TelnetInterfacer(socket)
-    const { appElement, cleanTerminal, flushable, renderInterval } = await setupClient({
+    const { appElement, cleanTerminal, flushable } = await setupClient({
       backend: this.backend,
       writable: socket,
       interfacer,
-      frameRate: 100,
       appConfig: {
         canControlPlayback: false,
         canControlQueue: true,
@@ -58,7 +57,6 @@ class TelnetServer extends EventEmitter {
     appElement.on('quitRequested', quit)
 
     socket.on('close', () => {
-      clearInterval(renderInterval)
       if (!closed) {
         flushable.end()
         closed = true
diff --git a/todo.txt b/todo.txt
index eba924c..6af1370 100644
--- a/todo.txt
+++ b/todo.txt
@@ -333,3 +333,40 @@ TODO: Default to 'after selected track' in context menu, and make pressing Q
       really only useful when you're building a queue at the start, assuming
       you don't start by shuffling your entire queue anyway).
       (Done!)
+
+TODO: When grouplike listings are fixLayout'd (e.g. the window is resized),
+      make sure the selected input is still visible!
+
+TODO: If you press any key which should select a particular element of the UI
+      (e.g. the menubar) while a context menu is open, currently the context
+      menu will close and then restore selection to the element which was
+      selected when the menu was opened. This seems to happen after the target
+      element is selected; the effect is that any key which is meant to focus
+      specific UI parts instead closes the context menu while one is open.
+      We already have a key for doing just that (the cancel key, i.e. esc or
+      period), so fix this!
+
+TODO: g/G should work for navigation in context menus! And probably all
+      ListScrollForms, tbh, but I'm kinda weary of introducing new behavior
+      like that to tui-lib.
+
+TODO: PageUp/PageDown support is complicated to implement (I've tried) and
+      should come eventually, but for now just make Home/End work as aliases
+      for g/G. This means making it possible for the keybinding system to let
+      a single input function return true for multiple different keys!
+
+TODO: Work out the BIGGER framerate shenanigans. Specifically: down with the
+      render interval! Or, more literally, get rid of it altogether. Only
+      render when we know the on-screen text is going to have changed.
+      (Done - Huzzah! We use 0% CPU when not actively updating the screen now.)
+
+TODO: Be more sneaky about how we decide what text to update on the screen,
+      in a way which saves us performance. Instead of going through the whole
+      ansi.interpret process, predict which regions of the screen are going to
+      have updated, and only process those spaces. We can guess reasonably by
+      looking at which elements caused the screen to update (keep track of this
+      in the Root, through the shouldRenderTo function).
+
+TODO: Create a basic Element class, which DisplayElement extends. We have a lot
+      of code related to scheduling displaying stuff on the screen, and it'd be
+      nice to keep it more separate from all the basic element logic.
diff --git a/tui-lib b/tui-lib
-Subproject 878e55e7c2a203d89fb1dad83ba6d6d8751b521
+Subproject 3f76094c554c23ee3519f41458a04d348f4f75a
diff --git a/ui.js b/ui.js
index c6bb448..f425048 100644
--- a/ui.js
+++ b/ui.js
@@ -1471,12 +1471,13 @@ class GrouplikeListingForm extends ListScrollForm {
   }
 
   set curIndex(newIndex) {
-    this._curIndex = newIndex
+    this.setDep('curIndex', newIndex)
     this.emit('selected input', this.inputs[this.curIndex])
+    return newIndex
   }
 
   get curIndex() {
-    return this._curIndex
+    return this.getDep('curIndex')
   }
 
   get firstItemIndex() {
@@ -1889,6 +1890,14 @@ class InlineListPickerElement extends FocusElement {
   }
 }
 
+// Quite hacky, but ATM I can't think of any way to neatly tie getDep/setDep
+// into the slider and toggle elements.
+const drawAfter = (fn, thisObj) => (...args) => {
+  const ret = fn(...args)
+  thisObj.scheduleDrawWithoutPropertyChange()
+  return ret
+}
+
 class SliderElement extends FocusElement {
   // Same general principle and usage as InlineListPickerElement, but for
   // changing a numeric value.
@@ -1896,7 +1905,7 @@ class SliderElement extends FocusElement {
   constructor(labelText, {setValue, getValue, maxValue = 100, percent = true, getEnabled = () => true}) {
     super()
     this.labelText = labelText
-    this.setValue = setValue
+    this.setValue = drawAfter(setValue, this)
     this.getValue = getValue
     this.getEnabled = getEnabled
     this.maxValue = maxValue
@@ -2036,7 +2045,7 @@ class ToggleControl extends FocusElement {
   constructor(labelText, {setValue, getValue, getEnabled = () => true}) {
     super()
     this.labelText = labelText
-    this.setValue = setValue
+    this.setValue = drawAfter(setValue, this)
     this.getValue = getValue
     this.getEnabled = getEnabled
     this.keyboardIdentifier = this.labelText
@@ -2549,8 +2558,12 @@ class PlaybackInfoElement extends DisplayElement {
   }
 
   updateProgress(timeData, player) {
-    this.timeData = timeData
     const {timeDone, duration, lenSecTotal, curSecTotal} = timeData
+    this.timeData = timeData
+    this.curSecTotal = curSecTotal
+    this.lenSecTotal = lenSecTotal
+    this.volume = player.volume
+    this.isLooping = player.isLooping
 
     this.progressBarLabel.text = '-'.repeat(Math.floor(this.w / lenSecTotal * curSecTotal))
     this.progressTextLabel.text = timeDone + ' / ' + duration
@@ -2585,6 +2598,15 @@ class PlaybackInfoElement extends DisplayElement {
     this.timeData = {}
     this.fixLayout()
   }
+
+  get curSecTotal() { return this.getDep('curSecTotal') }
+  set curSecTotal(v) { return this.setDep('curSecTotal', v) }
+  get lenSecTotal() { return this.getDep('lenSecTotal') }
+  set lenSecTotal(v) { return this.setDep('lenSecTotal', v) }
+  get volume() { return this.getDep('volume') }
+  set volume(v) { return this.setDep('volume', v) }
+  get isLooping() { return this.getDep('isLooping') }
+  set isLooping(v) { return this.setDep('isLooping', v) }
 }
 
 class OpenPlaylistDialog extends Dialog {
@@ -3310,6 +3332,10 @@ class PartyBanner extends DisplayElement {
   drawTo(writable) {
     writable.write(ansi.moveCursor(this.absTop, this.absLeft))
 
+    // TODO: Figure out how to connect this to the draw dependency system.
+    // Currently the party banner doesn't schedule any renders itself (meaning
+    // if you have nothing playing or otherwise rendering, it'll just stay
+    // still).
     const timerNum = Date.now() / 2000 * this.direction
     let lastAttribute = ''
     const updateAttribute = offsetNum => {