edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.67k stars 271 forks source link

Fragment with SubScene cause StackOverflowException #994

Closed balage1551 closed 5 years ago

balage1551 commented 5 years ago

When defining a Fragment with a SubScene control as a child cause infinite recusive error. Code:

package hu.vissy.subscenebug

import javafx.scene.SceneAntialiasing
import javafx.scene.SubScene
import tornadofx.*

class SubSceneBugApp: App(SubSceneBugView::class)

class SubSceneBugFragment : Fragment() {

    private lateinit var subScene: SubScene
    override val root = anchorpane {
        subScene = SubScene(this, 100.0, 100.0, true, SceneAntialiasing.BALANCED)
        add(subScene)
    }
}

class SubSceneBugView : View("SubScene bug") {
    private lateinit var viewer: SubSceneBugFragment

    override val root = borderpane {
        center {
            add<SubSceneBugFragment> {
                viewer = this
            }
        }
    }
}

fun main() {
    launch<SubSceneBugApp>()
}

Exception:

Exception in Application start method
máj. 16, 2019 12:28:51 DU tornadofx.DefaultErrorHandler uncaughtException
SEVERE: Uncaught error
java.lang.RuntimeException: Exception in Application start method
    at com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:917)
    at com.sun.javafx.application.LauncherImpl.lambda$launchApplication$154(LauncherImpl.java:182)
    at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.StackOverflowError
    at java.util.WeakHashMap.hash(WeakHashMap.java:298)
    at java.util.WeakHashMap.get(WeakHashMap.java:396)
    at com.sun.javafx.css.StyleManager.getCacheContainer(StyleManager.java:190)
    at com.sun.javafx.css.StyleManager.findMatchingStyles(StyleManager.java:1647)
    at javafx.scene.CssStyleHelper.createStyleHelper(CssStyleHelper.java:111)
    at javafx.scene.Node.reapplyCss(Node.java:8985)
    at javafx.scene.Node.reapplyCss(Node.java:9023)
    at javafx.scene.Node.reapplyCss(Node.java:9014)
    at javafx.scene.Node.reapplyCss(Node.java:9023)
edvin commented 5 years ago

It actually happens even if you don't embed the SubScene in a Fragment. This is enough to trigger it:

class SubSceneBugView : View("SubScene bug") {
    override val root = borderpane {
        center {
            add(SubScene(this, 100.0, 100.0, true, SceneAntialiasing.BALANCED))
        }
    }
}

Not sure if it's a TornadoFX issue or a JavaFX issue yet.

edvin commented 5 years ago

I think you might be using the SubScene wrong. You need to pass it a Node that should become it's root, and you must not give it a Node that is effectively a parent to the SubScene, that would make no sense, and is also what's causing the issue you're seeing. This code works:

class SubSceneBugView : View("SubScene bug") {
    override val root = vbox {
        add(SubScene(VBox(), 100.0, 100.0, true, SceneAntialiasing.BALANCED))
    }
}
balage1551 commented 5 years ago

As long as I used the AnchorPane as parent for the ViewerPane it worked fine (see first example in original issue. So it went wrong when I introduced Fragment (or TornadoFX).

edvin commented 5 years ago

The issue is not Fragment, the issue is that you're passing in a meaningless value for the SubScene root. I'll add a subscene builder to TornadoFX to make this easier to deal with.

balage1551 commented 5 years ago

You are right. The recursion is because when I refactored to Fragment I moved the Anchor pane to root (root was a simple Group before), but I kept the root of the SubScene the same. It is the cause of a simple recursion: root of the SubScene is the Anchor pane, then adding the SubScene to the AnchorPane as a child).

If I add correct root, it works fine:


class SubSceneBugFragment : Fragment() {

    private lateinit var subScene: SubScene
    private val ssRoot = Group()
    override val root = anchorpane {
        subScene = SubScene(ssRoot, 100.0, 100.0, true, SceneAntialiasing.BALANCED)
        add(subScene)
    }
}
edvin commented 5 years ago

Indeed. I've committed a subscene builder now, so you can write that same code in a much nicer way:

class SubSceneFragment : Fragment() {
    override val root = anchorpane {
        subscene(100, 100, true, SceneAntialiasing.BALANCED) {
            group {
                // Your components here, or skip the group and use another container
            }
        }
    }
}
balage1551 commented 5 years ago

Great! That was quick! One idea: it is inconvenient that one has to give the SubScene size. Consider the option to make these arguments optional (with another builder method or with special default values for width and height). When they are not specified, the size could be automatically align to parent:

subScene.heightProperty().bind(root.heightProperty())
subScene.widthProperty().bind(root.widthProperty())

It could reduce boiler code and would let the SubScene to behave the same way the other controls do.

edvin commented 5 years ago

Done :) To be clear, this is not what other controls do. Other controls adapt to their childrens's size. I suspect however that you're right that this is a good default.