nap icon indicating copy to clipboard operation
nap copied to clipboard

Make use of CMAKE_OSX_ARCHITECTURES variable

Open vykhovanets opened this issue 3 years ago • 12 comments

  • 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 avatar Jul 04 '21 19:07 vykhovanets

Similar change may be required for module creator template too

vykhovanets avatar Jul 04 '21 19:07 vykhovanets

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.

cklosters avatar Jul 05 '21 17:07 cklosters

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

vykhovanets avatar Jul 05 '21 18:07 vykhovanets

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:

  1. Make a Framework Release built on x86-64 usable on ARM (via cross complication/emulation on the ARM device)
  2. Make it possible to package a (x86-64) Framework Release on an ARM machine (cross compiled)
  3. Maybe: Make it possible to cross compile a Source build on ARM
  4. 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.

cheywood avatar Jul 06 '21 03:07 cheywood

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

cklosters avatar Jul 06 '21 16:07 cklosters

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 avatar Jul 13 '21 08:07 vykhovanets

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

  1. 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. 2. audioanalysis release build can't run because of Internal PortAudio error (can't connect to my speakers)

vykhovanets avatar Oct 28 '21 21:10 vykhovanets

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).

cklosters avatar Nov 10 '21 16:11 cklosters

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 avatar Nov 10 '21 22:11 vykhovanets

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.

vykhovanets avatar Nov 17 '21 16:11 vykhovanets

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?

cklosters avatar Nov 18 '21 09:11 cklosters

Hi, @cklosters !

The exact message from Xcode is (audio playback demo): Знімок екрана 2021-11-18 о 12 23 21

  • 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.

vykhovanets avatar Nov 18 '21 10:11 vykhovanets