BabylonReactNative icon indicating copy to clipboard operation
BabylonReactNative copied to clipboard

Remove AR from core Babylon React Native packages (in favor of plugin model)

Open nima-ap opened this issue 2 years ago • 5 comments

Is your feature request related to a problem? Please describe.

Babylon React Native provides a wide array of functionalities, including AR scenes. However, not all applications that might require the core rendering functionalities of Babylon on React Native might utilize advanced functionalities such as rendering an AR scene.

Due to the nature of AR requiring access to additional hardware, such as the camera and motion sensors, even if the application makes no usage of Babylon's AR functionalities, they must take extra steps to ensure adherence to public store policies like those found on Google Play and Apple App Store. Google is more relaxed about this but Apple requires that you explain camera usage even if the runtime does not use it.

As you can imagine, large apps might not be inclined to request additional permissions or include unnecessary metadata in their manifests that claim they use AR while they don't, and so this can be a serious blocker for usage.

Describe the solution you'd like All AR functionality and dependencies removed from the core Babylon React Native packages, moved into separate package(s) for use by consumers who wish to utilize that functionality.

This also has additional wins for the core packages including:

  • reduced bundle size
  • removal of peer-dependency to RN Permissions library

nima-ap avatar Aug 18 '23 17:08 nima-ap

This would apply equally to other features that require device permissions, like the the camera polyfill, correct?

Also, we already have an explosion of packages due to splitting the package per RN version and platform. Splitting out feature level packages would make this even worse. In the past, we’ve talked about the idea of having just a single NPM package, and platform specific packages for each platform and RN version that are downloaded at build time. For example, we’d have Maven packages published for Android, Cocoa Pods for iOS, and Nuget for Windows. This would hide the platform and RN version complexities, and would result in just one NPM package. If we have this setup, then splitting out feature level packages seems like it would be more manageable. We just end up with a few npm packages like @babylonjs/react-native, @bsbylonjs/react-native-ar, etc.

Thoughts?

ryantrem avatar Aug 19 '23 13:08 ryantrem

can we have a patch package to remove the use of AR since some people like me do not use the AR feature at all? but requires the user to grant the camera permission when not using it is not fine at all

namluu25 avatar Aug 22 '23 02:08 namluu25

I don't see a short term solution except releasing a new specific package without AR/Camera. This would add a new package per RN version and platform. I believe this change is one of many other linked needs. Like, as said Ryan, reducing the package count explosion, reducing package size (limit is 200Mb, Windows package size is >150Mb for 1 RN version) but also reducing build times (BabylonNative static libraries are built once per RN version for example) It's not possible to increase package size with different RN version, it's not possible to have more plugins without increasing build times, etc ... Having BabylonNative prebuilt would help reducing the build times but some libraries depending on JS engine need to be built specifically (see https://github.com/BabylonJS/BabylonNative/issues/1270 ) This looks like weeks or months of work. I guess we need a short term solution before.

EDIT: I think we discussed distributing source code (or at least the commands to download sources) and build BN/BRN along the app. what was its cons?

EDIT2: After looking at the build pipeline, it looks to me easier to add a new package for ios-android and windows, maybe just for a few RN versions that will contain just the engine without Camera polyfill or XR. We need to discuss the steps but that new package can be the default one and xr/camera plugins be optional is other repo/npm packages.

CedricGuillemet avatar Sep 04 '23 13:09 CedricGuillemet

I'm thinking about a simple way to get this plugin model to work. My idea is to have XR and camera code in main NPM. But it would not compile and link unless there is some other npm installed. Main NPM contains a file EnableXR.h that is empty. XR NPM contains a header named the same but containing #define EnableXR Depending on include paths order, 1 or the other is included. In the same vain, in the cmakelists.txt, if directory exists for XR plugin package, list of libraries to link are changed. On Windows, as it's using a vcxproj, this can be done with a #pragma comment(lib.... in the EnableXR.h header

CedricGuillemet avatar Nov 14 '23 15:11 CedricGuillemet

@CedricGuillemet I think having 'latent' code in the main package is totally fine, especially if its in a separate dll / so that can just be probed for at runtime.

My concern though is using a separate NPM package to control the linking and includes. Maybe this is easiest but I also wonder if that point, there should just be documentation on the main NPM package on what environment variables or config settings (maybe at build time you could read from something?) to control this as opposed to having another (empty) package be this config value

bbowman avatar Nov 14 '23 17:11 bbowman

Closing in favor of #614

bghgary avatar Sep 10 '24 22:09 bghgary