Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ThemeDB initialization in tests #81305

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Fix ThemeDB initialization in tests #81305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,9 +570,15 @@ Error Main::test_setup() {

ResourceLoader::load_path_remaps();

// Initialize ThemeDB early so that scene types can register their theme items.
// Default theme will be initialized later, after modules and ScriptServer are ready.
initialize_theme_db();

register_scene_types();
register_driver_types();

register_scene_singletons();

initialize_modules(MODULE_INITIALIZATION_LEVEL_SCENE);
GDExtensionManager::get_singleton()->initialize_extensions(GDExtension::INITIALIZATION_LEVEL_SCENE);

Expand All @@ -588,9 +594,7 @@ Error Main::test_setup() {
register_platform_apis();

// Theme needs modules to be initialized so that sub-resources can be loaded.
initialize_theme_db();
theme_db->initialize_theme();
register_scene_singletons();
theme_db->initialize_theme_noproject();

initialize_navigation_server();

Expand Down Expand Up @@ -2561,9 +2565,15 @@ Error Main::setup2() {

OS::get_singleton()->benchmark_begin_measure("scene");

// Initialize ThemeDB early so that scene types can register their theme items.
// Default theme will be initialized later, after modules and ScriptServer are ready.
initialize_theme_db();

register_scene_types();
register_driver_types();

register_scene_singletons();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust moving the theme stuff around, but I'm not confident about this. How does this interact with the module/GDExtension scene initialization level? This code is very core and moving stuff around typically breaks stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a method that I've added, it only registers ThemeDB right now and I moved it specifically so the singleton is available for other consumers as early as possible. 🙃


initialize_modules(MODULE_INITIALIZATION_LEVEL_SCENE);
GDExtensionManager::get_singleton()->initialize_extensions(GDExtension::INITIALIZATION_LEVEL_SCENE);

Expand All @@ -2581,11 +2591,6 @@ Error Main::setup2() {

register_platform_apis();

// Theme needs modules to be initialized so that sub-resources can be loaded.
// Default theme is initialized later, after ScriptServer is ready.
initialize_theme_db();
register_scene_singletons();

GLOBAL_DEF_BASIC(PropertyInfo(Variant::STRING, "display/mouse_cursor/custom_image", PROPERTY_HINT_FILE, "*.png,*.webp"), String());
GLOBAL_DEF_BASIC("display/mouse_cursor/custom_image_hotspot", Vector2());
GLOBAL_DEF_BASIC("display/mouse_cursor/tooltip_position_offset", Point2(10, 10));
Expand Down
12 changes: 12 additions & 0 deletions scene/theme/theme_db.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ void ThemeDB::initialize_theme_noproject() {
}
}

void ThemeDB::finalize_theme() {
if (!RenderingServer::get_singleton()) {
WARN_PRINT("Finalizing theme when there is no RenderingServer is an error; check the order of operations.");
}

default_theme.unref();

fallback_font.unref();
fallback_icon.unref();
fallback_stylebox.unref();
}

// Universal fallback Theme resources.

void ThemeDB::set_default_theme(const Ref<Theme> &p_default) {
Expand Down
1 change: 1 addition & 0 deletions scene/theme/theme_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class ThemeDB : public Object {
public:
void initialize_theme();
void initialize_theme_noproject();
void finalize_theme();

// Universal Theme resources

Expand Down
16 changes: 8 additions & 8 deletions tests/scene/test_viewport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@

namespace TestViewport {

class NotificationControl : public Control {
GDCLASS(NotificationControl, Control);
class NotificationControlViewport : public Control {
GDCLASS(NotificationControlViewport, Control);

protected:
void _notification(int p_what) {
Expand All @@ -63,11 +63,11 @@ class NotificationControl : public Control {
bool mouse_over = false;
};

// `NotificationControl`-derived class that additionally
// `NotificationControlViewport`-derived class that additionally
// - allows start Dragging
// - stores mouse information of last event
class DragStart : public NotificationControl {
GDCLASS(DragStart, NotificationControl);
class DragStart : public NotificationControlViewport {
GDCLASS(DragStart, NotificationControlViewport);

public:
MouseButton last_mouse_button;
Expand All @@ -93,9 +93,9 @@ class DragStart : public NotificationControl {
}
};

// `NotificationControl`-derived class that acts as a Drag and Drop target.
class DragTarget : public NotificationControl {
GDCLASS(DragTarget, NotificationControl);
// `NotificationControlViewport`-derived class that acts as a Drag and Drop target.
class DragTarget : public NotificationControlViewport {
GDCLASS(DragTarget, NotificationControlViewport);

public:
Variant drag_data;
Expand Down
6 changes: 3 additions & 3 deletions tests/scene/test_window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@

namespace TestWindow {

class NotificationControl : public Control {
GDCLASS(NotificationControl, Control);
class NotificationControlWindow : public Control {
GDCLASS(NotificationControlWindow, Control);

protected:
void _notification(int p_what) {
Expand Down Expand Up @@ -69,7 +69,7 @@ TEST_CASE("[SceneTree][Window]") {
w->set_content_scale_size(Size2i(200, 200));
w->set_content_scale_mode(Window::CONTENT_SCALE_MODE_CANVAS_ITEMS);
w->set_content_scale_aspect(Window::CONTENT_SCALE_ASPECT_KEEP);
NotificationControl *c = memnew(NotificationControl);
NotificationControlWindow *c = memnew(NotificationControlWindow);
w->add_child(c);
c->set_size(Size2i(100, 100));
c->set_position(Size2i(-50, -50));
Expand Down
17 changes: 8 additions & 9 deletions tests/test_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ struct GodotTestCaseListener : public doctest::IReporter {
PhysicsServer2D *physics_server_2d = nullptr;
NavigationServer3D *navigation_server_3d = nullptr;
NavigationServer2D *navigation_server_2d = nullptr;
ThemeDB *theme_db = nullptr;

void test_case_start(const doctest::TestCaseData &p_in) override {
reinitialize();
Expand All @@ -238,6 +237,10 @@ struct GodotTestCaseListener : public doctest::IReporter {
RenderingServerDefault::get_singleton()->init();
RenderingServerDefault::get_singleton()->set_render_loop_enabled(false);

// ThemeDB requires RenderingServer to initialize the default theme.
// So we have to do this for each test case.
ThemeDB::get_singleton()->initialize_theme_noproject();

physics_server_3d = PhysicsServer3DManager::get_singleton()->new_default_server();
physics_server_3d->init();

Expand All @@ -252,9 +255,6 @@ struct GodotTestCaseListener : public doctest::IReporter {
memnew(InputMap);
InputMap::get_singleton()->load_default();

theme_db = memnew(ThemeDB);
theme_db->initialize_theme_noproject();

memnew(SceneTree);
SceneTree::get_singleton()->initialize();
if (!DisplayServer::get_singleton()->has_feature(DisplayServer::Feature::FEATURE_SUBWINDOWS)) {
Expand Down Expand Up @@ -294,11 +294,6 @@ struct GodotTestCaseListener : public doctest::IReporter {
memdelete(SceneTree::get_singleton());
}

if (theme_db) {
memdelete(theme_db);
theme_db = nullptr;
}

if (navigation_server_3d) {
memdelete(navigation_server_3d);
navigation_server_3d = nullptr;
Expand Down Expand Up @@ -326,6 +321,10 @@ struct GodotTestCaseListener : public doctest::IReporter {
}

if (RenderingServer::get_singleton()) {
// ThemeDB requires RenderingServer to finalize the default theme.
// So we have to do this for each test case.
ThemeDB::get_singleton()->finalize_theme();

RenderingServer::get_singleton()->sync();
RenderingServer::get_singleton()->global_shader_parameters_clear();
RenderingServer::get_singleton()->finish();
Expand Down