Improve performance of worldmap panning (#7034)

* Refactor: change CrashHandlingStage to UncivStage

* Add possibility to disable pointer enter exit events temporarily

* Disable pointer enter/exit events and TileGroupMap.act while panning

* Change ZoomableScrollPane to be self-contained and reduce coupling

core/src/com/unciv/ui/UncivStage.kt

@@ -1,34 +1,64 @@
-package com.unciv.ui.crashhandling
+package com.unciv.ui
 
+import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.graphics.g2d.Batch
 import com.badlogic.gdx.scenes.scene2d.Stage
 import com.badlogic.gdx.utils.viewport.Viewport
-import com.unciv.ui.utils.*
+import com.unciv.ui.utils.wrapCrashHandling
+import com.unciv.ui.utils.wrapCrashHandlingUnit
 
 
-/** Stage that safely brings the game to a [CrashScreen] if any event handlers throw an exception or an error that doesn't get otherwise handled. */
+/** Main stage for the game. Safely brings the game to a [CrashScreen] if any event handlers throw an exception or an error that doesn't get otherwise handled. */
-class CrashHandlingStage(viewport: Viewport, batch: Batch) : Stage(viewport, batch) {
+class UncivStage(viewport: Viewport, batch: Batch) : Stage(viewport, batch) {
 
-    override fun draw() = { super.draw() }.wrapCrashHandlingUnit()()
+    /**
-    override fun act() = { super.act() }.wrapCrashHandlingUnit()()
+     * Enables/disables sending pointer enter/exit events to actors on this stage.
-    override fun act(delta: Float) = { super.act(delta) }.wrapCrashHandlingUnit()()
+     * Checking for the enter/exit bounds is a relatively expensive operation and may thus be disabled temporarily.
+     */
+    var performPointerEnterExitEvents: Boolean = false
 
-    override fun touchDown(screenX: Int, screenY: Int, pointer: Int, button: Int)
+    override fun draw() =
-        = { super.touchDown(screenX, screenY, pointer, button) }.wrapCrashHandling()() ?: true
+        { super.draw() }.wrapCrashHandlingUnit()()
-    override fun touchDragged(screenX: Int, screenY: Int, pointer: Int)
+
-        = { super.touchDragged(screenX, screenY, pointer) }.wrapCrashHandling()() ?: true
+    /** libGDX has no built-in way to disable/enable pointer enter/exit events. It is simply being done in [Stage.act]. So to disable this, we have
-    override fun touchUp(screenX: Int, screenY: Int, pointer: Int, button: Int)
+     * to replicate the [Stage.act] method without the code for pointer enter/exit events. This is of course inherently brittle, but the only way. */
-        = { super.touchUp(screenX, screenY, pointer, button) }.wrapCrashHandling()() ?: true
+    override fun act() = {
-    override fun mouseMoved(screenX: Int, screenY: Int)
+        /** We're replicating [Stage.act], so this value is simply taken from there */
-        = { super.mouseMoved(screenX, screenY) }.wrapCrashHandling()() ?: true
+        val delta = Gdx.graphics.deltaTime.coerceAtMost(1 / 30f)
-    override fun scrolled(amountX: Float, amountY: Float)
+
-        = { super.scrolled(amountX, amountY) }.wrapCrashHandling()() ?: true
+        if (performPointerEnterExitEvents) {
-    override fun keyDown(keyCode: Int)
+            super.act(delta)
-        = { super.keyDown(keyCode) }.wrapCrashHandling()() ?: true
+        } else {
-    override fun keyUp(keyCode: Int)
+            root.act(delta)
-        = { super.keyUp(keyCode) }.wrapCrashHandling()() ?: true
+        }
-    override fun keyTyped(character: Char)
+    }.wrapCrashHandlingUnit()()
-        = { super.keyTyped(character) }.wrapCrashHandling()() ?: true
+
+    override fun act(delta: Float) =
+        { super.act(delta) }.wrapCrashHandlingUnit()()
+
+    override fun touchDown(screenX: Int, screenY: Int, pointer: Int, button: Int) =
+        { super.touchDown(screenX, screenY, pointer, button) }.wrapCrashHandling()() ?: true
+
+    override fun touchDragged(screenX: Int, screenY: Int, pointer: Int) =
+        { super.touchDragged(screenX, screenY, pointer) }.wrapCrashHandling()() ?: true
+
+    override fun touchUp(screenX: Int, screenY: Int, pointer: Int, button: Int) =
+        { super.touchUp(screenX, screenY, pointer, button) }.wrapCrashHandling()() ?: true
+
+    override fun mouseMoved(screenX: Int, screenY: Int) =
+        { super.mouseMoved(screenX, screenY) }.wrapCrashHandling()() ?: true
+
+    override fun scrolled(amountX: Float, amountY: Float) =
+        { super.scrolled(amountX, amountY) }.wrapCrashHandling()() ?: true
+
+    override fun keyDown(keyCode: Int) =
+        { super.keyDown(keyCode) }.wrapCrashHandling()() ?: true
+
+    override fun keyUp(keyCode: Int) =
+        { super.keyUp(keyCode) }.wrapCrashHandling()() ?: true
+
+    override fun keyTyped(character: Char) =
+        { super.keyTyped(character) }.wrapCrashHandling()() ?: true
 
 }
 

core/src/com/unciv/ui/cityscreen/CityScreen.kt

@@ -308,11 +308,9 @@ class CityScreen(
             }
         }
 
-        val tileMapGroup = TileGroupMap(tileGroups, stage.width / 2, stage.height / 2, tileGroupsToUnwrap = tilesToUnwrap)
+        val tileMapGroup = TileGroupMap(tileGroups, tileGroupsToUnwrap = tilesToUnwrap)
         mapScrollPane.actor = tileMapGroup
         mapScrollPane.setSize(stage.width, stage.height)
-        mapScrollPane.setOrigin(stage.width / 2, stage.height / 2)
-        mapScrollPane.center(stage)
         stage.addActor(mapScrollPane)
 
         mapScrollPane.layout() // center scrolling

core/src/com/unciv/ui/crashhandling/CrashScreen.kt

@@ -21,7 +21,7 @@ import kotlin.concurrent.thread
 
 /*
 Crashes are now handled from:
-- Event listeners, by [CrashHandlingStage].
+- Event listeners, by [UncivStage].
 - The main rendering loop, by [UncivGame.render].
 - Threads, by [crashHandlingThread].
 - Main loop runnables, by [postCrashHandlingRunnable].

core/src/com/unciv/ui/map/TileGroupMap.kt

@@ -22,11 +22,14 @@ import kotlin.math.min
  */
 class TileGroupMap<T: TileGroup>(
     tileGroups: Iterable<T>,
-    private val leftAndRightPadding: Float,
-    private val topAndBottomPadding: Float,
     worldWrap: Boolean = false,
     tileGroupsToUnwrap: Set<T>? = null
 ): Group() {
+    /** If the [act] method should be performed. If this is false, every child within this [TileGroupMap] will not get their [act] method called
+     * and thus not perform any [com.badlogic.gdx.scenes.scene2d.Action]s.
+     * Most children here already do not do anything in their [act] methods. However, even iterating through all of them */
+    var shouldAct = true
+
     private var topX = -Float.MAX_VALUE
     private var topY = -Float.MAX_VALUE
     private var bottomX = Float.MAX_VALUE
@@ -72,7 +75,7 @@ class TileGroupMap<T: TileGroup>(
         }
 
         for (group in tileGroups) {
-            group.moveBy(-bottomX + leftAndRightPadding, -bottomY + topAndBottomPadding)
+            group.moveBy(-bottomX, -bottomY)
         }
 
         if (worldWrap) {
@@ -81,11 +84,11 @@ class TileGroupMap<T: TileGroup>(
 
                 mirrorTiles.first.setPosition(positionalVector.x * 0.8f * groupSize.toFloat(),
                         positionalVector.y * 0.8f * groupSize.toFloat())
-                mirrorTiles.first.moveBy(-bottomX + leftAndRightPadding - bottomX * 2, -bottomY + topAndBottomPadding)
+                mirrorTiles.first.moveBy(-bottomX - bottomX * 2, -bottomY )
 
                 mirrorTiles.second.setPosition(positionalVector.x * 0.8f * groupSize.toFloat(),
                         positionalVector.y * 0.8f * groupSize.toFloat())
-                mirrorTiles.second.moveBy(-bottomX + leftAndRightPadding + bottomX * 2, -bottomY + topAndBottomPadding)
+                mirrorTiles.second.moveBy(-bottomX + bottomX * 2, -bottomY)
             }
         }
 
@@ -146,8 +149,8 @@ class TileGroupMap<T: TileGroup>(
         // Map's width is reduced by groupSize if it is wrapped, because wrapped map will miss a tile on the right.
         // This ensures that wrapped maps have a smooth transition.
         // If map is not wrapped, Map's width doesn't need to be reduce by groupSize
-        if (worldWrap) setSize(topX - bottomX + leftAndRightPadding * 2 - groupSize, topY - bottomY + topAndBottomPadding * 2)
+        if (worldWrap) setSize(topX - bottomX - groupSize, topY - bottomY)
-        else setSize(topX - bottomX + leftAndRightPadding * 2, topY - bottomY + topAndBottomPadding * 2)
+        else setSize(topX - bottomX, topY - bottomY)
     }
 
     /**
@@ -155,7 +158,7 @@ class TileGroupMap<T: TileGroup>(
      */
     fun getPositionalVector(stageCoords: Vector2): Vector2 {
         val trueGroupSize = 0.8f * groupSize.toFloat()
-        return Vector2(bottomX - leftAndRightPadding, bottomY - topAndBottomPadding)
+        return Vector2(bottomX, bottomY)
                 .add(stageCoords)
                 .sub(groupSize.toFloat() / 2f, groupSize.toFloat() / 2f)
                 .scl(1f / trueGroupSize)
@@ -166,5 +169,9 @@ class TileGroupMap<T: TileGroup>(
     // For debugging purposes
      override fun draw(batch: Batch?, parentAlpha: Float) { super.draw(batch, parentAlpha) }
      @Suppress("RedundantOverride")
-     override fun act(delta: Float) { super.act(delta) }
+     override fun act(delta: Float) {
+         if(shouldAct) {
+             super.act(delta)
+         }
+     }
 }

core/src/com/unciv/ui/mapeditor/EditorMapHolder.kt

@@ -24,7 +24,7 @@ class EditorMapHolder(
     parentScreen: BaseScreen,
     internal val tileMap: TileMap,
     private val onTileClick: (TileInfo) -> Unit
-): ZoomableScrollPane() {
+): ZoomableScrollPane(20f, 20f) {
     val editorScreen = parentScreen as? MapEditorScreen
 
     val tileGroups = HashMap<TileInfo, List<TileGroup>>()
@@ -32,7 +32,6 @@ class EditorMapHolder(
     private val allTileGroups = ArrayList<TileGroup>()
 
     private val maxWorldZoomOut = UncivGame.Current.settings.maxWorldZoomOut
-    private val minZoomScale = 1f / maxWorldZoomOut
 
     private var blinkAction: Action? = null
 
@@ -53,8 +52,6 @@ class EditorMapHolder(
 
         tileGroupMap = TileGroupMap(
             daTileGroups,
-            stage.width * maxWorldZoomOut / 2,
-            stage.height * maxWorldZoomOut / 2,
             continuousScrollingX)
         actor = tileGroupMap
         val mirrorTileGroups = tileGroupMap.getMirrorTiles()
@@ -140,11 +137,6 @@ class EditorMapHolder(
         addAction(blinkAction) // Don't set it on the group because it's an actionless group
     }
 
-    override fun zoom(zoomScale: Float) {
-        if (zoomScale < minZoomScale || zoomScale > 2f) return
-        setScale(zoomScale)
-    }
-
     /*
     The ScrollPane interferes with the dragging listener of MapEditorToolsDrawer.
     Once the ZoomableScrollPane super is initialized, there are 3 listeners + 1 capture listener:
@@ -195,7 +187,7 @@ class EditorMapHolder(
                 if (!isPainting) return
 
                 editorScreen!!.hideSelection()
-                val stageCoords = actor.stageToLocalCoordinates(Vector2(event!!.stageX, event.stageY))
+                val stageCoords = actor?.stageToLocalCoordinates(Vector2(event!!.stageX, event.stageY)) ?: return
                 val centerTileInfo = getClosestTileTo(stageCoords)
                     ?: return
                 editorScreen.tabs.edit.paintTilesWithBrush(centerTileInfo)

core/src/com/unciv/ui/utils/BaseScreen.kt

@@ -10,9 +10,9 @@ import com.badlogic.gdx.scenes.scene2d.Stage
 import com.badlogic.gdx.scenes.scene2d.ui.*
 import com.badlogic.gdx.scenes.scene2d.utils.Drawable
 import com.badlogic.gdx.utils.viewport.ExtendViewport
-import com.unciv.ui.crashhandling.CrashHandlingStage
 import com.unciv.UncivGame
 import com.unciv.models.Tutorial
+import com.unciv.ui.UncivStage
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popup.hasOpenPopups
 import com.unciv.ui.tutorials.TutorialController
@@ -32,7 +32,7 @@ abstract class BaseScreen : Screen {
         val height = resolutions[1]
 
         /** The ExtendViewport sets the _minimum_(!) world size - the actual world size will be larger, fitted to screen/window aspect ratio. */
-        stage = CrashHandlingStage(ExtendViewport(height, height), SpriteBatch())
+        stage = UncivStage(ExtendViewport(height, height), SpriteBatch())
 
         if (enableSceneDebug) {
             stage.setDebugUnderMouse(true)

core/src/com/unciv/ui/utils/ZoomableScrollPane.kt

@@ -1,22 +1,122 @@
 package com.unciv.ui.utils
 
+import com.badlogic.gdx.math.Rectangle
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.Group
 import com.badlogic.gdx.scenes.scene2d.InputEvent
 import com.badlogic.gdx.scenes.scene2d.InputListener
 import com.badlogic.gdx.scenes.scene2d.ui.ScrollPane
 import com.badlogic.gdx.scenes.scene2d.utils.ActorGestureListener
+import com.badlogic.gdx.scenes.scene2d.utils.Cullable
 import kotlin.math.sqrt
 
 
-open class ZoomableScrollPane : ScrollPane(null) {
+open class ZoomableScrollPane(
+    val extraCullingX: Float = 0f,
+    val extraCullingY: Float = 0f,
+    var minZoom: Float = 0.5f,
+    var maxZoom: Float = 1 / minZoom // if we can halve the size, then by default also allow to double it
+) : ScrollPane(null) {
     var continuousScrollingX = false
 
-    init{
+    var onViewportChangedListener: ((width: Float, height: Float, viewport: Rectangle) -> Unit)? = null
+    var onPanStopListener: (() -> Unit)? = null
+    var onPanStartListener: (() -> Unit)? = null
+
+    /**
+     * Exists so that we are always able to set the center to the edge of the contained actor.
+     * Otherwise, the [ScrollPane] would always stop at the actor's edge, keeping the center always ([width or height]/2) away from the edge.
+     * This is lateinit because unfortunately [ScrollPane] uses [setActor] in its constructor, and we override [setActor], so paddingGroup has not been
+     * constructed at that moment, throwing a NPE.
+     */
+    @Suppress("UNNECESSARY_LATEINIT")
+    private lateinit var paddingGroup: Group
+
+    private val horizontalPadding get() = width / 2
+    private val verticalPadding get() = height / 2
+
+    init {
+        paddingGroup = Group()
+        super.setActor(paddingGroup)
+
         addZoomListeners()
     }
 
+    override fun setActor(actor: Actor?) {
+        if (!this::paddingGroup.isInitialized) return
+        paddingGroup.clearChildren()
+        paddingGroup.addActor(actor)
+    }
+
+    override fun getActor(): Actor? {
+        if (!this::paddingGroup.isInitialized || !paddingGroup.hasChildren()) return null
+        return paddingGroup.children[0]
+    }
+
+    override fun scrollX(pixelsX: Float) {
+        super.scrollX(pixelsX)
+        updateCulling()
+        onViewportChanged()
+    }
+
+    override fun scrollY(pixelsY: Float) {
+        super.scrollY(pixelsY)
+        updateCulling()
+        onViewportChanged()
+    }
+
+    override fun sizeChanged() {
+        updatePadding()
+        super.sizeChanged()
+        updateCulling()
+    }
+
+    private fun updatePadding() {
+        val content = actor
+        if (content == null) return
+        // Padding is always [dimension / 2] because we want to be able to have the center of the scrollPane at the very edge of the content
+        content.x = horizontalPadding
+        paddingGroup.width = content.width + horizontalPadding * 2
+        content.y = verticalPadding
+        paddingGroup.height = content.height + verticalPadding * 2
+    }
+
+    fun updateCulling() {
+        val content = actor
+        if (content !is Cullable) return
+
+        fun Rectangle.addInAllDirections(xDirectionIncrease: Float, yDirectionIncrease: Float): Rectangle {
+            x -= xDirectionIncrease
+            y -= yDirectionIncrease
+            width += xDirectionIncrease * 2
+            height += yDirectionIncrease * 2
+            return this
+        }
+
+        content.setCullingArea(
+            getViewport().addInAllDirections(extraCullingX, extraCullingY)
+        )
+    }
+
     open fun zoom(zoomScale: Float) {
-        if (zoomScale < 0.5f || zoomScale > 2f) return
+        if (zoomScale < minZoom || zoomScale > maxZoom) return
+
+        val previousScaleX = scaleX
+        val previousScaleY = scaleY
+
         setScale(zoomScale)
+
+        // When we scale, the width & height values stay the same. However, after scaling up/down, the width will be rendered wider/narrower than before.
+        // But we want to keep the size of the pane the same, so we do need to adjust the width & height: smaller if the scale increased, larger if it decreased.
+        val newWidth = width * previousScaleX / zoomScale
+        val newHeight = height * previousScaleY / zoomScale
+        setSize(newWidth, newHeight)
+
+        onViewportChanged()
+        // The size increase/decrease kept scrollX and scrollY (i.e. the top edge and left edge) the same - but changing the scale & size should have changed
+        // where the right and bottom edges are. This would mean our visual center moved. To correct this, we theoretically need to update the scroll position
+        // by half (i.e. middle) of what our size changed.
+        // However, we also changed the padding, which is exactly equal to half of our size change, so we actually don't need to move our center at all.
     }
     fun zoomIn() {
         zoom(scaleX / 0.8f)
@@ -57,7 +157,12 @@ open class ZoomableScrollPane : ScrollPane(null) {
         //This is mostly just Java code from the ScrollPane class reimplemented as Kotlin code
         //Had to change a few things to bypass private access modifiers
         return object : ActorGestureListener() {
+            private var wasPanning = false
             override fun pan(event: InputEvent, x: Float, y: Float, deltaX: Float, deltaY: Float) {
+                if (!wasPanning) {
+                    wasPanning = true
+                    onPanStartListener?.invoke()
+                }
                 setScrollbarsVisible(true)
                 scrollX -= deltaX
                 scrollY += deltaY
@@ -76,6 +181,27 @@ open class ZoomableScrollPane : ScrollPane(null) {
 
                 if ((isScrollX && deltaX != 0f || isScrollY && deltaY != 0f)) cancelTouchFocus()
             }
+
+            override fun panStop(event: InputEvent?, x: Float, y: Float, pointer: Int, button: Int) {
+                wasPanning = false
+                onPanStopListener?.invoke()
+            }
         }
     }
+
+    /** @return the currently scrolled-to viewport of the whole scrollable area */
+    fun getViewport(): Rectangle {
+        val viewportFromLeft = scrollX
+        /** In the default coordinate system, the y origin is at the bottom, but scrollY is from the top, so we need to invert. */
+        val viewportFromBottom = maxY - scrollY
+        return Rectangle(
+            viewportFromLeft - horizontalPadding,
+            viewportFromBottom - verticalPadding,
+            width,
+            height)
+    }
+
+    private fun onViewportChanged() {
+        onViewportChangedListener?.invoke(maxX, maxY, getViewport())
+    }
 }

core/src/com/unciv/ui/worldscreen/WorldMapHolder.kt

@@ -7,7 +7,11 @@ import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.graphics.g2d.Batch
 import com.badlogic.gdx.math.Interpolation
 import com.badlogic.gdx.math.Vector2
-import com.badlogic.gdx.scenes.scene2d.*
+import com.badlogic.gdx.scenes.scene2d.Action
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.Group
+import com.badlogic.gdx.scenes.scene2d.InputEvent
+import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.actions.Actions
 import com.badlogic.gdx.scenes.scene2d.actions.FloatAction
 import com.badlogic.gdx.scenes.scene2d.ui.Table
@@ -21,10 +25,14 @@ import com.unciv.logic.battle.Battle
 import com.unciv.logic.battle.MapUnitCombatant
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.civilization.CivilizationInfo
-import com.unciv.logic.map.*
+import com.unciv.logic.map.MapUnit
-import com.unciv.models.*
+import com.unciv.logic.map.TileInfo
+import com.unciv.logic.map.TileMap
+import com.unciv.models.AttackableTile
+import com.unciv.models.UncivSound
 import com.unciv.models.helpers.MapArrowType
 import com.unciv.models.helpers.MiscArrowTypes
+import com.unciv.ui.UncivStage
 import com.unciv.ui.audio.Sounds
 import com.unciv.ui.crashhandling.launchCrashHandling
 import com.unciv.ui.crashhandling.postCrashHandlingRunnable
@@ -33,11 +41,21 @@ import com.unciv.ui.map.TileGroupMap
 import com.unciv.ui.tilegroups.TileGroup
 import com.unciv.ui.tilegroups.TileSetStrings
 import com.unciv.ui.tilegroups.WorldTileGroup
-import com.unciv.ui.utils.*
+import com.unciv.ui.utils.UnitGroup
+import com.unciv.ui.utils.ZoomableScrollPane
+import com.unciv.ui.utils.center
+import com.unciv.ui.utils.colorFromRGB
+import com.unciv.ui.utils.darken
+import com.unciv.ui.utils.onClick
+import com.unciv.ui.utils.surroundWithCircle
+import com.unciv.ui.utils.toLabel
 import com.unciv.utils.Log
 
 
-class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap: TileMap): ZoomableScrollPane() {
+class WorldMapHolder(
+    internal val worldScreen: WorldScreen,
+    internal val tileMap: TileMap
+) : ZoomableScrollPane(20f, 20f) {
     internal var selectedTile: TileInfo? = null
     val tileGroups = HashMap<TileInfo, List<WorldTileGroup>>()
 
@@ -50,19 +68,40 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
 
     private val unitMovementPaths: HashMap<MapUnit, ArrayList<TileInfo>> = HashMap()
 
-    private var maxWorldZoomOut = 2f
+    private lateinit var tileGroupMap: TileGroupMap<WorldTileGroup>
-    private var minZoomScale = 0.5f
 
     init {
         if (Gdx.app.type == Application.ApplicationType.Desktop) this.setFlingTime(0f)
         continuousScrollingX = tileMap.mapParameters.worldWrap
         reloadMaxZoom()
+        disablePointerEventsAndActionsOnPan()
+    }
+
+    /**
+     * When scrolling the world map, there are two unnecessary (at least currently) things happening that take a decent amount of time:
+     *
+     * 1. Checking which [Actor]'s bounds the pointer (mouse/finger) entered+exited and sending appropriate events to these actors
+     * 2. Running all [Actor.act] methods of all child [Actor]s
+     *
+     * Disabling them while panning increases the frame rate while panning by approximately 100%.
+     */
+    private fun disablePointerEventsAndActionsOnPan() {
+        onPanStartListener = {
+            Log.debug("Disable pointer enter/exit events & TileGroupMap.act()")
+            (stage as UncivStage).performPointerEnterExitEvents = false
+            tileGroupMap.shouldAct = false
+        }
+        onPanStopListener = {
+            Log.debug("Enable pointer enter/exit events & TileGroupMap.act()")
+            (stage as UncivStage).performPointerEnterExitEvents = true
+            tileGroupMap.shouldAct = true
+        }
     }
 
     internal fun reloadMaxZoom() {
-        maxWorldZoomOut = UncivGame.Current.settings.maxWorldZoomOut
+        maxZoom = UncivGame.Current.settings.maxWorldZoomOut
-        minZoomScale = 1f / maxWorldZoomOut
+        minZoom = 1f / maxZoom
-        if (scaleX < minZoomScale) zoom(1f)   // since normally min isn't reached exactly, only powers of 0.8
+        if (scaleX < minZoom) zoom(1f)   // since normally min isn't reached exactly, only powers of 0.8
     }
 
     // Interface for classes that contain the data required to draw a button
@@ -75,10 +114,8 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
     internal fun addTiles() {
         val tileSetStrings = TileSetStrings()
         val daTileGroups = tileMap.values.map { WorldTileGroup(worldScreen, it, tileSetStrings) }
-        val tileGroupMap = TileGroupMap(
+        tileGroupMap = TileGroupMap(
             daTileGroups,
-            worldScreen.stage.width * maxWorldZoomOut / 2,
-            worldScreen.stage.height * maxWorldZoomOut / 2,
             continuousScrollingX)
         val mirrorTileGroups = tileGroupMap.getMirrorTiles()
 
@@ -125,12 +162,9 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
 
         actor = tileGroupMap
 
-        setSize(worldScreen.stage.width * maxWorldZoomOut, worldScreen.stage.height * maxWorldZoomOut)
+        setSize(worldScreen.stage.width, worldScreen.stage.height)
-        setOrigin(width / 2, height / 2)
-        center(worldScreen.stage)
 
         layout() // Fit the scroll pane to the contents - otherwise, setScroll won't work!
-
     }
 
     private fun onTileClicked(tileInfo: TileInfo) {
@@ -684,12 +718,10 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
         val originalScrollX = scrollX
         val originalScrollY = scrollY
 
-        // We want to center on the middle of the TileGroup (TG.getX()+TG.getWidth()/2)
+        val finalScrollX = tileGroup.x + tileGroup.width / 2
-        // and so the scroll position (== filter the screen starts) needs to be half the ScrollMap away
-        val finalScrollX = tileGroup.x + tileGroup.width / 2 - width / 2
 
-        // Here it's the same, only the Y axis is inverted - when at 0 we're at the top, not bottom - so we invert it back.
+        /** The Y axis of [scrollY] is inverted - when at 0 we're at the top, not bottom - so we invert it back. */
-        val finalScrollY = maxY - (tileGroup.y + tileGroup.width / 2 - height / 2)
+        val finalScrollY = maxY - (tileGroup.y + tileGroup.width / 2)
 
         if (finalScrollX == originalScrollX && finalScrollY == originalScrollY) return false
 
@@ -723,20 +755,29 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
     }
 
     override fun zoom(zoomScale: Float) {
-        if (zoomScale < minZoomScale || zoomScale > 2f) return
+        super.zoom(zoomScale)
-        setScale(zoomScale)
+
-        val scale = 1 / scaleX  // don't use zoomScale itself, in case it was out of bounds and not applied
+        clampCityButtonSize()
-        if (scale >= 1)
+    }
-            for (tileGroup in allWorldTileGroups)
+
+    /** We don't want the city buttons becoming too large when zooming out */
+    private fun clampCityButtonSize() {
+        // use scaleX instead of zoomScale itself, because zoomScale might have been outside minZoom..maxZoom and thus not applied
+        val clampedCityButtonZoom = 1 / scaleX
+        if (clampedCityButtonZoom >= 1) {
+            for (tileGroup in allWorldTileGroups) {
                 tileGroup.cityButtonLayerGroup.isTransform = false // to save on rendering time to improve framerate
-        if (scale < 1 && scale >= minZoomScale)
+            }
+        }
+        if (clampedCityButtonZoom < 1 && clampedCityButtonZoom >= minZoom) {
             for (tileGroup in allWorldTileGroups) {
                 // ONLY set those groups that have active city buttons as transformable!
                 // This is massively framerate-improving!
                 if (tileGroup.cityButtonLayerGroup.hasChildren())
                     tileGroup.cityButtonLayerGroup.isTransform = true
-                tileGroup.cityButtonLayerGroup.setScale(scale)
+                tileGroup.cityButtonLayerGroup.setScale(clampedCityButtonZoom)
             }
+        }
     }
 
     fun removeUnitActionOverlay() {

core/src/com/unciv/ui/worldscreen/WorldScreen.kt

@@ -615,10 +615,12 @@ class WorldScreen(val gameInfo: GameInfo, val viewingCiv:CivilizationInfo) : Bas
 
         // This is not the case if you have a multiplayer game where you play as 2 civs
         if (newWorldScreen.viewingCiv.civName == viewingCiv.civName) {
-            newWorldScreen.mapHolder.scrollX = mapHolder.scrollX
+            newWorldScreen.mapHolder.width = mapHolder.width
-            newWorldScreen.mapHolder.scrollY = mapHolder.scrollY
+            newWorldScreen.mapHolder.height = mapHolder.height
             newWorldScreen.mapHolder.scaleX = mapHolder.scaleX
             newWorldScreen.mapHolder.scaleY = mapHolder.scaleY
+            newWorldScreen.mapHolder.scrollX = mapHolder.scrollX
+            newWorldScreen.mapHolder.scrollY = mapHolder.scrollY
             newWorldScreen.mapHolder.updateVisualScroll()
         }
 
@@ -850,7 +852,6 @@ class WorldScreen(val gameInfo: GameInfo, val viewingCiv:CivilizationInfo) : Bas
         }
 //        topBar.selectedCivLabel.setText(Gdx.graphics.framesPerSecond) // for framerate testing
 
-        minimapWrapper.minimap.updateScrollPosition()
 
         super.render(delta)
     }

core/src/com/unciv/ui/worldscreen/minimap/Minimap.kt

@@ -10,7 +10,6 @@ import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.map.MapShape
 import com.unciv.logic.map.MapSize
 import com.unciv.ui.images.ClippingImage
-import com.unciv.ui.utils.*
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.worldscreen.WorldMapHolder
 import kotlin.math.max
@@ -56,6 +55,8 @@ class Minimap(val mapHolder: WorldMapHolder, minimapSize: Int) : Group() {
 
         setSize(tileLayer.width, tileLayer.height)
         addActor(tileLayer)
+
+        mapHolder.onViewportChangedListener = ::updateScrollPosition
     }
 
     private fun calcTileSize(minimapSize: Int): Float {
@@ -96,30 +97,20 @@ class Minimap(val mapHolder: WorldMapHolder, minimapSize: Int) : Group() {
     }
 
     /**### Transform and set coordinates for the scrollPositionIndicator.
-     *
-     *  Relies on the [MiniMap][MinimapHolder.minimap]'s copy of the main [WorldMapHolder] as input.
      *
      *  Requires [scrollPositionIndicator] to be a [ClippingImage] to keep the displayed portion of the indicator within the bounds of the minimap.
      */
-    fun updateScrollPosition() {
+    private fun updateScrollPosition(worldWidth: Float, worldHeight: Float, worldViewport: Rectangle) {
-        // Only mapHolder.scrollX/Y and mapHolder.scaleX/Y change. scrollX/Y will range from 0 to mapHolder.maxX/Y,
-        // with all extremes centering the corresponding map edge on screen. Y axis is 0 top, maxY bottom.
-        // Visible area relative to this coordinate system seems to be mapHolder.width/2 * mapHolder.height/2.
-        // Minimap coordinates are measured from the allTiles Group, which is a bounding box over the entire map, and (0,0) @ lower left.
-
-        // Helpers for readability - each single use, but they should help explain the logic
         operator fun Rectangle.times(other: Vector2) = Rectangle(x * other.x, y * other.y, width * other.x, height * other.y)
-
-        fun Vector2.centeredRectangle(size: Vector2) = Rectangle(x - size.x / 2, y - size.y / 2, size.x, size.y)
-        fun Rectangle.invertY(max: Float) = Rectangle(x, max - height - y, width, height)
         fun Actor.setViewport(rect: Rectangle) {
-            x = rect.x; y = rect.y; width = rect.width; height = rect.height
+            x = rect.x;
+            y = rect.y;
+            width = rect.width;
+            height = rect.height
         }
 
-        val worldToMiniFactor = Vector2(tileLayer.width / mapHolder.maxX, tileLayer.height / mapHolder.maxY)
+        val worldToMiniFactor = Vector2(tileLayer.width / worldWidth, tileLayer.height / worldHeight)
-        val worldVisibleArea = Vector2(mapHolder.width / 2 / mapHolder.scaleX, mapHolder.height / 2 / mapHolder.scaleY)
+        val miniViewport = worldViewport * worldToMiniFactor
-        val worldViewport = Vector2(mapHolder.scrollX, mapHolder.scrollY).centeredRectangle(worldVisibleArea)
-        val miniViewport = worldViewport.invertY(mapHolder.maxY) * worldToMiniFactor
         // This _could_ place parts of the 'camera' icon outside the minimap if it were a standard Image, thus the ClippingImage helper class
         scrollPositionIndicators[0].setViewport(miniViewport)