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

Bridge filament basic render infrastructure #9

Merged
merged 42 commits into from
Feb 29, 2024
Merged

Conversation

hannojg
Copy link
Member

@hannojg hannojg commented Feb 23, 2024

Changes

  • The EngineWrapper is creating all of the components we need for any scene to render (renderer, scene, view. camera, camera manipulator, swap chain, link to choreographer)
  • The JS layer is creating an EngineWrapper and linking it with the surface
  • The user can set a render callback
  • The engine provides getters for all internal state to update it e.g. in the render

Example
We can't see anything yet but the basic render infrastructure is there in FilamentView.tsx

    // Get Surface:
    const fView = FilamentProxy.findFilamentView(this.handle)
    const surfaceProvider = fView.getSurfaceProvider()

    // Create engine and link it to the surface:
    const engine = FilamentProxy.createEngine()
    engine.setSurfaceProvider(surfaceProvider)

    // Callback for rendering every frame
    engine.setRenderCallback(() => {
      engine.getCamera().lookAt(engine.getCameraManipulator())
    })

Note:

Android will crash once start rendering, as it can't render anything without any entities in the scene (not sure why its not an issue on iOS though)

@hannojg hannojg changed the title wip: bridge filament basic render infrastructure bridge filament basic render infrastructure Feb 23, 2024
@hannojg hannojg changed the title bridge filament basic render infrastructure [🚧WIP] bridge filament basic render infrastructure Feb 24, 2024
@mrousavy
Copy link
Member

this.choreographer.addOnFrameListener((timestamp) => {
  // ...
})

Make sure to keep a reference on the returned listener, otherwise it might be destroyed I think? Not sure about that.

const listener = this.choreographer.addOnFrameListener((timestamp) => {
  // ...
})

// .. later

listener.remove()

We can also change the implementation of Listener to not remove itself when it is garbage collected, then it will only remove the listener if the parent (in this case Choreographer) is collected.

@hannojg hannojg changed the title [🚧WIP] bridge filament basic render infrastructure Bridge filament basic render infrastructure Feb 27, 2024
Copy link
Member

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

looks pretty good - left some comments!

package/cpp/core/EngineWrapper.cpp Outdated Show resolved Hide resolved
package/cpp/core/EngineWrapper.cpp Outdated Show resolved Hide resolved
package/cpp/core/EngineWrapper.cpp Show resolved Hide resolved
package/cpp/core/EngineWrapper.cpp Outdated Show resolved Hide resolved
package/cpp/core/EngineWrapper.cpp Outdated Show resolved Hide resolved
package/cpp/core/RendererWrapper.cpp Show resolved Hide resolved
package/cpp/core/RendererWrapper.cpp Show resolved Hide resolved
package/cpp/core/SwapChainWrapper.cpp Outdated Show resolved Hide resolved
package/src/types/Entity.ts Outdated Show resolved Hide resolved
package/src/types/Manipulator.ts Show resolved Hide resolved
package/cpp/FilamentProxy.cpp Outdated Show resolved Hide resolved
}

_scene = scene;
_view->setScene(scene->getScene().get());
Copy link
Member

Choose a reason for hiding this comment

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

we need to think about this as well - I see this a lot in your code where we do set(get->get()) which might be a bit unsafe.

Copy link
Member

Choose a reason for hiding this comment

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

for now its fine

@hannojg hannojg merged commit 6367d7f into main Feb 29, 2024
@hannojg hannojg deleted the feat/first-render-api branch February 29, 2024 08:48
@hannojg hannojg restored the feat/first-render-api branch March 11, 2024 09:14
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.

2 participants