Godot: Crash on do `OS::set_window_size(Vector2(1024, 680))` when changing scene

[ryancheung]

Godot version:

3.0

OS/device including version:

MacBook Pro (Retina, 13-inch, Early 2015)
macOS 10.14.2
OpenGL ES 3.0 Renderer: Intel(R) Iris(TM) Graphics 6100

Issue description:

I'm in GDNative cpp.
Program crash on OS::set_window_size when changing to another scene with owner->get_tree()->change_scene("res://AnotherScene.tscn");.
The target scene's base node code exmaple:

void LVBase::_ready()
{
    OS::set_window_size(Vector2(1024, 680));
}

Steps to reproduce:
Given a GDNative base node of a Scene, a Node2D

#ifndef _LSBASE_H
#define _LSBASE_H

#include <Godot.hpp>
#include <Node2D.hpp>
#include <String.hpp>

using namespace godot;

class LSBase : public GodotScript<Node2D>
{
    GODOT_CLASS(LSBase);
public:
    LSBase() { }

    void _ready()
    {
        OS::set_window_size(Vector2(640, 438));
        OS::center_window();
    }

    void _process(float delta);

    static void _register_methods() {
        register_method("_ready", &LSBase::_ready);
        register_method("_process", &LSBase::_process);
    }
};

#endif

And another base node of another scene:

#ifndef _LVBASE_H
#define _LVBASE_H

#include <Godot.hpp>
#include <Node2D.hpp>
#include <String.hpp>

#include "WilDataLibrary.h"

using namespace godot;

class LVBase : public GodotScript<Node2D>
{
    GODOT_CLASS(LVBase);
public:
    LVBase() { }

    WilDataLibrary* WilDataLib = nullptr;

    void _ready()
    {
        OS::set_window_size(Vector2(1024, 680));
    }
    void _process(float delta);


    static void _register_methods() {
        register_method("_ready", &LVBase::_ready);
        register_method("_process", &LVBase::_process);
    }
};

#endif

change scene from one to another like:

    owner->get_tree()->change_scene("res://AnotherScene.tscn");

Minimal reproduction project:

[ryancheung]

Oh no, you mean I cannot change window size on switching scenes on godot 3.0?
There's no other way to do this?

[ryancheung]

I can't continue to make the game with all scenes with the same window size.
Anyone could help, please!

[akien-mga]

You've created a bug report, it will be worked on in due time by the contributors who are knowledgeable about it. Please be patient.

[ryancheung]

I know it's a bug, and would be fix by the contributors in some time.
But I just want to know if there's a workaround for it, if there's no, it's OK too. I'll wait.

[ryancheung]

Found a workaround: DONOT set window size on ::_ready function, set it on ::_process would work.

[reduz]

Would be very appreciated if you include operating system in your report, else it will be difficult to test

[bruvzg]

This has nothing to do with gdnative, same happens with gdscript.

Crashes on macOS 10.14.2 (18C54), works ok on Linux.

Triggered by Main::iteration(); here - platform/osx/os_osx.mm#L307 (code was added to redraw window content during resize), probably this Iteration call cause access to not fully initialized scene.

Stack trace:

godot!Object::notification(int, bool) (/Volumes/Seagate/SDK/godot_sdk/godot/core/object.cpp:955)
godot!Node::_add_child_nocheck(Node*, StringName const&) (/Volumes/Seagate/SDK/godot_sdk/godot/scene/main/node.cpp:1141)
godot!Node::add_child(Node*, bool) (/Volumes/Seagate/SDK/godot_sdk/godot/scene/main/node.cpp:1179)
godot!SceneTree::_change_scene(Node*) (/Volumes/Seagate/SDK/godot_sdk/godot/scene/main/scene_tree.cpp:1275)
godot!MethodBind1<Node*>::call(Object*, Variant const**, int, Variant::CallError&) (/Volumes/Seagate/SDK/godot_sdk/godot/core/method_bind.gen.inc:729)
godot!Object::call(StringName const&, Variant const**, int, Variant::CallError&) (/Volumes/Seagate/SDK/godot_sdk/godot/core/object.cpp:945)
godot!MessageQueue::_call_function(Object*, StringName const&, Variant const*, int, bool) (/Volumes/Seagate/SDK/godot_sdk/godot/core/message_queue.cpp:256)
godot!MessageQueue::flush() (/Volumes/Seagate/SDK/godot_sdk/godot/core/message_queue.cpp:300)
godot!SceneTree::idle(float) (/Volumes/Seagate/SDK/godot_sdk/godot/scene/main/scene_tree.cpp:505)
godot!Main::iteration() (/Volumes/Seagate/SDK/godot_sdk/godot/main/main.cpp:1864)

[bruvzg]

May be also relevant on Windows with set_window_position in _ready, since Windows have similar additional Iteration calls for move event - platform/windows/os_windows.cpp#L777-L788. (I did not tested this.)

[ryancheung]

Would be very appreciated if you include operating system in your report, else it will be difficult to test

OS Inlucded.

[akien-mga]

@reduz says this might be another variant of the call_deferred("_file_selected") issue outlined in #23572.

[bruvzg]

@reduz says this might be another variant of the call_deferred("_file_selected") issue outlined in #23572.

Looks similar, both #23572 and #24672 have Main::iteration() called directly (outside of normal main loop) in ProgressDialog::task_step() - editor/progress_dialog.cpp#L223.

[bruvzg]

@reduz says this might be another variant of the call_deferred("_file_selected") issue outlined in #23572.

I do not see and call_deferred involved here but situation is the same, new iteration() is called in the middle of previous iteration() processing message queue. I guess some interlocking is required.

[bruvzg]

Simple interlocking of iteration() call fixes this and #23572 for me.

I assume there's no reason to add real mutex since this lock is single-threaded and only serve to prevent recursive iteration() calls.

diff --git a/main/main.cpp b/main/main.cpp
index e0f9cfb9d..d83e295da 100644
--- a/main/main.cpp
+++ b/main/main.cpp
@@ -1786,6 +1786,7 @@ uint64_t Main::target_ticks = 0;
 uint32_t Main::frames = 0;
 uint32_t Main::frame = 0;
 bool Main::force_redraw_requested = false;
+bool Main::iteration_finished = true;

 // For performance metrics
 static uint64_t physics_process_max = 0;
@@ -1793,6 +1794,11 @@ static uint64_t idle_process_max = 0;

 bool Main::iteration() {

+   if (!iteration_finished)
+       return false;
+
+   iteration_finished = false;
+
    uint64_t ticks = OS::get_singleton()->get_ticks_usec();
    Engine::get_singleton()->_frame_ticks = ticks;
    main_timer_sync.set_cpu_ticks_usec(ticks);
@@ -1920,8 +1926,10 @@ bool Main::iteration() {
        frames = 0;
    }

-   if (fixed_fps != -1)
+   if (fixed_fps != -1) {
+       iteration_finished = true;
        return exit;
+   }

    if (OS::get_singleton()->is_in_low_processor_usage_mode() || !OS::get_singleton()->can_draw())
        OS::get_singleton()->delay_usec(OS::get_singleton()->get_low_processor_usage_mode_sleep_usec()); //apply some delay to force idle time (results in about 60 FPS max)
@@ -1945,11 +1953,13 @@ bool Main::iteration() {
    if (auto_build_solutions) {
        auto_build_solutions = false;
        if (!EditorNode::get_singleton()->call_build()) {
+           iteration_finished = true;
            ERR_FAIL_V(true);
        }
    }
 #endif

+   iteration_finished = true;
    return exit || auto_quit;
 }

diff --git a/main/main.h b/main/main.h
index 01fc259a8..7e4b79e3d 100644
--- a/main/main.h
+++ b/main/main.h
@@ -47,6 +47,7 @@ class Main {
    static uint32_t frames;
    static uint32_t frame;
    static bool force_redraw_requested;
+   static bool iteration_finished;

 public:
    static bool is_project_manager();

[akien-mga]

Should be fixed after 4bb0080b3df28a3003bfffd6285602664a4279c9, could you confirm?

[bruvzg]

Crash is fixed by 4bb0080, but it's now printing Condition ' flushing ' is true. error on resize, probably MessageQueue::get_singleton()->is_flushing() check should be added to all other out of main loop Main::iteration calls.

[reduz]

The original cause for this error (usage of call_deferred on select_file) was fixed in #25076 , so technically you should no longer see this. I wonder what's going on..

[reduz]

This is very confusing though, your original error seems to be unrelated calling iteration() recursively.. only ProgressDialog does this afaik, and you are not doing this from the editor, or are you?

[bruvzg]

The original cause for this error (usage of call_deferred on select_file) was fixed in #25076 , so technically you should no longer see this. I wonder what's going on..

This issue is not related to call_deferred/select_file, it's cause by direct call of Iteration from macOS window resize.

[bruvzg]

only ProgressDialog does this afaik

It's also called by window resize on macOS and window move on Windows.

[reduz]

Oh I see, I understand now. Thanks!

[bruvzg]

On macOS this call was added to redraw window, since main loop (process_events) is locked by OS during resize. I guess on Windows it's something similar.