Skip to content

Commit

Permalink
Fix XR Frame alloc (#1386)
Browse files Browse the repository at this point in the history
Context here : #1383

fixes #1383 

- Replace a weak pointer `const xr::System::Session::Frame*
m_frame{};`with a smart pointer.
- Rendering moved from Frame dtor to a specific, explicit method

PR tested ok :
https://forum.babylonjs.com/t/babylonreactnative-throws-exception-when-adding-anchor/50716/16?u=cedric
  • Loading branch information
CedricGuillemet committed Jun 26, 2024
1 parent 6cfd949 commit 6bb7702
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 18 deletions.
2 changes: 1 addition & 1 deletion Dependencies/xr/Include/XR.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ namespace xr
Plane& GetPlaneByID(Plane::Identifier) const;
Mesh& GetMeshByID(Mesh::Identifier) const;
ImageTrackingResult& GetImageTrackingResultByID(ImageTrackingResult::Identifier) const;

void Render();
private:
struct Impl;
std::unique_ptr<Impl> m_impl{};
Expand Down
5 changes: 3 additions & 2 deletions Dependencies/xr/Source/ARCore/XR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1471,8 +1471,9 @@ namespace xr
}


System::Session::Frame::~Frame()
{
System::Session::Frame::~Frame() {}

void System::Session::Frame::Render() {
m_impl->sessionImpl.CleanupFrameTrackables();
m_impl->sessionImpl.DrawFrame();
}
Expand Down
3 changes: 3 additions & 0 deletions Dependencies/xr/Source/ARKit/XR.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1737,6 +1737,9 @@ void GetHitTestResultsLegacy(std::vector<HitResult>& filteredResults, xr::HitTes
}

System::Session::Frame::~Frame() {
}

void System::Session::Frame::Render() {
m_impl->sessionImpl.DrawFrame();
}

Expand Down
4 changes: 3 additions & 1 deletion Dependencies/xr/Source/OpenXR/XR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,9 @@ namespace xr
// Stubbed out for now, should be implemented if we want to support OpenXR based passthrough AR devices.
}

System::Session::Frame::~Frame()
System::Session::Frame::~Frame() {}

void System::Session::Frame::Render()
{
if (!m_impl->sessionImpl.HmdImpl.Context.IsSessionRunning())
{
Expand Down
8 changes: 4 additions & 4 deletions Plugins/NativeXr/Source/NativeXrImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ namespace Babylon
{
// Block and burn frames until XR successfully shuts down.
m_sessionState->Frame = m_sessionState->Session->GetNextFrame(shouldEndSession, shouldRestartSession);
m_sessionState->Frame->Render();
m_sessionState->Frame.reset();
}
while (!shouldEndSession);
Expand All @@ -149,7 +150,7 @@ namespace Babylon
return m_endTask;
}

void NativeXr::Impl::ScheduleFrame(std::function<void(const xr::System::Session::Frame&)>&& callback)
void NativeXr::Impl::ScheduleFrame(std::function<void(const std::shared_ptr<const xr::System::Session::Frame>&)>&& callback)
{
if (m_sessionState->FrameScheduled)
{
Expand All @@ -175,7 +176,7 @@ namespace Babylon
auto callbacks{std::move(m_sessionState->ScheduleFrameCallbacks)};
for (auto& callback : callbacks)
{
callback(*m_sessionState->Frame);
callback(m_sessionState->Frame);
}
}

Expand All @@ -195,7 +196,6 @@ namespace Babylon
{
assert(m_sessionState != nullptr);
assert(m_sessionState->Session != nullptr);
assert(m_sessionState->Frame == nullptr);

arcana::trace_region beginFrameRegion{"NativeXR::BeginFrame"};

Expand Down Expand Up @@ -347,7 +347,7 @@ namespace Babylon

arcana::trace_region endFrameRegion{"NativeXR::EndFrame"};

m_sessionState->Frame.reset();
m_sessionState->Frame->Render();
}
} // Plugins
} // Babylon
6 changes: 3 additions & 3 deletions Plugins/NativeXr/Source/NativeXrImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Babylon
arcana::task<void, std::exception_ptr> BeginSessionAsync();
arcana::task<void, std::exception_ptr> EndSessionAsync();

void ScheduleFrame(std::function<void(const xr::System::Session::Frame&)>&& callback);
void ScheduleFrame(std::function<void(const std::shared_ptr<const xr::System::Session::Frame>&)>&& callback);

void SetRenderTextureFunctions(const Napi::Function& createFunction, const Napi::Function& destroyFunction)
{
Expand Down Expand Up @@ -123,10 +123,10 @@ namespace Babylon
std::unordered_map<ViewConfiguration*, uint32_t> ViewConfigurationStartViewIdx{};
std::unordered_map<void*, ViewConfiguration> TextureToViewConfigurationMap{};
std::shared_ptr<xr::System::Session> Session{};
std::unique_ptr<xr::System::Session::Frame> Frame{};
std::shared_ptr<xr::System::Session::Frame> Frame{};
arcana::cancellation_source CancellationSource{};
bool FrameScheduled{false};
std::vector<std::function<void(const xr::System::Session::Frame&)>> ScheduleFrameCallbacks{};
std::vector<std::function<void(const std::shared_ptr<const xr::System::Session::Frame>&)>> ScheduleFrameCallbacks{};
arcana::task<void, std::exception_ptr> FrameTask{arcana::task_from_result<std::exception_ptr>()};
};

Expand Down
4 changes: 2 additions & 2 deletions Plugins/NativeXr/Source/XRFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ namespace Babylon
m_jsJointPose.Set("transform", m_jsTransform.Value());
}

void XRFrame::Update(const Napi::Env& env, const xr::System::Session::Frame& frame, uint32_t timestamp)
void XRFrame::Update(const Napi::Env& env, const std::shared_ptr<const xr::System::Session::Frame>& frame, uint32_t timestamp)
{
// Store off a pointer to the frame so that the viewer pose can be updated later. We cannot
// update the viewer pose here because we don't yet know the desired reference space.
m_frame = &frame;
m_frame = frame;

// Update anchor positions.
UpdateAnchors();
Expand Down
4 changes: 2 additions & 2 deletions Plugins/NativeXr/Source/XRFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Babylon
static Napi::Object New(const Napi::CallbackInfo& info);

XRFrame(const Napi::CallbackInfo& info);
void Update(const Napi::Env& env, const xr::System::Session::Frame& frame, uint32_t timestamp);
void Update(const Napi::Env& env, const std::shared_ptr<const xr::System::Session::Frame>& frame, uint32_t timestamp);
Napi::Promise CreateNativeAnchor(const Napi::CallbackInfo& info, xr::Pose pose, xr::NativeTrackablePtr nativeTrackable);
Napi::Value DeclareNativeAnchor(const Napi::Env& env, xr::NativeAnchorPtr nativeAnchor);

Expand All @@ -30,7 +30,7 @@ namespace Babylon
Napi::Value GetJSSceneObjectFromID(const Napi::CallbackInfo& info, const xr::System::Session::Frame::SceneObject::Identifier objectID);

private:
const xr::System::Session::Frame* m_frame{};
std::shared_ptr<const xr::System::Session::Frame> m_frame{};
Napi::ObjectReference m_jsXRViewerPose{};
XRViewerPose& m_xrViewerPose;
std::vector<Napi::ObjectReference> m_trackedAnchors{};
Expand Down
6 changes: 3 additions & 3 deletions Plugins/NativeXr/Source/XRSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,9 @@ namespace Babylon
{
Napi::Function callback{ info[0].As<Napi::Function>() };

m_xr->ScheduleFrame([this, callbackPtr{ std::make_shared<Napi::FunctionReference>(Napi::Persistent(callback)) }](const auto& frame) {
ProcessEyeInputSource(frame, Env());
ProcessControllerInputSources(frame, Env());
m_xr->ScheduleFrame([this, callbackPtr{ std::make_shared<Napi::FunctionReference>(Napi::Persistent(callback)) }](const std::shared_ptr<const xr::System::Session::Frame>& frame) {
ProcessEyeInputSource(*frame.get(), Env());
ProcessControllerInputSources(*frame.get(), Env());

m_xrFrame.Update(Env(), frame, m_timestamp);

Expand Down

0 comments on commit 6bb7702

Please sign in to comment.