Skip to content

Commit 69f90c5

Browse files
CrissplCrisspl
committed
Some asset restore fixes wrt review
1 parent 4d73f2f commit 69f90c5

File tree

6 files changed

+142
-32
lines changed

6 files changed

+142
-32
lines changed

‎include/nbl/asset/IAssetManager.h

Lines changed: 87 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ class IAssetManager : public core::IReferenceCounted, public core::QuitSignallin
201201
@see IAssetLoader
202202
@see SAssetBundle
203203
*/
204-
SAssetBundle getAssetInHierarchy(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
204+
template <bool RestoreWholeBundle>
205+
SAssetBundle getAssetInHierarchy_impl(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
205206
{
206207
const uint32_t restoreLevels = (_hierarchyLevel >= _params.restoreLevels) ? 0u : (_params.restoreLevels - _hierarchyLevel);
207208

@@ -267,7 +268,14 @@ class IAssetManager : public core::IReferenceCounted, public core::QuitSignallin
267268
_override->insertAssetIntoCache(bundle, filename, ctx, _hierarchyLevel);
268269
}
269270

270-
if (bundle.getContents().empty() /*|| none of req levels is dummy*/)
271+
auto whole_bundle_not_dummy = [restoreLevels](const SAssetBundle& _b) {
272+
auto rng = _b.getContents();
273+
for (const auto& a : rng)
274+
if (a->isAnyDependencyDummy(restoreLevels))
275+
return false;
276+
return true;
277+
};
278+
if (bundle.getContents().empty() || whole_bundle_not_dummy(bundle))
271279
return bundle;
272280

273281
if (restoreLevels)
@@ -286,30 +294,37 @@ class IAssetManager : public core::IReferenceCounted, public core::QuitSignallin
286294
reloadParams.restoreLevels = 0u; // make sure it wont turn into infinite recursion
287295
reloadParams.reload = true; // TODO (consider): alternative to this flag: another method in override just to let user choose asset for restore
288296
}
289-
IAssetLoader::SAssetLoadContext ctx(params, file);
290-
auto asset = _override->chooseDefaultAsset(bundle, ctx);
291-
if (!asset->isAnyDependencyDummy(restoreLevels - 1u))
292-
return bundle;
293297

294-
auto reloadBundle = getAssetInHierarchy(_file, _supposedFilename, reloadParams, _hierarchyLevel, _override);
298+
auto reloadBundle = getAssetInHierarchy_impl<RestoreWholeBundle>(_file, _supposedFilename, reloadParams, _hierarchyLevel, _override);
295299

296-
bool restoreSucceeded = _override->handleRestore(std::move(asset), bundle, reloadBundle, restoreLevels);
297-
if (!restoreSucceeded) // hm? return empty bundle if restore was requested, but did not succeeded? or just return the bundle? (TODO consider)
298-
return {};
300+
if constexpr (RestoreWholeBundle)
301+
{
302+
IAssetLoader::SAssetLoadContext ctx(params, file);
303+
auto asset = _override->chooseDefaultAsset(bundle, ctx);
304+
305+
// user responsible for checking if assets he wanted to be restored are in fact restored
306+
_override->handleRestore(std::move(asset), bundle, reloadBundle, restoreLevels);
307+
}
308+
else
309+
{
310+
// user responsible for checking if assets he wanted to be restored are in fact restored
311+
_override->handleRestore(bundle, reloadBundle, restoreLevels);
312+
}
299313
}
300314

301315
return bundle;
302316
}
303317
//TODO change name
304-
SAssetBundle getAssetInHierarchy(const std::string& _filePath, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
318+
template <bool RestoreWholeBundle>
319+
SAssetBundle getAssetInHierarchy_impl(const std::string& _filePath, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
305320
{
306321
IAssetLoader::SAssetLoadContext ctx(_params, nullptr);
307322

308323
std::string filePath = _filePath;
309324
_override->getLoadFilename(filePath, ctx, _hierarchyLevel);
310325
io::IReadFile* file = m_fileSystem->createAndOpenFile(filePath.c_str());
311326

312-
SAssetBundle asset = getAssetInHierarchy(file, _filePath, _params, _hierarchyLevel, _override);
327+
SAssetBundle asset = getAssetInHierarchy_impl<RestoreWholeBundle>(file, _filePath, _params, _hierarchyLevel, _override);
313328

314329
if (file)
315330
file->drop();
@@ -318,15 +333,52 @@ class IAssetManager : public core::IReferenceCounted, public core::QuitSignallin
318333
}
319334

320335
//TODO change name
321-
SAssetBundle getAssetInHierarchy(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
336+
template <bool RestoreWholeBundle>
337+
SAssetBundle getAssetInHierarchy_impl(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
322338
{
323-
return getAssetInHierarchy(_file, _supposedFilename, _params, _hierarchyLevel, &m_defaultLoaderOverride);
339+
return getAssetInHierarchy_impl<RestoreWholeBundle>(_file, _supposedFilename, _params, _hierarchyLevel, &m_defaultLoaderOverride);
324340
}
325341

326342
//TODO change name
343+
template <bool RestoreWholeBundle>
344+
SAssetBundle getAssetInHierarchy_impl(const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
345+
{
346+
return getAssetInHierarchy_impl<RestoreWholeBundle>(_filename, _params, _hierarchyLevel, &m_defaultLoaderOverride);
347+
}
348+
349+
350+
SAssetBundle getAssetInHierarchy(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
351+
{
352+
return getAssetInHierarchy_impl<false>(_file, _supposedFilename, _params, _hierarchyLevel, _override);
353+
}
354+
SAssetBundle getAssetInHierarchy(const std::string& _filePath, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
355+
{
356+
return getAssetInHierarchy_impl<false>(_filePath, _params, _hierarchyLevel, _override);
357+
}
358+
SAssetBundle getAssetInHierarchy(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
359+
{
360+
return getAssetInHierarchy_impl<false>(_file, _supposedFilename, _params, _hierarchyLevel);
361+
}
327362
SAssetBundle getAssetInHierarchy(const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
328363
{
329-
return getAssetInHierarchy(_filename, _params, _hierarchyLevel, &m_defaultLoaderOverride);
364+
return getAssetInHierarchy_impl<false>(_filename, _params, _hierarchyLevel);
365+
}
366+
367+
SAssetBundle getAssetInHierarchyWholeBundleRestore(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
368+
{
369+
return getAssetInHierarchy_impl<true>(_file, _supposedFilename, _params, _hierarchyLevel, _override);
370+
}
371+
SAssetBundle getAssetInHierarchyWholeBundleRestore(const std::string& _filePath, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
372+
{
373+
return getAssetInHierarchy_impl<true>(_filePath, _params, _hierarchyLevel, _override);
374+
}
375+
SAssetBundle getAssetInHierarchyWholeBundleRestore(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
376+
{
377+
return getAssetInHierarchy_impl<true>(_file, _supposedFilename, _params, _hierarchyLevel);
378+
}
379+
SAssetBundle getAssetInHierarchyWholeBundleRestore(const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
380+
{
381+
return getAssetInHierarchy_impl<true>(_filename, _params, _hierarchyLevel);
330382
}
331383

332384
public:
@@ -342,19 +394,37 @@ class IAssetManager : public core::IReferenceCounted, public core::QuitSignallin
342394
{
343395
return getAssetInHierarchy(_file, _supposedFilename, _params, 0u, _override);
344396
}
345-
346397
//TODO change name
347398
SAssetBundle getAsset(const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params)
348399
{
349400
return getAsset(_filename, _params, &m_defaultLoaderOverride);
350401
}
351-
352402
//TODO change name
353403
SAssetBundle getAsset(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params)
354404
{
355405
return getAsset(_file, _supposedFilename, _params, &m_defaultLoaderOverride);
356406
}
357407

408+
SAssetBundle getAssetWholeBundleRestore(const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, IAssetLoader::IAssetLoaderOverride* _override)
409+
{
410+
return getAssetInHierarchyWholeBundleRestore(_filename, _params, 0u, _override);
411+
}
412+
//TODO change name
413+
SAssetBundle getAssetWholeBundleRestore(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, IAssetLoader::IAssetLoaderOverride* _override)
414+
{
415+
return getAssetInHierarchyWholeBundleRestore(_file, _supposedFilename, _params, 0u, _override);
416+
}
417+
//TODO change name
418+
SAssetBundle getAssetWholeBundleRestore(const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params)
419+
{
420+
return getAssetWholeBundleRestore(_filename, _params, &m_defaultLoaderOverride);
421+
}
422+
//TODO change name
423+
SAssetBundle getAssetWholeBundleRestore(io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params)
424+
{
425+
return getAssetWholeBundleRestore(_file, _supposedFilename, _params, &m_defaultLoaderOverride);
426+
}
427+
358428
//TODO change name
359429
//! Check whether Assets exist in cache using a key and optionally their types
360430
/*

‎include/nbl/asset/interchange/IAssetLoader.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class IAssetLoader : public virtual core::IReferenceCounted
192192
return { chooseDefaultAsset(bundle,ctx),bundle.getMetadata() };
193193
}
194194

195-
//!
195+
//! When choosing asset to be restored, ctx.params.reload flag is true
196196
inline virtual core::smart_refctd_ptr<IAsset> chooseDefaultAsset(const SAssetBundle& bundle, const SAssetLoadContext& ctx)
197197
{
198198
auto contents = bundle.getContents();
@@ -265,7 +265,12 @@ class IAssetLoader : public virtual core::IReferenceCounted
265265
//TODO change name
266266
virtual void insertAssetIntoCache(SAssetBundle& asset, const std::string& supposedKey, const SAssetLoadContext& ctx, const uint32_t hierarchyLevel);
267267

268-
virtual bool handleRestore(core::smart_refctd_ptr<IAsset>&& _chosenAsset, SAssetBundle& _bundle, SAssetBundle& _reloadedBundle, uint32_t _restoreLevels);
268+
//! Restores only the chosen asset
269+
//! The asset is chosen via chooseDefaultAsset()
270+
virtual void handleRestore(core::smart_refctd_ptr<IAsset>&& _chosenAsset, SAssetBundle& _bundle, SAssetBundle& _reloadedBundle, uint32_t _restoreLevels);
271+
272+
//! Restores all of assets in _bundle
273+
virtual void handleRestore(SAssetBundle& _bundle, SAssetBundle& _reloadedBundle, uint32_t _restoreLevels);
269274
};
270275

271276
public:
@@ -295,6 +300,11 @@ class IAssetLoader : public virtual core::IReferenceCounted
295300
SAssetBundle interm_getAssetInHierarchy(IAssetManager* _mgr, io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel);
296301
SAssetBundle interm_getAssetInHierarchy(IAssetManager* _mgr, const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel);
297302

303+
SAssetBundle interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override);
304+
SAssetBundle interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override);
305+
SAssetBundle interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel);
306+
SAssetBundle interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel);
307+
298308
void interm_setAssetMutability(const IAssetManager* _mgr, IAsset* _asset, IAsset::E_MUTABILITY _val);
299309
//void interm_restoreDummyAsset(IAssetManager* _mgr, SAssetBundle& _bundle);
300310
//void interm_restoreDummyAsset(IAssetManager* _mgr, IAsset* _asset, const std::string _path);

‎include/nbl/asset/interchange/SAssetBundle.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,6 @@ class SAssetBundle
109109
m_contents->operator[](offset) = std::move(_asset);
110110
assert(allSameTypeAndNotNull());
111111
}
112-
inline core::SRange<core::smart_refctd_ptr<IAsset>> getMutableContents()
113-
{
114-
return core::SRange<core::smart_refctd_ptr<IAsset>>(m_contents->begin(), m_contents->end());
115-
}
116112

117113

118114
core::smart_refctd_ptr<IAssetMetadata> m_metadata;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ CGraphicsPipelineLoaderMTL::image_views_set_t CGraphicsPipelineLoaderMTL::loadIm
530530
{
531531
// we need bumpmap restored to create derivative map from it
532532
const uint32_t restoreLevels = 3u; // 2 in case of image (image, texel buffer) and 3 in case of image view (view, image, texel buffer)
533-
lp.restoreLevels = hierarchyLevel + restoreLevels; // TODO what if `lp.restoreLevels` already had some non-zero value? ehhh this is complex, and this logic must be in some common function
533+
lp.restoreLevels = std::max(lp.restoreLevels, hierarchyLevel + restoreLevels);
534534
bundle = interm_getAssetInHierarchy(m_assetMgr, relDir+_mtl.maps[i], lp, hierarchyLevel, _ctx.loaderOverride);
535535
}
536536
auto asset = _ctx.loaderOverride->chooseDefaultAsset(bundle,_ctx.inner);

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

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,36 @@ void IAssetLoader::IAssetLoaderOverride::insertAssetIntoCache(SAssetBundle& asse
3535
m_manager->insertAssetIntoCache(asset, ASSET_MUTABILITY_ON_CACHE_INSERT);
3636
}
3737

38-
bool IAssetLoader::IAssetLoaderOverride::handleRestore(core::smart_refctd_ptr<IAsset>&& _chosenAsset, SAssetBundle& _bundle, SAssetBundle& _reloadedBundle, uint32_t _restoreLevels)
38+
void IAssetLoader::IAssetLoaderOverride::handleRestore(core::smart_refctd_ptr<IAsset>&& _chosenAsset, SAssetBundle& _bundle, SAssetBundle& _reloadedBundle, uint32_t _restoreLevels)
3939
{
4040
if (_bundle.getContents().size() != _reloadedBundle.getContents().size())
41-
return false;
41+
return;
4242

43-
auto dummies = _bundle.getMutableContents();
43+
auto dummies = _bundle.getContents();
4444
auto found_it = std::find(dummies.begin(), dummies.end(), _chosenAsset);
4545
if (found_it == dummies.end())
46-
return false;
46+
return;
4747
const uint32_t ix = found_it - dummies.begin();
4848

49-
auto reloaded = _reloadedBundle.getMutableContents();
49+
auto reloaded = _reloadedBundle.getContents();
5050
if (dummies.begin()[ix]->isADummyObjectForCache() && !dummies.begin()[ix]->canBeRestoredFrom(reloaded.begin()[ix].get()))
51-
return false;
51+
return;
5252

53-
return dummies.begin()[ix]->restoreFromDummy(reloaded.begin()[ix].get(), _restoreLevels);
53+
_chosenAsset->restoreFromDummy(reloaded.begin()[ix].get(), _restoreLevels);
54+
}
55+
56+
void IAssetLoader::IAssetLoaderOverride::handleRestore(SAssetBundle& _bundle, SAssetBundle& _reloadedBundle, uint32_t _restoreLevels)
57+
{
58+
if (_bundle.getContents().size() != _reloadedBundle.getContents().size())
59+
return;
60+
61+
auto dummies = _bundle.getContents();
62+
auto reloaded = _reloadedBundle.getContents();
63+
const uint32_t cnt = dummies.size();
64+
65+
for (uint32_t i = 0u; i < cnt; ++i)
66+
if (dummies.begin()[i]->isADummyObjectForCache() && dummies.begin()[i]->canBeRestoredFrom(reloaded.begin()[i].get()))
67+
dummies.begin()[i].get()->restoreFromDummy(reloaded.begin()[i].get(), _restoreLevels);
5468
}
5569

5670
SAssetBundle IAssetLoader::interm_getAssetInHierarchy(IAssetManager* _mgr, io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
@@ -73,6 +87,26 @@ SAssetBundle IAssetLoader::interm_getAssetInHierarchy(IAssetManager* _mgr, const
7387
return _mgr->getAssetInHierarchy(_filename, _params, _hierarchyLevel);
7488
}
7589

90+
SAssetBundle IAssetLoader::interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
91+
{
92+
return _mgr->getAssetInHierarchyWholeBundleRestore(_file, _supposedFilename, _params, _hierarchyLevel, _override);
93+
}
94+
95+
SAssetBundle IAssetLoader::interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel, IAssetLoader::IAssetLoaderOverride* _override)
96+
{
97+
return _mgr->getAssetInHierarchyWholeBundleRestore(_filename, _params, _hierarchyLevel, _override);
98+
}
99+
100+
SAssetBundle IAssetLoader::interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, io::IReadFile* _file, const std::string& _supposedFilename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
101+
{
102+
return _mgr->getAssetInHierarchyWholeBundleRestore(_file, _supposedFilename, _params, _hierarchyLevel);
103+
}
104+
105+
SAssetBundle IAssetLoader::interm_getAssetInHierarchyWholeBundleRestore(IAssetManager* _mgr, const std::string& _filename, const IAssetLoader::SAssetLoadParams& _params, uint32_t _hierarchyLevel)
106+
{
107+
return _mgr->getAssetInHierarchyWholeBundleRestore(_filename, _params, _hierarchyLevel);
108+
}
109+
76110
void IAssetLoader::interm_setAssetMutability(const IAssetManager* _mgr, IAsset* _asset, IAsset::E_MUTABILITY _val)
77111
{
78112
_mgr->setAssetMutability(_asset, _val);

‎src/nbl/ext/MitsubaLoader/CMitsubaLoader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,7 @@ SContext::tex_ass_type CMitsubaLoader::cacheTexture(SContext& ctx, uint32_t hier
962962
{
963963
const uint32_t restoreLevels = _restore ? 2u : 0u;
964964
auto loadParams = ctx.inner.params;
965-
loadParams.restoreLevels = hierarchyLevel + restoreLevels;
965+
loadParams.restoreLevels = std::max(loadParams.restoreLevels, hierarchyLevel + restoreLevels);
966966
asset::SAssetBundle imgBundle = interm_getAssetInHierarchy(m_assetMgr,tex->bitmap.filename.svalue,loadParams,hierarchyLevel,ctx.override_);
967967
auto contentRange = imgBundle.getContents();
968968
if (contentRange.begin() < contentRange.end())

0 commit comments

Comments
 (0)