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

M1 support #112

Closed
wants to merge 2 commits into from
Closed

M1 support #112

wants to merge 2 commits into from

Conversation

Tom-Ski
Copy link

@Tom-Ski Tom-Ski commented Feb 11, 2022

  • GHA is building arm64 slices
  • Library loader detecting arm and 64bit to change choice of native lib
  • gdx-jnigen is updated to 2.2.1
  • Freetype is now being compiled and built in GHA into a fat library, and then used to link against when building the bindings.

Copy link
Owner

@SpaiR SpaiR left a comment

Choose a reason for hiding this comment

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

You need to configure an additional matrix build for arm. As it's done for all platforms. Like, no need to build a separate freetype for an amd build. As well as to configure an arm workaround when there is no need of it. Also, check job artefacts and you'll see that now there is only one dylib - the arm one. When it should be two of them.

And don't add to the PR compiled arm binary, they are updated through the CI.

@SpaiR SpaiR added the feat New feature or request label Feb 12, 2022
@Tom-Ski
Copy link
Author

Tom-Ski commented Feb 12, 2022

Do you want to separate this at the generateLibs level? Atm type from the matrix is passed directly in, but another entry for arm will need that part adapting too.

Or should we just do some logic and feed in 'mac' still to generateLibs for that arm type?

Im not sure what you mean by not needing a separate build for arm, that is required. Do you mean no need to separate it when adding a new entry to the build matrix?

Comment on lines +69 to +90
- if: matrix.os == 'macos-latest'
name: Build Setup Mac xcode
run: |
# See https://github.com/actions/virtual-environments/issues/2557
sudo mv /Library/Developer/CommandLineTools/SDKs/* /tmp
sudo mv /Applications/Xcode.app /Applications/Xcode.app.bak
sudo mv /Applications/Xcode_12.4.app /Applications/Xcode.app
sudo xcode-select -switch /Applications/Xcode.app
/usr/bin/xcodebuild -version
sudo mkdir /opt/freetype

- if: matrix.os == 'macos-latest' && matrix.freetype == true
name: Download FreeType - Compile & Install
working-directory: /opt/freetype
run: |
# freetype download and untar and install
sudo wget -O freetype.tar.gz ${{ env.FREETYPE_URL }}
sudo tar -xzf freetype.tar.gz -C . --strip-components=1

sudo ./configure CFLAGS="-arch arm64 -arch x86_64" --with-zlib=no --with-bzip2=no --with-png=no --with-harfbuzz=no --with-brotli=no
sudo make
sudo make install
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need it for amd64 builds.

@@ -25,7 +25,10 @@
public class ImGui {
private static final String LIB_PATH_PROP = "imgui.library.path";
private static final String LIB_NAME_PROP = "imgui.library.name";
private static final String LIB_NAME_DEFAULT = System.getProperty("os.arch").contains("64") ? "imgui-java64" : "imgui-java";
private static final String LIB_NAME_BASE = "imgui-java";
private static final boolean is64Bit = System.getProperty("os.arch").contains("64") || System.getProperty("os.arch").startsWith("armv8");
Copy link
Owner

Choose a reason for hiding this comment

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

Can be simplified: if it's no ARM, then it's 64 for sure.

@@ -137,6 +147,10 @@ class GenerateLibs extends DefaultTask {
break
case BuildTarget.TargetOs.MacOsX:
target.cppFlags += ' -I/usr/local/include/freetype2'
if (isARM) {
//For GHA
target.cppFlags += ' -I/usr/local/arm64/include/freetype2'
Copy link
Owner

Choose a reason for hiding this comment

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

That what I was talking about: split builds for different archs.

@nCore
Copy link

nCore commented Mar 2, 2024

hey, any news on that topic? :)

@SpaiR
Copy link
Owner

SpaiR commented Aug 4, 2024

Closing in favor of #223
I'll mention this PR in changelog as well. Thanks for the contrib! 🙏

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

Successfully merging this pull request may close these issues.

3 participants