Skip to content

Commit 6c1af7c

Browse files
AnastaZIukAnastaZIuk
committed
Optimize memory management in PLY loader (do not waste time on memcpy, limit attributes memory usage to minimum)
1 parent 8bb4a2c commit 6c1af7c

File tree

3 files changed

+146
-60
lines changed

3 files changed

+146
-60
lines changed

‎examples_tests/27.LoaderFixes/main.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ int main()
8686

8787
auto getMeshDependentDrawData = [&](core::smart_refctd_ptr<asset::ICPUMesh> cpuMesh, bool isPLY) -> DependentDrawData
8888
{
89-
asset::ICPUMeshBuffer* const firstMeshBuffer = cpuMesh->getMeshBuffers().begin()[0];
90-
asset::ICPUDescriptorSetLayout* ds1layout = firstMeshBuffer->getPipeline()->getLayout()->getDescriptorSetLayout(1u); //! DS1
89+
const asset::ICPUMeshBuffer* const firstMeshBuffer = cpuMesh->getMeshBuffers().begin()[0];
90+
const asset::ICPUDescriptorSetLayout* ds1layout = firstMeshBuffer->getPipeline()->getLayout()->getDescriptorSetLayout(1u); //! DS1
9191
const asset::IRenderpassIndependentPipelineMetadata* pipelineMetadata;
9292
{
9393
if(isPLY)

‎src/nbl/asset/interchange/CPLYMeshFileLoader.cpp

Lines changed: 142 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ void CPLYMeshFileLoader::initialize()
133133
const auto currentBitmask = core::createBitmask({ attrib });
134134
inputParams.enabledBindingFlags |= currentBitmask;
135135
inputParams.enabledAttribFlags |= currentBitmask;
136-
inputParams.bindings[attrib] = { 16, EVIR_PER_VERTEX };
136+
inputParams.bindings[attrib] = { asset::getTexelOrBlockBytesize(static_cast<E_FORMAT>(vertexAttribParamsAllOptions[attrib].format)), EVIR_PER_VERTEX };
137137
inputParams.attributes[attrib] = vertexAttribParamsAllOptions[attrib];
138138
}
139139

@@ -353,7 +353,7 @@ asset::SAssetBundle CPLYMeshFileLoader::loadAsset(io::IReadFile* _file, const as
353353

354354
mb->setNormalnAttributeIx(3u);
355355

356-
core::vector<core::vectorSIMDf> attribs[4];
356+
asset::SBufferBinding<asset::ICPUBuffer> attributes[4];
357357
core::vector<uint32_t> indices;
358358

359359
bool hasNormals = true;
@@ -364,14 +364,56 @@ asset::SAssetBundle CPLYMeshFileLoader::loadAsset(io::IReadFile* _file, const as
364364
// do we want this element type?
365365
if (ctx.ElementList[i]->Name == "vertex")
366366
{
367+
auto& plyVertexElement = *ctx.ElementList[i];
368+
369+
for (auto& vertexProperty : plyVertexElement.Properties)
370+
{
371+
const auto propertyName = vertexProperty.Name;
372+
373+
if (propertyName == "x" || propertyName == "y" || propertyName == "z")
374+
{
375+
if (!attributes[ET_POS].buffer)
376+
{
377+
attributes[ET_POS].offset = 0u;
378+
attributes[ET_POS].buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(asset::getTexelOrBlockBytesize(EF_R32G32B32_SFLOAT) * plyVertexElement.Count);
379+
}
380+
}
381+
else if(propertyName == "nx" || propertyName == "ny" || propertyName == "nz")
382+
{
383+
if (!attributes[ET_NORM].buffer)
384+
{
385+
attributes[ET_NORM].offset = 0u;
386+
attributes[ET_NORM].buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(asset::getTexelOrBlockBytesize(EF_R32G32B32_SFLOAT) * plyVertexElement.Count);
387+
}
388+
}
389+
else if (propertyName == "u" || propertyName == "s" || propertyName == "v" || propertyName == "t")
390+
{
391+
if (!attributes[ET_UV].buffer)
392+
{
393+
attributes[ET_UV].offset = 0u;
394+
attributes[ET_UV].buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(asset::getTexelOrBlockBytesize(EF_R32G32_SFLOAT) * plyVertexElement.Count);
395+
}
396+
}
397+
else if (propertyName == "red" || propertyName == "green" || propertyName == "blue" || propertyName == "alpha")
398+
{
399+
if (!attributes[ET_COL].buffer)
400+
{
401+
attributes[ET_COL].offset = 0u;
402+
attributes[ET_COL].buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(asset::getTexelOrBlockBytesize(EF_R32G32B32A32_SFLOAT) * plyVertexElement.Count);
403+
}
404+
}
405+
}
406+
367407
// loop through vertex properties
368408
for (uint32_t j=0; j<ctx.ElementList[i]->Count; ++j)
369-
hasNormals &= readVertex(ctx, *ctx.ElementList[i], attribs, _params);
409+
hasNormals &= readVertex(ctx, plyVertexElement, attributes, j, _params);
370410
}
371411
else if (ctx.ElementList[i]->Name == "face")
372412
{
413+
const size_t indicesCount = ctx.ElementList[i]->Count;
414+
373415
// read faces
374-
for (uint32_t j=0; j < ctx.ElementList[i]->Count; ++j)
416+
for (uint32_t j=0; j < indicesCount; ++j)
375417
readFace(ctx, *ctx.ElementList[i], indices);
376418
}
377419
else
@@ -386,22 +428,22 @@ asset::SAssetBundle CPLYMeshFileLoader::loadAsset(io::IReadFile* _file, const as
386428

387429
if (indices.size())
388430
{
389-
asset::SBufferBinding<ICPUBuffer> indexBinding = {0, core::make_smart_refctd_ptr<asset::ICPUBuffer>(indices.size() * sizeof(uint32_t))};
390-
memcpy(indexBinding.buffer->getPointer(), indices.data(), indexBinding.buffer->getSize());
391-
auto DATA = reinterpret_cast<uint32_t*>(indexBinding.buffer->getPointer());
431+
asset::SBufferBinding<ICPUBuffer> indexBinding = { 0, core::make_smart_refctd_ptr<asset::ICPUBuffer>(indices.size() * sizeof(uint32_t)) };
432+
memcpy(indexBinding.buffer->getPointer(), indices.data(), indexBinding.buffer->getSize());
433+
392434
mb->setIndexCount(indices.size());
393435
mb->setIndexBufferBinding(std::move(indexBinding));
394-
mb->setIndexType(asset::EIT_32BIT);
436+
mb->setIndexType(asset::EIT_32BIT);
395437

396-
if (!genVertBuffersForMBuffer(mb.get(), attribs, ctx))
438+
if (!genVertBuffersForMBuffer(mb.get(), attributes, ctx))
397439
return {};
398440
}
399441
else
400442
{
401-
mb->setIndexCount(attribs[ET_POS].size());
443+
mb->setIndexCount(attributes[ET_POS].buffer->getSize());
402444
mb->setIndexType(EIT_UNKNOWN);
403445

404-
if (!genVertBuffersForMBuffer(mb.get(), attribs, ctx))
446+
if (!genVertBuffersForMBuffer(mb.get(), attributes, ctx))
405447
return {};
406448
}
407449

@@ -429,7 +471,7 @@ static void performActionBasedOnOrientationSystem(const asset::IAssetLoader::SAs
429471
performOnLeftHanded();
430472
}
431473

432-
bool CPLYMeshFileLoader::readVertex(SContext& _ctx, const SPLYElement& Element, core::vector<core::vectorSIMDf> _outAttribs[4], const asset::IAssetLoader::SAssetLoadParams& _params)
474+
bool CPLYMeshFileLoader::readVertex(SContext& _ctx, const SPLYElement& Element, asset::SBufferBinding<asset::ICPUBuffer> outAttributes[4], const uint32_t& currentVertexIndex, const asset::IAssetLoader::SAssetLoadParams& _params)
433475
{
434476
if (!_ctx.IsBinaryFile)
435477
getNextLine(_ctx);
@@ -438,94 +480,151 @@ bool CPLYMeshFileLoader::readVertex(SContext& _ctx, const SPLYElement& Element,
438480
attribs[ET_COL].second.W = 1.f;
439481
attribs[ET_NORM].second.Y = 1.f;
440482

483+
constexpr auto ET_POS_BYTESIZE = asset::getTexelOrBlockBytesize<EF_R32G32B32_SFLOAT>();
484+
constexpr auto ET_NORM_BYTESIZE = asset::getTexelOrBlockBytesize<EF_R32G32B32_SFLOAT>();
485+
constexpr auto ET_UV_BYTESIZE = asset::getTexelOrBlockBytesize<EF_R32G32_SFLOAT>();
486+
constexpr auto ET_COL_BYTESIZE = asset::getTexelOrBlockBytesize<EF_R32G32B32A32_SFLOAT>();
487+
441488
bool result = false;
442489
for (uint32_t i = 0; i < Element.Properties.size(); ++i)
443490
{
444491
E_PLY_PROPERTY_TYPE t = Element.Properties[i].Type;
445492

446493
if (Element.Properties[i].Name == "x")
447494
{
448-
attribs[ET_POS].second.X = getFloat(_ctx, t);
495+
auto& value = attribs[ET_POS].second.X = getFloat(_ctx, t);
449496
attribs[ET_POS].first = true;
497+
498+
if (_params.loaderFlags & E_LOADER_PARAMETER_FLAGS::ELPF_RIGHT_HANDED_MESHES)
499+
performActionBasedOnOrientationSystem<float>(value, [](float& varToFlip) { varToFlip = -varToFlip; });
500+
501+
const size_t propertyOffset = ET_POS_BYTESIZE * currentVertexIndex;
502+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_POS].buffer->getPointer()) + propertyOffset;
503+
504+
reinterpret_cast<float*>(data)[0] = value;
450505
}
451506
else if (Element.Properties[i].Name == "y")
452507
{
453-
attribs[ET_POS].second.Y = getFloat(_ctx, t);
508+
auto& value = attribs[ET_POS].second.Y = getFloat(_ctx, t);
454509
attribs[ET_POS].first = true;
510+
511+
const size_t propertyOffset = ET_POS_BYTESIZE * currentVertexIndex;
512+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_POS].buffer->getPointer()) + propertyOffset;
513+
514+
reinterpret_cast<float*>(data)[1] = value;
455515
}
456516
else if (Element.Properties[i].Name == "z")
457517
{
458-
attribs[ET_POS].second.Z = getFloat(_ctx, t);
518+
auto& value = attribs[ET_POS].second.Z = getFloat(_ctx, t);
459519
attribs[ET_POS].first = true;
520+
521+
const size_t propertyOffset = ET_POS_BYTESIZE * currentVertexIndex;
522+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_POS].buffer->getPointer()) + propertyOffset;
523+
524+
reinterpret_cast<float*>(data)[2] = value;
460525
}
461526
else if (Element.Properties[i].Name == "nx")
462527
{
463-
attribs[ET_NORM].second.X = getFloat(_ctx, t);
528+
auto& value = attribs[ET_NORM].second.X = getFloat(_ctx, t);
464529
attribs[ET_NORM].first = result = true;
530+
531+
if (_params.loaderFlags & E_LOADER_PARAMETER_FLAGS::ELPF_RIGHT_HANDED_MESHES)
532+
performActionBasedOnOrientationSystem<float>(attribs[ET_NORM].second.X, [](float& varToFlip) { varToFlip = -varToFlip; });
533+
534+
const size_t propertyOffset = ET_NORM_BYTESIZE * currentVertexIndex;
535+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_NORM].buffer->getPointer()) + propertyOffset;
536+
537+
reinterpret_cast<float*>(data)[0] = value;
465538
}
466539
else if (Element.Properties[i].Name == "ny")
467540
{
468-
attribs[ET_NORM].second.Y = getFloat(_ctx, t);
541+
auto& value = attribs[ET_NORM].second.Y = getFloat(_ctx, t);
469542
attribs[ET_NORM].first = result = true;
543+
544+
const size_t propertyOffset = ET_NORM_BYTESIZE * currentVertexIndex;
545+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_NORM].buffer->getPointer()) + propertyOffset;
546+
547+
reinterpret_cast<float*>(data)[1] = value;
470548
}
471549
else if (Element.Properties[i].Name == "nz")
472550
{
473-
attribs[ET_NORM].second.Z = getFloat(_ctx, t);
551+
auto& value = attribs[ET_NORM].second.Z = getFloat(_ctx, t);
474552
attribs[ET_NORM].first = result = true;
553+
554+
const size_t propertyOffset = ET_NORM_BYTESIZE * currentVertexIndex;
555+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_NORM].buffer->getPointer()) + propertyOffset;
556+
557+
reinterpret_cast<float*>(data)[2] = value;
475558
}
476559
// there isn't a single convention for the UV, some softwares like Blender or Assimp use "st" instead of "uv"
477560
else if (Element.Properties[i].Name == "u" || Element.Properties[i].Name == "s")
478561
{
479-
attribs[ET_UV].second.X = getFloat(_ctx, t);
562+
auto& value = attribs[ET_UV].second.X = getFloat(_ctx, t);
480563
attribs[ET_UV].first = true;
564+
565+
const size_t propertyOffset = ET_UV_BYTESIZE * currentVertexIndex;
566+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_UV].buffer->getPointer()) + propertyOffset;
567+
568+
reinterpret_cast<float*>(data)[0] = value;
481569
}
482570
else if (Element.Properties[i].Name == "v" || Element.Properties[i].Name == "t")
483571
{
484-
attribs[ET_UV].second.Y = getFloat(_ctx, t);
572+
auto& value = attribs[ET_UV].second.Y = getFloat(_ctx, t);
485573
attribs[ET_UV].first = true;
574+
575+
const size_t propertyOffset = ET_UV_BYTESIZE * currentVertexIndex;
576+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_UV].buffer->getPointer()) + propertyOffset;
577+
578+
reinterpret_cast<float*>(data)[1] = value;
486579
}
487580
else if (Element.Properties[i].Name == "red")
488581
{
489582
float value = Element.Properties[i].isFloat() ? getFloat(_ctx, t) : float(getInt(_ctx, t)) / 255.f;
490583
attribs[ET_COL].second.X = value;
491584
attribs[ET_COL].first = true;
585+
586+
const size_t propertyOffset = ET_COL_BYTESIZE * currentVertexIndex;
587+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_COL].buffer->getPointer()) + propertyOffset;
588+
589+
reinterpret_cast<float*>(data)[0] = value;
492590
}
493591
else if (Element.Properties[i].Name == "green")
494592
{
495593
float value = Element.Properties[i].isFloat() ? getFloat(_ctx, t) : float(getInt(_ctx, t)) / 255.f;
496594
attribs[ET_COL].second.Y = value;
497595
attribs[ET_COL].first = true;
596+
597+
const size_t propertyOffset = ET_COL_BYTESIZE * currentVertexIndex;
598+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_COL].buffer->getPointer()) + propertyOffset;
599+
600+
reinterpret_cast<float*>(data)[1] = value;
498601
}
499602
else if (Element.Properties[i].Name == "blue")
500603
{
501604
float value = Element.Properties[i].isFloat() ? getFloat(_ctx, t) : float(getInt(_ctx, t)) / 255.f;
502605
attribs[ET_COL].second.Z = value;
503606
attribs[ET_COL].first = true;
607+
608+
const size_t propertyOffset = ET_COL_BYTESIZE * currentVertexIndex;
609+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_COL].buffer->getPointer()) + propertyOffset;
610+
611+
reinterpret_cast<float*>(data)[2] = value;
504612
}
505613
else if (Element.Properties[i].Name == "alpha")
506614
{
507615
float value = Element.Properties[i].isFloat() ? getFloat(_ctx, t) : float(getInt(_ctx, t)) / 255.f;
508616
attribs[ET_COL].second.W = value;
509617
attribs[ET_COL].first = true;
618+
619+
const size_t propertyOffset = ET_COL_BYTESIZE * currentVertexIndex;
620+
uint8_t* data = reinterpret_cast<uint8_t*>(outAttributes[ET_COL].buffer->getPointer()) + propertyOffset;
621+
622+
reinterpret_cast<float*>(data)[3] = value;
510623
}
511624
else
512625
skipProperty(_ctx, Element.Properties[i]);
513626
}
514627

515-
for (size_t i = 0u; i < 4u; ++i)
516-
if (attribs[i].first)
517-
{
518-
if (_params.loaderFlags & E_LOADER_PARAMETER_FLAGS::ELPF_RIGHT_HANDED_MESHES)
519-
{
520-
if (i == ET_POS)
521-
performActionBasedOnOrientationSystem<float>(attribs[ET_POS].second.X, [](float& varToFlip) { varToFlip = -varToFlip; });
522-
else if (i == ET_NORM)
523-
performActionBasedOnOrientationSystem<float>(attribs[ET_NORM].second.X, [](float& varToFlip) { varToFlip = -varToFlip; });
524-
}
525-
526-
_outAttribs[i].push_back(attribs[i].second);
527-
}
528-
529628
return result;
530629
}
531630

@@ -688,32 +787,25 @@ void CPLYMeshFileLoader::moveForward(SContext& _ctx, uint32_t bytes)
688787

689788
bool CPLYMeshFileLoader::genVertBuffersForMBuffer(
690789
asset::ICPUMeshBuffer* _mbuf,
691-
const core::vector<core::vectorSIMDf> _attribs[4],
790+
const asset::SBufferBinding<asset::ICPUBuffer> attributes[4],
692791
SContext& context
693792
) const
694793
{
695794
core::vector<uint8_t> availableAttributes;
696795
for (auto i = 0; i < 4; ++i)
697-
if (!_attribs[i].empty())
796+
if (attributes[i].buffer)
698797
availableAttributes.push_back(i);
699798

700799
{
701-
size_t check = _attribs[0].size();
800+
size_t check = attributes[0].buffer->getSize();
702801
for (size_t i = 1u; i < 4u; ++i)
703802
{
704-
if (_attribs[i].size() != 0u && _attribs[i].size() != check)
803+
if (attributes[i].buffer && attributes[i].buffer->getSize() != check)
705804
return false;
706-
else if (_attribs[i].size() != 0u)
707-
check = _attribs[i].size();
805+
else if (attributes[i].buffer)
806+
check = attributes[i].buffer->getSize();
708807
}
709808
}
710-
711-
auto putAttr = [&_attribs](asset::ICPUMeshBuffer* _buf, size_t _attr)
712-
{
713-
size_t i = 0u;
714-
for (const core::vectorSIMDf& v : _attribs[_attr])
715-
_buf->setAttribute(v, _attr, i++);
716-
};
717809

718810
auto getPipeline = [&]() -> core::smart_refctd_ptr<asset::ICPURenderpassIndependentPipeline>
719811
{
@@ -756,15 +848,9 @@ bool CPLYMeshFileLoader::genVertBuffersForMBuffer(
756848

757849
for (auto index = 0; index < 4; ++index)
758850
{
759-
auto attribute = _attribs[index];
760-
if (!attribute.empty())
761-
{
762-
const auto bufferByteSize = attribute.size() * 16ull;
763-
auto buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(bufferByteSize);
764-
memcpy(buffer->getPointer(), attribute.data(), bufferByteSize); // TODO refactor input to take SBufferBinding to avoid memcpy
765-
766-
_mbuf->setVertexBufferBinding({ 0ul, std::move(buffer) }, index);
767-
}
851+
auto attribute = attributes[index];
852+
if (attribute.buffer)
853+
_mbuf->setVertexBufferBinding(std::move(attribute), index);
768854
}
769855

770856
_mbuf->setPipeline(std::move(mbPipeline));

‎src/nbl/asset/interchange/CPLYMeshFileLoader.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ class CPLYMeshFileLoader : public IAssetLoader, public IRenderpassIndependentPip
189189
void fillBuffer(SContext& _ctx);
190190
E_PLY_PROPERTY_TYPE getPropertyType(const char* typeString) const;
191191

192-
bool readVertex(SContext& _ctx, const SPLYElement &Element, core::vector<core::vectorSIMDf> _attribs[4], const IAssetLoader::SAssetLoadParams& _params);
192+
bool readVertex(SContext& _ctx, const SPLYElement &Element, asset::SBufferBinding<asset::ICPUBuffer> outAttributes[4], const uint32_t& currentVertexIndex, const IAssetLoader::SAssetLoadParams& _params);
193193
bool readFace(SContext& _ctx, const SPLYElement &Element, core::vector<uint32_t>& _outIndices);
194194

195195
void skipElement(SContext& _ctx, const SPLYElement &Element);
@@ -200,7 +200,7 @@ class CPLYMeshFileLoader : public IAssetLoader, public IRenderpassIndependentPip
200200

201201
bool genVertBuffersForMBuffer(
202202
ICPUMeshBuffer* _mbuf,
203-
const core::vector<core::vectorSIMDf> _attribs[4],
203+
const asset::SBufferBinding<asset::ICPUBuffer> attributes[4],
204204
SContext& context
205205
) const;
206206

0 commit comments

Comments
 (0)