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 visionOS support #1384

Merged
merged 6 commits into from
Aug 29, 2024
Merged

Conversation

okwasniewski
Copy link
Contributor

@okwasniewski okwasniewski commented May 29, 2024

Hey,

This PR introduces visionOS windowed rendering for Babylon Native.

Recording

ScreenRecording_08-14-2024.12-57-06_1.MP4

TODO

NativeXr plugin integration will be a separate PR.

@okwasniewski
Copy link
Contributor Author

I'm not sure if the latest BGFX changes make Win32 fail or if it's something else... Probably it would be best to separate bgfx bump

@bghgary
Copy link
Contributor

bghgary commented Aug 5, 2024

I'm not sure if the latest BGFX changes make Win32 fail or if it's something else... Probably it would be best to separate bgfx bump

It's probably bgfx. We normally verify that bgfx is working before submitting a PR to the bgfx fork. Here is one of the errors:

##[error]buildUWP_arm64\_deps\bgfx.cmake-src\bgfx\src\shader_spirv.cpp(43,73): Error C2065: 'LiteralNumber': undeclared identifier
    98>D:\a\1\s\buildUWP_arm64\_deps\bgfx.cmake-src\bgfx\src\shader_spirv.cpp(43,73): error C2065: 'LiteralNumber': undeclared identifier [D:\a\1\s\buildUWP_arm64\_deps\bgfx.cmake-build\bgfx.vcxproj]
         (compiling source file 'CMakeFiles/bgfx.dir/Unity/unity_2_cxx.cxx')

@okwasniewski
Copy link
Contributor Author

okwasniewski commented Aug 6, 2024

Okay looks like this change fixed build issues:

diff --git a/src/shader_spirv.cpp b/src/shader_spirv.cpp
index dc4f12282..185fdfd02 100644
--- a/src/shader_spirv.cpp
+++ b/src/shader_spirv.cpp
@@ -17,12 +17,8 @@ namespace bgfx
 #define SPV_OPERAND_7(_a0, _a1, _a2, _a3, _a4, _a5, _a6) SPV_OPERAND_1(_a0), SPV_OPERAND_6(_a1, _a2, _a3, _a4, _a5, _a6)
 #define SPV_OPERAND_8(_a0, _a1, _a2, _a3, _a4, _a5, _a6, _a7) SPV_OPERAND_1(_a0), SPV_OPERAND_7(_a1, _a2, _a3, _a4, _a5, _a6, _a7)
 #define SPV_OPERAND_9(_a0, _a1, _a2, _a3, _a4, _a5, _a6, _a7, _a8) SPV_OPERAND_1(_a0), SPV_OPERAND_8(_a1, _a2, _a3, _a4, _a5, _a6, _a7, _a8)
-#if BX_COMPILER_MSVC
-// Workaround MSVS bug...
-#	define SPV_OPERAND(...) { BX_MACRO_DISPATCHER(SPV_OPERAND_, __VA_ARGS__) BX_VA_ARGS_PASS(__VA_ARGS__) }
-#else
-#	define SPV_OPERAND(...) { BX_MACRO_DISPATCHER(SPV_OPERAND_, __VA_ARGS__)(__VA_ARGS__) }
-#endif // BX_COMPILER_MSVC
+#define SPV_OPERAND(...) { BX_MACRO_DISPATCHER(SPV_OPERAND_, __VA_ARGS__)(__VA_ARGS__) }
+
 #define _ Count
 
 	bool isDebug(SpvOpcode::Enum _opcode)

Here is a PR bringing this back: BabylonJS/bgfx#39

@okwasniewski
Copy link
Contributor Author

Hey, FYI we will first make windowed rendering work in this PR and then create a separate one with the NativeXr plugin.

@okwasniewski okwasniewski force-pushed the feat/visionos branch 3 times, most recently from 84bad1e to c3d27f1 Compare August 14, 2024 11:30
@okwasniewski
Copy link
Contributor Author

Hey @ryantrem @CedricGuillemet this one is ready for re-review!

Copy link
Contributor

@docEdub docEdub left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. I'm getting ready to test on-device, now. Please add a link in README.md to the new VisionOS section in BUILDING.md.

@docEdub
Copy link
Contributor

docEdub commented Aug 28, 2024

I'm getting a bgfx build error

[...]/build/visionOS/_deps/bgfx.cmake-src/bgfx/src/renderer_mtl.mm:3528:5: error: 'MTKView' is unavailable: not available on visionOS
                                MTKView *view = (MTKView *)_nwh;
                                ^

@okwasniewski
Copy link
Contributor Author

@docEdub

Which version of CMake are you using? Can you try with at least 3.29.0?

And also just to make sure did you use this command to generate project?

cmake -B build/visionOS -G Xcode -D VISIONOS=ON

@ryantrem
Copy link
Member

@okwasniewski in the PR description you have a link to a closed bgfx.cmake PR. Will you update that to point to the merged PR(s) so we have a good record of all the related changes across repos?

@okwasniewski
Copy link
Contributor Author

@okwasniewski in the PR description you have a link to a closed bgfx.cmake PR. Will you update that to point to the merged PR(s) so we have a good record of all the related changes across repos?

Fixed

@docEdub
Copy link
Contributor

docEdub commented Aug 28, 2024

@docEdub

Which version of CMake are you using? Can you try with at least 3.29.0?

And also just to make sure did you use this command to generate project?

cmake -B build/visionOS -G Xcode -D VISIONOS=ON

I'm have CMake 3.29.3 installed and I'm using cmake -B build/visionOS -G Xcode -D VISIONOS=ON to generate the Xcode project.

@docEdub
Copy link
Contributor

docEdub commented Aug 28, 2024

I'm getting a bgfx build error

[...]/build/visionOS/_deps/bgfx.cmake-src/bgfx/src/renderer_mtl.mm:3528:5: error: 'MTKView' is unavailable: not available on visionOS
                                MTKView *view = (MTKView *)_nwh;
                                ^

Is MTKView expected to be available? ...or is this section of renderer_mtl.mm supposed to be skipped for VisionOS builds?

@okwasniewski
Copy link
Contributor Author

MTKView *view = (MTKView *)_nwh;

It's supposed to be skipped, in that file (renderer_mtl.mm) from BGFX do you have any signs of BX_PLATFORM_VISIONOS macro? Maybe the dependency got cached somehow?

@okwasniewski
Copy link
Contributor Author

I just did another test of installing fresh deps:

rm -rf build/visionOS

And then:

cmake -B build/visionOS -G Xcode -D VISIONOS=ON

And everything worked fine:
CleanShot 2024-08-28 at 16 41 38@2x

@docEdub
Copy link
Contributor

docEdub commented Aug 28, 2024

I just did another test of installing fresh deps:

rm -rf build/visionOS

And then:

cmake -B build/visionOS -G Xcode -D VISIONOS=ON

And everything worked fine: CleanShot 2024-08-28 at 16 41 38@2x

Which visionOS SDK are you using? I'm using visionOS SDK 1.2.

@okwasniewski
Copy link
Contributor Author

Oh okay I got the issue, I was running on visionOS 2.0 and switched to older Xcode for simulator build and got the same issue as you did.

Damn I guess I need to submit one more PR to bgfx. Going to work on this tomorrow

@okwasniewski
Copy link
Contributor Author

@docEdub Can you try if adding ifdefs to this piece of code make bgfx compile? Im hoping you could test out Babylon Native while we wait for this PR to get merged into bgfx

#if !BX_PLATFORM_VISIONOS
			if (NULL != NSClassFromString(@"MTKView") )
			{
				MTKView *view = (MTKView *)_nwh;
				
				if (NULL != view
					&&  [view isKindOfClass:NSClassFromString(@"MTKView")])
				{
					m_metalLayer = (CAMetalLayer *)view.layer;
				}
			}
#endif

@okwasniewski
Copy link
Contributor Author

Here is the PR which should fix it: bkaradzic/bgfx#3347 going to do sync one more time once it merges

@docEdub
Copy link
Contributor

docEdub commented Aug 28, 2024

@docEdub Can you try if adding ifdefs to this piece of code make bgfx compile? Im hoping you could test out Babylon Native while we wait for this PR to get merged into bgfx

#if !BX_PLATFORM_VISIONOS
			if (NULL != NSClassFromString(@"MTKView") )
			{
				MTKView *view = (MTKView *)_nwh;
				
				if (NULL != view
					&&  [view isKindOfClass:NSClassFromString(@"MTKView")])
				{
					m_metalLayer = (CAMetalLayer *)view.layer;
				}
			}
#endif

Yep, that gets it building and it's working on device. Nice!

@ryantrem
Copy link
Member

Looks like we're good to go on this one?

Copy link
Contributor

@docEdub docEdub left a comment

Choose a reason for hiding this comment

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

👍 LGTM. Tested on device with most recent changes and it works as expected.

@docEdub docEdub merged commit fdcd2f9 into BabylonJS:master Aug 29, 2024
19 checks passed
@bghgary
Copy link
Contributor

bghgary commented Aug 29, 2024

Congrats @okwasniewski (and @matthargett) on getting this in! Thanks for the contribution!

@matthargett
Copy link

Thanks for your partnership (and patience)! Next up is getting Babylon React Native to support React Native visionOS, then immersive mode and 6DOF head tracking 🥽

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.

6 participants