From b68e9fef03e4ee82a57e8bfec83fb30cf6c26fe4 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Thu, 13 Feb 2025 15:32:05 -0800 Subject: [PATCH] froxel: make froxel buffer size consistent (#8437) The definition of the froxel buffer sizes were spread across multiple places and for different use cases (including allocating the buffer, setting to a descriptor set, and adjusting the relevant spec constant). In this commit, we try to unify the size definitions into Froxelizer::getFroxelBufferByteCount(). We also remove the vulkan-only special case for froxel buffer UBO. This case was introduced because spec constant couldn't be used to size arrays for some drivers in the past, but we introduced a workaround that replaced spec constants with constants. --- filament/src/Froxelizer.cpp | 16 +++++++---- filament/src/Froxelizer.h | 18 +++++++----- filament/src/details/Material.cpp | 6 ++-- filament/src/details/View.cpp | 1 + filament/src/ds/ColorPassDescriptorSet.cpp | 4 +-- filament/src/ds/ColorPassDescriptorSet.h | 1 + libs/filamat/src/shaders/CodeGenerator.cpp | 33 ++++------------------ 7 files changed, 34 insertions(+), 45 deletions(-) diff --git a/filament/src/Froxelizer.cpp b/filament/src/Froxelizer.cpp index 0feed65717..6bf2beedf1 100644 --- a/filament/src/Froxelizer.cpp +++ b/filament/src/Froxelizer.cpp @@ -116,6 +116,12 @@ static bool fuzzyEqual(mat4f const& UTILS_RESTRICT l, mat4f const& UTILS_RESTRIC return result == 0; } +size_t Froxelizer::getFroxelBufferByteCount(FEngine::DriverApi& driverApi) noexcept { + // Make sure that targetSize is 16-byte aligned so that it'll fit properly into an array of + // uvec4. + size_t const targetSize = (driverApi.getMaxUniformBufferSize() / 16) * 16; + return std::min(FROXEL_BUFFER_MAX_ENTRY_COUNT * sizeof(FroxelEntry), targetSize); +} Froxelizer::Froxelizer(FEngine& engine) : mArena("froxel", PER_FROXELDATA_ARENA_SIZE), @@ -131,14 +137,14 @@ Froxelizer::Froxelizer(FEngine& engine) return; } - mFroxelBufferEntryCount = std::min( - FROXEL_BUFFER_MAX_ENTRY_COUNT, - engine.getDriverApi().getMaxUniformBufferSize() / 16u); + size_t const froxelBufferByteCount = getFroxelBufferByteCount(engine.getDriverApi()); + mFroxelBufferEntryCount = froxelBufferByteCount / sizeof(FroxelEntry); mRecordsBuffer = driverApi.createBufferObject(RECORD_BUFFER_ENTRY_COUNT, BufferObjectBinding::UNIFORM, BufferUsage::DYNAMIC); - mFroxelsBuffer = driverApi.createBufferObject(getFroxelBufferEntryCount() * 16u, + mFroxelsBuffer = driverApi.createBufferObject( + froxelBufferByteCount, BufferObjectBinding::UNIFORM, BufferUsage::DYNAMIC); } @@ -527,7 +533,7 @@ std::pair Froxelizer::clipToIndices(float2 const& clip) const no void Froxelizer::commit(DriverApi& driverApi) { // send data to GPU driverApi.updateBufferObject(mFroxelsBuffer, - { mFroxelBufferUser.data(), getFroxelBufferEntryCount() * 16u }, 0); + { mFroxelBufferUser.data(), getFroxelBufferEntryCount() * sizeof(FroxelEntry) }, 0); driverApi.updateBufferObject(mRecordsBuffer, { mRecordBufferUser.data(), RECORD_BUFFER_ENTRY_COUNT }, 0); diff --git a/filament/src/Froxelizer.h b/filament/src/Froxelizer.h index d052b7241b..440ded442f 100644 --- a/filament/src/Froxelizer.h +++ b/filament/src/Froxelizer.h @@ -35,15 +35,16 @@ namespace filament { -// Max number of froxels limited by: -// - max ubo size [min 16KiB] -// -// Also, increasing the number of froxels adds more pressure on the "record buffer" which stores -// the light indices per froxel. The record buffer is limited to min(16K[ubo], 64K[uint16]) entries, -// so with 8192 froxels, we can store 2 lights per froxels assuming they're all used. In practice, -// some froxels are not used, so we can store more. +// The number of froxel buffer entries is determined by max UBO size (see +// getFroxelBufferByteCount()). We also introduce the limit below because increasing the number of +// froxels adds more pressure on the "record buffer" which stores the light indices per froxel. The +// record buffer is limited to min(16K[ubo], 64K[uint16]) entries. In practice some froxels are not +// used, so we can store more. constexpr size_t FROXEL_BUFFER_MAX_ENTRY_COUNT = 8192; +// Froxel buffer UBO is an array of uvec4. Make sure that the buffer is properly aligned. +static_assert(FROXEL_BUFFER_MAX_ENTRY_COUNT % 4 == 0u); + class FEngine; class FCamera; class FTexture; @@ -146,6 +147,7 @@ public: inline uint16_t offset() const noexcept { return u32 >> 16u; } uint32_t u32 = 0; }; + static_assert(sizeof(FroxelEntry) == 4u); // we can't change this easily because the shader expects 16 indices per uint4 using RecordBufferType = uint8_t; @@ -157,6 +159,8 @@ public: // with 256 lights this implies 8 jobs (256 / 32) for froxelization. using LightGroupType = uint32_t; + static size_t getFroxelBufferByteCount(FEngine::DriverApi& driverApi) noexcept; + private: size_t getFroxelBufferEntryCount() const noexcept { return mFroxelBufferEntryCount; diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index d67fc11233..13dfc3c245 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -980,9 +980,9 @@ void FMaterial::processSpecializationConstants(FEngine& engine, Builder const& b int const maxInstanceCount = (engine.getActiveFeatureLevel() == FeatureLevel::FEATURE_LEVEL_0) ? 1 : CONFIG_MAX_INSTANCES; - int const maxFroxelBufferHeight = int(std::min( - FROXEL_BUFFER_MAX_ENTRY_COUNT / 4, - engine.getDriverApi().getMaxUniformBufferSize() / 16u)); + // The 16u below denotes the 16 bytes in a uvec4, which is how the froxel buffer is stored. + int const maxFroxelBufferHeight = + int(Froxelizer::getFroxelBufferByteCount(engine.getDriverApi()) / 16u); bool const staticTextureWorkaround = engine.getDriverApi().isWorkaroundNeeded(Workaround::METAL_STATIC_TEXTURE_TARGET_ERROR); diff --git a/filament/src/details/View.cpp b/filament/src/details/View.cpp index 4aa9b67532..c06e1667ef 100644 --- a/filament/src/details/View.cpp +++ b/filament/src/details/View.cpp @@ -126,6 +126,7 @@ FView::FView(FEngine& engine) mDefaultColorGrading = mColorGrading = engine.getDefaultColorGrading(); mColorPassDescriptorSet.init( + engine, mLightUbh, mFroxelizer.getRecordBuffer(), mFroxelizer.getFroxelBuffer()); diff --git a/filament/src/ds/ColorPassDescriptorSet.cpp b/filament/src/ds/ColorPassDescriptorSet.cpp index 29bb4b6eb9..b63efdbd43 100644 --- a/filament/src/ds/ColorPassDescriptorSet.cpp +++ b/filament/src/ds/ColorPassDescriptorSet.cpp @@ -126,6 +126,7 @@ ColorPassDescriptorSet::ColorPassDescriptorSet(FEngine& engine, } void ColorPassDescriptorSet::init( + FEngine& engine, BufferObjectHandle lights, BufferObjectHandle recordBuffer, BufferObjectHandle froxelBuffer) noexcept { @@ -135,7 +136,7 @@ void ColorPassDescriptorSet::init( descriptorSet.setBuffer(+PerViewBindingPoints::RECORD_BUFFER, recordBuffer, 0, sizeof(FroxelRecordUib)); descriptorSet.setBuffer(+PerViewBindingPoints::FROXEL_BUFFER, - froxelBuffer, 0, sizeof(FroxelsUib)); + froxelBuffer, 0, Froxelizer::getFroxelBufferByteCount(engine.getDriverApi())); } } @@ -573,4 +574,3 @@ void ColorPassDescriptorSet::setBuffer(descriptor_binding_t const binding, } } // namespace filament - diff --git a/filament/src/ds/ColorPassDescriptorSet.h b/filament/src/ds/ColorPassDescriptorSet.h index db0bfa2217..f4b86db529 100644 --- a/filament/src/ds/ColorPassDescriptorSet.h +++ b/filament/src/ds/ColorPassDescriptorSet.h @@ -79,6 +79,7 @@ public: TypedUniformBuffer& uniforms) noexcept; void init( + FEngine& engine, backend::BufferObjectHandle lights, backend::BufferObjectHandle recordBuffer, backend::BufferObjectHandle froxelBuffer) noexcept; diff --git a/libs/filamat/src/shaders/CodeGenerator.cpp b/libs/filamat/src/shaders/CodeGenerator.cpp index 4b41e7974d..614d8db4cc 100644 --- a/libs/filamat/src/shaders/CodeGenerator.cpp +++ b/libs/filamat/src/shaders/CodeGenerator.cpp @@ -295,35 +295,12 @@ utils::io::sstream& CodeGenerator::generateCommonProlog(utils::io::sstream& out, generateSpecializationConstant(out, "BACKEND_FEATURE_LEVEL", +ReservedSpecializationConstants::BACKEND_FEATURE_LEVEL, 1); - if (mTargetApi == TargetApi::VULKAN) { - // Note: This is a hack for a hack. - // - // Vulkan doesn't support sizing arrays within a block with specialization constants, - // as per this paragraph of the ARB_spir_v specification: - // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_gl_spirv.txt - // - // Arrays inside a block may be sized with a specialization constant, - // but the block will have a static layout. Changing the specialized size will - // not re-layout the block. In the absence of explicit offsets, the layout will be - // based on the default size of the array. - // - // CONFIG_MAX_INSTANCES is only needed for WebGL, so we can replace it with a constant. - // CONFIG_FROXEL_BUFFER_HEIGHT can be hardcoded to 2048 because only 3% of Android devices - // only support 16KiB buffer or less (1024 lines). - // - // We *could* leave these as a specialization constant, but this triggers a crashing bug with - // some Adreno drivers on Android. see: https://github.com/google/filament/issues/6444 - // - out << "const int CONFIG_MAX_INSTANCES = " << (int)CONFIG_MAX_INSTANCES << ";\n"; - out << "const int CONFIG_FROXEL_BUFFER_HEIGHT = 2048;\n"; - } else { - generateSpecializationConstant(out, "CONFIG_MAX_INSTANCES", - +ReservedSpecializationConstants::CONFIG_MAX_INSTANCES, (int)CONFIG_MAX_INSTANCES); + generateSpecializationConstant(out, "CONFIG_MAX_INSTANCES", + +ReservedSpecializationConstants::CONFIG_MAX_INSTANCES, (int)CONFIG_MAX_INSTANCES); - // the default of 1024 (16KiB) is needed for 32% of Android devices - generateSpecializationConstant(out, "CONFIG_FROXEL_BUFFER_HEIGHT", - +ReservedSpecializationConstants::CONFIG_FROXEL_BUFFER_HEIGHT, 1024); - } + // the default of 1024 (16KiB) is needed for 32% of Android devices + generateSpecializationConstant(out, "CONFIG_FROXEL_BUFFER_HEIGHT", + +ReservedSpecializationConstants::CONFIG_FROXEL_BUFFER_HEIGHT, 1024); // directional shadowmap visualization generateSpecializationConstant(out, "CONFIG_DEBUG_DIRECTIONAL_SHADOWMAP",