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

Android render #30

Closed
wants to merge 71 commits into from

Conversation

CedricGuillemet
Copy link
Contributor

@CedricGuillemet CedricGuillemet commented Aug 19, 2019

New features

  • Android Native activity. Basically looks like a Win32 project (messages, main,..). This second Android project shows how to use libnative without the Java activity and content view. Depending on the needs, user might chose one or the other as a startup example
  • width/height parameters for the runtime init. Suggest to change. I added the parameters because I needed them for the activity init.
  • bgfx callback : bgfx trace messages are now routed to the console. testapp has to set the correct output depending on the platform . Outputting done for Android and Win32
  • debug output of backend rendering api
  • screen rotation support
  • shader preprocessor: shaders work out of the box on d3d11, opengl, opengles
  • library/bgfx/… build is handled by Android studio. No more the need to generate ninja build files

Issues

- face winding seem inverted with opengl. I didn't find any flag in bgfx that declares it. Not a bug issue for now
- Vertex Attribute names: I had to change it manually because by default they are not remapped (and apparently no remapping functions in glslang)
- Uniform struct: for VS and FS, uniforms are part of a structure called 'Frame'. There is a size conflict (and content) during linking. I'll try to make it work by renaming both structure (FrameVS and FrameFS) but another solution is needed
- uniform types: bgfx only allow vec4/mat4 uniforms. Any other type results in a runtime error and no update of the uniforms. I can't think of another solution of using an AST and parsing the shader source. Any idea?
- Shaders are not used the same depending on Android and Windows Opengl. For Windows, I made it work by adding #version 430. Not a stopping issue.

  • Android event management/life cycle/app switching,... Main concern is the context switching. When switching to another app, the window (and its context) are destroyed. At least with the Java activity. I'll check with the native activity to see if it behaves the same. --> will be fixed in another PR

Android Java activity project update
@syntheticmagus
Copy link
Contributor

Looks like Git has confused itself somehow about Win32/App.cpp. Can we revert whatever it thinks it did to that file?

Library/CMakeLists.txt Outdated Show resolved Hide resolved
Library/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -10,7 +10,8 @@ namespace babylon
{
public:
explicit RuntimeAndroid(ANativeWindow* nativeWindowPtr);
explicit RuntimeAndroid(ANativeWindow* nativeWindowPtr, const std::string& rootUrl);
explicit RuntimeAndroid(ANativeWindow* nativeWindowPtr, const std::string& rootUrl, uint32_t width, uint32_t height);
Copy link
Contributor

Choose a reason for hiding this comment

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

@bghgary API change. I think I like this better than what we were doing before, which (I think) was to call UpdateSize() immediately from outside. Is there a case where you want to create a renderer, but don't know the size yet? If so, could we just add some defaults here?

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this change? Is there not a way to get the width and height of the native window on Android?

Library/Source/Console.h Outdated Show resolved Hide resolved
Library/Source/NativeEngine.cpp Outdated Show resolved Hide resolved
Library/Source/ShaderCompilerOpenGL.cpp Outdated Show resolved Hide resolved
Library/Source/NativeEngine.cpp Outdated Show resolved Hide resolved
Library/Source/ShaderCompilerOpenGL.cpp Outdated Show resolved Hide resolved
Library/Source/ShaderCompilerOpenGL.cpp Outdated Show resolved Hide resolved
TestApp/Source/Android/app/CMakeLists.txt Outdated Show resolved Hide resolved
…nto AndroidRender

# Conflicts:
#	Library/CMakeLists.txt
#	TestApp/Source/Win32/App.cpp
Library/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -74,7 +74,7 @@ target_include_directories(napi PUBLIC "include")

if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Chakra")
target_link_libraries(napi PRIVATE "chakrart.lib")
elseif(NAPI_JAVASCRIPT_ENGINE STREQUAL "V8")
elseif(NAPI_JAVASCRIPT_ENGINE STREQUAL "V8" AND NOT ANDROID)
target_link_libraries(napi PRIVATE v8)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should rename the v8 target to v8windows or something

@@ -10,7 +10,8 @@ namespace babylon
{
public:
explicit RuntimeAndroid(ANativeWindow* nativeWindowPtr);
explicit RuntimeAndroid(ANativeWindow* nativeWindowPtr, const std::string& rootUrl);
explicit RuntimeAndroid(ANativeWindow* nativeWindowPtr, const std::string& rootUrl, uint32_t width, uint32_t height);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this change? Is there not a way to get the width and height of the native window on Android?

@@ -377,19 +377,25 @@ namespace babylon

bx::DefaultAllocator m_allocator;
uint64_t m_engineState;

void *m_nativeWindow{};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void *m_nativeWindow{};
void* m_nativeWindow{};

…nto AndroidRender

# Conflicts:
#	Library/CMakeLists.txt
#	Library/Source/NativeEngine.cpp
#	TestApp/Scripts/babylon.max.js
…nto AndroidRender

# Conflicts:
#	Library/CMakeLists.txt
…nNative into AndroidRender

# Conflicts:
#	Library/Dependencies/bgfx.cmake
#	Library/Source/RuntimeAndroid.cpp
#	TestApp/Source/Android/app/CMakeLists.txt
#	TestApp/Source/Android/app/build.gradle
@CedricGuillemet
Copy link
Contributor Author

Closing PR as it's not meant to be merged in master.

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.

None yet

3 participants