From bb87e9e6280b321708f631f286b57a67343c769e Mon Sep 17 00:00:00 2001 From: Matt Williams Date: Sun, 28 Oct 2012 15:41:53 +0000 Subject: [PATCH] Fix Ambient Occlusion Calculator to accept functors, functions and lambdas By changing the 'pass by value' to be a 'pass by const reference' (and adding some const qualifiers) the calculator can take any of the three types. Performance could be improved further using C++11 perfect forwarding to pass the function on without changing a thing. I added a comment to remind us of this. Also added a test for passing a function and a (commented out) test for passing a lambda. --- .../PolyVoxCore/AmbientOcclusionCalculator.h | 13 +++++++++---- .../PolyVoxCore/AmbientOcclusionCalculator.inl | 2 +- tests/TestAmbientOcclusionGenerator.cpp | 13 ++++++++++++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.h b/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.h index 341c0cff..0cbb08da 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.h +++ b/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.h @@ -49,9 +49,8 @@ namespace PolyVox class AmbientOcclusionCalculatorRaycastCallback { public: - AmbientOcclusionCalculatorRaycastCallback(IsVoxelTransparentCallback isVoxelTransparentCallback) + AmbientOcclusionCalculatorRaycastCallback(IsVoxelTransparentCallback isVoxelTransparentCallback) : mIsVoxelTransparentCallback(isVoxelTransparentCallback) { - mIsVoxelTransparentCallback = isVoxelTransparentCallback; } bool operator()(const SimpleVolume::Sampler& sampler) @@ -65,13 +64,19 @@ namespace PolyVox return direct; } - IsVoxelTransparentCallback mIsVoxelTransparentCallback; + const IsVoxelTransparentCallback& mIsVoxelTransparentCallback; }; // NOTE: The callback needs to be a functor not a function. I haven't been // able to work the required template magic to get functions working as well. + // + // Matt: If you make the function take a "IsVoxelTransparentCallback&&" then + // it will forward it on. Works for functors, functions and lambdas. + // This will be 'perfect forwarding' using 'universal references' + // This will require C++11 rvalue references which is why I haven't made the + // change yet. template - void calculateAmbientOcclusion(VolumeType* volInput, Array<3, uint8_t>* arrayResult, Region region, float fRayLength, uint8_t uNoOfSamplesPerOutputElement, IsVoxelTransparentCallback isVoxelTransparentCallback); + void calculateAmbientOcclusion(VolumeType* volInput, Array<3, uint8_t>* arrayResult, Region region, float fRayLength, uint8_t uNoOfSamplesPerOutputElement, const IsVoxelTransparentCallback& isVoxelTransparentCallback); } #include "PolyVoxCore/AmbientOcclusionCalculator.inl" diff --git a/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.inl b/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.inl index b5b814fd..bdb1237c 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.inl @@ -24,7 +24,7 @@ freely, subject to the following restrictions: namespace PolyVox { template - void calculateAmbientOcclusion(VolumeType* volInput, Array<3, uint8_t>* arrayResult, Region region, float fRayLength, uint8_t uNoOfSamplesPerOutputElement, IsVoxelTransparentCallback isVoxelTransparentCallback) + void calculateAmbientOcclusion(VolumeType* volInput, Array<3, uint8_t>* arrayResult, Region region, float fRayLength, uint8_t uNoOfSamplesPerOutputElement, const IsVoxelTransparentCallback& isVoxelTransparentCallback) { typename VolumeType::Sampler m_sampVolume(volInput); diff --git a/tests/TestAmbientOcclusionGenerator.cpp b/tests/TestAmbientOcclusionGenerator.cpp index 42f34a1f..667acce4 100644 --- a/tests/TestAmbientOcclusionGenerator.cpp +++ b/tests/TestAmbientOcclusionGenerator.cpp @@ -33,12 +33,17 @@ using namespace PolyVox; class IsVoxelTransparent { public: - bool operator()(uint8_t voxel) + bool operator()(uint8_t voxel) const { return voxel == 0; } }; +bool isVoxelTransparentFunction(uint8_t voxel) +{ + return voxel == 0; +} + void TestAmbientOcclusionGenerator::testExecute() { const int32_t g_uVolumeSideLength = 64; @@ -78,6 +83,12 @@ void TestAmbientOcclusionGenerator::testExecute() QCOMPARE(static_cast(ambientOcclusionResult[16][16][16]), 103); QCOMPARE(static_cast(ambientOcclusionResult[16][24][16]), 123); QCOMPARE(static_cast(ambientOcclusionResult[16][31][16]), 173); + + //Just run a quick test to make sure that it compiles when taking a function pointer + calculateAmbientOcclusion(&volData, &ambientOcclusionResult, volData.getEnclosingRegion(), 32.0f, 8, &isVoxelTransparentFunction); + + //Also test it using a lambda + //calculateAmbientOcclusion(&volData, &ambientOcclusionResult, volData.getEnclosingRegion(), 32.0f, 8, [](uint8_t voxel){return voxel == 0;}); } QTEST_MAIN(TestAmbientOcclusionGenerator)