Synchronize Render thread access to FrameState

This fixes the occasional flickering issue

app/src/androidTest/java/com/gaurav/avnc/ui/vnc/FrameStateTest.kt

@@ -188,4 +188,24 @@ class FrameStateTest {
         assertEquals(-100f, state.frameX) //Right side of frame and Viewport are aligned
         assertEquals(-100f, state.frameY) //Bottom side of frame and Viewport are aligned
     }
+
+    //Sanity check for snapshot
+    @Test
+    fun snapshotTest() {
+        val state = FrameState()
+        state.setWindowSize(10f, 20f)
+        state.setViewportSize(10f, 20f)
+        state.setFramebufferSize(40f, 80f)
+        state.updateZoom(2f)
+        state.pan(-2f, -3f)
+
+        val snapshot = state.getSnapshot()
+        assertEquals(10f, snapshot.vpWidth)
+        assertEquals(20f, snapshot.vpHeight)
+        assertEquals(40f, snapshot.fbWidth)
+        assertEquals(80f, snapshot.fbHeight)
+        assertEquals(.5f, snapshot.scale)    // (10/40) * 2
+        assertEquals(-2f, snapshot.frameX)
+        assertEquals(-3f, snapshot.frameY)
+    }
 }

app/src/main/java/com/gaurav/avnc/ui/vnc/FrameState.kt

@@ -10,6 +10,7 @@ package com.gaurav.avnc.ui.vnc
 
 import android.graphics.PointF
 import android.graphics.RectF
+import com.gaurav.avnc.ui.vnc.FrameState.Snapshot
 import kotlin.math.max
 import kotlin.math.min
 
@@ -95,10 +96,13 @@ import kotlin.math.min
  * Thread safety
  * =============
  *
- * Frame state is accessed from two threads: Its properties are updated in
- * UI thread and consumed by the renderer thread. There is a "slight" chance that
- * Renderer thread may see half-updated state. But it should "eventually" settle down
- * because any change in frame state is usually followed by a new render request.
+ * Frame state is accessed from two threads: Its properties are updated in UI thread
+ * and consumed by the renderer thread. There is a chance that Renderer thread may see
+ * half-updated state (e.g. [frameX] is changed inside [pan] but [coerceValues] is not yet called).
+ * This half-updated state can cause flickering issues.
+ *
+ * To avoid this we use [Snapshot]. All updates to frame state are guarded by [lock].
+ * Render thread uses [getSnapshot] to retrieve a consistent state to render the frame.
  */
 class FrameState(
         private val minZoomScale: Float = 0.5F,
@@ -136,28 +140,43 @@ class FrameState(
 
     val scale get() = baseScale * zoomScale
 
-
-    fun setFramebufferSize(w: Float, h: Float) {
+    /**
+     * Immutable wrapper for frame state
+     */
+    data class Snapshot(
+            val frameX: Float,
+            val frameY: Float,
+            val fbWidth: Float,
+            val fbHeight: Float,
+            val vpWidth: Float,
+            val vpHeight: Float,
+            val scale: Float
+    )
+
+    private val lock = Any()
+    private inline fun <T> withLock(block: () -> T) = synchronized(lock) { block() }
+
+    fun setFramebufferSize(w: Float, h: Float) = withLock {
         fbWidth = w
         fbHeight = h
         calculateBaseScale()
         coerceValues()
     }
 
-    fun setViewportSize(w: Float, h: Float) {
+    fun setViewportSize(w: Float, h: Float) = withLock {
         vpWidth = w
         vpHeight = h
         coerceValues()
     }
 
-    fun setWindowSize(w: Float, h: Float) {
+    fun setWindowSize(w: Float, h: Float) = withLock {
         windowWidth = w
         windowHeight = h
         calculateBaseScale()
         coerceValues()
     }
 
-    fun setSafeArea(rect: RectF) {
+    fun setSafeArea(rect: RectF) = withLock {
         safeArea = RectF(rect)
         coerceValues()
     }
@@ -167,7 +186,7 @@ class FrameState(
      *
      * Returns 'how much' scale factor is actually applied (after coercing).
      */
-    fun updateZoom(scaleFactor: Float): Float {
+    fun updateZoom(scaleFactor: Float): Float = withLock {
         val oldScale = zoomScale
 
         zoomScale *= scaleFactor
@@ -176,7 +195,7 @@ class FrameState(
         return zoomScale / oldScale //Applied scale factor
     }
 
-    fun setZoom(zoom1: Float, zoom2: Float) {
+    fun setZoom(zoom1: Float, zoom2: Float) = withLock {
         zoomScale1 = zoom1
         zoomScale2 = zoom2
         coerceValues()
@@ -185,7 +204,7 @@ class FrameState(
     /**
      * Shift frame by given delta.
      */
-    fun pan(deltaX: Float, deltaY: Float) {
+    fun pan(deltaX: Float, deltaY: Float) = withLock {
         frameX += deltaX
         frameY += deltaY
         coerceValues()
@@ -194,7 +213,7 @@ class FrameState(
     /**
      * Move frame to given position.
      */
-    fun moveTo(x: Float, y: Float) {
+    fun moveTo(x: Float, y: Float) = withLock {
         frameX = x
         frameY = y
         coerceValues()
@@ -226,6 +245,14 @@ class FrameState(
         return PointF(fbPoint.x * scale + frameX, fbPoint.y * scale + frameY)
     }
 
+    /**
+     * Returns immutable & consistent snapshot of frame state.
+     */
+    fun getSnapshot(): Snapshot = withLock {
+        return Snapshot(frameX = frameX, frameY = frameY,
+                        fbWidth = fbWidth, fbHeight = fbHeight,
+                        vpWidth = vpWidth, vpHeight = vpHeight, scale = scale)
+    }
 
     private fun calculateBaseScale() {
         if (fbHeight == 0F || fbWidth == 0F || windowHeight == 0F)

app/src/main/java/com/gaurav/avnc/ui/vnc/gl/Renderer.kt

@@ -24,7 +24,6 @@ import javax.microedition.khronos.opengles.GL10
 class Renderer(val viewModel: VncViewModel) : GLSurfaceView.Renderer {
 
     private val projectionMatrix = FloatArray(16)
-    private val state = viewModel.frameState
     private val hideCursor = viewModel.pref.input.hideRemoteCursor
     private lateinit var program: FrameProgram
     private lateinit var frame: Frame
@@ -78,6 +77,7 @@ class Renderer(val viewModel: VncViewModel) : GLSurfaceView.Renderer {
         if (!viewModel.client.connected)
             return
 
+        val state = viewModel.frameState.getSnapshot()
         if (state.vpWidth == 0f || state.vpHeight == 0f)
             return