Optimization: only rebuild the grid view and its associated cells if the grid changes structurally, otherwise only rebind the cells.

netomi · netomi · commit 889256d14182 · 2020-09-26T22:56:42.000+02:00
diff --git a/desktop/src/main/kotlin/com/github/netomi/sudoku/trainer/view/CellEditView.kt b/desktop/src/main/kotlin/com/github/netomi/sudoku/trainer/view/CellEditView.kt
@@ -59,7 +59,10 @@ class CellEditView : View()
             }
         }
 
-    private fun rebuildViewFromModel() {
+    private fun rebuildViewFromModel(oldGrid: Grid?, newGrid: Grid) {
+        // if the grid size has not changed, we dont have to change anything
+        if (oldGrid?.gridSize == newGrid.gridSize) return
+
         valuesPane.children.clear()
         valuesPane.rowConstraints.clear()
         valuesPane.columnConstraints.clear()
@@ -188,7 +191,8 @@ class CellEditView : View()
 
     init {
         // rebuild the view when the model has changed
-        gridProperty.onChange { rebuildViewFromModel() }
+        gridProperty.addListener(ChangeListener { _, oldGrid: Grid?, newGrid: Grid -> rebuildViewFromModel(oldGrid, newGrid) })
+
         cellProperty.onChange {
             valuesPane.children.forEach { it.isDisable = true }
             refreshView()
diff --git a/desktop/src/main/kotlin/com/github/netomi/sudoku/trainer/view/CellFragment.kt b/desktop/src/main/kotlin/com/github/netomi/sudoku/trainer/view/CellFragment.kt
@@ -23,8 +23,6 @@
 
 package com.github.netomi.sudoku.trainer.view
 
-import javafx.beans.property.IntegerProperty
-import javafx.beans.property.SimpleIntegerProperty
 import javafx.collections.FXCollections
 import javafx.collections.ObservableIntegerArray
 import javafx.event.EventHandler
@@ -45,8 +43,7 @@ import com.github.netomi.sudoku.solver.LinkType.WEAK
 import com.github.netomi.sudoku.trainer.Styles
 import com.github.netomi.sudoku.trainer.model.DisplayOptions
 import com.github.netomi.sudoku.trainer.pseudoClass
-import javafx.beans.property.BooleanProperty
-import javafx.beans.property.SimpleBooleanProperty
+import javafx.beans.property.*
 import javafx.css.PseudoClass
 import javafx.geometry.Insets
 import javafx.scene.layout.*
@@ -56,8 +53,11 @@ import kotlin.math.sign
 /**
  * The view to display the state of an individual cell within a sudoku grid.
  */
-class CellFragment(private val cell: Cell) : Fragment()
+class CellFragment(cellArgument: Cell) : Fragment()
 {
+    private val cellProperty: ObjectProperty<Cell> = SimpleObjectProperty(cellArgument)
+    var cell: Cell by cellProperty
+
     private val valueProperty: IntegerProperty = SimpleIntegerProperty(0)
     private var value by valueProperty
 
@@ -66,12 +66,77 @@ class CellFragment(private val cell: Cell) : Fragment()
     val selectedProperty: BooleanProperty = SimpleBooleanProperty(false)
     var selected by selectedProperty
 
-    private val candidatesPane: GridPane
-    private val assignedValueLabel: Label
+    private var candidatesPane     by singleAssign<GridPane>()
+    private var assignedValueLabel by singleAssign<Label>()
     // a simple pane to easily indicate the currently focused cell.
-    private val selectPane: Pane
+    private var selectPane         by singleAssign<Pane>()
+
+    override val root = stackpane {
+        addClass(Styles.sudokuCell)
+
+        selectPane = pane {
+            useMaxSize = true
+            id = Styles.cellSelectPane.name
+        }
+
+        setMinSize(30.0, 30.0)
+        useMaxSize = true
+
+        val rows = when (cell.owner.gridSize) {
+            4    -> 2
+            6    -> 2
+            9    -> 3
+            else -> kotlin.error("unexpected grid size ${cell.owner.gridSize}")
+        }
+        val cols = cell.owner.gridSize / rows
+
+        val percentageRow = 100.0 / rows
+        val percentageCol = 100.0 / cols
+
+        candidatesPane = gridpane {
+            var possibleValue = 1
+            for (i in 0 until rows) {
+                for (j in 0 until cols) {
+                    label {
+                        useMaxHeight = true
+                        maxWidth     = 36.0
+
+                        id = Styles.cellCandidate.name
+                        text = possibleValue.toString()
+
+                        gridpaneConstraints {
+                            margin = Insets(3.0)
+                            columnRowIndex(j, i)
+                        }
+                    }
+                    possibleValue++
+                }
+            }
 
-    override val root = stackpane {}
+            for (i in 0 until rows) {
+                val row = RowConstraints(10.0, 20.0, Double.MAX_VALUE)
+                row.vgrow         = Priority.ALWAYS
+                row.valignment    = VPos.CENTER
+                row.percentHeight = percentageRow
+                rowConstraints.add(row)
+            }
+
+            for (i in 0 until cols) {
+                val col = ColumnConstraints(10.0, 20.0, Double.MAX_VALUE)
+                col.hgrow        = Priority.ALWAYS
+                col.halignment   = HPos.CENTER
+                col.percentWidth = percentageCol
+                columnConstraints.add(col)
+            }
+        }
+
+        assignedValueLabel = label {
+            id = Styles.cellValue.name
+            isVisible = false
+
+            textProperty().bind(valueProperty.asString())
+        }
+    }
 
     fun refreshView(conflicts: Array<Conflict>, displayedHint: Hint?) {
         assignedValueLabel.apply {
@@ -112,6 +177,12 @@ class CellFragment(private val cell: Cell) : Fragment()
         }
     }
 
+    fun resetView() {
+        selected = false
+        value    = 0
+        possibleValuesProperty.clear()
+    }
+
     private fun processHint(hint: Hint?): Array<CandidateState> {
         val result = Array(cell.owner.gridSize + 1) { _ -> CandidateState.NONE }
 
@@ -314,74 +385,9 @@ class CellFragment(private val cell: Cell) : Fragment()
     }
 
     init {
-        with(root) {
-            addClass(Styles.sudokuCell)
-
-            selectPane = pane {
-                useMaxSize = true
-                id = Styles.cellSelectPane.name
-            }
-
-            style += getBorderStyle(cell)
-
-            setMinSize(30.0, 30.0)
-            useMaxSize = true
-
-            val rows = when (cell.owner.gridSize) {
-                4    -> 2
-                6    -> 2
-                9    -> 3
-                else -> kotlin.error("unexpected grid size $cell.owner.gridSize")
-            }
-            val cols = cell.owner.gridSize / rows
-
-            val percentageRow = 100.0 / rows
-            val percentageCol = 100.0 / cols
-
-            candidatesPane = gridpane {
-                var possibleValue = 1
-                for (i in 0 until rows) {
-                    for (j in 0 until cols) {
-                        label {
-                            useMaxHeight = true
-                            maxWidth     = 36.0
-
-                            id = Styles.cellCandidate.name
-                            text = possibleValue.toString()
-
-                            gridpaneConstraints {
-                                margin = Insets(3.0)
-                                columnRowIndex(j, i)
-                            }
-                        }
-                        possibleValue++
-                    }
-                }
-
-                for (i in 0 until rows) {
-                    val row = RowConstraints(10.0, 20.0, Double.MAX_VALUE)
-                    row.vgrow         = Priority.ALWAYS
-                    row.valignment    = VPos.CENTER
-                    row.percentHeight = percentageRow
-                    rowConstraints.add(row)
-                }
-
-                for (i in 0 until cols) {
-                    val col = ColumnConstraints(10.0, 20.0, Double.MAX_VALUE)
-                    col.hgrow        = Priority.ALWAYS
-                    col.halignment   = HPos.CENTER
-                    col.percentWidth = percentageCol
-                    columnConstraints.add(col)
-                }
-            }
-
-            assignedValueLabel = label {
-                id = Styles.cellValue.name
-                isVisible = false
-            }
-        }
-
-        assignedValueLabel.textProperty().bind(valueProperty.asString())
+        val updateStyle: (Cell) -> Unit = { root.style = getBorderStyle(it) }
+        cellProperty.onChange { it?.let(updateStyle) }
+        updateStyle.invoke(cell)
 
         valueProperty.onChange { newValue ->
             candidatesPane.isVisible     = newValue == 0
diff --git a/desktop/src/main/kotlin/com/github/netomi/sudoku/trainer/view/GridView.kt b/desktop/src/main/kotlin/com/github/netomi/sudoku/trainer/view/GridView.kt
@@ -52,7 +52,10 @@ class GridView : View()
     private val cellFragmentList = ArrayList<CellFragment>()
 
     private val selectedCellFragmentProperty: ObjectProperty<CellFragment?> = SimpleObjectProperty()
+    private var selectedCellFragment: CellFragment? by selectedCellFragmentProperty
+
     private val selectedCellProperty: ObjectProperty<Cell?> = SimpleObjectProperty()
+    private var selectedCell: Cell? by selectedCellProperty
 
     override val root =
         borderpane {
@@ -76,7 +79,22 @@ class GridView : View()
             bottom = cellEditView.root
         }
 
-    private fun rebuildViewFromModel() {
+    private fun rebuildViewFromModel(oldGrid: Grid?, newGrid: Grid) {
+        selectedCellFragment = null
+        selectedCell = null
+
+        if (oldGrid?.type == newGrid.type && cellFragmentList.isNotEmpty()) {
+            newGrid.cells.forEachIndexed { index, cell ->
+                cellFragmentList[index].apply {
+                    this.cell = cell
+                    this.resetView()
+                }
+            }
+            newGrid.onUpdate { refreshView() }
+            refreshView()
+            return
+        }
+
         gridPane.children.clear()
         gridPane.rowConstraints.clear()
         gridPane.columnConstraints.clear()
@@ -94,9 +112,9 @@ class GridView : View()
 
                 cellFragment.selectedProperty.onChange { selected ->
                     if (selected) {
-                        selectedCellFragmentProperty.get()?.selected = false
-                        selectedCellFragmentProperty.set(cellFragment)
-                        selectedCellProperty.set(cell)
+                        selectedCellFragment?.selected = false
+                        selectedCellFragment = cellFragment
+                        selectedCell = cellFragment.cell
                     }
                 }
             }
@@ -157,7 +175,7 @@ class GridView : View()
     }
 
     init {
-        gridController.gridProperty.onChange { rebuildViewFromModel() }
+        gridController.gridProperty.addListener(ChangeListener { _, oldGrid: Grid?, newGrid: Grid -> rebuildViewFromModel(oldGrid, newGrid) })
 
         // bind the current model and selected cell to the cell edit view
         cellEditView.gridProperty.bind(gridController.gridProperty)
