Skip to content

Commit 802dd1e

Browse files
bkaradzicCopilot
andcommitted
Address review: track async cube-load task + validate cube container faces/mips
- loadCubeTexture single-buffer path: capture TrackAsyncTask() into the inline continuation that uploads to bgfx, so NativeEngine::Dispose() drains it before teardown frees the graphics resources it touches (matches the multi-buffer path). - LoadCubeTextureFromContainer: validate every (side, mip) is present before uploading and fail fast (freeing the container) if not, so the single release callback attached to the last upload always fires - no leak or partially initialized texture on a truncated container. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 383b8a3 commit 802dd1e

1 file changed

Lines changed: 29 additions & 13 deletions

File tree

Plugins/NativeEngine/Source/NativeEngine.cpp

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -628,28 +628,44 @@ namespace Babylon
628628
texture->CreateCube(static_cast<uint16_t>(size), hasMips, 1, format, flags);
629629
}
630630

631+
// The single release callback is attached to the last (side, mip) upload, so every
632+
// expected face/mip must be present; otherwise the container would leak (the callback
633+
// would never fire) and the texture would be left partially initialized. Validate the
634+
// whole face/mip grid up front and fail fast.
635+
const uint8_t numMips{static_cast<uint8_t>(image->m_numMips)};
636+
for (uint8_t side = 0; side < 6; ++side)
637+
{
638+
for (uint8_t mip = 0; mip < numMips; ++mip)
639+
{
640+
bimg::ImageMip imageMip{};
641+
if (!bimg::imageGetRawData(*image, side, mip, image->m_data, image->m_size, imageMip))
642+
{
643+
bimg::imageFree(image);
644+
throw std::runtime_error{"Cubemap container is missing one or more faces/mips."};
645+
}
646+
}
647+
}
648+
631649
// Every (side, mip) view points into the single container's backing
632650
// store, so the allocation is released exactly once, after bgfx has
633651
// consumed the final upload.
634-
const uint8_t numMips{static_cast<uint8_t>(image->m_numMips)};
635652
for (uint8_t side = 0; side < 6; ++side)
636653
{
637654
for (uint8_t mip = 0; mip < numMips; ++mip)
638655
{
639656
bimg::ImageMip imageMip{};
640-
if (bimg::imageGetRawData(*image, side, mip, image->m_data, image->m_size, imageMip))
641-
{
642-
bgfx::ReleaseFn releaseFn{};
643-
if (side == 5 && mip == numMips - 1)
644-
{
645-
releaseFn = [](void*, void* userData) {
646-
bimg::imageFree(static_cast<bimg::ImageContainer*>(userData));
647-
};
648-
}
657+
bimg::imageGetRawData(*image, side, mip, image->m_data, image->m_size, imageMip);
649658

650-
const bgfx::Memory* mem{bgfx::makeRef(imageMip.m_data, imageMip.m_size, releaseFn, image)};
651-
texture->UpdateCube(0, side, mip, 0, 0, static_cast<uint16_t>(imageMip.m_width), static_cast<uint16_t>(imageMip.m_height), mem);
659+
bgfx::ReleaseFn releaseFn{};
660+
if (side == 5 && mip == numMips - 1)
661+
{
662+
releaseFn = [](void*, void* userData) {
663+
bimg::imageFree(static_cast<bimg::ImageContainer*>(userData));
664+
};
652665
}
666+
667+
const bgfx::Memory* mem{bgfx::makeRef(imageMip.m_data, imageMip.m_size, releaseFn, image)};
668+
texture->UpdateCube(0, side, mip, 0, 0, static_cast<uint16_t>(imageMip.m_width), static_cast<uint16_t>(imageMip.m_height), mem);
653669
}
654670
}
655671
}
@@ -1780,7 +1796,7 @@ namespace Babylon
17801796
arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource, [dataSpan]() {
17811797
return ParseCubeImage(Graphics::DeviceContext::GetDefaultAllocator(), dataSpan);
17821798
})
1783-
.then(arcana::inline_scheduler, *m_cancellationSource, [texture, srgb, cancellationSource{m_cancellationSource}](bimg::ImageContainer* image) {
1799+
.then(arcana::inline_scheduler, *m_cancellationSource, [texture, srgb, cancellationSource{m_cancellationSource}, asyncTaskScope{TrackAsyncTask()}](bimg::ImageContainer* image) {
17841800
// Compute the spherical harmonics from the decoded top mip before the upload
17851801
// hands the container's memory to bgfx.
17861802
auto sphericalPolynomial = ComputeCubeSphericalPolynomial(Graphics::DeviceContext::GetDefaultAllocator(), image);

0 commit comments

Comments
 (0)