Skip to content

Commit

Permalink
Static engine allocator (#1327)
Browse files Browse the repository at this point in the history 
When shutting down the engine while having some textures, it's possible
for NativeEngine to be disposed before the textures.
In this lambda:


https://github.com/BabylonJS/BabylonNative/blob/95240a59f2e1483d4c4b92b329b84ff333f0de84/Plugins/NativeEngine/Source/NativeEngine.cpp#L216

The imageContainer has still a ref to NativeEngine::m_allocator that
gets invalid


https://github.com/bkaradzic/bimg/blob/5d61905eb1aa8074ee62553b4a7bd2dc16f4cf24/src/image.cpp#L3335

Setting m_allocator as static fixes the issue. It only contains a vtable
so it's threadsafe.
  • Loading branch information
@CedricGuillemet
CedricGuillemet authored Nov 30, 2023
1 parent 5865dc3 commit 661a5a2
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 24 deletions.
4 changes: 4 additions & 0 deletions Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "BgfxCallback.h"
#include <bx/allocator.h>
#include "continuation_scheduler.h"
#include "SafeTimespanGuarantor.h"

Expand Down Expand Up @@ -114,6 +115,7 @@ namespace Babylon::Graphics
void AddTexture(bgfx::TextureHandle handle, uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format);
void RemoveTexture(bgfx::TextureHandle handle);
TextureInfo GetTextureInfo(bgfx::TextureHandle handle);
static bx::AllocatorI& GetDefaultAllocator() { return m_allocator; }

private:
friend UpdateToken;
Expand All @@ -122,5 +124,7 @@ namespace Babylon::Graphics

std::unordered_map<uint16_t, TextureInfo> m_textureHandleToInfo{};
std::mutex m_textureHandleToInfoMutex{};

static inline bx::DefaultAllocator m_allocator{};
};
}
32 changes: 16 additions & 16 deletions Plugins/NativeEngine/Source/NativeEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1224,9 +1224,9 @@ namespace Babylon
const auto dataSpan = gsl::make_span(static_cast<uint8_t*>(data.ArrayBuffer().Data()) + data.ByteOffset(), data.ByteLength());

arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource,
[this, dataSpan, generateMips, invertY, srgb, texture, cancellationSource{m_cancellationSource}]() {
bimg::ImageContainer* image{ParseImage(m_allocator, dataSpan)};
image = PrepareImage(m_allocator, image, invertY, srgb, generateMips);
[dataSpan, generateMips, invertY, srgb, texture, cancellationSource{m_cancellationSource}]() {
bimg::ImageContainer* image{ParseImage(Graphics::DeviceContext::GetDefaultAllocator(), dataSpan)};
image = PrepareImage(Graphics::DeviceContext::GetDefaultAllocator(), image, invertY, srgb, generateMips);
LoadTextureFromImage(texture, image, srgb);
})
.then(m_runtimeScheduler, *m_cancellationSource, [dataRef{Napi::Persistent(data)}, onSuccessRef{Napi::Persistent(onSuccess)}, onErrorRef{Napi::Persistent(onError)}, cancellationSource{m_cancellationSource}](arcana::expected<void, std::exception_ptr> result) {
Expand Down Expand Up @@ -1275,8 +1275,8 @@ namespace Babylon
throw Napi::Error::New(Env(), "The data size does not match width, height, and format");
}

bimg::ImageContainer* image{bimg::imageAlloc(&m_allocator, format, width, height, 1, 1, false, false, bytes)};
image = PrepareImage(m_allocator, image, invertY, false, generateMips);
bimg::ImageContainer* image{bimg::imageAlloc(&Graphics::DeviceContext::GetDefaultAllocator(), format, width, height, 1, 1, false, false, bytes)};
image = PrepareImage(Graphics::DeviceContext::GetDefaultAllocator(), image, invertY, false, generateMips);
LoadTextureFromImage(texture, image, false);
}

Expand Down Expand Up @@ -1342,9 +1342,9 @@ namespace Babylon
const auto typedArray{data[face].As<Napi::TypedArray>()};
const auto dataSpan{gsl::make_span(static_cast<uint8_t*>(typedArray.ArrayBuffer().Data()) + typedArray.ByteOffset(), typedArray.ByteLength())};
dataRefs[face] = Napi::Persistent(typedArray);
tasks[face] = arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource, [this, dataSpan, invertY, generateMips, srgb]() {
bimg::ImageContainer* image{ParseImage(m_allocator, dataSpan)};
image = PrepareImage(m_allocator, image, invertY, srgb, generateMips);
tasks[face] = arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource, [dataSpan, invertY, generateMips, srgb]() {
bimg::ImageContainer* image{ParseImage(Graphics::DeviceContext::GetDefaultAllocator(), dataSpan)};
image = PrepareImage(Graphics::DeviceContext::GetDefaultAllocator(), image, invertY, srgb, generateMips);
return image;
});
}
Expand Down Expand Up @@ -1385,9 +1385,9 @@ namespace Babylon
const auto typedArray = faceData[face].As<Napi::TypedArray>();
const auto dataSpan = gsl::make_span(static_cast<uint8_t*>(typedArray.ArrayBuffer().Data()) + typedArray.ByteOffset(), typedArray.ByteLength());
dataRefs[(face * numMips) + mip] = Napi::Persistent(typedArray);
tasks[(face * numMips) + mip] = arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource, [this, dataSpan, invertY, srgb]() {
bimg::ImageContainer* image{ParseImage(m_allocator, dataSpan)};
image = PrepareImage(m_allocator, image, invertY, srgb, false);
tasks[(face * numMips) + mip] = arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource, [dataSpan, invertY, srgb]() {
bimg::ImageContainer* image{ParseImage(Graphics::DeviceContext::GetDefaultAllocator(), dataSpan)};
image = PrepareImage(Graphics::DeviceContext::GetDefaultAllocator(), image, invertY, srgb, false);
return image;
});
}
Expand Down Expand Up @@ -1563,12 +1563,12 @@ namespace Babylon

// Read the source texture.
m_graphicsContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel)
.then(arcana::inline_scheduler, *m_cancellationSource, [this, textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable {
.then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable {
// If the source texture format does not match the target texture format, convert it.
if (targetTextureInfo.format != sourceTextureInfo.format)
{
std::vector<uint8_t> convertedTextureBuffer(targetTextureInfo.storageSize);
if (!bimg::imageConvert(&m_allocator, convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1))
if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1))
{
throw std::runtime_error{"Texture conversion to RBGA8 failed."};
}
Expand Down Expand Up @@ -1849,7 +1849,7 @@ namespace Babylon
throw Napi::Error::New(env, "CreateImageBitmap array buffer is empty.");
}

image = ParseImage(m_allocator, gsl::make_span(static_cast<uint8_t*>(data.Data()), data.ByteLength()));
image = ParseImage(Graphics::DeviceContext::GetDefaultAllocator(), gsl::make_span(static_cast<uint8_t*>(data.Data()), data.ByteLength()));
allocatedImage = true;
}
else if (info[0].IsObject())
Expand Down Expand Up @@ -1905,7 +1905,7 @@ namespace Babylon

const Napi::Env env{info.Env()};

bimg::ImageContainer* image = bimg::imageAlloc(&m_allocator, format, static_cast<uint16_t>(width), static_cast<uint16_t>(height), 1, 1, false, false, data.Data());
bimg::ImageContainer* image = bimg::imageAlloc(&Graphics::DeviceContext::GetDefaultAllocator(), format, static_cast<uint16_t>(width), static_cast<uint16_t>(height), 1, 1, false, false, data.Data());
if (image == nullptr)
{
throw Napi::Error::New(env, "Unable to allocate image for ResizeImageBitmap.");
Expand All @@ -1917,7 +1917,7 @@ namespace Babylon
{
image->m_format = bimg::TextureFormat::A8;
}
bimg::ImageContainer* rgba = bimg::imageConvert(&m_allocator, bimg::TextureFormat::RGBA8, *image, false);
bimg::ImageContainer* rgba = bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), bimg::TextureFormat::RGBA8, *image, false);
if (rgba == nullptr)
{
throw Napi::Error::New(env, "Unable to convert image to RGBA pixel format for ResizeImageBitmap.");
Expand Down
1 change: 0 additions & 1 deletion Plugins/NativeEngine/Source/NativeEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ namespace Babylon
void ScheduleRequestAnimationFrameCallbacks();
bool m_requestAnimationFrameCallbacksScheduled{};

bx::DefaultAllocator m_allocator{};
uint64_t m_engineState{BGFX_STATE_DEFAULT};
uint32_t m_stencilState{BGFX_STENCIL_TEST_ALWAYS | BGFX_STENCIL_FUNC_REF(0) | BGFX_STENCIL_FUNC_RMASK(0xFF) | BGFX_STENCIL_OP_FAIL_S_KEEP | BGFX_STENCIL_OP_FAIL_Z_KEEP | BGFX_STENCIL_OP_PASS_Z_REPLACE};

Expand Down
5 changes: 3 additions & 2 deletions Plugins/TestUtils/Source/TestUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <functional>
#include <sstream>
#include <Babylon/JsRuntime.h>
#include <Babylon/Graphics/DeviceContext.h>

#define STRINGIZEX(x) #x
#define STRINGIZE(x) STRINGIZEX(x)
Expand All @@ -31,7 +32,7 @@ namespace Babylon::Plugins::Internal
return;
}

bx::MemoryBlock mb(&allocator);
bx::MemoryBlock mb(&Graphics::DeviceContext::GetDefaultAllocator());
bx::FileWriter writer;
bx::FilePath filepath(filename.c_str());
bx::FilePath filedir(filepath.getPath());
Expand All @@ -49,7 +50,7 @@ namespace Babylon::Plugins::Internal
Image* image = new Image;
const auto buffer = info[0].As<Napi::ArrayBuffer>();

image->m_Image = bimg::imageParse(&allocator, buffer.Data(), static_cast<uint32_t>(buffer.ByteLength()));
image->m_Image = bimg::imageParse(&Graphics::DeviceContext::GetDefaultAllocator(), buffer.Data(), static_cast<uint32_t>(buffer.ByteLength()));

auto finalizer = [](Napi::Env, Image* image) { delete image; };
return Napi::External<Image>::New(info.Env(), image, std::move(finalizer));
Expand Down
1 change: 0 additions & 1 deletion Plugins/TestUtils/Source/TestUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ namespace Babylon::Plugins::Internal
static inline Napi::FunctionReference constructor{};

inline static std::shared_ptr<ImplData> m_implData;
inline static bx::DefaultAllocator allocator{};

void Exit(const Napi::CallbackInfo& info);
void UpdateSize(const Napi::CallbackInfo& info);
Expand Down
2 changes: 1 addition & 1 deletion Polyfills/Canvas/Source/Image.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ namespace Babylon::Polyfills::Internal
return;
}

m_imageContainer = bimg::imageParse(&m_allocator, buffer.data(), static_cast<uint32_t>(buffer.size_bytes()), bimg::TextureFormat::RGBA8);
m_imageContainer = bimg::imageParse(&Graphics::DeviceContext::GetDefaultAllocator(), buffer.data(), static_cast<uint32_t>(buffer.size_bytes()), bimg::TextureFormat::RGBA8);

if (m_imageContainer == nullptr)
{
Expand Down
1 change: 0 additions & 1 deletion Polyfills/Canvas/Source/Image.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ namespace Babylon::Polyfills::Internal
Napi::FunctionReference m_onloadHandlerRef;
Napi::FunctionReference m_onerrorHandlerRef;
std::shared_ptr<arcana::cancellation_source> m_cancellationSource{};
bx::DefaultAllocator m_allocator{};
bimg::ImageContainer* m_imageContainer{};
};
}
3 changes: 1 addition & 2 deletions Polyfills/Canvas/Source/nanovg_babylon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1106,8 +1106,7 @@ NVGcontext* nvgCreate(int32_t _edgeaa, bx::AllocatorI* _allocator)
{
if (NULL == _allocator)
{
static bx::DefaultAllocator allocator;
_allocator = &allocator;
_allocator = &Babylon::Graphics::DeviceContext::GetDefaultAllocator();
}

struct NVGparams params;
Expand Down

0 comments on commit 661a5a2

Please sign in to comment.