Asset loading #13

hannojg · 2024-02-26T17:23:15Z

Todo

iOS asset loading bridgen
Rename loadAsset in proxy to smth else
Clean up create indirect light function
Fix assets / resources not loading completely
Set camera focal length / point correctly

Changes

Goal of this PR is to make it possible to load assets and add them to the scene.
Note that we also need assets to load ktx files for lighting.

Right now the assets need to be bundled within the app's assets / bundle (there is no loading from JS code yet).

…feat/first-render-api

…feat/asset-loader

hannojg · 2024-03-02T11:00:56Z

package/android/src/main/java/com/margelo/filament/FilamentProxy.java

+ // Allocate a *direct* ByteBuffer and put the bytes into it.
+ ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.length);
+ buffer.put(bytes);
+ // Reset position to 0 to be ready for reading
+ buffer.flip();


@mrousavy I had to do this for the buffer to work as I am using fbjni ByteBuffer implementation which only supports direct(?) buffers. And I think in this step I am copying the buffer again.
In general I am wondering whether I am doing this here the most efficient way (I doubt it). Would appreciate if you could take a stab at making this as copy-less as possible 😄

mrousavy · 2024-03-04T13:32:06Z

package/android/src/main/cpp/java-bindings/JFilamentProxy.cpp

+std::shared_ptr<FilamentBuffer> JFilamentProxy::getAssetByteBuffer(const std::string& path) {
+ static const auto method = javaClassLocal()->getMethod<jni::alias_ref<jni::JByteBuffer>(jni::alias_ref<jstring>)>("getAssetByteBuffer");
+ jni::local_ref<jni::JByteBuffer> localRef = method(_javaPart, jni::make_jstring(path));
+ jni::global_ref<jni::JByteBuffer> globalRef = jni::make_global(localRef);


memory leak!

mrousavy · 2024-03-04T13:33:01Z

package/android/src/main/java/com/margelo/filament/FilamentProxy.java

- return 13;
+ ByteBuffer getAssetByteBuffer(String assetName) throws IOException {
+ InputStream input = reactContext.getAssets().open(assetName);
+ byte[] bytes = new byte[input.available()];


allocates buffer twice!

I think I found a better solution for it, please have a look

mrousavy · 2024-03-04T13:37:34Z

package/cpp/core/EngineWrapper.cpp

@@ -24,6 +35,13 @@ EngineWrapper::EngineWrapper(std::shared_ptr<Choreographer> choreographer) {
 _engine = References<Engine>::adoptRef(Engine::create(), [](Engine* engine) { Engine::destroy(&engine); });
 _materialProvider = gltfio::createUbershaderProvider(_engine.get(), UBERARCHIVE_DEFAULT_DATA, UBERARCHIVE_DEFAULT_SIZE);
 _assetLoader = gltfio::AssetLoader::create(filament::gltfio::AssetConfiguration{.engine = _engine.get(), .materials = _materialProvider});
+ _resourceLoader = new filament::gltfio::ResourceLoader({.engine = _engine.get(), .normalizeSkinningWeights = true});


memory leak!

References<T>::

Converted _materialProvider, _assetLoader and _resourceLoader to be shared_ptrs using the references library now!

mrousavy · 2024-03-04T13:39:52Z

package/cpp/core/EngineWrapper.cpp

-void EngineWrapper::createDefaultLight() {
- // Create default directional light (In ModelViewer this is the default, so we use it here as well)
+void EngineWrapper::loadAsset(std::shared_ptr<FilamentBuffer> modelBuffer) {
+ filament::gltfio::FilamentAsset* asset = _assetLoader->createAsset(modelBuffer->getData(), modelBuffer->getSize());


how does cleanup work here? it's a pointer, does it need to be delete'd?

To use the References I feel like we'd need a AdoptAssetLoader reference 🤔

mrousavy · 2024-03-04T13:42:21Z

package/cpp/core/EngineWrapper.cpp

+ Texture* cubemap = ktxreader::Ktx1Reader::createTexture(
+ _engine.get(), *iblBundle, false,
+ [](void* userdata) {
+ auto* bundle = (image::Ktx1Bundle*)userdata;


Use dynamic_cast and check for null (sucessful cast)

package/cpp/core/EngineWrapper.cpp

mrousavy · 2024-03-05T09:35:35Z

package/android/src/main/java/com/margelo/filament/FilamentProxy.java

- buffer.put(bytes);
+ // Create a channel from the input stream
+ ReadableByteChannel channel = Channels.newChannel(input);
+ int estimatedSize = input.available(); // This is not always accurate for the actual size


what? 😄 you cannot estimate an array size - this needs to be the exact number.

@mrousavy

>[!WARNING] > Based on the following PR which should be merged first > - #13 **Goal:** To provide an API from JS that can be used to setup the scene and control the transform of the elements. (Most important) **Changes:** - **Camera:** - Renamed `lookAt` -> `lookAtCameraManipulator` Can be used to make camera look at camera manipulator - New `lookAt` function, which can be used to manually control the position, target and up vector of the camera - New `setLensProjection` & `setProjection` function to Control the lens configuration from JS - **⚠️ Note:** @mrousavy The user would also need to call this when the surface size changes (as we need to pass the aspect ratio to this function). Lets discuss in a quick meeting how to solve this best - **Engine:** - Renamed `createDefaultLight` -> `setIndirectLight`, only sets the indirect light from an FilamentBuffer now (decouples the function from setting a default directional light, which can be now configured by the user) - New `createLightEntity` Function to create a light with the configuration you want - Exposed `transformToUnitCube`. Pass an asset and it will transform the asset to fit into a unit cube - New `setEntityPosition`, `setEntityRotation` and `setEntityScale` to set or update an entities transform - `loadAsset` now returns a `FilamentAsset`. The asset can be used for various operations in the scene. --------- Co-authored-by: Marc Rousavy <me@mrousavy.com>

This reverts commit 9dde746.

@mrousavy

>[!WARNING] > Based on the following PR which should be merged first > - #13 **Goal:** To provide an API from JS that can be used to setup the scene and control the transform of the elements. (Most important) **Changes:** - **Camera:** - Renamed `lookAt` -> `lookAtCameraManipulator` Can be used to make camera look at camera manipulator - New `lookAt` function, which can be used to manually control the position, target and up vector of the camera - New `setLensProjection` & `setProjection` function to Control the lens configuration from JS - **⚠️ Note:** @mrousavy The user would also need to call this when the surface size changes (as we need to pass the aspect ratio to this function). Lets discuss in a quick meeting how to solve this best - **Engine:** - Renamed `createDefaultLight` -> `setIndirectLight`, only sets the indirect light from an FilamentBuffer now (decouples the function from setting a default directional light, which can be now configured by the user) - New `createLightEntity` Function to create a light with the configuration you want - Exposed `transformToUnitCube`. Pass an asset and it will transform the asset to fit into a unit cube - New `setEntityPosition`, `setEntityRotation` and `setEntityScale` to set or update an entities transform - `loadAsset` now returns a `FilamentAsset`. The asset can be used for various operations in the scene. --------- Co-authored-by: Marc Rousavy <me@mrousavy.com>

@mrousavy

>[!WARNING] > Based on the following PR which should be merged first > - #13 **Goal:** To provide an API from JS that can be used to setup the scene and control the transform of the elements. (Most important) **Changes:** - **Camera:** - Renamed `lookAt` -> `lookAtCameraManipulator` Can be used to make camera look at camera manipulator - New `lookAt` function, which can be used to manually control the position, target and up vector of the camera - New `setLensProjection` & `setProjection` function to Control the lens configuration from JS - **⚠️ Note:** @mrousavy The user would also need to call this when the surface size changes (as we need to pass the aspect ratio to this function). Lets discuss in a quick meeting how to solve this best - **Engine:** - Renamed `createDefaultLight` -> `setIndirectLight`, only sets the indirect light from an FilamentBuffer now (decouples the function from setting a default directional light, which can be now configured by the user) - New `createLightEntity` Function to create a light with the configuration you want - Exposed `transformToUnitCube`. Pass an asset and it will transform the asset to fit into a unit cube - New `setEntityPosition`, `setEntityRotation` and `setEntityScale` to set or update an entities transform - `loadAsset` now returns a `FilamentAsset`. The asset can be used for various operations in the scene. --------- --------- Co-authored-by: Hanno J. Gödecke <hanno@margelo.io> Co-authored-by: Hanno J. Gödecke <die.drei99@yahoo.de>

@mrousavy

>[!WARNING] > Based on the following PR which should be merged first > - #13 **Goal:** To provide an API from JS that can be used to setup the scene and control the transform of the elements. (Most important) **Changes:** - **Camera:** - Renamed `lookAt` -> `lookAtCameraManipulator` Can be used to make camera look at camera manipulator - New `lookAt` function, which can be used to manually control the position, target and up vector of the camera - New `setLensProjection` & `setProjection` function to Control the lens configuration from JS - **⚠️ Note:** @mrousavy The user would also need to call this when the surface size changes (as we need to pass the aspect ratio to this function). Lets discuss in a quick meeting how to solve this best - **Engine:** - Renamed `createDefaultLight` -> `setIndirectLight`, only sets the indirect light from an FilamentBuffer now (decouples the function from setting a default directional light, which can be now configured by the user) - New `createLightEntity` Function to create a light with the configuration you want - Exposed `transformToUnitCube`. Pass an asset and it will transform the asset to fit into a unit cube - New `setEntityPosition`, `setEntityRotation` and `setEntityScale` to set or update an entities transform - `loadAsset` now returns a `FilamentAsset`. The asset can be used for various operations in the scene. --------- Co-authored-by: Marc Rousavy <me@mrousavy.com>

hannojg added 30 commits February 23, 2024 14:01

create engine + createRenderer

cc3eb54

format cpp

0f7de42

create camera

87a2ebb

scene + camera + view scaffolding

7562bf3

wip: asset loading

8064705

wip: create default light + add entity to scene

84ddaf3

wip

1f35415

use getter camera

7588a0d

Merge branch 'main' of github.com:margelo/react-native-filament into …

4be6e4c

…feat/first-render-api

using new reference heuristics

6e3b6eb

wip: adding missing types

6794753

setup swap chain

2c109ac

cleanup

88b8a4c

implemented renderer methods

7b0ca1a

cleanup

628084c

implemented camera manipulator

639249b

safe guard code + make internal private

7dbbebf

cleanup listener

2026812

wip: FilamentBuffer

df1028d

wip: example assets

ffabde0

fix reading bytes broken

d9fd876

load asset resources

2306932

first render output of the pengu

6c08d06

remove black background

85884ac

wip: rendering indirect light

5151a2c

restructure, create everything in the EngineWrapper

23ad358

new render implementation

f1118e0

cleanup

5ff5f8c

remove choreographer listener

73ef9f9

export the types

a62bb78

hannojg added 6 commits February 28, 2024 18:35

update todo

a68986e

check all

2bd25ee

fix swap chain wrapper

0de95a7

Merge branch 'feat/first-render-api' into feat/asset-loader

fa3ae0f

cleanup camera logic

f35b6d3

type cleanup

7d1be07

hannojg changed the title ~~[🚧 WIP] Asset loading~~ Asset loading Feb 28, 2024

hannojg requested a review from mrousavy February 28, 2024 19:38

Base automatically changed from feat/first-render-api to main February 29, 2024 08:48

hannojg added 2 commits February 29, 2024 09:56

Merge branch 'main' of github.com:margelo/react-native-filament into …

8584933

…feat/asset-loader

fix issues after merge

4f8fccf

hannojg mentioned this pull request Feb 29, 2024

Feat/API for controlling scene #15

Merged

hannojg commented Mar 2, 2024

View reviewed changes

mrousavy reviewed Mar 4, 2024

View reviewed changes

package/cpp/core/EngineWrapper.cpp Outdated Show resolved Hide resolved

hannojg added 3 commits March 5, 2024 09:27

don't use pointer

cddfde1

shared_ptr + using Reference

2f72c1f

improve asset loading, only use one buffer

d953282

mrousavy reviewed Mar 5, 2024

View reviewed changes

hannojg and others added 2 commits March 5, 2024 13:24

Revert "feat: Add API for controlling the scene (#15)"

886b7ce

This reverts commit 9dde746.

mrousavy merged commit 58984d7 into main Mar 5, 2024

mrousavy mentioned this pull request Mar 5, 2024

feat: Add API for controlling the scene (#15) #27

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Asset loading #13

Asset loading #13

hannojg commented Feb 26, 2024 •

edited

Loading

hannojg Mar 2, 2024

mrousavy Mar 4, 2024

mrousavy Mar 4, 2024

hannojg Mar 5, 2024

mrousavy Mar 4, 2024

mrousavy Mar 4, 2024

hannojg Mar 5, 2024

mrousavy Mar 4, 2024

hannojg Mar 5, 2024

mrousavy Mar 4, 2024

mrousavy Mar 5, 2024

Asset loading #13

Asset loading #13

Conversation

hannojg commented Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannojg commented Feb 26, 2024 •

edited

Loading