From 6c740f060e64cffd02ffe3aa7bda2a35bd0bb157 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 23 May 2022 13:21:06 -0700 Subject: [PATCH] move skinning info into PrimitiveInfo By design we shouldn't access the scene SOA when executing the high level commands. This change increases the size of a command to 64 bytes and PrimitiveInfo to 48 bytes (both with some small padding left). --- filament/src/RenderPass.cpp | 30 ++++++++++--------- filament/src/RenderPass.h | 27 +++++++++-------- filament/src/details/Scene.cpp | 11 ++++--- filament/src/details/Scene.h | 4 +-- .../include/private/filament/UibStructs.h | 8 ++++- libs/utils/include/utils/StructureOfArrays.h | 2 +- 6 files changed, 47 insertions(+), 35 deletions(-) diff --git a/filament/src/RenderPass.cpp b/filament/src/RenderPass.cpp index aef4046f33..4ff9cbe22a 100644 --- a/filament/src/RenderPass.cpp +++ b/filament/src/RenderPass.cpp @@ -51,6 +51,7 @@ RenderPass::RenderPass(RenderPass const& rhs) = default; RenderPass::~RenderPass() noexcept = default; RenderPass::Command* RenderPass::append(size_t count) noexcept { + // this is like a "in-place" realloc(). Works only with LinearAllocator. Command* const curr = mCommandArena.alloc(count); assert_invariant(mCommandBegin == nullptr || curr == mCommandEnd); if (mCommandBegin == nullptr) { @@ -301,9 +302,10 @@ void RenderPass::generateCommandsImpl(uint32_t extraFlags, auto const* const UTILS_RESTRICT soaWorldAABBCenter = soa.data(); auto const* const UTILS_RESTRICT soaVisibility = soa.data(); auto const* const UTILS_RESTRICT soaPrimitives = soa.data(); + auto const* const UTILS_RESTRICT soaSkinning = soa.data(); auto const* const UTILS_RESTRICT soaMorphing = soa.data(); auto const* const UTILS_RESTRICT soaVisibilityMask = soa.data(); - auto const* const UTILS_RESTRICT soaInstanceCount = soa.data(); + auto const* const UTILS_RESTRICT soaInstanceCount = soa.data(); const bool hasShadowing = renderFlags & HAS_SHADOWING; const bool viewInverseFrontFaces = renderFlags & HAS_INVERSE_FRONT_FACES; @@ -396,6 +398,7 @@ void RenderPass::generateCommandsImpl(uint32_t extraFlags, const bool writeDepthForShadowCasters = depthContainsShadowCasters & shadowCaster; const Slice& primitives = soaPrimitives[i]; + const FRenderableManager::SkinningBindingInfo& skinning = soaSkinning[i]; const FRenderableManager::MorphingBindingInfo& morphing = soaMorphing[i]; /* @@ -412,6 +415,8 @@ void RenderPass::generateCommandsImpl(uint32_t extraFlags, cmdColor.primitive.primitiveHandle = primitive.getHwHandle(); RenderPass::setupColorCommand(cmdColor, variant, mi, inverseFrontFaces); + cmdColor.primitive.skinningHandle = skinning.handle; + cmdColor.primitive.skinningOffset = skinning.offset; cmdColor.primitive.morphWeightBuffer = morphing.handle; cmdColor.primitive.morphTargetBuffer = morphTargets.buffer->getHwHandle(); @@ -506,6 +511,8 @@ void RenderPass::generateCommandsImpl(uint32_t extraFlags, cmdDepth.primitive.mi = mi; cmdDepth.primitive.rasterState.culling = mi->getCullingMode(); + cmdDepth.primitive.skinningHandle = skinning.handle; + cmdDepth.primitive.skinningOffset = skinning.offset; cmdDepth.primitive.morphWeightBuffer = morphing.handle; cmdDepth.primitive.morphTargetBuffer = morphTargets.buffer->getHwHandle(); @@ -552,22 +559,18 @@ void RenderPass::Executor::execute(const char* name, engine.flush(); driver.beginRenderPass(renderTarget, params); - recordDriverCommands(engine, driver, mBegin, mEnd, mRenderableSoa, params.readOnlyDepthStencil); + recordDriverCommands(engine, driver, mBegin, mEnd, params.readOnlyDepthStencil); driver.endRenderPass(); } UTILS_NOINLINE // no need to be inlined -void RenderPass::Executor::recordDriverCommands(FEngine& engine, - backend::DriverApi& driver, - const Command* first, const Command* last, - FScene::RenderableSoa const& soa, uint16_t readOnlyDepthStencil) const noexcept { +void RenderPass::Executor::recordDriverCommands(FEngine& engine, backend::DriverApi& driver, + const Command* first, const Command* last, uint16_t readOnlyDepthStencil) const noexcept { SYSTRACE_CALL(); if (first != last) { SYSTRACE_VALUE32("commandCount", last - first); - auto const* const UTILS_RESTRICT soaSkinning = soa.data(); - PolygonOffset dummyPolyOffset; PipelineState pipeline{ .polygonOffset = mPolygonOffset }; PolygonOffset* const pPipelinePolygonOffset = @@ -614,14 +617,13 @@ void RenderPass::Executor::recordDriverCommands(FEngine& engine, driver.bindUniformBufferRange(BindingPoints::PER_RENDERABLE, uboHandle, offset, sizeof(PerRenderableUib)); - auto skinning = soaSkinning[info.index]; - if (UTILS_UNLIKELY(skinning.handle)) { + if (UTILS_UNLIKELY(info.skinningHandle)) { // note: we can't bind less than CONFIG_MAX_BONE_COUNT due to glsl limitations driver.bindUniformBufferRange(BindingPoints::PER_RENDERABLE_BONES, - skinning.handle, - skinning.offset * sizeof(PerRenderableUibBone), + info.skinningHandle, + info.skinningOffset * sizeof(PerRenderableUibBone), CONFIG_MAX_BONE_COUNT * sizeof(PerRenderableUibBone)); - // note: even if skinning is only enabled, binding morphTargetBuffer is needed. + // note: even if only skinning is enabled, binding morphTargetBuffer is needed. driver.bindSamplers(BindingPoints::PER_RENDERABLE_MORPHING, info.morphTargetBuffer); } @@ -643,7 +645,7 @@ void RenderPass::Executor::recordDriverCommands(FEngine& engine, // ------------------------------------------------------------------------------------------------ RenderPass::Executor::Executor(RenderPass const* pass, Command const* b, Command const* e) noexcept - : mEngine(pass->mEngine), mBegin(b), mEnd(e), mRenderableSoa(*pass->mRenderableSoa), + : mEngine(pass->mEngine), mBegin(b), mEnd(e), mCustomCommands(pass->mCustomCommands), mUboHandle(pass->mUboHandle), mPolygonOffset(pass->mPolygonOffset), mPolygonOffsetOverride(pass->mPolygonOffsetOverride) { diff --git a/filament/src/RenderPass.h b/filament/src/RenderPass.h index 2fa6c23cc6..fe46110f29 100644 --- a/filament/src/RenderPass.h +++ b/filament/src/RenderPass.h @@ -213,25 +213,28 @@ public: return boolish ? std::numeric_limits::max() : uint64_t(0); } - struct PrimitiveInfo { // 40 bytes + struct PrimitiveInfo { // 48 bytes union { FMaterialInstance const* mi; - uint64_t reserved0 = {}; // ensures mi is 8 bytes on all archs + uint64_t padding = {}; // ensures mi is 8 bytes on all archs }; // 8 bytes + backend::RasterState rasterState; // 8 bytes backend::Handle primitiveHandle; // 4 bytes + backend::Handle skinningHandle; // 4 bytes backend::Handle morphWeightBuffer; // 4 bytes backend::Handle morphTargetBuffer; // 4 bytes - uint16_t index = 0; // 2 bytes + uint32_t index = 0; // 4 bytes + uint32_t skinningOffset = 0; // 4 bytes uint16_t instanceCount; // 2 bytes - backend::RasterState rasterState; // 8 bytes Variant materialVariant; // 1 byte - uint8_t reserved1[7] = {}; // 7 bytes + uint8_t reserved[5] = {}; // 5 bytes }; - static_assert(sizeof(PrimitiveInfo) == 40); + static_assert(sizeof(PrimitiveInfo) == 48); - struct alignas(8) Command { // 40 bytes + struct alignas(8) Command { // 64 bytes CommandKey key = 0; // 8 bytes - PrimitiveInfo primitive; // 40 bytes + PrimitiveInfo primitive; // 48 bytes + uint64_t reserved = 0; // 8 bytes bool operator < (Command const& rhs) const noexcept { return key < rhs.key; } // placement new declared as "throw" to avoid the compiler's null-check inline void* operator new (std::size_t, void* ptr) { @@ -239,7 +242,7 @@ public: return ptr; } }; - static_assert(sizeof(Command) == 48); + static_assert(sizeof(Command) == 64); static_assert(std::is_trivially_destructible_v, "Command isn't trivially destructible"); @@ -249,7 +252,7 @@ public: // Arena used for commands using Arena = utils::Arena< - utils::LinearAllocator, + utils::LinearAllocator, // note: can't change this allocator utils::LockingPolicy::NoLock, utils::TrackingPolicy::HighWatermark, utils::AreaPolicy::StaticArea>; @@ -319,7 +322,6 @@ public: FEngine& mEngine; Command const* mBegin; Command const* mEnd; - FScene::RenderableSoa const& mRenderableSoa; const CustomCommandVector mCustomCommands; const backend::Handle mUboHandle; const backend::PolygonOffset mPolygonOffset; @@ -328,8 +330,7 @@ public: Executor(RenderPass const* pass, Command const* b, Command const* e) noexcept; void recordDriverCommands(FEngine& engine, backend::DriverApi& driver, - const Command* first, const Command* last, - FScene::RenderableSoa const& soa, uint16_t readOnlyDepthStencil) const noexcept; + const Command* first, const Command* last, uint16_t readOnlyDepthStencil) const noexcept; public: Executor(Executor const& rhs); diff --git a/filament/src/details/Scene.cpp b/filament/src/details/Scene.cpp index 8b8a102ca8..f7edec4f6f 100644 --- a/filament/src/details/Scene.cpp +++ b/filament/src/details/Scene.cpp @@ -32,6 +32,7 @@ #include +using namespace filament::backend; using namespace filament::math; using namespace utils; @@ -196,7 +197,9 @@ void FScene::prepare(const mat4& worldOriginTransform, bool shadowReceiversAreCa } } -void FScene::updateUBOs(utils::Range visibleRenderables, backend::Handle renderableUbh) noexcept { +void FScene::updateUBOs( + Range visibleRenderables, + Handle renderableUbh) noexcept { FEngine::DriverApi& driver = mEngine.getDriverApi(); FRenderableManager& rcm = mEngine.getRenderableManager(); @@ -219,7 +222,7 @@ void FScene::updateUBOs(utils::Range visibleRenderables, backend::Hand // Using mat3f::getTransformForNormals handles non-uniform scaling, but DOESN'T guarantee that // the transformed normals will have unit-length, therefore they need to be normalized - // in the shader (that's already the case anyways, since normalization is needed after + // in the shader (that's already the case anyway, since normalization is needed after // interpolation). // // We pre-scale normals by the inverse of the largest scale factor to avoid @@ -287,7 +290,7 @@ void FScene::terminate(FEngine& engine) { } void FScene::prepareDynamicLights(const CameraInfo& camera, ArenaScope& rootArena, - backend::Handle lightUbh) noexcept { + Handle lightUbh) noexcept { FEngine::DriverApi& driver = mEngine.getDriverApi(); FLightManager& lcm = mEngine.getLightManager(); FScene::LightSoa& lightData = getLightData(); @@ -450,7 +453,7 @@ bool FScene::hasContactShadows() const noexcept { } UTILS_NOINLINE -void FScene::forEach(Invocable&& functor) const noexcept { +void FScene::forEach(Invocable&& functor) const noexcept { std::for_each(mEntities.begin(), mEntities.end(), std::move(functor)); } diff --git a/filament/src/details/Scene.h b/filament/src/details/Scene.h index a0f1240c0e..3365a3bbb7 100644 --- a/filament/src/details/Scene.h +++ b/filament/src/details/Scene.h @@ -84,8 +84,8 @@ public: enum { RENDERABLE_INSTANCE, // 4 | instance of the Renderable component WORLD_TRANSFORM, // 16 | instance of the Transform component - VISIBILITY_STATE, // 1 | visibility data of the component - SKINNING_BUFFER, // 8 | bones uniform buffer handle, count, offset + VISIBILITY_STATE, // 2 | visibility data of the component + SKINNING_BUFFER, // 8 | bones uniform buffer handle, offset MORPHING_BUFFER, // 16 | weights uniform buffer handle, count, morph targets WORLD_AABB_CENTER, // 12 | world-space bounding box center of the renderable VISIBLE_MASK, // 2 | each bit represents a visibility in a pass diff --git a/libs/filabridge/include/private/filament/UibStructs.h b/libs/filabridge/include/private/filament/UibStructs.h index 9677ee988e..55eb86188c 100644 --- a/libs/filabridge/include/private/filament/UibStructs.h +++ b/libs/filabridge/include/private/filament/UibStructs.h @@ -180,8 +180,13 @@ struct alignas(256) PerRenderableUib { // NOLINT(cppcoreguidelines-pro-type-memb uint32_t flags; // see packFlags() below uint32_t channels; // 0x000000ll uint32_t objectId; // used for picking + // TODO: We need a better solution, this currently holds the average local scale for the renderable float userData; + float reserved0; + float reserved1; + float reserved2; + math::float4 reserved[7]; static uint32_t packFlags(bool skinning, bool morphing, bool contactShadows) noexcept { return (skinning ? 1 : 0) | @@ -189,7 +194,8 @@ struct alignas(256) PerRenderableUib { // NOLINT(cppcoreguidelines-pro-type-memb (contactShadows ? 4 : 0); } }; -static_assert(sizeof(PerRenderableUib) % 256 == 0, "sizeof(Transform) should be a multiple of 256"); + +static_assert(sizeof(PerRenderableUib) == 256, "sizeof(PerRenderableUib) must be 256 bytes"); struct LightsUib { // NOLINT(cppcoreguidelines-pro-type-member-init) static constexpr utils::StaticString _name{ "LightsUniforms" }; diff --git a/libs/utils/include/utils/StructureOfArrays.h b/libs/utils/include/utils/StructureOfArrays.h index e5cbb422e8..dd879a6fa3 100644 --- a/libs/utils/include/utils/StructureOfArrays.h +++ b/libs/utils/include/utils/StructureOfArrays.h @@ -509,7 +509,7 @@ private: mSize = needed; } - // this calculate the offset adjusted for all data alignment of a given array + // this calculates the offset adjusted for all data alignment of a given array static inline size_t getOffset(size_t index, size_t capacity) noexcept { auto offsets = getOffsets(capacity); return offsets[index];