scenerygraphics / scenery

Flexible VR Visualisation for Volumetric and Geometric Data on the Java VM, powered by Kotlin and Vulkan.
https://scenery.graphics
GNU Lesser General Public License v3.0
130 stars 32 forks source link

Node.runRecursive should use a copy of children #552

Closed kephale closed 1 year ago

kephale commented 1 year ago

Describe the bug

https://github.com/scenerygraphics/scenery/blob/b607fd00a297963ebfddc15b14ab685f21f1f70d/src/main/kotlin/graphics/scenery/DefaultNode.kt#L123

runRecursive should make a copy of children then process those. Otherwise, if the lambda adds nodes to a child this leads to a trivial infinite loop.

To Reproduce

I ran into this while working on https://github.com/scenerygraphics/sciview/issues/241

where I tried to introduce a runRecursive call into the ToggleBoundingGrid command in sciview like this:

        if (applyToChildren) {
            node.runRecursive({ node -> toggleBoundingGrid(node)})
        } else {
            toggleBoundingGrid(node)
        }

However, toggleBoundingGrid is adding children to node and this leads to runRecursive running forever.

Expected behavior

runRecursive shouldn't lead to an infinite loop if the function being applied adds children to the node.

Additional context

sciview https://github.com/scenerygraphics/sciview/commit/28f4fb17bd0a596cf51d7060da78e70167fab396

kephale commented 1 year ago

https://github.com/scenerygraphics/sciview/pull/482 depends on this being fixed in scenery