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.

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

feat: Add builds for iOS Silicon Simulator #7704

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

chrfalch
Copy link

On iOS The current build system only builds for one architecture for iPhone (arm64) and one for iPhone Simulator (x86-64). This worked before the new Apple Silicon Macs arrived - since Xcode/iOS Simulator now supports both x86-64 and arm64 architectures which is not directly supported by the current build setup in Google/Filament.

Multiple iOS simulator architectures makes it a bit harder to package on iOS, since we can no longer just create fat binaries with the two architectures - we now have three that we somehow need to fit into the build:

The old list of library architectures and platforms were:
iPhone - arm64
iOS Simulator - x86-64

The new list on iOS will now be:
iPhone - arm64
iOS Simulator - x86-64
iOS Simulator - arm64

To accomplish distributing the above binaries we need to change from distributing our libraries as .a files in universal format (containing multiple architectures in one .a file), and move over to XC Frameworks. An XC Framework is a way for Xcode to consume multiple architectures and multiple platforms (where a platform would typically be iPhone or iPhoneSimulator which both have different SDKs).

This pull request fixes this by introducing platform as a directory element so that we can build multiple outputs configured for the same architecture, and also changes the build so the final iOS build step is to create the XC framework-files. The Cocoapods spec file is also updated so that it now references the XC frameworks instead of the library files directly.

This PR fixes #6714

- Added target platform to directory name generation
- Added simulator build steps for both arm64 and x86_64
- Added building frameworks when BUILD_UNIVERSAL_LIBRARIES is set.
Added platform name as part of destination directory if PLATFORM_NAME is set.
Added script with support for building XC.Frameworks
Appended -target parameter for the iOS parameters when building for simulator to ensure the emitted libraries are targetting the simulator.
- Removed fix for XCode 12 linking error
- Updated podspec to include XCFrameworks instead of .a files directly to support multiple archs and platforms.
@mrousavy
Copy link

Wow!! 🥳

@romainguy romainguy requested a review from bejado March 25, 2024 16:56
hannojg added a commit to margelo/react-native-filament that referenced this pull request Mar 25, 2024
Copy link
Member

@bejado bejado left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I took a quick look -- so far everything is on the right path. The samples in ios/samples do need to be updated to reflect the build directory renaming.

I think this build.sh help message should be updated to reflect that "universal" libraries now means XC Frameworks and will contain 3 different platform/arch combinations.

Finally, do we need to keep the create-universal-libs.sh script? Could we run your new create-xc-frameworks.sh script like:

build/ios/create-xc-frameworks.sh \
    -o out/ios-debug/filament/lib \
    out/ios-debug/filament/lib/arm64-iphoneos \
    out/ios-debug/filament/lib/arm64-iphonesimulator \
    out/ios-debug/filament/lib/x86_64-iphonesimulator

Thanks again!

@chrfalch chrfalch changed the title Feature/6714: Add builds for iOS Silicon Simulator feat: Add builds for iOS Silicon Simulator Mar 26, 2024
@chrfalch
Copy link
Author

chrfalch commented Mar 26, 2024

Thank you for your contribution! I took a quick look -- so far everything is on the right path. The samples in ios/samples do need to be updated to reflect the build directory renaming.

I think this build.sh help message should be updated to reflect that "universal" libraries now means XC Frameworks and will contain 3 different platform/arch combinations.

Finally, do we need to keep the create-universal-libs.sh script? Could we run your new create-xc-frameworks.sh script like:

build/ios/create-xc-frameworks.sh \
    -o out/ios-debug/filament/lib \
    out/ios-debug/filament/lib/arm64-iphoneos \
    out/ios-debug/filament/lib/arm64-iphonesimulator \
    out/ios-debug/filament/lib/x86_64-iphonesimulator

Thanks again!

Np! Super fun task to work on :)

The reason we still have to use the create-universal-libs.sh script is that an XC Framework cannot contain two library files built for the same platform (simulator) - these two needs to be distributed as a universal library - built with create-universal-libs.sh.

So the process now will become (with this PR):

  1. Run build/install as usual
  2. Create universal libraries of all platform simulator into the lib/universal folder
  3. Create XC Frameworks with the universal libraries + the arm64 iPhoneOS libraries.

Reagarding the ios/samples/app-template.yml file and building the samples - I'll take a quick look at this now.

It is not possible to have different xcframeworks depending on build type (release/debug) so release frameworks have been added.
@chrfalch
Copy link
Author

chrfalch commented Mar 26, 2024

I've updated release_notes and updated example projects on iOS - hope it is in line with how you expect this. It seems to build correctly locally - excited to see how the tests are working.

Btw, I was not sure if the generated project files in iOS should be committed - so I left them out.

Added iOS sample project files after running generate - CI runs these projects without re-generating so project files needs to be committed.

@chrfalch chrfalch requested a review from bejado March 26, 2024 10:53
Looks like CI is building without running generate - so then we need to check in and submit changes to these projects as well.
Copy link
Member

@bejado bejado left a comment

Choose a reason for hiding this comment

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

We also need to support the case where we don't build universal libraries. For example:

./build.sh -i -p ios debug

In that case, no XCFrameworks are generated and the iOS sample builds fail. I'm not sure what the best solution is. Maybe we always generate XCFrameworks, but for non-simulator builds they only contain a single architecture/platform?

"-lzstd", "-lcamutils", "-ldracodec", "-lmeshoptimizer", "-lviewer", "-lcivetweb",
"-luberzlib"]
dependencies:
- framework: $(SRCROOT)/../../../out/ios-release/filament/lib/libgltfio_core.xcframework
Copy link
Member

@bejado bejado Mar 27, 2024

Choose a reason for hiding this comment

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

Same here (and other project.yml files). Is it not sufficient to list these frameworks under the root app-template.yml file?

Copy link
Author

Choose a reason for hiding this comment

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

Absolutely, it can be done - just tried to follow the current setup where common libraries are defined in app-template.yml and any specific (used by only one or a few examples) are defined in each example project.yml file.

@@ -17,25 +17,27 @@ targetTemplates:
configVariants:
- Metal
- OpenGL
dependencies:
- framework: "../../out/ios-release/filament/lib/libbackend.xcframework"
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to hardcode this to ios-release, as then this won't work with debug builds. I think we might need to move this under settings.config.release and modify it for settings.config.debug

Copy link
Member

Choose a reason for hiding this comment

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

I think you should also be able to use FRAMEWORK_SEARCH_PATHS here instead of listing each one individually.

Copy link
Author

Choose a reason for hiding this comment

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

  1. Release/Debug: Makes sense to move these in settings.config files

  2. Unfortunately FRAMEWORK_SEARCH_PATHS does not work with XCFrameworks, they are used by older framework versions - I've already tested it without any luck.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes that must be a different type of framework path. Confusing!

@chrfalch
Copy link
Author

chrfalch commented Apr 2, 2024

Sorry for the delayed feedback here - been having a few days of during easter.

I can look into fixing moving release/debug paths into settings.config files - but we need to discuss the build process now that we make XCFrameworks.

Either we switch to always building all platforms, or we only build those defined by the input parameters and then add some logic to the build scripts to include the libraries that was actually built into XCFramework files.

What do you think?

@bejado
Copy link
Member

bejado commented Apr 4, 2024

Either we switch to always building all platforms

We definitely don't want to do that. It's common for us while developing to just build debug, i.e. run build.sh -i -p ios debug. The extra cost of building all platforms would be too high.

add some logic to the build scripts to include the libraries that was actually built into XCFramework files

I think this is the best solution. How does that sound?

chrfalch and others added 3 commits April 7, 2024 15:39
Since samples, pods etc. are now using XC Frameworks, we need to build them when building release modes, not just when asking if we should build simulator builds.

This fixes this and makes sure we build universal libs and xc frameworks correctly when building in release mode.
@chrfalch
Copy link
Author

chrfalch commented Apr 7, 2024

@bejado The last commit I pushed will now make sure we always build XCFrameworks in Release mode - making the builds to (hopefully) pass even though simulator builds was not requested!

It was not possible from my research to change the build scripts on iOS (project.yml etc) to support XCFrameworks in debug/release version - so they are now linked against the release versions.

Hope this is sufficient :)

@bejado
Copy link
Member

bejado commented Apr 11, 2024

It was not possible from my research to change the build scripts on iOS (project.yml etc) to support XCFrameworks in debug/release version

Do you know if this is a limitation of Xcode itself, or of XcodeGen (the Xcode project generator tool we use)?

so they are now linked against the release versions.

I think we're close to a solution, but unfortunately building and running our samples against a Debug build of Filament is important for us. Maybe we need to do a symlink or something.

Thanks for all your work on this, I'm happy to take a look myself when I get a chance.

@chrfalch
Copy link
Author

Do you know if this is a limitation of Xcode itself, or of XcodeGen (the Xcode project generator tool we use)?

To my knowledge this is an issue with how Xcode embeds XC Frameworks - not XCodeGen.

so they are now linked against the release versions.

I think we're close to a solution, but unfortunately building and running our samples against a Debug build of Filament is important for us. Maybe we need to do a symlink or something.

Yeah, let me think a bit on this one, I understand your use case - let me see if I can come up with something, maybe change the script that builds the samples or something.

Thanks for all your work on this, I'm happy to take a look myself when I get a chance.

It is super fun to be able to help :)

@BStringhamVRSK
Copy link
Contributor

Just adding my 2 cents of support for this PR to get approved and merged. We want to integrate ARM64 simulator testing into our development cycle. Is this feature still alive?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for iOS Apple Silicon Simulator
4 participants