Skip to content

Commit

Permalink
Fix broken layerCount for some types of textures (#8151)
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
z3moon authored Sep 24, 2024
1 parent fe15a30 commit 9d57ced
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 25 deletions.
17 changes: 7 additions & 10 deletions filament/backend/include/backend/TargetBufferInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<HwTexture> handle, uint8_t level, uint16_t layer, uint8_t baseViewIndex) noexcept
: handle(handle), baseViewIndex(baseViewIndex), level(level), layer(layer) {
}

TargetBufferInfo(Handle<HwTexture> handle, uint8_t level, uint16_t layer) noexcept
: handle(handle), level(level), layer(layer) {
}
Expand All @@ -51,14 +47,15 @@ struct TargetBufferInfo {
// texture to be used as render target
Handle<HwTexture> 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;
};

Expand Down Expand Up @@ -103,7 +100,7 @@ class MRT {

// this is here for backward compatibility
MRT(Handle<HwTexture> handle, uint8_t level, uint16_t layer) noexcept
: mInfos{{ handle, level, layer, 0 }} {
: mInfos{{ handle, level, layer }} {
}
};

Expand Down
2 changes: 1 addition & 1 deletion filament/backend/src/opengl/OpenGLDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
1 change: 0 additions & 1 deletion filament/backend/src/ostream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 << "}";
}
Expand Down
1 change: 0 additions & 1 deletion filament/backend/src/vulkan/VulkanContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 0 additions & 3 deletions filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,6 @@ void VulkanDriver::createRenderTargetR(Handle<HwRenderTarget> rth,
colorTargets[i] = {
.texture = mResourceAllocator.handle_cast<VulkanTexture*>(color[i].handle),
.level = color[i].level,
.baseViewIndex = color[i].baseViewIndex,
.layerCount = layerCount,
.layer = color[i].layer,
};
Expand All @@ -611,7 +610,6 @@ void VulkanDriver::createRenderTargetR(Handle<HwRenderTarget> rth,
depthStencil[0] = {
.texture = mResourceAllocator.handle_cast<VulkanTexture*>(depth.handle),
.level = depth.level,
.baseViewIndex = depth.baseViewIndex,
.layerCount = layerCount,
.layer = depth.layer,
};
Expand All @@ -625,7 +623,6 @@ void VulkanDriver::createRenderTargetR(Handle<HwRenderTarget> rth,
depthStencil[1] = {
.texture = mResourceAllocator.handle_cast<VulkanTexture*>(stencil.handle),
.level = stencil.level,
.baseViewIndex = stencil.baseViewIndex,
.layerCount = layerCount,
.layer = stencil.layer,
};
Expand Down
22 changes: 20 additions & 2 deletions filament/include/filament/RenderTarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -127,14 +127,32 @@ 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.
* @return A reference to this Builder for chaining calls.
*/
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.
*
Expand Down
37 changes: 30 additions & 7 deletions filament/src/details/RenderTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -105,28 +116,40 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) {
uint32_t maxWidth = 0;
uint32_t minHeight = std::numeric_limits<uint32_t>::max();
uint32_t maxHeight = 0;
uint32_t minDepth = std::numeric_limits<uint32_t>::max();
uint32_t maxDepth = 0;
uint32_t minLayerCount = std::numeric_limits<uint32_t>::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);
}

Expand All @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions filament/src/details/RenderTarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions samples/hellostereo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down

0 comments on commit 9d57ced

Please sign in to comment.