Fix FullscreenAdapter to be independent of the order of its listener …

…calls (#689)
JetBrains · Apr 19, 2023 · 8b73b67 · 8b73b67
1 parent 7cdd091
commit 8b73b67
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 16 deletions.
diff --git a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/PlatformOperations.kt b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/PlatformOperations.kt
@@ -6,34 +6,52 @@ import java.awt.event.ComponentAdapter
 import java.awt.event.ComponentEvent
 import javax.swing.SwingUtilities
 
-internal open class FullscreenAdapter(
+/**
+ * A proxy between [SkiaLayer] and [HardwareLayer] that keeps the assigned "fullscreen" state while the layer doesn't
+ * exist, and applies it once it does.
+ */
+internal class FullscreenAdapter(
     val backedLayer: HardwareLayer
 ): ComponentAdapter() {
-    private var _isFullscreenDispatched = false
-    private var _isFullscreen: Boolean = false
+
+    // Keep a `localFullscreen` flag which stores the virtual fullscreen state when the window is not actually
+    // visible, and apply it when the window does become visible.
+    private var localFullscreen = false
+
+    // Additionally, keep an `isWindowShown` flag that says whether `backedLayer` or `localFullscreen` is currently
+    // the source of truth. This flag must be used, rather than the real state (e.g. with `window.isVisible)
+    // because otherwise the code becomes dependent on the order of listener calls. For example, `componentResized`
+    // can be called when the window is already visible but before `componentShown` has been called. If the test
+    // in `componentResized` was on `window.isVisible`, it would reset the value of `localFullscreen` before it had
+    // applied in `componentShown`.
+    private var isWindowShown = false
+
     var fullscreen: Boolean
-        get() = _isFullscreen
+        // If window is shown, return backedLayer.fullscreen; localFullscreen may not have updated yet
+        get() = if (isWindowShown) backedLayer.fullscreen else localFullscreen
         set(value) {
-            _isFullscreen = value
-            val window = SwingUtilities.getRoot(backedLayer)
-            if ( window == null || !window.isVisible) {
-                _isFullscreenDispatched = value
-            } else {
+            localFullscreen = value
+            if (isWindowShown) {
                 backedLayer.fullscreen = value
             }
         }
 
     override fun componentShown(e: ComponentEvent) {
-        backedLayer.fullscreen = _isFullscreenDispatched
+        isWindowShown = true
+        backedLayer.fullscreen = localFullscreen
     }
 
-    override fun componentHidden(e: ComponentEvent) {
-        _isFullscreenDispatched = _isFullscreen
+    override fun componentHidden(e: ComponentEvent?) {
+        isWindowShown = false
+        localFullscreen = backedLayer.fullscreen
     }
 
     override fun componentResized(e: ComponentEvent) {
-        _isFullscreen = backedLayer.fullscreen
+        if (isWindowShown) {
+            localFullscreen = backedLayer.fullscreen
+        }
     }
+
 }
 
 internal interface PlatformOperations {

diff --git a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt
@@ -4,7 +4,6 @@ import org.jetbrains.skia.*
 import org.jetbrains.skiko.redrawer.Redrawer
 import java.awt.Color
 import java.awt.Component
-import java.awt.Window
 import java.awt.event.*
 import java.awt.geom.AffineTransform
 import java.awt.im.InputMethodRequests
@@ -146,7 +145,7 @@ actual open class SkiaLayer internal constructor(
 
     override fun removeNotify() {
         Logger.debug { "SkiaLayer.awt#removeNotify $this" }
-        val window = SwingUtilities.getRoot(this) as Window
+        val window = SwingUtilities.getWindowAncestor(this)
         window.removeComponentListener(fullscreenAdapter)
         dispose()
         super.removeNotify()
@@ -155,7 +154,7 @@ actual open class SkiaLayer internal constructor(
     override fun addNotify() {
         Logger.debug { "SkiaLayer.awt#addNotify $this" }
         super.addNotify()
-        val window = SwingUtilities.getRoot(this) as Window
+        val window = SwingUtilities.getWindowAncestor(this)
         window.addComponentListener(fullscreenAdapter)
         checkShowing()
         init(isInited)

diff --git a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt
@@ -23,6 +23,8 @@ import org.junit.Rule
 import org.junit.Test
 import java.awt.Color
 import java.awt.Dimension
+import java.awt.event.ComponentAdapter
+import java.awt.event.ComponentEvent
 import java.awt.event.WindowEvent
 import javax.swing.JFrame
 import javax.swing.JLayeredPane
@@ -244,6 +246,34 @@ class SkiaLayerTest {
         }
     }
 
+    @Test
+    fun `window fullscreen state in componentResized`() = uiTest {
+        val window = UiTestWindow()
+        try {
+            window.defaultCloseOperation = WindowConstants.DISPOSE_ON_CLOSE
+            window.layer.fullscreen = true
+            var stateRemainsFullscreen = true
+            window.addComponentListener(object: ComponentAdapter(){
+                override fun componentResized(e: ComponentEvent?) {
+                    if (!window.layer.fullscreen)
+                        stateRemainsFullscreen = false
+                }
+            })
+            window.isVisible = true
+
+            delay(1000)
+            assertEquals(true, stateRemainsFullscreen)
+        } finally {
+            window.close()
+
+            // Delay before starting next test to let the window animation to complete, and allow the next window
+            // to become fullscreen
+            if (hostOs == OS.MacOS) {
+                delay(1000)
+            }
+        }
+    }
+
     @Test
     fun `should call onRender after init, after resize, and only once after needRedraw`() = uiTest {
         var renderCount = 0