Skip to content

Commit 3650da9

Browse files
AnastaZIukAnastaZIuk
authored
Merge pull request #1022 from AnastaZIuk/nscFixes
Clean up Wave preprocessor handling
2 parents 38e7b5b + a091ca9 commit 3650da9

3 files changed

Lines changed: 205 additions & 74 deletions

File tree

‎.github/workflows/build-nabla.yml‎

Lines changed: 103 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77

88
permissions:
99
contents: read
10-
checks: write
10+
actions: read
1111

1212
concurrency:
1313
group: push-lock-${{ github.ref }}
@@ -219,73 +219,81 @@ jobs:
219219
--profiling-format=google-trace
220220
221221
- name: Container – Build & Install Nabla
222+
id: build-nabla
222223
run: |
223224
docker exec orphan `
224225
${{ env.entry }} ${{ env.cmd }} -Command cmake --build `
225226
--preset ci-build-dynamic-${{ matrix.vendor }} `
226227
--target install `
227228
--config ${{ matrix.config }}
228229
229-
- name: API / Examples / Check Run (Create)
230-
id: check-run-create
231-
uses: actions/github-script@v6
232-
with:
233-
github-token: ${{ secrets.GITHUB_TOKEN }}
234-
result-encoding: string
235-
script: |
236-
const headSha = context.payload.pull_request ? context.payload.pull_request.head.sha : context.sha;
237-
const response = await github.rest.checks.create({
238-
owner: context.repo.owner,
239-
repo: context.repo.repo,
240-
name: `Examples (${{ matrix.os }}, ${{ matrix.vendor }}-${{ matrix.tag }}, ${{ matrix.config }})`,
241-
head_sha: headSha,
242-
status: 'in_progress'
243-
});
244-
return response.data.id;
245-
246230
- name: Container – Build & Install Examples
247231
id: build-examples
248232
continue-on-error: true
233+
shell: pwsh
249234
run: |
250-
docker exec orphan `
235+
$statusDir = Join-Path $env:RUNNER_TEMP "examples-status"
236+
New-Item -ItemType Directory -Force -Path $statusDir | Out-Null
237+
238+
$buildLog = Join-Path $statusDir "build-examples.log"
239+
$installLog = Join-Path $statusDir "install-examples.log"
240+
241+
& docker exec orphan `
251242
${{ env.entry }} ${{ env.cmd }} -Command cmake --build `
252243
--preset ci-build-dynamic-${{ matrix.vendor }} `
253244
-t examples_tests\all --config ${{ matrix.config }} `
254-
-- -k 0
245+
-- -k 0 2>&1 | Tee-Object -FilePath $buildLog
246+
$buildExitCode = $LASTEXITCODE
247+
248+
if ($buildExitCode -eq 0) {
249+
& docker exec orphan `
250+
${{ env.entry }} ${{ env.cmd }} -Command cmake --install `
251+
${{ env.binary }}\examples_tests --config ${{ matrix.config }} `
252+
--prefix ${{ env.install }} 2>&1 | Tee-Object -FilePath $installLog
253+
$installExitCode = $LASTEXITCODE
254+
} else {
255+
"Skipped because the Examples build failed." | Set-Content -Path $installLog -Encoding ascii
256+
$installExitCode = 1
257+
}
255258
256-
docker exec orphan `
257-
${{ env.entry }} ${{ env.cmd }} -Command cmake --install `
258-
${{ env.binary }}\examples_tests --config ${{ matrix.config }} `
259-
--prefix ${{ env.install }}
259+
if ($buildExitCode -ne 0) {
260+
exit $buildExitCode
261+
}
262+
263+
if ($installExitCode -ne 0) {
264+
exit $installExitCode
265+
}
260266
261-
- name: API / Examples / Check Run (Conclusion)
262-
id: outcome-examples
267+
- name: Record Examples result
268+
if: ${{ always() }}
269+
shell: pwsh
263270
run: |
264-
$completedAt = (Get-Date).ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ")
265-
if ("${{ steps.build-examples.outcome }}" -eq "success") {
266-
"conclusion=success" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
271+
$examplesResult = if (
272+
"${{ steps.build-nabla.outcome }}" -eq "success" -and
273+
"${{ steps.build-examples.outcome }}" -eq "success"
274+
) {
275+
"success"
267276
} else {
268-
"conclusion=failure" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
277+
"failure"
269278
}
270-
"completed_at=$completedAt" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
271-
272-
- name: API / Examples / Check Run (Update)
273-
uses: actions/github-script@v6
279+
$statusDir = Join-Path $env:RUNNER_TEMP "examples-status"
280+
New-Item -ItemType Directory -Force -Path $statusDir | Out-Null
281+
$statusFile = Join-Path $statusDir "status.txt"
282+
$detailsFile = Join-Path $statusDir "details.txt"
283+
$examplesResult | Set-Content -Path $statusFile -Encoding ascii -NoNewline
284+
@(
285+
"build-nabla=${{ steps.build-nabla.outcome }}"
286+
"build-examples=${{ steps.build-examples.outcome }}"
287+
"result=$examplesResult"
288+
) | Set-Content -Path $detailsFile -Encoding ascii
289+
290+
- name: Upload Examples result
291+
if: ${{ always() }}
292+
uses: actions/upload-artifact@v4
274293
with:
275-
github-token: ${{ secrets.GITHUB_TOKEN }}
276-
script: |
277-
await github.rest.checks.update({
278-
owner: context.repo.owner,
279-
repo: context.repo.repo,
280-
check_run_id: ${{ steps.check-run-create.outputs.result }},
281-
status: 'completed',
282-
conclusion: '${{ steps.outcome-examples.outputs.conclusion }}',
283-
completed_at: '${{ steps.outcome-examples.outputs.completed_at }}',
284-
output: {
285-
title: '',
286-
summary: '[View logs](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) to see details.'
287-
}
288-
});
294+
name: examples-status-${{ matrix.os }}-${{ matrix.vendor }}-${{ matrix.tag }}-${{ matrix.config }}
295+
path: ${{ runner.temp }}/examples-status
296+
if-no-files-found: error
289297

290298
- name: Container – Save NSC Image
291299
run: |
@@ -436,6 +444,53 @@ jobs:
436444
run: |
437445
docker push ${{ steps.set-prefix.outputs.nscTargetTaggedImageLatest }}
438446
447+
examples-status:
448+
name: Examples (${{ matrix.os }}, ${{ matrix.vendor }}-${{ matrix.tag }}, ${{ matrix.config }})
449+
needs: build-windows
450+
if: ${{ always() }}
451+
runs-on: ubuntu-latest
452+
strategy:
453+
fail-fast: false
454+
matrix:
455+
vendor: [msvc]
456+
config: [Release, Debug, RelWithDebInfo]
457+
tag: ['17.13.6']
458+
os: [windows-2022]
459+
460+
steps:
461+
- name: Download Examples result
462+
uses: actions/download-artifact@v4
463+
with:
464+
name: examples-status-${{ matrix.os }}-${{ matrix.vendor }}-${{ matrix.tag }}-${{ matrix.config }}
465+
path: examples-status
466+
467+
- name: Fail if Examples did not succeed
468+
shell: bash
469+
run: |
470+
status_file="examples-status/status.txt"
471+
if [[ ! -f "$status_file" ]]; then
472+
echo "Missing Examples status artifact"
473+
exit 1
474+
fi
475+
476+
result="$(tr -d '\r\n' < "$status_file")"
477+
echo "Examples result: $result"
478+
479+
if [[ "$result" != "success" ]]; then
480+
if [[ -f "examples-status/details.txt" ]]; then
481+
echo "Details:"
482+
cat "examples-status/details.txt"
483+
fi
484+
for file in "examples-status/build-examples.log" "examples-status/install-examples.log"; do
485+
if [[ -f "$file" ]]; then
486+
echo "::group::$(basename "$file")"
487+
cat "$file"
488+
echo "::endgroup::"
489+
fi
490+
done
491+
exit 1
492+
fi
493+
439494
update-badges:
440495
name: Update Build & Image Badges
441496
if: ${{ always() && github.ref == 'refs/heads/master' }}

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

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
#include <combaseapi.h>
2121
#include <sstream>
2222
#include <dxc/dxcapi.h>
23-
#include <boost/algorithm/string/predicate.hpp>
24-
#include <boost/algorithm/string/trim.hpp>
2523

2624
using namespace nbl;
2725
using namespace nbl::asset;
@@ -363,6 +361,91 @@ namespace nbl::wave
363361
extern nbl::core::string preprocess(std::string& code, const IShaderCompiler::SPreprocessorOptions& preprocessOptions, bool withCaching, std::function<void(nbl::wave::context&)> post);
364362
}
365363

364+
static bool isHorizontalWhitespace(const char c)
365+
{
366+
return c == ' ' || c == '\t' || c == '\r' || c == '\v' || c == '\f';
367+
}
368+
369+
static bool consumeIdentifier(std::string_view line, size_t& pos, const std::string_view identifier)
370+
{
371+
if (line.substr(pos, identifier.size()) != identifier)
372+
return false;
373+
374+
const size_t end = pos + identifier.size();
375+
if (end < line.size())
376+
{
377+
const char next = line[end];
378+
if ((next >= 'a' && next <= 'z') || (next >= 'A' && next <= 'Z') || (next >= '0' && next <= '9') || next == '_')
379+
return false;
380+
}
381+
382+
pos = end;
383+
return true;
384+
}
385+
386+
static void normalizeLegacyShaderStagePragmas(std::string& code)
387+
{
388+
std::string normalized;
389+
normalized.reserve(code.size());
390+
391+
size_t lineStart = 0u;
392+
while (lineStart < code.size())
393+
{
394+
const size_t lineEnd = code.find('\n', lineStart);
395+
const bool hasNewline = lineEnd != std::string::npos;
396+
const size_t lineSize = hasNewline ? (lineEnd - lineStart) : (code.size() - lineStart);
397+
const std::string_view line(code.data() + lineStart, lineSize);
398+
399+
size_t pos = 0u;
400+
while (pos < line.size() && isHorizontalWhitespace(line[pos]))
401+
++pos;
402+
403+
bool normalizedLegacyPragma = false;
404+
if (pos < line.size() && line[pos] == '#')
405+
{
406+
++pos;
407+
while (pos < line.size() && isHorizontalWhitespace(line[pos]))
408+
++pos;
409+
410+
if (consumeIdentifier(line, pos, "pragma"))
411+
{
412+
while (pos < line.size() && isHorizontalWhitespace(line[pos]))
413+
++pos;
414+
415+
const size_t pragmaArgumentPos = pos;
416+
if (consumeIdentifier(line, pos, "shader_stage"))
417+
normalizedLegacyPragma = true;
418+
else
419+
pos = pragmaArgumentPos;
420+
}
421+
}
422+
423+
if (normalizedLegacyPragma)
424+
{
425+
normalized.append(line.substr(0u, pos - std::string_view("shader_stage").size()));
426+
normalized += "wave ";
427+
normalized.append(line.substr(pos - std::string_view("shader_stage").size()));
428+
}
429+
else
430+
{
431+
normalized.append(line);
432+
}
433+
434+
if (hasNewline)
435+
normalized.push_back('\n');
436+
437+
lineStart = hasNewline ? (lineEnd + 1u) : code.size();
438+
}
439+
440+
code = std::move(normalized);
441+
}
442+
443+
static void ensureTrailingNewline(std::string& code)
444+
{
445+
if (!code.empty() && code.back() != '\n' && code.back() != '\r')
446+
code.push_back('\n');
447+
}
448+
366449
static std::string preprocessShaderImpl(
367450
std::string&& code,
368451
IShader::E_SHADER_STAGE& stage,
@@ -386,18 +469,8 @@ static std::string preprocessShaderImpl(
386469
if (depfileEnabled && !dependenciesOut)
387470
dependenciesOut = &localDependencies;
388471

389-
// HACK: we do a pre-pre-process here to add \n after every #pragma to neutralize boost::wave's actions
390-
// See https://github.com/Devsh-Graphics-Programming/Nabla/issues/746
391-
size_t line_index = 0;
392-
for (size_t i = 0; i < code.size(); i++) {
393-
if (code[i] == '\n') {
394-
auto line = code.substr(line_index, i - line_index);
395-
boost::trim(line);
396-
if (boost::starts_with(line, "#pragma"))
397-
code.insert(i++, 1, '\n');
398-
line_index = i;
399-
}
400-
}
472+
normalizeLegacyShaderStagePragmas(code);
473+
ensureTrailingNewline(code);
401474

402475
// preprocess
403476
core::string resolvedString = nbl::wave::preprocess(code, preprocessOptions, bool(dependenciesOut),

‎src/nbl/asset/utils/waveContext.h‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ struct load_to_string final
2929
static void init_iterators(IterContextT& iter_ctx, PositionT const& act_pos, boost::wave::language_support language)
3030
{
3131
iter_ctx.instring = iter_ctx.ctx.get_located_include_content();
32+
if (!iter_ctx.instring.empty() && iter_ctx.instring.back() != '\n' && iter_ctx.instring.back() != '\r')
33+
iter_ctx.instring.push_back('\n');
3234

3335
using iterator_type = IterContextT::iterator_type;
3436
iter_ctx.first = iterator_type(iter_ctx.instring.begin(),iter_ctx.instring.end(),PositionT(iter_ctx.filename),language);
@@ -72,7 +74,6 @@ struct preprocessing_hooks final : public boost::wave::context_policies::default
7274
typename ContextT::token_type const& act_token
7375
)
7476
{
75-
hash_token_occurences++;
7677
auto optionStr = option.get_value().c_str();
7778
if (strcmp(optionStr,"shader_stage")==0)
7879
{
@@ -118,12 +119,13 @@ struct preprocessing_hooks final : public boost::wave::context_policies::default
118119
std::string compiler_option_s = std::string(valueIter->get_value().c_str());
119120
// the compiler_option_s is a token thus can be only part of the actual argument, i.e. "-spirv" will be split into tokens [ "-", "spirv" ]
120121
// for dxc_compile_flags just join the strings until it finds a whitespace or end of args
121-
122-
if (compiler_option_s == " ")
122+
if (IS_CATEGORY(*valueIter, WhiteSpaceTokenType))
123123
{
124-
// push argument and reset
125-
m_dxc_compile_flags_override.push_back(arg);
126-
arg.clear();
124+
if (!arg.empty())
125+
{
126+
m_dxc_compile_flags_override.push_back(arg);
127+
arg.clear();
128+
}
127129
}
128130
else
129131
{
@@ -196,12 +198,13 @@ class context : private boost::noncopyable
196198
, current_relative_filename(fname)
197199
, macros(*this_())
198200
, language(language_support(
199-
support_cpp20
200-
| support_option_preserve_comments
201-
| support_option_emit_line_directives
202-
| support_option_emit_pragma_directives
203-
// | support_option_emit_contnewlines
204-
// | support_option_insert_whitespace
201+
support_cpp20 // C++20 lexer mode. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L56-L59
202+
| support_option_prefer_pp_numbers // Prefer pp-number lexing before retokenization. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L71
203+
| support_option_preserve_comments // Keep comments in the token stream. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L67
204+
| support_option_emit_line_directives // Emit #line directives in the output. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L72
205+
| support_option_emit_pragma_directives // Keep pragma directives in the output. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L74
206+
// | support_option_emit_contnewlines // Emit escaped line continuations. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L65
207+
// | support_option_insert_whitespace // Let Wave inject separator whitespace. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L66
205208
))
206209
, hooks(hooks_)
207210
{

0 commit comments

Comments
 (0)