Skip to content

Commit cdbd33c

Browse files
committed
IShaderCompiler Fix Caching behavior when we have both read and write cache
+ ISystem delete file and other changes to have proper shader caching mechanism
1 parent bf7f6e6 commit cdbd33c

File tree

6 files changed

+36
-19
lines changed

6 files changed

+36
-19
lines changed

include/nbl/asset/utils/IShaderCompiler.h

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
182182
public:
183183
// Used to check compatibility of Caches before reading
184184
constexpr static inline std::string_view VERSION = "1.0.0";
185-
185+
186186
using hash_t = std::array<uint64_t,4>;
187187
static auto const SHADER_BUFFER_SIZE_BYTES = sizeof(uint64_t) / sizeof(uint8_t); // It's obviously 8
188188

@@ -362,7 +362,7 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
362362
// Making the copy constructor deep-copy everything but the shader
363363
inline SEntry(const SEntry& other)
364364
: mainFileContents(other.mainFileContents), compilerArgs(other.compilerArgs), hash(other.hash), lookupHash(other.lookupHash),
365-
dependencies(other.dependencies), value(other.value) {}
365+
dependencies(other.dependencies), cpuShader(other.cpuShader) {}
366366

367367
inline SEntry& operator=(SEntry& other) = delete;
368368
inline SEntry(SEntry&& other) = default;
@@ -375,7 +375,7 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
375375
std::array<uint64_t,4> hash;
376376
size_t lookupHash;
377377
dependency_container_t dependencies;
378-
core::smart_refctd_ptr<asset::ICPUShader> value;
378+
core::smart_refctd_ptr<asset::ICPUShader> cpuShader;
379379
};
380380

381381
inline void insert(SEntry&& entry)
@@ -399,7 +399,7 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
399399
return retVal;
400400
}
401401

402-
NBL_API2 core::smart_refctd_ptr<asset::ICPUShader> find(const SEntry& mainFile, const CIncludeFinder* finder) const;
402+
NBL_API2 SEntry find(const SEntry& mainFile, const CIncludeFinder* finder) const;
403403

404404
inline CCache() {}
405405

@@ -433,24 +433,32 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
433433
{
434434
CCache::SEntry entry;
435435
std::vector<CCache::SEntry::SPreprocessingDependency> dependencies;
436-
if (options.readCache or options.writeCache)
436+
if (options.readCache || options.writeCache)
437437
entry = std::move(CCache::SEntry(code, options));
438+
438439
if (options.readCache)
439440
{
440441
auto found = options.readCache->find(entry, options.preprocessorOptions.includeFinder);
441-
if (found)
442-
return found;
442+
auto cpuShader = found.cpuShader;
443+
if (cpuShader)
444+
{
445+
if (options.writeCache)
446+
options.writeCache->insert(std::move(found));
447+
return cpuShader;
448+
}
443449
}
450+
444451
auto retVal = compileToSPIRV_impl(code, options, options.writeCache ? &dependencies : nullptr);
445452
// compute the SPIR-V shader content hash
446453
{
447454
auto backingBuffer = retVal->getContent();
448455
const_cast<ICPUBuffer*>(backingBuffer)->setContentHash(backingBuffer->computeContentHash());
449456
}
457+
450458
if (options.writeCache)
451459
{
452460
entry.dependencies = std::move(dependencies);
453-
entry.value = retVal;
461+
entry.cpuShader = retVal;
454462
options.writeCache->insert(std::move(entry));
455463
}
456464
return retVal;

include/nbl/system/ISystem.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ class NBL_API2 ISystem : public core::IReferenceCounted
9494

9595
// can only perform operations on non-virtual filesystem paths
9696
bool deleteDirectory(const system::path& p);
97+
98+
// can only perform operations on non-virtual filesystem paths
99+
bool deleteFile(const system::path& p);
97100

98101
// can only perform operations on non-virtual filesystem paths
99102
std::error_code moveFileOrDirectory(const system::path& oldPath, const system::path& newPath);

src/nbl/asset/utils/IShaderCompiler.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ auto IShaderCompiler::CIncludeFinder::tryIncludeGenerators(const std::string& in
216216
return {};
217217
}
218218

219-
core::smart_refctd_ptr<asset::ICPUShader> IShaderCompiler::CCache::find(const SEntry& mainFile, const IShaderCompiler::CIncludeFinder* finder) const
219+
SEntry IShaderCompiler::CCache::find(const SEntry& mainFile, const IShaderCompiler::CIncludeFinder* finder) const
220220
{
221221
auto foundRange = m_container.equal_range(mainFile);
222222
for (auto& found = foundRange.first; found != foundRange.second; found++)
@@ -240,10 +240,10 @@ core::smart_refctd_ptr<asset::ICPUShader> IShaderCompiler::CCache::find(const SE
240240
}
241241
}
242242
if (allDependenciesMatch) {
243-
return found->value;
243+
return *found;
244244
}
245245
}
246-
return nullptr;
246+
return SEntry();
247247
}
248248

249249
core::smart_refctd_ptr<ICPUBuffer> IShaderCompiler::CCache::serialize() const
@@ -263,10 +263,10 @@ core::smart_refctd_ptr<ICPUBuffer> IShaderCompiler::CCache::serialize() const
263263
// We keep a copy of the offsets and the sizes of each shader. This is so that later on, when we add the shaders to the buffer after json creation
264264
// (where the params array has been moved) we don't have to read the json to get the offsets again
265265
offsets[i] = shaderBufferSize;
266-
sizes[i] = entry.value->getContent()->getSize();
266+
sizes[i] = entry.cpuShader->getContent()->getSize();
267267

268268
// And add the params to the shader creation parameters array
269-
shaderCreationParams.emplace_back(entry.value->getStage(), entry.value->getContentType(), entry.value->getFilepathHint(), sizes[i], shaderBufferSize);
269+
shaderCreationParams.emplace_back(entry.cpuShader->getStage(), entry.cpuShader->getContentType(), entry.cpuShader->getFilepathHint(), sizes[i], shaderBufferSize);
270270
// Enlarge the shader buffer by the size of the current shader
271271
shaderBufferSize += sizes[i];
272272
i++;
@@ -290,7 +290,7 @@ core::smart_refctd_ptr<ICPUBuffer> IShaderCompiler::CCache::serialize() const
290290
// Loop over entries again, adding each one's shader to the buffer.
291291
i = 0u;
292292
for (auto& entry : m_container) {
293-
memcpy(retVal.data() + SHADER_BUFFER_SIZE_BYTES + offsets[i], entry.value->getContent()->getPointer(), sizes[i]);
293+
memcpy(retVal.data() + SHADER_BUFFER_SIZE_BYTES + offsets[i], entry.cpuShader->getContent()->getPointer(), sizes[i]);
294294
i++;
295295
}
296296

@@ -335,9 +335,7 @@ core::smart_refctd_ptr<IShaderCompiler::CCache> IShaderCompiler::CCache::deseria
335335
// Copy the shader bytecode into the buffer
336336
memcpy(code->getPointer(), serializedCache.data() + SHADER_BUFFER_SIZE_BYTES + shaderCreationParams[i].offset, shaderCreationParams[i].codeByteSize);
337337
// Create the ICPUShader
338-
auto value = core::make_smart_refctd_ptr<ICPUShader>(std::move(code), shaderCreationParams[i].stage, shaderCreationParams[i].contentType, std::move(shaderCreationParams[i].filepathHint));
339-
340-
entries[i].value = std::move(value);
338+
entries[i].cpuShader = core::make_smart_refctd_ptr<ICPUShader>(std::move(code), shaderCreationParams[i].stage, shaderCreationParams[i].contentType, std::move(shaderCreationParams[i].filepathHint));
341339

342340
retVal->insert(std::move(entries[i]));
343341
}

src/nbl/asset/utils/shaderCompiler_serialization.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ inline void from_json(const json& j, SEntry& entry)
192192
j.at("hash").get_to(entry.hash);
193193
j.at("lookupHash").get_to(entry.lookupHash);
194194
j.at("dependencies").get_to(entry.dependencies);
195-
entry.value = nullptr;
195+
entry.cpuShader = nullptr;
196196
}
197197

198198
}

src/nbl/system/ISystem.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ bool ISystem::deleteDirectory(const system::path& p)
120120
return false;
121121
}
122122

123+
bool nbl::system::ISystem::deleteFile(const system::path& p)
124+
{
125+
if (std::filesystem::exists(p))
126+
return std::filesystem::remove(p);
127+
else
128+
return false;
129+
}
130+
123131
std::error_code ISystem::moveFileOrDirectory(const system::path& oldPath, const system::path& newPath)
124132
{
125133
std::error_code ec;

0 commit comments

Comments
 (0)