Allow disabling 2D when compiling export templates

[aaronfranke]

This PR is marked as draft for the moment because GraphEdit's recent changes prevent this PR from fully functioning.

Implements and closes godotengine/godot-proposals#1653.

This PR allows disabling 2D when compiling export templates. The 2D rendering stuff isn't disabled as it's needed for Control nodes, but all of the nodes under scene/2d are disabled (except for one), as well as 2D physics and many 2D-only resources.

With optimized production=yes builds of export templates, with 2D enabled the file size is 56.4 MB, and with 2D disabled the file size is 50.8 MB. This is a reduction of 5.6 MB, about 9.9%.

Disabling 2D with this PR actually gives even better gains than the current disable_3d=yes option for 3D, which optimized production=yes builds of export templates, with disable_3d=yes, the size is 52.4 MB (a reduction of 4.0 MB or 7.1%).

This PR adds the #ifndefs and SCons code to add support for disabling 2D when compiling export templates. When compiling with disable_2d=yes, _2D_DISABLED is defined. Everything is disabled in scene/2d (except VisibilityNotifier2D), scene/resources/2d, and GraphEdit is disabled (dependency on Line2D).

This PR has two main use cases: 3D games (which only need 3D and UI, not 2D), and UI-only projects (which only need UI, not 2D or 3D, and 3D can already be disabled), which helps solve godotengine/godot-proposals#190.

[groud]

I'm not really convinced this is very useful. It requires a significant maintenance work (all cases of disabling/enabling things should be tested for compilation), while not allowing a very significant gain on the executable size (though i guess that's debatable).

At a given point, Godot is a game engine. I know it can be used to make tools, but this is not and should not be a priority. So basically I am not convinced that adding this compilation option, for a 7% size decrease that would only matter in some corner cases, is worth the maintenance work needed.

[Calinou]

On my Ubuntu system, with debug builds of export templates, with 2D enabled the file size is 376.7 MB, and with 2D disabled the file size is 348.5 MB. This is a reduction of 28.2 MB, about 7.5%.

Do you have size figures for fully optimized release + LTO builds? I think this is the most important metric as this is what impacts distribution size.

Also, what matters even more is compressed distribution size (such as packing the binary into a ZIP archive), since this is what affects the download speed for a given project. But I have to agree with @groud here – even if we can save 7% on a release binary, saving 7% on a compressed release binary is unlikely to amount to more than 1.5 MB of savings (assuming the compression efficiency is identical).

[Zireael07]

The one place where the savings might be important is WASM (HTML5) export, though.

[groud]

The one place where the savings might be important is WASM (HTML5) export, though.

Yeah, but we have to consider the big picture. Even the gain goes up to 10% on WASM export, this is still only for the binary part of the game. There's still the game data to be loaded along with it, which, reduces the relative gain even more.

[aaronfranke]

It requires a significant maintenance work (all cases of disabling/enabling things should be tested for compilation), while not allowing a very significant gain on the executable size (though i guess that's debatable).

I don't think that it will be significant, and I'm willing to take on this maintenance burden. We don't have to test every single case, as long as the cases for official builds work it's fine, and then others are "use at your own risk" so to speak. We already allow disabling most modules, some combinations of which may not be valid, but we don't disallow disabling modules because some combinations don't work. We also already allow disabling 3D.

The worst case scenario if something stops working in the future is that you just can't use the option, at which point it's not a downside to anyone who isn't using it, and anyway it's probably only a few #ifndefs away from being fixed.

Long-term I would like to allow disabling even more things, so the gain will be much more than 7.5%. Godot is an engine that prides itself on being a small engine, lots of users love this and are sensitive to new features being added due to possible bloat, and so I think we should give users freedom to make it even smaller if they want to ("at their own risk").

At a given point, Godot is a game engine. I know it can be used to make tools, but this is not and should not be a priority.

That was just one of the two use cases I mentioned above. The other is 3D games with only 3D and UI. However, I will add on one more use case: There are some games that can be made with only UI nodes, such as Cookie Clicker.

[groud]

I don't think that it will be significant, and I'm willing to take on this maintenance burden.

This is not about you. You may not being here forever like anyone of us. So any maintenance work has to be justified.

Even if it's small, adding a new flag which is spread over the whole codebase is significant maintenance work (your PR modifies 55 files...). It means we will have to verify every times someone open a PR if they made sure to surround their code with the proper ifdef, this is work on every maintainers shoulder.

If we can't justify both a common enough use case and significant gains on the binary size, then I don't think this feature is worth.

There are some games that can be made with only UI nodes, such as Cookie Clicker.

Sorry but there's no way Cookie Clicker would be made efficiently without 2D nodes. It has animations and all, which Control node are very bad for.

[aaronfranke]

It means we will have to verify every times someone open a PR

Again, we don't have to do this. We already don't do this. We don't test every combination of disabling modules for every PR. We don't even test disabling 3D for every PR. Worst case scenario, the disable 2D option stops working and someone else can fix it (probably easily). It doesn't affect anyone who isn't using the disable 2D option.

Sorry but there's no way Cookie Clicker would be made efficiently without 2D nodes. It has animations and all, which Control node are very bad for.

I don't see why not. You can do animations with Control nodes, even if it's not as nice. You can always change positions and sizes and scales of things manually through script if needed.

[groud]

I don't see why not. You can do animations with Control nodes, even if it's not as nice. You can always change positions and sizes and scales of things manually through script if needed.

Indeed, you could, but that's why I specified "efficiently".

[Xrayez]

If Godot is going to have this option, then I'd certainly find this useful in Goost, where extensibility and customization are at the core of the development philosophy.

For instance, if you look at https://github.com/goostengine/goost/blob/739851575d46cbff3291c1d50699014ac0ccdaf9/scene/SCsub#L5-L8, then you'll see that I can only reuse the existing disable_3d SCons option. Yet I haven't introduced disable_2d option in Goost simply because I wasn't sure whether Godot plans to add it in the future. So currently, I'd have to add additional disable_2d in the module's SCSub file, and the option would only apply for Goost itself, which would make the code somewhat inconsistent and prone to confusion. It would be better that Godot itself had this option out of the box.

Of course, I could as well add goost_2d_enabled and goost_3d_enabled options specifically for Goost ecosystem, but I don't quite see a reason why we cannot make Godot a reusable piece of software in this regard, so users already familiar with Godot can easily disable 2D and 3D parts of the extension even without reading documentation. In any case, it's something which could make Godot more complete.

In Goost, I went a little bit further and introduced a build dependency system for classes goostengine/goost#49, so that each and every class from the public API can be disabled at build time (not just runtime, but also on the level of source). Now, I think that this would be certainly overkill for Godot, yet adding disable_2d option is quite an innocent change in comparison.

The reason why I have to maintain such a system is to allow the extension to grow almost without limits, but at the same time make it possible to optimize the build for size, because I realize that not all people would be interested in a particular feature set. My development policy is that if something gets little support, it simply gets easily disabled, and eventually removed if nobody needs a particular feature, so really there's not much problem when it comes to maintenance (given that everything is in place and pretty). Interested contributors can chime in and resurrect needed features.

---

We already allow disabling most modules, some combinations of which may not be valid, but we don't disallow disabling modules because some combinations don't work. We also already allow disabling 3D.

Confirmed by existence of #39196. But I'd say that some modules are crucial and are checked at build time via simplistic dependency check, like this:

godot/SConstruct

Lines 627 to 634 in dc038bd

	editor_module_list = ["freetype", "regex"]
	if env["tools"] and not env.module_check_dependencies("tools", editor_module_list):
	print(
	"Build option 'module_"
	+ x
	+ "_enabled=no' cannot be used with 'tools=yes' (editor), only with 'tools=no' (export template)."
	)
	Exit(255)

---

Calinou

even if we can save 7% on a release binary, saving 7% on a compressed release binary is unlikely to amount to more than 1.5 MB of savings (assuming the compression efficiency is identical).

aaronfranke

Long-term I would like to allow disabling even more things, so the gain will be much more than 7.5%.

Yes, those savings add up with each similar PR like this one, have to look at the bigger picture here. In any case, the savings from this PR would outweigh savings in #29587, for instance (when comparing potential savings, not necessarily custom vs official builds).

[aaronfranke]

@Calinou I tried testing with an optimized release binary, but I'm not able to test this on the master branch (so I can't test it on this PR either). scons p=linuxbsd target=release tools=no use_lto=yes gives this error during linking:

thirdparty/icu4c/common/udata.cpp:646: warning: 'icudt68_dat' violates the C++ One Definition Rule [-Wodr]
  646 | extern "C" const DataHeader U_DATA_API U_ICUDATA_ENTRY_POINT;
      | 
modules/text_server_adv/icu_data/icudata_stub.cpp:44: note: type name 'ICU_data_header' should match type name 'DataHeader'
   44 | extern "C" U_EXPORT const ICU_data_header U_ICUDATA_ENTRY_POINT = {
      | 
modules/text_server_adv/icu_data/icudata_stub.cpp:44: note: 'icudt68_dat' was previously declared here
In function 'strncpy',
    inlined from 'ultag_parse' at thirdparty/icu4c/common/uloc_tag.cpp:2101:21,
    inlined from 'ulocimp_forLanguageTag_68' at thirdparty/icu4c/common/uloc_tag.cpp:2766:88,
    inlined from 'uloc_forLanguageTag_68.constprop' at thirdparty/icu4c/common/uloc_tag.cpp:2736:27:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:34: warning: '__builtin_strncpy' specified bound depends on the length of the source argument [-Wstringop-overflow=]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |                                  ^
thirdparty/icu4c/common/uloc_tag.cpp: In function 'uloc_forLanguageTag_68.constprop':
thirdparty/icu4c/common/uloc_tag.cpp:2100:46: note: length computed here
 2100 |                     size_t preferredTagLen = uprv_strlen(preferredTag);
      |  

[bruvzg]

I tried testing with an optimized release binary, but I'm not able to test this on the master branch (so I can't test it on this PR either). scons p=linuxbsd target=release tools=no use_lto=yes gives this error during linking:

Here's a fix for ICU warnings - https://github.com/bruvzg/godot/tree/icu_lto_define, but there are few other -Wstringop-overflow= warnings in Vulkan loader.

Changing third party code just for a cleaner seems like a bad idea to me, probably they should be suppressed instead, but I'm not sure how to do it.

None of these warnings is an actual issue, and should not prevent linking. Make sure you are using a machine/container with the sufficient amount of memory (at least 12 GB) for the build.

[aaronfranke]

@bruvzg Thanks for pointing that out, I do hope it gets solved eventually. But yes they are just warnings, the build does actually succeed, I just didn't look closely enough to notice. I decided to do production=yes builds that include LTO and static C++.

With optimized production=yes builds of export templates, with 2D enabled the file size is 56.4 MB, and with 2D disabled the file size is 50.8 MB. This is a reduction of 5.6 MB, about 9.9%.

Disabling 2D with this PR actually gives even better gains than the current disable_3d=yes option for 3D, which optimized production=yes builds of export templates, with disable_3d=yes, the size is 52.4 MB (a reduction of 4.0 MB or 7.1%). To me, this seems like a very strong argument in favor of adding disable_2d=yes, since disable_3d=yes gives smaller gains but it was deemed useful enough to be included in the engine.

[MewPurPur]

All objects disabled by this option, aside from nodes, are specialized for physics or navigation. But this PR also removes Curve2D, which seems unnecessary, as it's a generic way to represent a 2D curve.

[aaronfranke]

@MewPurPur Yeah, and it also disables Line2D, which used to work fine, but as of the GraphEdit refactor, removing this breaks GraphEdit. So I will probably change the PR to keep Node2D, Line2D, and Curve2D always compiled.

Anyway, I have not focused on this PR much because I first and foremost want to get other PRs in as prerequisite, like #66744, and several more that have already been merged in the last few weeks.

[MewPurPur]

Curve2D is a resource, not a node. Line2D is a node.

That's quite odd that GraphEdit is relying on Line2D. I have a strong feeling that whatever GraphEdit is using Line2D for, should be much more efficient with draw_polyline_colors() where points are grabbed by a tessellated Curve2D, and colors are grabbed by a sampled gradient.

Edit: So it seems like there's a good reasoning behind using Line2D, actually, but still, we have an unexposed LineBuilder class that can be used for this without a node.

[MewPurPur]

I tried to get GraphEdit to use the LineBuilder class, to remove the Line2D baggage, but couldn't manage.

This PR should likely now account for this: #95261