From 9d57ced452f9c91cffa2f31db24787ccd7691738 Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Tue, 24 Sep 2024 10:24:32 -0700 Subject: [PATCH] Fix broken layerCount for some types of textures (#8151) Currently mLayerCount for RenderTarget is always updated to the number of depth for attachments, which incurs unintended behaviors for some types of textures. i.e., 2d array, cubemap array, and 3d textures. Fix this by updating mLayerCount only for multiview use case. BUGS=[369165616] --- .../include/backend/TargetBufferInfo.h | 17 ++++----- filament/backend/src/opengl/OpenGLDriver.cpp | 2 +- filament/backend/src/ostream.cpp | 1 - filament/backend/src/vulkan/VulkanContext.h | 1 - filament/backend/src/vulkan/VulkanDriver.cpp | 3 -- filament/include/filament/RenderTarget.h | 22 ++++++++++- filament/src/details/RenderTarget.cpp | 37 +++++++++++++++---- filament/src/details/RenderTarget.h | 3 ++ samples/hellostereo.cpp | 2 + 9 files changed, 63 insertions(+), 25 deletions(-) diff --git a/filament/backend/include/backend/TargetBufferInfo.h b/filament/backend/include/backend/TargetBufferInfo.h index c4d284ccce52..00bbd025cb6b 100644 --- a/filament/backend/include/backend/TargetBufferInfo.h +++ b/filament/backend/include/backend/TargetBufferInfo.h @@ -30,10 +30,6 @@ namespace filament::backend { struct TargetBufferInfo { // note: the parameters of this constructor are not in the order of this structure's fields - TargetBufferInfo(Handle handle, uint8_t level, uint16_t layer, uint8_t baseViewIndex) noexcept - : handle(handle), baseViewIndex(baseViewIndex), level(level), layer(layer) { - } - TargetBufferInfo(Handle handle, uint8_t level, uint16_t layer) noexcept : handle(handle), level(level), layer(layer) { } @@ -51,14 +47,15 @@ struct TargetBufferInfo { // texture to be used as render target Handle handle; - // Starting layer index for multiview. This value is only used when the `layerCount` for the - // render target is greater than 1. - uint8_t baseViewIndex = 0; - // level to be used uint8_t level = 0; - // For cubemaps and 3D textures. See TextureCubemapFace for the face->layer mapping + // - For cubemap textures, this indicates the face of the cubemap. See TextureCubemapFace for + // the face->layer mapping) + // - For 2d array, cubemap array, and 3d textures, this indicates an index of a single layer of + // them. + // - For multiview textures (i.e., layerCount for the RenderTarget is greater than 1), this + // indicates a starting layer index of the current 2d array texture for multiview. uint16_t layer = 0; }; @@ -103,7 +100,7 @@ class MRT { // this is here for backward compatibility MRT(Handle handle, uint8_t level, uint16_t layer) noexcept - : mInfos{{ handle, level, layer, 0 }} { + : mInfos{{ handle, level, layer }} { } }; diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 4be6a412b78f..c753125d3999 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -1213,7 +1213,7 @@ void OpenGLDriver::framebufferTexture(TargetBufferInfo const& binfo, if (layerCount > 1) { // if layerCount > 1, it means we use the multiview extension. glFramebufferTextureMultiviewOVR(GL_FRAMEBUFFER, attachment, - t->gl.id, 0, binfo.baseViewIndex, layerCount); + t->gl.id, 0, binfo.layer, layerCount); } else #endif // !defined(__EMSCRIPTEN__) && !defined(IOS) { diff --git a/filament/backend/src/ostream.cpp b/filament/backend/src/ostream.cpp index 7c32636b354f..36d93dfbec39 100644 --- a/filament/backend/src/ostream.cpp +++ b/filament/backend/src/ostream.cpp @@ -410,7 +410,6 @@ io::ostream& operator<<(io::ostream& out, const RasterState& rs) { io::ostream& operator<<(io::ostream& out, const TargetBufferInfo& tbi) { return out << "TargetBufferInfo{" << "handle=" << tbi.handle - << ", baseViewIndex=" << tbi.baseViewIndex << ", level=" << tbi.level << ", layer=" << tbi.layer << "}"; } diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h index 46fe475612ab..a820087d7a2a 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -43,7 +43,6 @@ struct VulkanCommandBuffer; struct VulkanAttachment { VulkanTexture* texture = nullptr; uint8_t level = 0; - uint8_t baseViewIndex = 0; uint8_t layerCount = 1; uint16_t layer = 0; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 262780db2cde..587e4c823bc2 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -595,7 +595,6 @@ void VulkanDriver::createRenderTargetR(Handle rth, colorTargets[i] = { .texture = mResourceAllocator.handle_cast(color[i].handle), .level = color[i].level, - .baseViewIndex = color[i].baseViewIndex, .layerCount = layerCount, .layer = color[i].layer, }; @@ -611,7 +610,6 @@ void VulkanDriver::createRenderTargetR(Handle rth, depthStencil[0] = { .texture = mResourceAllocator.handle_cast(depth.handle), .level = depth.level, - .baseViewIndex = depth.baseViewIndex, .layerCount = layerCount, .layer = depth.layer, }; @@ -625,7 +623,6 @@ void VulkanDriver::createRenderTargetR(Handle rth, depthStencil[1] = { .texture = mResourceAllocator.handle_cast(stencil.handle), .level = stencil.level, - .baseViewIndex = stencil.baseViewIndex, .layerCount = layerCount, .layer = stencil.layer, }; diff --git a/filament/include/filament/RenderTarget.h b/filament/include/filament/RenderTarget.h index fc76111da74d..d432f63ed770 100644 --- a/filament/include/filament/RenderTarget.h +++ b/filament/include/filament/RenderTarget.h @@ -118,7 +118,7 @@ class UTILS_PUBLIC RenderTarget : public FilamentAPI { Builder& mipLevel(AttachmentPoint attachment, uint8_t level) noexcept; /** - * Sets the cubemap face for a given attachment point. + * Sets the face for cubemap textures at the given attachment point. * * @param attachment The attachment point. * @param face The associated cubemap face. @@ -127,7 +127,12 @@ class UTILS_PUBLIC RenderTarget : public FilamentAPI { Builder& face(AttachmentPoint attachment, CubemapFace face) noexcept; /** - * Sets the layer for a given attachment point (for 3D textures). + * Sets an index of a single layer for 2d array, cubemap array, and 3d textures at the given + * attachment point. + * + * For cubemap array textures, layer is translated into an array index and face according to + * - index: layer / 6 + * - face: layer % 6 * * @param attachment The attachment point. * @param layer The associated cubemap layer. @@ -135,6 +140,19 @@ class UTILS_PUBLIC RenderTarget : public FilamentAPI { */ Builder& layer(AttachmentPoint attachment, uint32_t layer) noexcept; + /** + * Sets the starting index of the 2d array textures for multiview at the given attachment + * point. + * + * This requires COLOR and DEPTH attachments (if set) to be of 2D array textures. + * + * @param attachment The attachment point. + * @param layerCount The number of layers used for multiview, starting from baseLayer. + * @param baseLayer The starting index of the 2d array texture. + * @return A reference to this Builder for chaining calls. + */ + Builder& multiview(AttachmentPoint attachment, uint8_t layerCount, uint8_t baseLayer = 0) noexcept; + /** * Creates the RenderTarget object and returns a pointer to it. * diff --git a/filament/src/details/RenderTarget.cpp b/filament/src/details/RenderTarget.cpp index 942887e2f95b..6e460133e489 100644 --- a/filament/src/details/RenderTarget.cpp +++ b/filament/src/details/RenderTarget.cpp @@ -44,6 +44,9 @@ struct RenderTarget::BuilderDetails { uint32_t mWidth{}; uint32_t mHeight{}; uint8_t mSamples = 1; // currently not settable in the public facing API + // The number of layers for the render target. The value should be 1 except for multiview. + // If multiview is enabled, this value is appropriately updated based on the layerCount value + // from each attachment. Hence, #>1 means using multiview uint8_t mLayerCount = 1; }; @@ -75,6 +78,14 @@ RenderTarget::Builder& RenderTarget::Builder::layer(AttachmentPoint pt, uint32_t return *this; } +RenderTarget::Builder& RenderTarget::Builder::multiview(AttachmentPoint pt, uint8_t layerCount, + uint8_t baseLayer/*= 0*/) noexcept { + mImpl->mAttachments[(size_t)pt].layer = baseLayer; + mImpl->mAttachments[(size_t)pt].layerCount = layerCount; + return *this; +} + + RenderTarget* RenderTarget::Builder::build(Engine& engine) { using backend::TextureUsage; const FRenderTarget::Attachment& color = mImpl->mAttachments[(size_t)AttachmentPoint::COLOR0]; @@ -105,28 +116,40 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) { uint32_t maxWidth = 0; uint32_t minHeight = std::numeric_limits::max(); uint32_t maxHeight = 0; - uint32_t minDepth = std::numeric_limits::max(); - uint32_t maxDepth = 0; + uint32_t minLayerCount = std::numeric_limits::max(); + uint32_t maxLayerCount = 0; for (auto const& attachment : mImpl->mAttachments) { if (attachment.texture) { const uint32_t w = attachment.texture->getWidth(attachment.mipLevel); const uint32_t h = attachment.texture->getHeight(attachment.mipLevel); const uint32_t d = attachment.texture->getDepth(attachment.mipLevel); + const uint32_t l = attachment.layerCount; + if (l > 0) { + FILAMENT_CHECK_PRECONDITION( + attachment.texture->getTarget() == Texture::Sampler::SAMPLER_2D_ARRAY) + << "Texture sampler must be of 2d array for multiview"; + } + FILAMENT_CHECK_PRECONDITION(attachment.layer + l <= d) + << "layer + layerCount cannot exceed the number of depth"; minWidth = std::min(minWidth, w); minHeight = std::min(minHeight, h); - minDepth = std::min(minDepth, d); + minLayerCount = std::min(minLayerCount, l); maxWidth = std::max(maxWidth, w); maxHeight = std::max(maxHeight, h); - maxDepth = std::max(maxDepth, d); + maxLayerCount = std::max(maxLayerCount, l); } } FILAMENT_CHECK_PRECONDITION(minWidth == maxWidth && minHeight == maxHeight - && minDepth == maxDepth) << "All attachments dimensions must match"; + && minLayerCount == maxLayerCount) << "All attachments dimensions must match"; mImpl->mWidth = minWidth; mImpl->mHeight = minHeight; - mImpl->mLayerCount = minDepth; + if (minLayerCount > 0) { + // mLayerCount should be 1 except for multiview use where we update this variable + // to the number of layerCount for multiview. + mImpl->mLayerCount = minLayerCount; + } return downcast(engine).createRenderTarget(*this); } @@ -141,7 +164,7 @@ FRenderTarget::FRenderTarget(FEngine& engine, const RenderTarget::Builder& build backend::MRT mrt{}; TargetBufferInfo dinfo{}; - auto setAttachment = [this](TargetBufferInfo& info, AttachmentPoint attachmentPoint) { + auto setAttachment = [&](TargetBufferInfo& info, AttachmentPoint attachmentPoint) { Attachment const& attachment = mAttachments[(size_t)attachmentPoint]; auto t = downcast(attachment.texture); info.handle = t->getHwHandle(); diff --git a/filament/src/details/RenderTarget.h b/filament/src/details/RenderTarget.h index 3b5dca98cb07..1b57b5acc9b1 100644 --- a/filament/src/details/RenderTarget.h +++ b/filament/src/details/RenderTarget.h @@ -39,6 +39,9 @@ class FRenderTarget : public RenderTarget { uint8_t mipLevel = 0; CubemapFace face = RenderTarget::CubemapFace::POSITIVE_X; uint32_t layer = 0; + // Indicates the number of layers used for multiview, starting from the `layer` (baseIndex). + // This means `layer` + `layerCount` cannot exceed the number of depth for the attachment. + uint16_t layerCount = 0; }; FRenderTarget(FEngine& engine, const Builder& builder); diff --git a/samples/hellostereo.cpp b/samples/hellostereo.cpp index fff1616ace06..06936241a850 100644 --- a/samples/hellostereo.cpp +++ b/samples/hellostereo.cpp @@ -224,6 +224,8 @@ int main(int argc, char** argv) { app.stereoRenderTarget = RenderTarget::Builder() .texture(RenderTarget::AttachmentPoint::COLOR, app.stereoColorTexture) .texture(RenderTarget::AttachmentPoint::DEPTH, app.stereoDepthTexture) + .multiview(RenderTarget::AttachmentPoint::COLOR, eyeCount, 0) + .multiview(RenderTarget::AttachmentPoint::DEPTH, eyeCount, 0) .build(*engine); app.stereoView->setRenderTarget(app.stereoRenderTarget); app.stereoView->setViewport({0, 0, vp.width, vp.height});