ATOM-15658 Better option of CreateCommonBuffer requires unique buffer name (#1133)

* ATOM-15658 Better option of CreateCommonBuffer requires unique buffer name
- Change the CreateCommonBuffer function to not require an unique  name by default.
- Remove the code for generating unique buffer names.
- Add buffer name to BufferAsset so it can be used for device object name instead of using asset file name.
- Change RPI::Buffer to use BufferName_AssetUuid as attachment id.
This commit is contained in:
Qing Tao
2021-06-04 10:28:56 -07:00
committed by GitHub
parent 5d4226df16
commit dcdd63966e
25 changed files with 104 additions and 77 deletions
@@ -311,14 +311,9 @@ namespace AZ
{
auto tileBufferResolution = GetTileDataBufferResolution();
// generate a UUID for the buffer name to keep it unique when there are multiple render pipelines
AZ::Uuid uuid = AZ::Uuid::CreateRandom();
AZStd::string uuidString;
uuid.ToString(uuidString);
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadWrite;
desc.m_bufferName = AZStd::string::format("LightList_%s", uuidString.c_str());
desc.m_bufferName = "LightList";
desc.m_elementSize = sizeof(uint32_t);
desc.m_byteCount = tileBufferResolution.m_width * tileBufferResolution.m_height * 256 * sizeof(uint32_t);
m_lightList = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
@@ -118,14 +118,9 @@ namespace AZ
void LightCullingRemap::CreateRemappedLightListBuffer()
{
// generate a UUID for the buffer name to keep it unique when there are multiple render pipelines
AZ::Uuid uuid = AZ::Uuid::CreateRandom();
AZStd::string uuidString;
uuid.ToString(uuidString);
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadWrite;
desc.m_bufferName = AZStd::string::format("LightListRemapped_%s", uuidString.c_str());
desc.m_bufferName = "LightListRemapped";
desc.m_elementSize = RHI::GetFormatSize(LightListRemappedFormat);
desc.m_byteCount = m_tileDim.m_width * m_tileDim.m_height * NumBins * MaxLightsPerTile * desc.m_elementSize;
m_lightListRemapped = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
@@ -74,6 +74,7 @@ namespace AZ
desc.m_elementFormat = filters.front()->GetElementFormat();
desc.m_byteCount = totalElementCount * elementSize;
desc.m_bufferData = data.data();
desc.m_isUniqueName = true;
auto buffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
@@ -100,16 +100,9 @@ namespace AZ
bool ExposureControlSettings::InitCommonBuffer()
{
// generate a UUID for the buffer name to keep it unique
AZ::Uuid uuid = AZ::Uuid::CreateRandom();
AZStd::string uuidString;
uuid.ToString(uuidString);
AZStd::string bufferName = AZStd::string::format("%s_%s", ExposureControlBufferBaseName, uuidString.c_str());
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::Constant;
desc.m_bufferName = bufferName;
desc.m_bufferName = ExposureControlBufferName;
desc.m_byteCount = sizeof(ShaderParameters);
desc.m_elementSize = sizeof(ShaderParameters);
@@ -117,7 +110,7 @@ namespace AZ
if (!m_buffer)
{
AZ_Assert(false, "Failed to create the RPI::Buffer[%s] which is used for the exposure control feature.", bufferName.c_str());
AZ_Assert(false, "Failed to create the RPI::Buffer[%s] which is used for the exposure control feature.", desc.m_bufferName.c_str());
return false;
}
@@ -28,8 +28,8 @@ namespace AZ
{
class PostProcessSettings;
// Base name of the buffer used for the exposure control feature. Usually distinct identifier will be added to this name for each exposure control settings.
static const char* const ExposureControlBufferBaseName = "ExposureControlBuffer";
// Name of the buffer used for the exposure control feature
static const char* const ExposureControlBufferName = "ExposureControlBuffer";
// The post process sub-settings class for the exposure control feature
class ExposureControlSettings final
@@ -47,9 +47,8 @@ namespace AZ
m_getDepthPass = static_cast<DepthOfFieldWriteFocusDepthFromGpuPass*>(pass.get());
// Create buffer for read back focus depth. We append static counter to avoid name conflicts.
AZStd::string bufferName = AZStd::string::format("DepthOfFieldReadBackAutoFocusDepthBuffer_%d", s_bufferInstance++);
RPI::CommonBufferDescriptor desc;
desc.m_bufferName = bufferName;
desc.m_bufferName = "DepthOfFieldReadBackAutoFocusDepthBuffer";
desc.m_poolType = RPI::CommonBufferPoolType::ReadWrite;
desc.m_byteCount = sizeof(float);
desc.m_elementSize = aznumeric_cast<uint32_t>(desc.m_byteCount);
@@ -71,6 +71,7 @@ namespace AZ
desc.m_bufferName = bufferName;
desc.m_byteCount = sizeof(ShaderParameters);
desc.m_elementSize = sizeof(ShaderParameters);
desc.m_isUniqueName = true;
m_buffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
}
@@ -34,7 +34,7 @@ namespace AZ
{
namespace Render
{
static const char* const EyeAdaptationBufferBaseName = "EyeAdaptationBuffer";
static const char* const EyeAdaptationBufferName = "EyeAdaptationBuffer";
RPI::Ptr<EyeAdaptationPass> EyeAdaptationPass::Create(const RPI::PassDescriptor& descriptor)
{
@@ -49,12 +49,10 @@ namespace AZ
void EyeAdaptationPass::InitBuffer()
{
AZStd::string bufferName = AZStd::string::format("%s_%p", EyeAdaptationBufferBaseName, this);
ExposureCalculationData defaultData;
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadWrite;
desc.m_bufferName = bufferName;
desc.m_bufferName = EyeAdaptationBufferName;
desc.m_byteCount = sizeof(ExposureCalculationData);
desc.m_elementSize = aznumeric_cast<uint32_t>(desc.m_byteCount);
desc.m_bufferData = &defaultData;
@@ -62,14 +62,9 @@ namespace AZ
void LuminanceHistogramGeneratorPass::CreateHistogramBuffer()
{
// generate a UUID for the buffer name to keep it unique when there are multiple render pipelines
AZ::Uuid uuid = AZ::Uuid::CreateRandom();
AZStd::string uuidString;
uuid.ToString(uuidString);
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadWrite;
desc.m_bufferName = AZStd::string::format("LuminanceHistogramBuffer_%s", uuidString.c_str());
desc.m_bufferName = "LuminanceHistogramBuffer";
desc.m_elementSize = sizeof(uint32_t);
desc.m_byteCount = NumHistogramBins * sizeof(uint32_t);
desc.m_elementFormat = RHI::Format::R32_UINT;
@@ -210,12 +210,10 @@ namespace AZ
if (m_meshInfoBuffer == nullptr)
{
AZStd::string uuidString = AZ::Uuid::CreateRandom().ToString<AZStd::string>();
// allocate the MeshInfo structured buffer
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc.m_bufferName = AZStd::string::format("RayTracingMeshInfo_%s", uuidString.c_str());
desc.m_bufferName = "RayTracingMeshInfo";
desc.m_byteCount = newMeshByteCount;
desc.m_elementSize = sizeof(MeshInfo);
m_meshInfoBuffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
@@ -283,12 +281,10 @@ namespace AZ
if (m_materialInfoBuffer == nullptr)
{
AZStd::string uuidString = AZ::Uuid::CreateRandom().ToString<AZStd::string>();
// allocate the MaterialInfo structured buffer
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc.m_bufferName = AZStd::string::format("RayTracingMaterialInfo_%s", uuidString.c_str());
desc.m_bufferName = "RayTracingMaterialInfo";
desc.m_byteCount = newMaterialByteCount;
desc.m_elementSize = sizeof(MaterialInfo);
m_materialInfoBuffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
@@ -193,7 +193,7 @@ namespace AZ
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::Constant;
desc.m_bufferName = AZStd::string::format("SkyboxBuffer_%p", this);
desc.m_bufferName = "SkyboxBuffer";
desc.m_byteCount = byteCount;
desc.m_elementSize = byteCount;
desc.m_bufferData = &m_physicalSkyData;
@@ -89,13 +89,13 @@ namespace AZ
// Create the transform buffer, grow by powers of two
RPI::CommonBufferDescriptor desc2;
desc2.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc2.m_bufferName = AZStd::string::format("'m_objectToWorldBuffer_%" PRIXPTR, reinterpret_cast<uintptr_t>(this));
desc2.m_bufferName = "m_objectToWorldBuffer";
desc2.m_byteCount = byteCount;
desc2.m_elementSize = elementSize;
m_objectToWorldBuffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc2);
desc2.m_bufferName = AZStd::string::format("'m_objectToWorldHistoryBuffer_%p", this);
desc2.m_bufferName = "m_objectToWorldHistoryBuffer";
m_objectToWorldHistoryBuffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc2);
}
else
@@ -119,7 +119,7 @@ namespace AZ
// Create the normal buffer, grow by powers of two
RPI::CommonBufferDescriptor desc2;
desc2.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc2.m_bufferName = AZStd::string::format("'m_objectToWorldInverseTransposeBuffer_%" PRIXPTR, reinterpret_cast<uintptr_t>(this));
desc2.m_bufferName = "m_objectToWorldInverseTransposeBuffer";
desc2.m_byteCount = byteCount;
desc2.m_elementSize = elementSize;
@@ -40,13 +40,11 @@ namespace AZ
if (m_bufferIndex.IsValid())
{
AZStd::string bufferName = AZStd::string::format("%s_%" PRIXPTR, descriptor.m_bufferName.c_str(), reinterpret_cast<uintptr_t>(this));
uint32_t byteCount = RHI::NextPowerOfTwo(GetMax<uint32_t>(BufferMinSize, m_elementCount * m_elementSize));
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc.m_bufferName = bufferName;
desc.m_bufferName = descriptor.m_bufferName;
desc.m_byteCount = byteCount;
desc.m_elementSize = descriptor.m_elementSize;
@@ -112,6 +112,8 @@ namespace AZ
AZStd::mutex m_pendingUploadMutex;
RHI::BufferViewDescriptor m_bufferViewDescriptor;
RHI::AttachmentId m_attachmentId;
};
template <class structureType>
@@ -35,7 +35,7 @@ namespace AZ
// BufferSystemInterface overrides...
RHI::Ptr<RHI::BufferPool> GetCommonBufferPool(CommonBufferPoolType poolType) override;
Data::Instance<Buffer> CreateBufferFromCommonPool(const CommonBufferDescriptor& descriptor) override;
Data::Instance<Buffer> FindCommonBuffer(AZStd::string_view bufferName) override;
Data::Instance<Buffer> FindCommonBuffer(AZStd::string_view uniqueBufferName) override;
void Init();
void Shutdown();
@@ -53,6 +53,9 @@ namespace AZ
RHI::Format m_elementFormat = RHI::Format::Unknown; //<! [optional] If it's specified with a valid format, the size of this format will be used instead of m_elementSize
AZ::u64 m_byteCount = 0;
const void* m_bufferData = nullptr; //<! [optional] Initial data content of this buffer. This data buffer size needs to be same as m_bufferSizeInbytes
//! Set to true if you want this buffer to be discoverable by BufferSystemInterface::FindCommonBuffer using m_bufferName.
//! Note that create buffer may fail if there is a buffer with the same name.
bool m_isUniqueName = false;
};
class BufferSystemInterface
@@ -78,7 +81,7 @@ namespace AZ
virtual Data::Instance<Buffer> CreateBufferFromCommonPool(const CommonBufferDescriptor& descriptor) = 0;
//! Find a buffer by name. The buffer has to be created by CreateBufferFromCommonPool function
virtual Data::Instance<Buffer> FindCommonBuffer(AZStd::string_view bufferName) = 0;
virtual Data::Instance<Buffer> FindCommonBuffer(AZStd::string_view uniqueBufferName) = 0;
};
} // namespace RPI
} // namespace AZ
@@ -60,11 +60,15 @@ namespace AZ
const Data::Asset<ResourcePoolAsset>& GetPoolAsset() const;
CommonBufferPoolType GetCommonPoolType() const;
const AZStd::string& GetName() const;
private:
// Called by asset creators to assign the asset to a ready state.
void SetReady();
AZStd::string m_name;
AZStd::vector<uint8_t> m_buffer;
RHI::BufferDescriptor m_bufferDescriptor;
@@ -114,7 +114,7 @@ namespace AZ
if (auto* serialize = azrtti_cast<SerializeContext*>(context))
{
serialize->Class<ModelAssetBuilderComponent, SceneAPI::SceneCore::ExportingComponent>()
->Version(26); // [ATOM-14992]
->Version(27); // [ATOM-15658]
}
}
@@ -32,10 +32,6 @@ namespace AZ
auto buffer = Data::InstanceDatabase<Buffer>::Instance().FindOrCreate(
Data::InstanceId::CreateFromAssetId(bufferAsset.GetId()),
bufferAsset);
if (buffer && buffer->m_rhiBuffer)
{
buffer->m_rhiBuffer->SetName(Name(bufferAsset.GetHint()));
}
return buffer;
}
@@ -170,6 +166,16 @@ namespace AZ
return resultCode;
}
}
m_rhiBuffer->SetName(Name(bufferAsset.GetName()));
// Only generate buffer's attachment id if the buffer is writable
if (RHI::CheckBitsAny(m_rhiBuffer->GetDescriptor().m_bindFlags,
RHI::BufferBindFlags::ShaderWrite | RHI::BufferBindFlags::CopyWrite | RHI::BufferBindFlags::DynamicInputAssembly))
{
// attachment id = bufferName_bufferInstanceId
m_attachmentId = Name(bufferAsset.GetName() + "_" + bufferAsset.GetId().m_guid.ToString<AZStd::string>(false, false));
}
return RHI::ResultCode::Success;
}
@@ -312,7 +318,8 @@ namespace AZ
const RHI::AttachmentId& Buffer::GetAttachmentId() const
{
return m_rhiBuffer->GetName();
AZ_Assert(!m_attachmentId.GetStringView().empty(), "Read-only buffer doesn't need attachment id");
return m_attachmentId;
}
const RHI::BufferViewDescriptor& Buffer::GetBufferViewDescriptor() const
@@ -152,15 +152,22 @@ namespace AZ
}
Data::Instance<Buffer> BufferSystem::CreateBufferFromCommonPool(const CommonBufferDescriptor& descriptor)
{
Uuid bufferId = Uuid::CreateName(descriptor.m_bufferName.c_str());
// Report error if there is a buffer with same name.
// Note: this shouldn't return the existing buffer because users are expecting a newly created buffer.
if (Data::InstanceDatabase<Buffer>::Instance().Find(Data::InstanceId(bufferId)))
{
Uuid bufferId;
if (descriptor.m_isUniqueName)
{
AZ_Error("BufferSystem", false, "Buffer with same name '%s' already exist", descriptor.m_bufferName.c_str());
return nullptr;
bufferId = Uuid::CreateName(descriptor.m_bufferName.c_str());
// Report error if there is a buffer with same name.
// Note: this shouldn't return the existing buffer because users are expecting a newly created buffer.
if (Data::InstanceDatabase<Buffer>::Instance().Find(Data::InstanceId(bufferId)))
{
AZ_Error("BufferSystem", false, "Buffer with same name '%s' already exist", descriptor.m_bufferName.c_str());
return nullptr;
}
}
else
{
bufferId = Uuid::CreateRandom();
}
RHI::Ptr<RHI::BufferPool> bufferPool = GetCommonBufferPool(descriptor.m_poolType);
@@ -207,9 +214,9 @@ namespace AZ
return nullptr;
}
Data::Instance<Buffer> BufferSystem::FindCommonBuffer(AZStd::string_view bufferName)
Data::Instance<Buffer> BufferSystem::FindCommonBuffer(AZStd::string_view uniqueBufferName)
{
Uuid bufferId = Uuid::CreateName(bufferName.data());
Uuid bufferId = Uuid::CreateName(uniqueBufferName.data());
return Data::InstanceDatabase<Buffer>::Instance().Find(Data::InstanceId(bufferId));
}
} // namespace RPI
@@ -30,7 +30,7 @@ namespace AZ
// Create the ring buffer from common pool
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::DynamicInputAssembly;
desc.m_bufferName = AZStd::string::format("DyanmicBufferRing_%p", this);
desc.m_bufferName = "DyanmicBufferRing";
desc.m_elementSize = 1;
desc.m_byteCount = ringBufferSize;
m_ringBuffer = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
@@ -30,7 +30,8 @@ namespace AZ
if (auto* serializeContext = azrtti_cast<SerializeContext*>(context))
{
serializeContext->Class<BufferAsset>()
->Version(1)
->Version(2)
->Field("Name", &BufferAsset::m_name)
->Field("Buffer", &BufferAsset::m_buffer)
->Field("BufferDescriptor", &BufferAsset::m_bufferDescriptor)
->Field("BufferViewDescriptor", &BufferAsset::m_bufferViewDescriptor)
@@ -80,5 +81,10 @@ namespace AZ
{
return m_poolType;
}
const AZStd::string& BufferAsset::GetName() const
{
return m_name;
}
} //namespace RPI
} // namespace AZ
@@ -152,7 +152,10 @@ namespace AZ
void BufferAssetCreator::SetBufferName(AZStd::string_view name)
{
m_asset.SetHint(name);
if (ValidateIsReady())
{
m_asset->m_name = name;
}
}
bool BufferAssetCreator::Clone(const Data::Asset<BufferAsset>& sourceAsset, Data::Asset<BufferAsset>& clonedResult, Data::AssetId& inOutLastCreatedAssetId)
@@ -474,6 +474,7 @@ namespace UnitTest
desc.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc.m_bufferName = "Buffer1";
desc.m_byteCount = bufferInfo.m_bufferDescriptor.m_byteCount;
desc.m_isUniqueName = true;
Data::Instance<RPI::Buffer> bufferInst = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
// buffer created
@@ -488,8 +489,33 @@ namespace UnitTest
EXPECT_EQ(bufferFound2.get(), nullptr);
}
// Failed if creates a buffer with duplicated name with existing buffer
TEST_F(BufferTests, BufferSystem_CreateDuplicatedNamedBuffer_Fail)
// Failed if creates a buffe which has a same name with existing buffer
// and has m_isUniqueName is enabled
TEST_F(BufferTests, BufferSystem_CreateDuplicatedNamedBufferEnableUniqueName_Fail)
{
using namespace AZ;
ExpectedBuffer bufferInfo = CreateValidBuffer();
RPI::CommonBufferDescriptor desc;
desc.m_poolType = RPI::CommonBufferPoolType::ReadOnly;
desc.m_bufferName = "Buffer1";
desc.m_byteCount = bufferInfo.m_bufferDescriptor.m_byteCount;
desc.m_isUniqueName = true;
Data::Instance<RPI::Buffer> bufferInst = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
// buffer created
EXPECT_NE(bufferInst.get(), nullptr);
AZ_TEST_START_ASSERTTEST;
Data::Instance<RPI::Buffer> bufferInst2 = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
AZ_TEST_STOP_ASSERTTEST(1);
// buffer NOT created
EXPECT_EQ(bufferInst2.get(), nullptr);
}
// create a buffer which has a same name with existing buffer
TEST_F(BufferTests, BufferSystem_CreateDuplicatedNamedBuffers_Success)
{
using namespace AZ;
@@ -503,12 +529,10 @@ namespace UnitTest
Data::Instance<RPI::Buffer> bufferInst = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
// buffer created
EXPECT_NE(bufferInst.get(), nullptr);
AZ_TEST_START_ASSERTTEST;
Data::Instance<RPI::Buffer> bufferInst2 = RPI::BufferSystemInterface::Get()->CreateBufferFromCommonPool(desc);
AZ_TEST_STOP_ASSERTTEST(1);
// buffer NOT created
EXPECT_EQ(bufferInst2.get(), nullptr);
// buffer created
EXPECT_NE(bufferInst2.get(), nullptr);
}
// Buffer instance creation unit tests
@@ -595,7 +595,7 @@ namespace AZ
// Create a buffer and populate it with the transforms
RPI::CommonBufferDescriptor descriptor;
descriptor.m_bufferData = boneTransforms.data();
descriptor.m_bufferName = AZStd::string::format("BoneTransformBuffer_%s_%s", actorInstance->GetActor()->GetName(), Uuid::CreateRandom().ToString<AZStd::string>().c_str());
descriptor.m_bufferName = AZStd::string::format("BoneTransformBuffer_%s", actorInstance->GetActor()->GetName());
descriptor.m_byteCount = boneTransforms.size() * sizeof(float);
descriptor.m_elementSize = floatsPerBone * sizeof(float);
descriptor.m_poolType = RPI::CommonBufferPoolType::ReadOnly;