Make use of CMAKE_OSX_ARCHITECTURES variable

vykhovanets commented 3 years ago

if (APPLE) is skipped because this variable is ignored on other OSes
this line is placed before project statement according to CMake docs
enables APPLE branch in targetarch.cmake
this change is made because all compiled libraries for MacOS are based on x86_64 architecture
this option makes possible to cross-compile and run NAP-based project on Apple Silicon

vykhovanets commented 3 years ago

Similar change may be required for module creator template too

cklosters commented 3 years ago

Thanks for the PR! What do you think @cheywood ?

From what I can tell this change only works when creating a project against a packaged release of NAP. Demos & Apps in source will not be included, am I correct? In that case this change won't result in the requested behavior across the entire code base, including source. That's a requirement unfortunately. I can't test this change because I don't have an M1 and we currently only officially support 'Catalina'. However, package validation succeeds.

Regarding this change applied to the module creator template: I don't think that's necessary because the property is applied to the project and cached.

vykhovanets commented 3 years ago

I completely agree with you! I checked demos and understood that this template is required to be changed in many places, so it is not an option. I am going to place this flag in regenerate script and update commit

cheywood commented 3 years ago

Hey, yeah, unfortunately no M1 here to test with either, but if changes are needed to allow x86-64-built NAP and its projects to work on M1 that sounds like work worth doing.

We could look at some high level phases of macOS ARM support being:

Make a Framework Release built on x86-64 usable on ARM (via cross complication/emulation on the ARM device)
Make it possible to package a (x86-64) Framework Release on an ARM machine (cross compiled)
Maybe: Make it possible to cross compile a Source build on ARM
Native ARM support: The full changes to be able to compile and run as native ARM in all contexts (Source, Framework Release, and Packaged Apps)

What we're talking about with this PR addresses the first phase, or part of it. I think it makes sense for the first PR for M1 to include all of the first phase. It should pass the packaging dependencies tester on M1 (if all current libraries will work emulated). Changes similar to what's in this PR would be required if addressing phases 1 through 3.

However as Coen noted, at the moment NAP currently targets Catalina so either ARM changes would wait for macOS v11.0 support or would be unofficial for now.

Another take on all of this would be to skip the effort involved in phases 1 thru 3 and only, at some point, do 4. But hopefully #1 at least might not be too much work.

cklosters commented 3 years ago

Thanks for taking the time to reply @vykhovanets and @cheywood.

We're currently discussing the option to provide native ARM support, but until then, having a Framework Release built on x86-64 usable on ARM (via cross complication/emulation on the ARM device) is a good compromise. For that to work the package must be validated, but since we don't have an M1 and we're currently on Catalina we can't do this. But if we tag the functionality as 'experimental', I'm willing to accept potential changes, until we can properly build and validate NAP on M1 systems. As long as package validation succeeds for the current supported setup.

@vykhovanets: I hope this feedback helps, looking forward to your changes. To test them, you can always run the packaged dependencies tester.py in build_tools\packaged_dependencies_tester, by pointing it to the extracted packaged version of NAP:

python3 packaged_dependencies_tester.py ../../NAP-0.4.2-Win64

vykhovanets commented 3 years ago

Hello, @cklosters and @cheywood ! Thank you for your responses!I am going to look into this, but can be not so fast, because of my travel right now.

Also about full M1 support - I am able to compile Qt5.15.2 for arm. I want to look into remaining libs, and try to make native binaries.

vykhovanets commented 2 years ago

Hello! After running packaged dependencies tester.py everything is working except:

artnetreceive debug build can't run because of
```
Failed to load and deserialize artnetreceive.json
Failed to extract primitive type: Mode, object: Window
```
I will look into this error.. with release app everything is correct.
audioanalysis release build can't run because of Internal PortAudio error (can't connect to my speakers)

cklosters commented 2 years ago

Thanks for the update @vykhovanets.

The 'Mode' property is an rtti defined enum in nap::RenderWindow and should be de-serializable.

I think it fails because it can't convert the basic (deserialized) value into the extracted property type (jsonreader.cpp, line 275):

default:
{
    // Basic JSON type, read value and copy to target
    rtti::Variant extracted_value = readBasicType(json_value);
    if (!errorState.check(extracted_value.convert(wrapped_type),
        "Failed to extract primitive type: %s, object: %s", readState.mCurrentRTTIPath.toString().c_str(),
        rootObject->mID.c_str()))
    {
        if (readState.mCurrentRTTIPath.toString() == "Type")
            errorState.fail("Type is a reserved keyword");
        return false;
    }

    property.set_value(compound, extracted_value);
}

Why this is I can't say, but worth stepping through the code from there. Looks related to I could build helloworld with as well as without the flag, however it fails to deserialize enums from the app structure somehow when trying to run: https://community.napframework.com/t/possibility-to-build-for-m1-mac/344/5

I also expect this error to occur with other demos that declare a nap::RenderWindow together with the mode property (in json).

vykhovanets commented 2 years ago

To be able to look into this issue further I built NAP Framework from source (phase 3) and packaged it (phase 2)

And now I can reproduce the issue mentioned by Stijn. I think here I had encountered the same issue.

vykhovanets commented 2 years ago

Issue with port audio can be resolved by updating library version in thirdparty repo

Audioanalysis and AudioPlayback demos are now working on macOS bigsur/monterey with M1 chip.

cklosters commented 2 years ago

Issue with port audio can be resolved by updating library version in thirdparty repo

Audioanalysis and AudioPlayback demos are now working on macOS bigsur/monterey with M1 chip.

Thanks for the feedback @vykhovanets, updating the portaudio third party library is already on our TODO list (NAP 0.5.0), I'm assigning it higher priority based on your findings. Do you know what bug it resolved specifically?

vykhovanets commented 2 years ago

Hi, @cklosters !

The exact message from Xcode is (audio playback demo):

I think the resolution of this issue was in this MR, but I checked this after linking against newer version of PortAudio.
this is another MR

So the bug was in config script, when you set SDK version to 11.0 the version of MacOS was dropped to 10.4, as a result deprecated API was chosen. No problems with Catalina though.

cklosters commented 8 months ago

We decided to officially deprecate and remove macOS as a target from NAP 0.7 onwards. We are not mac developers and don't have the time (and resources) to properly support it. @stijnvanbeek will continue supporting macOS on his own fork but unless properly tested and maintained for the full NAP release we won't merge it back in. To prevent code rot we will start to remove macOS directives from the code base.

We will continue support for Windows (x86_64) and Linux (x86_64, armhf, arm64).

napframework / nap

Make use of CMAKE_OSX_ARCHITECTURES variable #8