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

Allow configuring iOS build with CMake #3234

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

Conversation

louwers
Copy link
Collaborator

@louwers louwers commented Feb 17, 2025

  • Allows configuring Xcode with CMake for iOS builds.
  • Add new job to ios-ci that builds the mbgl-core, ios-sdk-static and app CMake targets for iOS.
  • Move some files that are implementations for the core to platform/darwin/core to better separate out the iOS SDK.
  • Fix some build issues (unused/uninitialized variables).
  • Add license for SMCalloutView.
  • Fix typo MLN_PUBLIC_IOS_APP_SOPURCE.
  • Update MapLibre Developer Documentation to reflect changes in this PR.
  • Remove some unused files.
  • Add CMakePresets.json. Other platforms will be added in a follow-up PR. Info here: https://martin-fieber.de/blog/cmake-presets/

@louwers louwers requested a review from alexcristici February 17, 2025 15:32
Copy link

github-actions bot commented Feb 17, 2025

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0  -0.0%     -24    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-3234-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +31% +36.2Mi  +438% +26.2Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-3234-compared-to-legacy.txt

Copy link

github-actions bot commented Feb 17, 2025

Benchmark Results ⚡

Benchmark                                                     Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------
OVERALL_GEOMEAN                                            +0.0059         +0.0059             0             0             0             0

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/benchmark-results/pr-3234-compared-to-main.txt

Copy link

github-actions bot commented Feb 18, 2025

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%      +8  [ = ]       0    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-3234-compared-to-main.txt

@louwers louwers changed the title Allow configuring iOS build with CMake again Allow configuring iOS build with CMake Feb 23, 2025
@@ -252,6 +252,7 @@ void Drawable::draw(PaintParameters& parameters) const {
const auto stencilMode = enableStencil ? parameters.stencilModeForClipping(tileID->toUnwrapped())
: gfx::StencilMode::disabled();
impl->depthStencilState = context.makeDepthStencilState(depthMode, stencilMode, renderable);
assert(newStencilMode);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Issue: #3248

@@ -86,7 +86,6 @@ void cancel() {
public:
Impl(const ResourceOptions& resourceOptions_, const ClientOptions& clientOptions_)
: resourceOptions(resourceOptions_.clone()), clientOptions(clientOptions_.clone()) {
@autoreleasepool {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was causing a crash. I don't think this @autoreleasepool is needed because we use ARC?


add_custom_command(
OUTPUT ${MLN_GENERATED_DARWIN_STYLE_SOURCE} ${MLN_GENERATED_DARWIN_STYLE_HEADERS}
COMMAND ${BAZEL} build //platform/darwin:generated_code
Copy link
Collaborator Author

@louwers louwers Feb 23, 2025

Choose a reason for hiding this comment

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

I use Bazel to generate the code so I don't have to redo all that build logic in CMake... Also nice way to make sure that the output is the same as for the Bazel build.

@@ -0,0 +1,164 @@
enable_language(OBJC OBJCXX Swift)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Common code ios.cmake and macos.cmake.

@louwers louwers requested a review from cgalvan February 23, 2025 01:18
@TimSylvester
Copy link
Collaborator

I had to upgrade to the latest CMake, maybe the cmake_minimum_required should be increased?

The Team ID QMC7N4BDVZ is embedded in platform/ios/app/CMakeLists.txt. I guess that needs to come from a config file like config.bzl or environment variable.

@louwers
Copy link
Collaborator Author

louwers commented Feb 25, 2025

@TimSylvester Yes if you use the CMakePresets.json you need CMake 3.31.

@TimSylvester That's the one on CI. Yes I'll fix that.

@louwers louwers enabled auto-merge (squash) February 26, 2025 11:49
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.

3 participants