search

godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.75k stars 577 forks source link

macOS and iOS arch build behavior does not match Godot upstream #1633

Open Ivorforce opened 3 weeks ago

Ivorforce commented 3 weeks ago

We recently found in a discussion that godot-cpp arch build behavior does not match Godot upstream.

In particular, godot-cpp defaults to building universal binaries (which passes the appropriate flags to the compiler to build both). The gdextension usually loads the same binary for both arm64 and x86_64 (as per godot-cpp-template).

Godot, on the other hand, builds arm64 and x86_64 separately and joins them afterwards with lipo. It does not support a 'universal' arch target.

One problem with the universal approach used by godot-cpp is that for universal builds, no architecture-specific build flags can be passed to the compilers (such as -mavx), because then the other architecture won't build (e.g. -mavx is not arm64 compatible). Notably, godot-cpp already supports compiling for the arches separately.

Changing the default behavior would affect all current macOS compatible godot-cpp extensions, because they will (likely) build with the default universal target right now (as per godot-cpp-template). We will have to figure out whether a change to separate the binaries will affect exports, especially for universal macOS apps.

We discussed this briefly at the last GDExtension meeting, and would like to have input on this before proceeding (especially from @Faless).

Ivorforce commented 2 weeks ago

I just tested exports on macOS.

When using feature tags, in-editor it tries to load the binary for the matching architecture (i.e. x86_64 or arm64, but not universal). When exporting, it tries to add the binary for the universal architecture, but not the arch-specific ones (x86_64 or arm64). (Edit: I opened a proposal for this issue)

This means that if we want to move to building the arches separately, we need to either patch godot-upstream to handle arch-separate binary exports, or lipo the binaries together to a universal as part of the build process.

Faless commented 2 weeks ago

I think universal builds will be completely deprecated soon as apple stop supporting intel Mac.

As far as I understand modern "frameworks" format should support using different libraries for different architectures, and Apple is trying really hard to force everyone to use frameworks for everything.

I don't think it's worth doing the lipo part in the godot-cpp build system because that won't include any third party dependency (which the user would likely have to lipo manually like we do for webrtc-native).

In the end, I think the solution is to start defaulting to arm64 (or auto-detect if building from macOS directly), and update the examples to use a multi-arch framework (we should confirm this is actually doable, I don't have a Mac, and I'm not an Apple fan as you've probably guessed, so I can't really verify any of this as their documentation is close to non-existent).

Ivorforce commented 2 weeks ago

I think universal builds will be completely deprecated soon as apple stop supporting intel Mac.

Deprecated perhaps, but while Apple's support is a decent guideline for what Godot should support, there are still a lot of intel macs in circulation (and will be for a while). (speaking of which, this may be a good question to add for the next Godot user survey...)

I don't think it's worth doing the lipo part in the godot-cpp build system because that won't include any third party dependency (which the user would likely have to lipo manually like we do for webrtc-native). In the end, I think the solution is to start defaulting to arm64 (or auto-detect if building from macOS directly), and update the examples to use a multi-arch framework (we should confirm this is actually doable, I don't have a Mac, and I'm not an Apple fan as you've probably guessed, so I can't really verify any of this as their documentation is close to non-existent).

If I understand correctly, you're hoping we can find a way to bundle the frameworks with separate binaries for separate arches? I'm not sure that's possible, but I'm also not deeply familiar with framework bundles in the first place. Apple seems to prefer creating universal bundles.

I don't see anything wrong with doing a lipo step in the GitHub runner to create a universal arch from both separate arches. But if you have any reservations against this approach please share them, as I'm not sure I wholly understood the point you were making. But either way, we seem to agree that building x86_64 and arm64 separately is preferable, correct?

Faless commented 2 weeks ago

I don't see anything wrong with doing a lipo step in the GitHub runner to create a universal arch from both separate arches.

I'm not sure how that would help, the GH artifacts are not supposed to be used by extension developers, but yeah, we can totally add the lipo step there.

But either way, we seem to agree that building x86_64 and arm64 separately is preferable, correct?

Yeah, I think that's fine.

We don't have to necessarily remove the "universal" option, we can just change the default (but I'm not against removal)

Ivorforce commented 2 weeks ago

I'm not sure how that would help, the GH artifacts are not supposed to be used by extension developers, but yeah, we can totally add the lipo step there.

Ah, i'm just proposing to do that for releases (ie. The GitHub runner). For local builds non-universal binaries are of course totally fine.

And yeah i agree there's no reason to remove the universal build option!

I'm happy to take on the next steps. Building arches on the GitHub runner separately can be done now (edit: actually https://github.com/godotengine/godot-cpp-template/pull/55 should be addressed first). Changing the default to use the local arch should probably wait until https://github.com/godotengine/godot/pull/98809 is merged, just to avoid somebody accidentally creating a non exportable build.

Faless commented 2 weeks ago

Ah, i'm just proposing to do that for releases (ie. The GitHub runner).

Again, not sure why you call them releases, the artifacts are not released in any way.

Ivorforce commented 2 weeks ago

I guess that's correct, though I'm not sure what else the artifacts would be used for? I use the github runner from godot-cpp-template to create binaries for my GDExtension, which I can publish as a release. Does that clear it up?

Faless commented 2 weeks ago

I guess that's correct, though I'm not sure what else the artifacts would be used for?

Mostly for debugging CI issues themselves.

I use the github runner from godot-cpp-template to create binaries for my GDExtension, which I can publish as a release. Does that clear it up?

Yeah, I think it makes sense to have that for the godot-cpp-template since I assume people will rely on the CI file from that repository as a template for CI builds.