Skip to content

Commit

Permalink
Fix asserts and build buster in debug build.
Browse files Browse the repository at this point in the history
- Fixed assert() in Tests/FileSystemModel_Test.cpp that broke the
  debug build.
- Fixed asserts in Engine/EffectInstanceRenderRoI.cpp so that they
  are only checked if tiling is supported. The assert was
  accidentally applied to all cases when the code was refactored
  by NatronGitHub#918
- Updated Linux CI to build and test release AND debug builds.
  This should help avoid such issues from sneaking in going forward.
  • Loading branch information
acolwell committed Oct 17, 2023
1 parent 525a5d5 commit 676bacb
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 23 deletions.
39 changes: 27 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,34 +66,49 @@ jobs:
tar xzf Natron-v${OCIO_CONFIG_VERSION}.tar.gz
mv OpenColorIO-Configs-Natron-v${OCIO_CONFIG_VERSION} OpenColorIO-Configs
- name: Build Unix
- name: Download Plugins
run: |
mkdir build && cd build
mkdir Plugins && cd Plugins
wget https://github.com/NatronGitHub/openfx-io/releases/download/natron_testing/openfx-io-build-ubuntu_22-testing.zip
unzip openfx-io-build-ubuntu_22-testing.zip
cd ..
- name: Build Unix (debug)
run: |
mkdir debug && cd debug
cmake ../
make -j2
- name: Run Unix Tests
id: run-unix-tests
- name: Run Unix Tests (debug)
id: run-unix-tests-debug
# Allow continuing after error so logs can be uploaded.
continue-on-error: true
run: |
cd build
cd debug
OFX_PLUGIN_PATH=$PWD/../Plugins OCIO=$PWD/../OpenColorIO-Configs/blender/config.ocio ctest -V
mkdir Plugins && cd Plugins
wget https://github.com/NatronGitHub/openfx-io/releases/download/natron_testing/openfx-io-build-ubuntu_22-testing.zip
unzip openfx-io-build-ubuntu_22-testing.zip
cd ..
- name: Build Unix (release)
run: |
mkdir release && cd release
cmake ../
make -j2
OFX_PLUGIN_PATH=$PWD/Plugins OCIO=$PWD/../OpenColorIO-Configs/blender/config.ocio ctest -V
- name: Run Unix Tests (release)
id: run-unix-tests-release
# Allow continuing after error so logs can be uploaded.
continue-on-error: true
run: |
cd release
OFX_PLUGIN_PATH=$PWD/../Plugins OCIO=$PWD/../OpenColorIO-Configs/blender/config.ocio ctest -V
- name: Upload ${{ matrix.os }} Test Log artifacts
uses: actions/upload-artifact@v3
with:
name: ${{ matrix.os }} Test Logs
path: ${{ github.workspace }}/build/Testing/Temporary/LastTest.log
path: ${{ github.workspace }}/release/Testing/Temporary/LastTest.log

- name: Check for test failures
if: steps.run-unix-tests.outcome == 'failure'
if: steps.run-unix-tests-debug.outcome == 'failure' || steps.run-unix-tests-release.outcome == 'failure'
run: exit 1

win-test:
Expand Down
16 changes: 10 additions & 6 deletions Engine/EffectInstanceRenderRoI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,17 +737,21 @@ EffectInstance::renderRoI(const RenderRoIArgs & args,
//renderRoIInternal should check the bitmap of 'image' and not downscaledImage!
roi = args.roi.toNewMipmapLevel(args.mipmapLevel, 0, par, rod);

if (frameArgs->tilesSupported && !roi.clipIfOverlaps(upscaledImageBoundsNc)) {
return eRenderRoIRetCodeOk;
if (frameArgs->tilesSupported) {
if (!roi.clipIfOverlaps(upscaledImageBoundsNc)) {
return eRenderRoIRetCodeOk;
}
assert(upscaledImageBoundsNc.contains(roi));
}
assert( upscaledImageBoundsNc.contains(roi));
} else {
roi = args.roi;

if (frameArgs->tilesSupported && !roi.clipIfOverlaps(downscaledImageBoundsNc)) {
return eRenderRoIRetCodeOk;
if (frameArgs->tilesSupported) {
if (!roi.clipIfOverlaps(downscaledImageBoundsNc)) {
return eRenderRoIRetCodeOk;
}
assert(downscaledImageBoundsNc.contains(roi));
}
assert(downscaledImageBoundsNc.contains(roi));
}

/*
Expand Down
8 changes: 3 additions & 5 deletions Tests/FileSystemModel_Test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,8 @@ TEST(FileSystemModelTest, CleanPath) {
expectedOutput = testCase.input;
}
#endif
// Make sure the expectation was actually set to something.
assert(!expectedOutput.isNull());

const auto output = FileSystemModel::cleanPath(input).toStdString();
EXPECT_EQ(expectedOutput, output) << " input '" << testCase.input << "'";
const QString output = FileSystemModel::cleanPath(input);
ASSERT_TRUE(!output.isNull());
EXPECT_EQ(expectedOutput, output.toStdString()) << " input '" << testCase.input << "'";
}
}

0 comments on commit 676bacb

Please sign in to comment.