Improve scroll persistence (#104)

* Refactor so buffer operations directly trigger window rendering This allows all buffer operations to be able to attempt to preserve scroll position without having to go through the Window object. Window.append should probably be deprecated, except perhaps as a convenience extension * Treat onBufModifyPre/Post as a renderer transaction in JLineWindow This drastically simplifies the JLineBuffer implementation; we may even be able to just replace it with a factory function * Handle buffer clearing * Remove Buffer append marshalling through the Window * Minor Cleanup
dhleong · Jan 8, 2020 · 3307e59 · 3307e59
1 parent 4159b1f
commit 3307e59
Show file tree

Hide file tree

Showing 8 changed files with 222 additions and 90 deletions.
diff --git a/core/src/main/kotlin/net/dhleong/judo/render/JudoBuffer.kt b/core/src/main/kotlin/net/dhleong/judo/render/JudoBuffer.kt
@@ -3,6 +3,7 @@ package net.dhleong.judo.render
 import net.dhleong.judo.DelegateStateMap
 import net.dhleong.judo.IStateMap
 import net.dhleong.judo.util.CircularArrayList
+import java.util.concurrent.atomic.AtomicInteger
 
 open class JudoBuffer(
     override val id: Int,
@@ -17,6 +18,7 @@ open class JudoBuffer(
     ) : this(ids.newBuffer(), settings, scrollbackSize)
 
     private val contents = CircularArrayList<FlavorableCharSequence>(scrollbackSize)
+    private val windows = mutableListOf<IJudoWindow>()
 
     override val settings: IStateMap = DelegateStateMap(settings)
 
@@ -28,40 +30,41 @@ open class JudoBuffer(
     override fun get(index: Int): FlavorableCharSequence = contents[index]
 
     @Synchronized
-    override fun append(text: FlavorableCharSequence) {
+    override fun append(text: FlavorableCharSequence) = notifyingChanges {
         text.splitAtNewlines(contents, continueIncompleteLines = true)
     }
 
     @Synchronized
-    override fun appendLine(line: FlavorableCharSequence) {
+    override fun appendLine(line: FlavorableCharSequence) = notifyingChanges {
         if (!line.endsWith('\n')) {
             line += '\n'
         }
         line.splitAtNewlines(contents, continueIncompleteLines = false)
     }
 
     @Synchronized
-    override fun clear() {
+    override fun clear() = notifyingChanges{
         contents.clear()
     }
 
     @Synchronized
-    override fun deleteLast() =
+    override fun deleteLast() = notifyingChanges {
         contents.removeLast()
+    }
 
     @Synchronized
-    override fun replaceLastLine(result: FlavorableCharSequence) {
+    override fun replaceLastLine(result: FlavorableCharSequence) = notifyingChanges {
         contents[contents.lastIndex] = result
     }
 
     @Synchronized
-    override fun set(newContents: List<FlavorableCharSequence>) {
+    override fun set(newContents: List<FlavorableCharSequence>) = notifyingChanges {
         clear()
         newContents.forEach(this::appendLine)
     }
 
     @Synchronized
-    override fun set(index: Int, line: FlavorableCharSequence) {
+    override fun set(index: Int, line: FlavorableCharSequence) = notifyingChanges {
         val newLine = line.indexOf('\n')
         require(newLine == -1 || newLine == line.lastIndex) {
             "Line must not have any newline characters in it"
@@ -72,6 +75,44 @@ open class JudoBuffer(
         contents[index] = line
     }
 
+    override fun attachWindow(window: IJudoWindow) {
+        windows += window
+    }
+
+    override fun detachWindow(window: IJudoWindow) {
+        windows -= window
+    }
+
+    private val changeWorkspace = arrayListOf<Any?>()
+    private val changeDepth = AtomicInteger(0)
+    protected fun beginChange() {
+        if (changeDepth.getAndIncrement() > 0) return
+
+        changeWorkspace.ensureCapacity(windows.size)
+        for (i in changeWorkspace.size until windows.size) {
+            changeWorkspace.add(null)
+        }
+
+        for (i in windows.indices) {
+            changeWorkspace[i] = windows[i].onBufModifyPre()
+        }
+    }
+    protected fun endChange() {
+        if (changeDepth.decrementAndGet() > 0) return
+
+        for (i in windows.indices) {
+            windows[i].onBufModifyPost(changeWorkspace[i])
+        }
+        changeWorkspace.fill(null)
+    }
+
+    protected inline fun <R> notifyingChanges(block: () -> R): R = try {
+        beginChange()
+        block()
+    } finally {
+        endChange()
+    }
+
     companion object {
         const val DEFAULT_SCROLLBACK_SIZE = 20_000
     }

diff --git a/core/src/main/kotlin/net/dhleong/judo/script/Jsr223ScriptingEngine.kt b/core/src/main/kotlin/net/dhleong/judo/script/Jsr223ScriptingEngine.kt
@@ -190,7 +190,7 @@ class Jsr223JudoCore(
     override var buffer: Any
 //        get() = engine.toScript(judo.renderer.currentTabpage.currentWindow.currentBuffer)
         get() = judo.renderer.currentTabpage.currentWindow.let { w ->
-            Jsr223Buffer(w, w.currentBuffer)
+            Jsr223Buffer(w.currentBuffer)
         }
         set(_) {
             throw UnsupportedOperationException("TODO change buffer in Window")
@@ -207,7 +207,7 @@ class Jsr223Window(
     override val width: Int get() = window.width
     override val height: Int get() = window.visibleHeight
     override val buffer: IScriptBuffer
-        get() = Jsr223Buffer(window, window.currentBuffer)
+        get() = Jsr223Buffer(window.currentBuffer)
 
     override var hidden: Boolean
         get() = window.isWindowHidden
@@ -231,7 +231,6 @@ class Jsr223Window(
 }
 
 class Jsr223Buffer(
-    private val window: IJudoWindow,
     private val buffer: IJudoBuffer
 ) : IScriptBuffer {
     override val id: Int = buffer.id
@@ -240,7 +239,7 @@ class Jsr223Buffer(
         get() = buffer.size
 
     override fun append(line: String) {
-        line.appendAsFlavorableTo(window)
+        line.appendAsFlavorableTo(buffer)
     }
 
     override fun clear() {

diff --git a/core/src/main/kotlin/net/dhleong/judo/script/JythonScriptingEngine.kt b/core/src/main/kotlin/net/dhleong/judo/script/JythonScriptingEngine.kt
@@ -523,7 +523,6 @@ internal fun createPyCore(
                     judo.renderer.currentTabpage.currentWindow
                 )
                 "buffer" -> JythonBuffer.from(
-                    judo.renderer.currentTabpage.currentWindow,
                     judo.renderer.currentTabpage.currentWindow.currentBuffer
                 )
                 else -> super.__findattr_ex__(name)
@@ -597,7 +596,7 @@ internal fun createPyWindow(
     return object : PyObject() {
         override fun __findattr_ex__(name: String?): PyObject? =
             when (name ?: "") {
-                "buffer" -> JythonBuffer.from(window, window.currentBuffer) // cache?
+                "buffer" -> JythonBuffer.from(window.currentBuffer) // cache?
                 "height" -> Py.java2py(window.visibleHeight)
                 "width" -> Py.java2py(window.width)
                 "id" -> Py.java2py(window.id)
@@ -643,7 +642,7 @@ private class JythonWindow(
 ) : JythonWrapperPyObject<IJudoWindow>(window, IJudoWindow::class.java) {
 
     override fun __findattr_ex__(name: String?): PyObject = when (name) {
-        "buffer" -> JythonBuffer.from(window, window.currentBuffer) // cache?
+        "buffer" -> JythonBuffer.from(window.currentBuffer) // cache?
 
         else -> base.__findattr_ex__(name)
     }
@@ -680,11 +679,8 @@ private class JythonBuffer(
         base.__findattr_ex__(name)
 
     companion object {
-        fun from(
-            window: IJudoWindow,
-            buffer: IJudoBuffer
-        ): PyObject {
-            val jsrBase = Jsr223Buffer(window, buffer)
+        fun from(buffer: IJudoBuffer): PyObject {
+            val jsrBase = Jsr223Buffer(buffer)
             val base = Py.java2py(jsrBase)
             return JythonBuffer(buffer, jsrBase, base)
         }

diff --git a/jline/src/main/kotlin/net/dhleong/judo/jline/JLineBuffer.kt b/jline/src/main/kotlin/net/dhleong/judo/jline/JLineBuffer.kt
@@ -1,47 +1,17 @@
 package net.dhleong.judo.jline
 
-import net.dhleong.judo.inTransaction
-import net.dhleong.judo.render.FlavorableCharSequence
 import net.dhleong.judo.render.IdManager
 import net.dhleong.judo.render.JudoBuffer
 
 /**
  * @author dhleong
  */
 class JLineBuffer(
-    private val renderer: JLineRenderer,
+    renderer: JLineRenderer,
     ids: IdManager,
     scrollbackSize: Int = DEFAULT_SCROLLBACK_SIZE
 ) : JudoBuffer(
     ids,
     renderer.settings,
     scrollbackSize
-) {
-    override fun append(text: FlavorableCharSequence) = renderer.inTransaction {
-        super.append(text)
-    }
-
-    override fun appendLine(line: FlavorableCharSequence) = renderer.inTransaction {
-        super.appendLine(line)
-    }
-
-    override fun clear() = renderer.inTransaction {
-        super.clear()
-    }
-
-    override fun deleteLast(): FlavorableCharSequence  = renderer.inTransaction {
-        return super.deleteLast()
-    }
-
-    override fun replaceLastLine(result: FlavorableCharSequence) = renderer.inTransaction {
-        super.replaceLastLine(result)
-    }
-
-    override fun set(newContents: List<FlavorableCharSequence>) = renderer.inTransaction {
-        super.set(newContents)
-    }
-
-    override fun set(index: Int, line: FlavorableCharSequence) = renderer.inTransaction {
-        super.set(index, line)
-    }
-}
+)
diff --git a/jline/src/main/kotlin/net/dhleong/judo/jline/JLineWindow.kt b/jline/src/main/kotlin/net/dhleong/judo/jline/JLineWindow.kt
@@ -15,6 +15,7 @@ import org.jline.utils.AttributedString
 import org.jline.utils.AttributedStringBuilder
 import org.jline.utils.AttributedStyle
 import kotlin.math.abs
+import kotlin.properties.Delegates
 
 /**
  * @author dhleong
@@ -34,7 +35,17 @@ class JLineWindow(
     isFocusable, statusLineOverlaysOutput
 ), IJLineWindow {
 
-    override var currentBuffer: IJudoBuffer = initialBuffer
+    override var currentBuffer: IJudoBuffer by Delegates.observable(
+        initialBuffer
+    ) { _, oldValue, newValue ->
+        oldValue.detachWindow(this)
+        newValue.attachWindow(this)
+    }
+
+    init {
+        initialBuffer.attachWindow(this)
+    }
+
     override var isFocused: Boolean = false
     override val visibleHeight
         get() = when {
@@ -77,14 +88,6 @@ class JLineWindow(
 
     private var renderWorkspace = ArrayList<AttributedString>(initialHeight + 8)
 
-    override fun append(text: FlavorableCharSequence) = adjustingScrollingIfNecessary {
-        super.append(text)
-    }
-
-    override fun appendLine(line: FlavorableCharSequence) = adjustingScrollingIfNecessary {
-        super.appendLine(line)
-    }
-
     override fun render(display: JLineDisplay, x: Int, y: Int) {
         // synchronize on the buffer to protect against concurrent
         // modification
@@ -382,37 +385,58 @@ class JLineWindow(
         }
     }
 
-    private inline fun adjustingScrollingIfNecessary(
-        block: () -> Unit
-    ) = renderer.inTransaction {
+    override fun onBufModifyPre(): Any? {
         val b = currentBuffer
         val wordWrap = settings[WORD_WRAP]
 
-        val linesBefore = b.size
-        val lastLineRenderHeight = when {
-            b.size > 0 -> b[b.lastIndex].computeRenderedLinesCount(width, wordWrap)
-            else -> 0
-        }
+        renderer.beginUpdate()
+        return ScrollAdjustmentState(
+            linesBefore = b.size,
+            lastLineRenderHeight = when {
+                b.size > 0 -> b[b.lastIndex].computeRenderedLinesCount(width, wordWrap)
+                else -> 0
+            }
+        )
+    }
 
-        block()
+    override fun onBufModifyPost(preState: Any?) {
+        try {
+            val b = currentBuffer
+            val wordWrap = settings[WORD_WRAP]
 
-        if (scrollbackBottom == 0 && scrollbackOffset == 0) {
-            // no need to maintain scroll position
-            return
-        }
-        if (linesBefore == 0) {
-            // could not have scrolled; quick reject
-            return
-        }
+            if (b.size == 0) {
+                // easy case
+                scrollbackBottom = 0
+                scrollbackOffset = 0
+                return
+            }
 
-        val linesAfter = b.size
+            val (linesBefore, lastLineRenderHeight) = preState as ScrollAdjustmentState
+            if (scrollbackBottom == 0 && scrollbackOffset == 0) {
+                // no need to maintain scroll position
+                return
+            }
+            if (linesBefore == 0) {
+                // could not have scrolled; quick reject
+                return
+            }
 
-        if (linesBefore == linesAfter) {
-            // appending to last line
-            val newRenderHeight = b[b.lastIndex].computeRenderedLinesCount(width, wordWrap)
-            scrollbackOffset += (newRenderHeight - lastLineRenderHeight)
-        } else {
-            scrollbackBottom += linesAfter - linesBefore
+            val linesAfter = b.size
+
+            if (linesBefore == linesAfter) {
+                // appending to last line
+                val newRenderHeight = b[b.lastIndex].computeRenderedLinesCount(width, wordWrap)
+                scrollbackOffset += (newRenderHeight - lastLineRenderHeight)
+            } else {
+                scrollbackBottom += linesAfter - linesBefore
+            }
+
+            if (scrollbackBottom < 0) {
+                scrollbackBottom = 0
+                scrollbackOffset = 0
+            }
+        } finally {
+            renderer.finishUpdate()
         }
     }
 
@@ -460,3 +484,8 @@ internal fun FlavorableCharSequence.splitLinesInto(
         target.add(subSequence(start, end))
     }
 }
+
+private data class ScrollAdjustmentState(
+    val linesBefore: Int,
+    val lastLineRenderHeight: Int
+)