Skip to content

Commit f64d491

Browse files
deprilula28deprilula28
committed
Apply PR reviews
1 parent aea2204 commit f64d491

File tree

6 files changed

+96
-90
lines changed

6 files changed

+96
-90
lines changed

‎include/nbl/asset/utils/CGLSLCompiler.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -128,27 +128,6 @@ class NBL_API2 CGLSLCompiler final : public IShaderCompiler
128128

129129
std::string preprocessShader(std::string&& code, IShader::E_SHADER_STAGE& stage, const SPreprocessorOptions& preprocessOptions) const override;
130130

131-
static constexpr const char* PREPROC_GL__DISABLER = "_this_is_a_GL__prefix_";
132-
static constexpr const char* PREPROC_GL__ENABLER = PREPROC_GL__DISABLER;
133-
static constexpr const char* PREPROC_LINE_CONTINUATION_DISABLER = "_this_is_a_line_continuation_\n";
134-
static constexpr const char* PREPROC_LINE_CONTINUATION_ENABLER = "_this_is_a_line_continuation_";
135-
136-
static void disableGlDirectives(std::string& _code)
137-
{
138-
std::regex glMacro("[ \t\r\n\v\f]GL_");
139-
auto result = std::regex_replace(_code, glMacro, PREPROC_GL__DISABLER);
140-
std::regex lineContinuation("\\\\[ \t\r\n\v\f]*\n");
141-
_code = std::regex_replace(result, lineContinuation, PREPROC_LINE_CONTINUATION_DISABLER);
142-
}
143-
144-
static void reenableGlDirectives(std::string& _code)
145-
{
146-
std::regex lineContinuation(PREPROC_LINE_CONTINUATION_ENABLER);
147-
auto result = std::regex_replace(_code, lineContinuation, " \\");
148-
std::regex glMacro(PREPROC_GL__ENABLER);
149-
_code = std::regex_replace(result, glMacro, " GL_");
150-
}
151-
152131
protected:
153132

154133
void insertIntoStart(std::string& code, std::ostringstream&& ins) const override;

‎include/nbl/asset/utils/CHLSLCompiler.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ using Microsoft::WRL::ComPtr;
1414

1515
class IDxcUtils;
1616
class IDxcCompiler3;
17+
class DxcCompilationResult;
1718

1819
namespace nbl::asset
1920
{
@@ -48,15 +49,16 @@ class NBL_API2 CHLSLCompiler final : public IShaderCompiler
4849
std::string preprocessShader(std::string&& code, IShader::E_SHADER_STAGE& stage, const SPreprocessorOptions& preprocessOptions) const override;
4950

5051
void insertIntoStart(std::string& code, std::ostringstream&& ins) const override;
51-
52-
IDxcUtils* getDxcUtils() const { return m_dxcUtils.Get(); }
53-
IDxcCompiler3* getDxcCompiler() const { return m_dxcCompiler.Get(); }
5452
protected:
5553

56-
// TODO do we want to use ComPtr?
5754
ComPtr<IDxcUtils> m_dxcUtils;
5855
ComPtr<IDxcCompiler3> m_dxcCompiler;
5956

57+
DxcCompilationResult dxcCompile(std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options) const;
58+
59+
IDxcUtils* getDxcUtils() const { return m_dxcUtils.Get(); }
60+
IDxcCompiler3* getDxcCompiler() const { return m_dxcCompiler.Get(); }
61+
6062
static CHLSLCompiler::SOptions option_cast(const IShaderCompiler::SCompilerOptions& options)
6163
{
6264
CHLSLCompiler::SOptions ret = {};

‎include/nbl/asset/utils/IShaderCompiler.h

Lines changed: 3 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -254,68 +254,11 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
254254
}
255255
}
256256

257-
//string to be replaced with all "#" except those in "#include"
258-
static constexpr const char* PREPROC_DIRECTIVE_DISABLER = "_this_is_a_hash_";
259-
static constexpr const char* PREPROC_DIRECTIVE_ENABLER = PREPROC_DIRECTIVE_DISABLER;
260-
261-
// TODO make sure there are no GLSL specifics for these functions in the HLSL compiler
262-
//
263-
//all "#", except those in "#include"/"#version"/"#pragma shader_stage(...)", replaced with `PREPROC_DIRECTIVE_DISABLER`
264-
static void disableAllDirectivesExceptIncludes(std::string& _code)
265-
{
266-
// TODO: replace this with a proper-ish proprocessor and includer one day
267-
std::regex directive("#(?!(include|version|pragma shader_stage))");//all # not followed by "include" nor "version" nor "pragma shader_stage"
268-
//`#pragma shader_stage(...)` is needed for determining shader stage when `_stage` param of IShaderCompiler functions is set to ESS_UNKNOWN
269-
_code = std::regex_replace(_code, directive, PREPROC_DIRECTIVE_DISABLER);
270-
}
257+
static void disableAllDirectivesExceptIncludes(std::string& _code);
271258

272-
static void reenableDirectives(std::string& _code)
273-
{
274-
std::regex directive(PREPROC_DIRECTIVE_ENABLER);
275-
_code = std::regex_replace(_code, directive, "#");
276-
}
259+
static void reenableDirectives(std::string& _code);
277260

278-
static std::string encloseWithinExtraInclGuards(std::string&& _code, uint32_t _maxInclusions, const char* _identifier)
279-
{
280-
assert(_maxInclusions != 0u);
281-
282-
using namespace std::string_literals;
283-
std::string defBase_ = "_GENERATED_INCLUDE_GUARD_"s + _identifier + "_";
284-
std::replace_if(defBase_.begin(), defBase_.end(), [](char c) ->bool { return !::isalpha(c) && !::isdigit(c); }, '_');
285-
286-
auto genDefs = [&defBase_, _maxInclusions, _identifier] {
287-
auto defBase = [&defBase_](uint32_t n) { return defBase_ + std::to_string(n); };
288-
std::string defs = "#ifndef " + defBase(0) + "\n\t#define " + defBase(0) + "\n";
289-
for (uint32_t i = 1u; i <= _maxInclusions; ++i) {
290-
const std::string defname = defBase(i);
291-
defs += "#elif !defined(" + defname + ")\n\t#define " + defname + "\n";
292-
}
293-
defs += "#endif\n";
294-
return defs;
295-
};
296-
auto genUndefs = [&defBase_, _maxInclusions, _identifier] {
297-
auto defBase = [&defBase_](int32_t n) { return defBase_ + std::to_string(n); };
298-
std::string undefs = "#ifdef " + defBase(_maxInclusions) + "\n\t#undef " + defBase(_maxInclusions) + "\n";
299-
for (int32_t i = _maxInclusions - 1; i >= 0; --i) {
300-
const std::string defname = defBase(i);
301-
undefs += "#elif defined(" + defname + ")\n\t#undef " + defname + "\n";
302-
}
303-
undefs += "#endif\n";
304-
return undefs;
305-
};
306-
307-
return
308-
genDefs() +
309-
"\n"
310-
"#ifndef " + defBase_ + std::to_string(_maxInclusions) +
311-
"\n" +
312-
"#line 1 \"" + _identifier + "\"\n" +
313-
_code +
314-
"\n"
315-
"#endif"
316-
"\n\n" +
317-
genUndefs();
318-
}
261+
static std::string encloseWithinExtraInclGuards(std::string&& _code, uint32_t _maxInclusions, const char* _identifier);
319262

320263
virtual IShader::E_CONTENT_TYPE getCodeContentType() const = 0;
321264

‎src/nbl/asset/utils/CGLSLCompiler.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,27 @@
1212
using namespace nbl;
1313
using namespace nbl::asset;
1414

15+
static constexpr const char* PREPROC_GL__DISABLER = "_this_is_a_GL__prefix_";
16+
static constexpr const char* PREPROC_GL__ENABLER = PREPROC_GL__DISABLER;
17+
static constexpr const char* PREPROC_LINE_CONTINUATION_DISABLER = "_this_is_a_line_continuation_\n";
18+
static constexpr const char* PREPROC_LINE_CONTINUATION_ENABLER = "_this_is_a_line_continuation_";
19+
20+
static void disableGlDirectives(std::string& _code)
21+
{
22+
std::regex glMacro("[ \t\r\n\v\f]GL_");
23+
auto result = std::regex_replace(_code, glMacro, PREPROC_GL__DISABLER);
24+
std::regex lineContinuation("\\\\[ \t\r\n\v\f]*\n");
25+
_code = std::regex_replace(result, lineContinuation, PREPROC_LINE_CONTINUATION_DISABLER);
26+
}
27+
28+
static void reenableGlDirectives(std::string& _code)
29+
{
30+
std::regex lineContinuation(PREPROC_LINE_CONTINUATION_ENABLER);
31+
auto result = std::regex_replace(_code, lineContinuation, " \\");
32+
std::regex glMacro(PREPROC_GL__ENABLER);
33+
_code = std::regex_replace(result, glMacro, " GL_");
34+
}
35+
1536

1637
namespace nbl::asset::impl
1738
{
@@ -67,7 +88,7 @@ namespace nbl::asset::impl
6788
else {
6889
//employ encloseWithinExtraInclGuards() in order to prevent infinite loop of (not necesarilly direct) self-inclusions while other # directives (incl guards among them) are disabled
6990
IShaderCompiler::disableAllDirectivesExceptIncludes(res_str);
70-
CGLSLCompiler::disableGlDirectives(res_str);
91+
disableGlDirectives(res_str);
7192
res_str = IShaderCompiler::encloseWithinExtraInclGuards(std::move(res_str), m_maxInclCnt, name.string().c_str());
7293

7394
res->content_length = res_str.size();
@@ -108,7 +129,7 @@ std::string CGLSLCompiler::preprocessShader(std::string&& code, IShader::E_SHADE
108129
if (preprocessOptions.includeFinder != nullptr)
109130
{
110131
IShaderCompiler::disableAllDirectivesExceptIncludes(code);
111-
CGLSLCompiler::disableGlDirectives(code);
132+
disableGlDirectives(code);
112133
shaderc::Compiler comp;
113134
shaderc::CompileOptions options;
114135
options.SetTargetSpirv(shaderc_spirv_version_1_6);
@@ -124,7 +145,7 @@ std::string CGLSLCompiler::preprocessShader(std::string&& code, IShader::E_SHADE
124145

125146
auto resolvedString = std::string(res.cbegin(), std::distance(res.cbegin(), res.cend()));
126147
IShaderCompiler::reenableDirectives(resolvedString);
127-
CGLSLCompiler::reenableGlDirectives(resolvedString);
148+
reenableGlDirectives(resolvedString);
128149
return resolvedString;
129150
}
130151
else

‎src/nbl/asset/utils/CHLSLCompiler.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,9 @@ class DxcCompilationResult
9292
}
9393
};
9494

95-
DxcCompilationResult dxcCompile(const CHLSLCompiler* compiler, std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options)
95+
DxcCompilationResult CHLSLCompiler::dxcCompile(std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options) const
9696
{
97+
auto compiler = this;
9798
if (options.genDebugInfo)
9899
{
99100
std::ostringstream insertion;
@@ -292,7 +293,7 @@ core::smart_refctd_ptr<ICPUShader> CHLSLCompiler::compileToSPIRV(const char* cod
292293
const uint32_t nonDebugArgs = 5;
293294
const uint32_t allArgs = nonDebugArgs + 4;
294295

295-
auto compileResult = dxcCompile(this, newCode, &arguments[0], hlslOptions.genDebugInfo ? allArgs : nonDebugArgs, hlslOptions);
296+
auto compileResult = dxcCompile(newCode, &arguments[0], hlslOptions.genDebugInfo ? allArgs : nonDebugArgs, hlslOptions);
296297

297298
if (!compileResult.objectBlob)
298299
{

‎src/nbl/asset/utils/IShaderCompiler.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,66 @@
1212
using namespace nbl;
1313
using namespace nbl::asset;
1414

15+
//string to be replaced with all "#" except those in "#include"
16+
static constexpr const char* PREPROC_DIRECTIVE_DISABLER = "_this_is_a_hash_";
17+
static constexpr const char* PREPROC_DIRECTIVE_ENABLER = PREPROC_DIRECTIVE_DISABLER;
18+
19+
//all "#", except those in "#include"/"#version"/"#pragma shader_stage(...)", replaced with `PREPROC_DIRECTIVE_DISABLER`
20+
void IShaderCompiler::disableAllDirectivesExceptIncludes(std::string& _code)
21+
{
22+
// TODO: replace this with a proper-ish proprocessor and includer one day
23+
std::regex directive("#(?!(include|version|pragma shader_stage))");//all # not followed by "include" nor "version" nor "pragma shader_stage"
24+
//`#pragma shader_stage(...)` is needed for determining shader stage when `_stage` param of IShaderCompiler functions is set to ESS_UNKNOWN
25+
_code = std::regex_replace(_code, directive, PREPROC_DIRECTIVE_DISABLER);
26+
}
27+
28+
void IShaderCompiler::reenableDirectives(std::string& _code)
29+
{
30+
std::regex directive(PREPROC_DIRECTIVE_ENABLER);
31+
_code = std::regex_replace(_code, directive, "#");
32+
}
33+
34+
std::string IShaderCompiler::encloseWithinExtraInclGuards(std::string&& _code, uint32_t _maxInclusions, const char* _identifier)
35+
{
36+
assert(_maxInclusions != 0u);
37+
38+
using namespace std::string_literals;
39+
std::string defBase_ = "_GENERATED_INCLUDE_GUARD_"s + _identifier + "_";
40+
std::replace_if(defBase_.begin(), defBase_.end(), [](char c) ->bool { return !::isalpha(c) && !::isdigit(c); }, '_');
41+
42+
auto genDefs = [&defBase_, _maxInclusions, _identifier] {
43+
auto defBase = [&defBase_](uint32_t n) { return defBase_ + std::to_string(n); };
44+
std::string defs = "#ifndef " + defBase(0) + "\n\t#define " + defBase(0) + "\n";
45+
for (uint32_t i = 1u; i <= _maxInclusions; ++i) {
46+
const std::string defname = defBase(i);
47+
defs += "#elif !defined(" + defname + ")\n\t#define " + defname + "\n";
48+
}
49+
defs += "#endif\n";
50+
return defs;
51+
};
52+
auto genUndefs = [&defBase_, _maxInclusions, _identifier] {
53+
auto defBase = [&defBase_](int32_t n) { return defBase_ + std::to_string(n); };
54+
std::string undefs = "#ifdef " + defBase(_maxInclusions) + "\n\t#undef " + defBase(_maxInclusions) + "\n";
55+
for (int32_t i = _maxInclusions - 1; i >= 0; --i) {
56+
const std::string defname = defBase(i);
57+
undefs += "#elif defined(" + defname + ")\n\t#undef " + defname + "\n";
58+
}
59+
undefs += "#endif\n";
60+
return undefs;
61+
};
62+
63+
return
64+
genDefs() +
65+
"\n"
66+
"#ifndef " + defBase_ + std::to_string(_maxInclusions) +
67+
"\n" +
68+
"#line 1 \"" + _identifier + "\"\n" +
69+
_code +
70+
"\n"
71+
"#endif"
72+
"\n\n" +
73+
genUndefs();
74+
}
1575

1676
IShaderCompiler::IShaderCompiler(core::smart_refctd_ptr<system::ISystem>&& system)
1777
: m_system(std::move(system))

0 commit comments

Comments
 (0)