From f9d0cc52f1fbcf576045d074d9130f7882f0af42 Mon Sep 17 00:00:00 2001 From: Michele Renzullo Date: Wed, 25 Dec 2024 19:35:00 -0300 Subject: [PATCH] Add CPPCHECK and Clang Tidy in CMake --- .clang-tidy | 35 +++++++++++++++++++ .cppcheck-suppressions | 3 ++ CMakeLists.txt | 51 ++++++++++++++++++++++++---- Dockerfile | 4 +-- include/gaussianblur/helpers.hpp | 8 ++--- src/gaussianblur.cpp | 14 ++++---- tests/test_gaussianblur.cpp | 58 ++++++++++++++++---------------- 7 files changed, 124 insertions(+), 49 deletions(-) create mode 100644 .clang-tidy create mode 100644 .cppcheck-suppressions diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..8fdc9fe --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,35 @@ +--- +Checks: '*, +-clang-diagnostic-error, +-clang-diagnostic-unused-variable, +-cppcoreguidelines-avoid-magic-numbers, +-cppcoreguidelines-non-private-member-variables-in-classes, +-cppcoreguidelines-pro-bounds-array-to-pointer-decay, +-cppcoreguidelines-pro-bounds-pointer-arithmetic, +-cppcoreguidelines-pro-type-cstyle-cast, +-cppcoreguidelines-pro-type-vararg, +-cppcoreguidelines-special-member-functions, +-fuchsia-default-arguments-calls, +-fuchsia-default-arguments-declarations, +-google-readability-avoid-underscore-in-googletest-name, +-google-readability-namespace-comments, +-google-readability-todo, +-google-runtime-references, +-hicpp-avoid-c-arrays, +-hicpp-no-array-decay, +-hicpp-no-malloc, +-hicpp-special-member-functions, +-hicpp-vararg, +-llvm-header-guard, +-llvm-namespace-comment, +-llvmlibc-*, +-misc-non-private-member-variables-in-classes, +-modernize-avoid-c-arrays, +-modernize-use-trailing-return-type, +-readability-else-after-return, +-readability-implicit-bool-conversion, +-readability-convert-member-functions-to-static, +-readability-magic-numbers, +' +HeaderFilterRegex: '.*(include/gaussianblur).*' +ExcludeHeaderFilterRegex: '.*(external/googletest/googletest/include|include/pffft_pommier).*' \ No newline at end of file diff --git a/.cppcheck-suppressions b/.cppcheck-suppressions new file mode 100644 index 0000000..51916fb --- /dev/null +++ b/.cppcheck-suppressions @@ -0,0 +1,3 @@ +*:*/stb_image* +*:*/pffft* +*:*/googletest* diff --git a/CMakeLists.txt b/CMakeLists.txt index b439c86..6d7fd91 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,9 +28,17 @@ endif() # If compiling to WebAssembly if(WASM) message(STATUS "Compiling to WebAssembly") - if(NOT DEFINED CMAKE_TOOLCHAIN_FILE) - set(CMAKE_TOOLCHAIN_FILE "$ENV{HOME}/emsdk/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake") + if(NOT CMAKE_TOOLCHAIN_FILE OR NOT CMAKE_TOOLCHAIN_FILE MATCHES "Emscripten.cmake") + find_program(EMCC_EXE emcc) + if (EMCC_EXE) + message ("-- Found Emscripten") + get_filename_component(EMSDK_DIR ${EMCC_EXE} DIRECTORY) + set(CMAKE_TOOLCHAIN_FILE "${EMSDK_DIR}/cmake/Modules/Platform/Emscripten.cmake") + else() + message(FATAL_ERROR "-- Emscripten not found! Aborting...") + endif() endif() + message(STATUS "Using Emscripten toolchain file: ${CMAKE_TOOLCHAIN_FILE}") set(CMAKE_EXECUTABLE_SUFFIX ".wasm") endif() @@ -60,15 +68,39 @@ if(APPLE AND NOT WASM) set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -framework Accelerate") endif() +# Cpp check for static analysis +find_program(CPPCHECK_EXE cppcheck) +if (CPPCHECK_EXE) + message("-- Program cppcheck found: ${CPPCHECK_EXE}") + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + add_custom_target(cppcheck + COMMAND "${CPPCHECK_EXE}" --std=c++${CMAKE_CXX_STANDARD} "--suppressions-list=${CMAKE_SOURCE_DIR}/.cppcheck-suppressions" --enable=all "--project=${CMAKE_BINARY_DIR}/compile_commands.json") +else () + message("-- Program cppcheck NOT found!") +endif () + +# Clang tidy +if (CLANG_TIDY_ENABLED) + find_program(CLANG_TIDY_EXE clang-tidy) + if (CLANG_TIDY_EXE) + message("-- Program clang-tidy found: ${CLANG_TIDY_EXE}") + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + set(CMAKE_C_CLANG_TIDY "${CLANG_TIDY_EXE}" --quiet "--config-file=${CMAKE_SOURCE_DIR}/.clang-tidy" -p "${CMAKE_BINARY_DIR}/compile_commands.json") + set(CMAKE_CXX_CLANG_TIDY "${CMAKE_C_CLANG_TIDY}") + else () + message("-- Program clang-tidy NOT found!") + endif () +endif () + # Project name project(GaussianBlur) include_directories(include) # Add library if(WASM) - add_library(GaussianBlurLib STATIC "include/pffft_pommier/pffft.c" "src/gaussianblur.cpp") + add_library(GaussianBlurLib STATIC "${CMAKE_SOURCE_DIR}/include/pffft_pommier/pffft.c" "${CMAKE_SOURCE_DIR}/src/gaussianblur.cpp") else() - add_library(GaussianBlurLib "include/pffft_pommier/pffft.c" "src/gaussianblur.cpp") + add_library(GaussianBlurLib "${CMAKE_SOURCE_DIR}/include/pffft_pommier/pffft.c" "${CMAKE_SOURCE_DIR}/src/gaussianblur.cpp") endif() # Set properties for the library @@ -77,7 +109,7 @@ set_target_properties(GaussianBlurLib PROPERTIES OUTPUT_NAME "Gaussianblur") if(WITH_EXAMPLES) message(STATUS "Building examples") if(WASM) - add_executable(GaussianBlur "examples/wasm/main.cpp") + add_executable(GaussianBlur "${CMAKE_SOURCE_DIR}/examples/wasm/main.cpp") set(WASM_COMMON_LINK_OPTIONS "SHELL: --closure 1" "-sALLOW_MEMORY_GROWTH=1" @@ -104,7 +136,7 @@ if(WITH_EXAMPLES) # Install the WASM file install(FILES "${CMAKE_BINARY_DIR}/GaussianBlur.wasm" DESTINATION bin) else() - add_executable(GaussianBlur "examples/desktop/main.cpp") + add_executable(GaussianBlur "${CMAKE_SOURCE_DIR}/examples/desktop/main.cpp") endif() target_link_libraries(GaussianBlur GaussianBlurLib) @@ -127,12 +159,17 @@ if(WITH_TESTS) set(BUILD_SHARED_LIBS_SAVE ${BUILD_SHARED_LIBS}) set(BUILD_SHARED_LIBS OFF) add_subdirectory(external/googletest) + + # Skip clang-tidy for Google Test + set_target_properties(gtest PROPERTIES CXX_CLANG_TIDY "" C_CLANG_TIDY "") + set_target_properties(gtest_main PROPERTIES CXX_CLANG_TIDY "" C_CLANG_TIDY "") + set(BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS_SAVE}) include_directories(${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR}) # Add test executable - add_executable(GaussianBlurTests tests/test_gaussianblur.cpp) + add_executable(GaussianBlurTests ${CMAKE_SOURCE_DIR}/tests/test_gaussianblur.cpp) target_link_libraries(GaussianBlurTests GaussianBlurLib gtest_main) add_test(NAME GaussianBlurTests COMMAND GaussianBlurTests) install(TARGETS GaussianBlurTests DESTINATION bin) diff --git a/Dockerfile b/Dockerfile index 09a929e..efc9123 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,8 +2,8 @@ FROM debian:bookworm-slim AS base # Install dependencies RUN apt-get -qq update; \ - apt-get install -qqy --no-install-recommends \ - gnupg2 wget ca-certificates apt-transport-https curl unzip make cmake xz-utils + apt-get install -qqy --no-install-recommends gnupg2 wget ca-certificates \ + apt-transport-https curl unzip make cmake xz-utils cppcheck # Install LLVM RUN echo "deb https://apt.llvm.org/bookworm llvm-toolchain-bookworm-16 main" \ diff --git a/include/gaussianblur/helpers.hpp b/include/gaussianblur/helpers.hpp index 8aa0ffc..1c9f920 100644 --- a/include/gaussianblur/helpers.hpp +++ b/include/gaussianblur/helpers.hpp @@ -216,7 +216,7 @@ void deinterleave_channels(const T *const interleaved, U **const deinterleaved, // Cache-friendly deinterleave, splitting for blocks of 16 MB, inspired by // flip-block constexpr float round = - std::is_integral_v ? std::is_integral_v ? 0 : 0.5f : 0; + std::is_integral_v ? std::is_integral_v ? 0 : 0.5F : 0; constexpr uint32_t block = L2_CACHE_SIZE / (Channels * std::max(sizeof(T), sizeof(U))); const uint32_t num_blocks = std::ceil(total_size / (float)block); @@ -229,12 +229,12 @@ void deinterleave_channels(const T *const interleaved, U **const deinterleaved, for (uint32_t c = 0; c < Channels; ++c) { channel_ptrs[c] = deinterleaved[c] + x; } - const T *const interleaved_ptr = interleaved + x * Channels; + const T *const interleaved_ptr = interleaved + (x * Channels); const int blockx = (n == num_blocks - 1) ? last_block_size : block; for (int xx = 0; xx < blockx; ++xx) { for (uint32_t c = 0; c < Channels; ++c) { - channel_ptrs[c][xx] = interleaved_ptr[xx * Channels + c] + round; + channel_ptrs[c][xx] = interleaved_ptr[(xx * Channels) + c] + round; } } }); @@ -244,7 +244,7 @@ template void interleave_channels(const U **const deinterleaved, T *const interleaved, const uint32_t total_size) { constexpr float round = - std::is_integral_v ? std::is_integral_v ? 0 : 0.5f : 0; + std::is_integral_v ? std::is_integral_v ? 0 : 0.5F : 0; constexpr uint32_t block = L2_CACHE_SIZE / (Channels * std::max(sizeof(T), sizeof(U))); const uint32_t num_blocks = std::ceil(total_size / (float)block); diff --git a/src/gaussianblur.cpp b/src/gaussianblur.cpp index cd1f2e9..22a810b 100644 --- a/src/gaussianblur.cpp +++ b/src/gaussianblur.cpp @@ -15,7 +15,7 @@ int gaussian_window(const float sigma, const int max_width = 0) { // bigger than it const float radius = sigma * sqrt(2 * log(255)) - 1; - int width = radius * 2 + 0.5f; + int width = radius * 2 + 0.5F; if (max_width) width = std::min(width, max_width); if (width % 2 == 0) ++width; @@ -34,8 +34,8 @@ void get_gaussian(T &kernel, const float sigma, int width = 0, kernel.resize(FFT_length ? FFT_length : width); - const float mid_w = (width - 1) / 2.0f; - const float s = 2.0f * sigma * sigma; + const float mid_w = (width - 1) / 2.0F; + const float s = 2.0F * sigma * sigma; int i = 0; @@ -43,10 +43,10 @@ void get_gaussian(T &kernel, const float sigma, int width = 0, kernel.at(i) = (exp(-(y * y) / s)) / (std::numbers::pi_v * s); const float sum = - 1.0f / std::accumulate(kernel.begin(), kernel.begin() + width, 0.0f); + 1.0F / std::accumulate(kernel.begin(), kernel.begin() + width, 0.0F); std::transform(kernel.begin(), kernel.begin() + width, kernel.begin(), - [&sum](auto &i) { return i * sum; }); + [&sum](const auto &i) { return i * sum; }); // fit the kernel in the FFT_length and shift the center to avoid circular // convolution @@ -248,8 +248,8 @@ void pffft(const ImgGeom image_geometry, const KernelDFT kernelDFT, std::chrono::steady_clock::now(); const int maxsize = std::max(kernelDFT.kerf_1D_row.size(), kernelDFT.kerf_1D_col.size()); - const float divisor_col = 1.0f / kernelDFT.kerf_1D_col.size(); - const float divisor_row = 1.0f / kernelDFT.kerf_1D_row.size(); + const float divisor_col = 1.0F / kernelDFT.kerf_1D_col.size(); + const float divisor_row = 1.0F / kernelDFT.kerf_1D_row.size(); AlignedVector tmp; tmp.reserve(maxsize); diff --git a/tests/test_gaussianblur.cpp b/tests/test_gaussianblur.cpp index 767af84..3edb64b 100644 --- a/tests/test_gaussianblur.cpp +++ b/tests/test_gaussianblur.cpp @@ -1,17 +1,17 @@ -#include -#include - +#include #include #include +#include #include +#include +#include #include - #include "test_helpers.hpp" // Test case for prepare_kernel_DFT TEST(GaussianBlurTest, PrepareKernelDFT) { - ImgGeom image_geom = {4, 4, 3}; // 4x4 image with 3 channels (RGB) - float sigma = 2.0f; + const ImgGeom image_geom = {4, 4, 3}; // 4x4 image with 3 channels (RGB) + float sigma = 2.0F; // Prepare the kernel DFT KernelDFT kernel_dft = gaussianblur::prepare_kernel_DFT(image_geom, sigma); @@ -37,10 +37,10 @@ TEST(GaussianBlurTest, BasicTestRGB) { // Row 3 128, 0, 0, 0, 128, 0, 0, 0, 128}; - ImgGeom image_geom = {3, 3, 3}; // 3x3 image with 3 channels (RGB) + const ImgGeom image_geom = {3, 3, 3}; // 3x3 image with 3 channels (RGB) Image image = {image_data, image_geom}; - float sigma = 3.0f; + float sigma = 3.0F; bool apply_to_alpha = false; // Calculate the average color and variance of the original image @@ -72,7 +72,7 @@ TEST(GaussianBlurTest, InvalidChannelCount) { ImgGeom image_geom = {3, 3, 2}; // 3x3 image with 2 channels Image image = {image_data, image_geom}; - float sigma = 3.0f; + float sigma = 3.0F; bool apply_to_alpha = false; gaussianblur::gaussianblur(image, sigma, apply_to_alpha); @@ -84,7 +84,7 @@ TEST(GaussianBlurTest, InvalidChannelCount) { // Test case for Gaussian blur without applying to alpha channel TEST(GaussianBlurTest, BasicTestRGBAWithoutAlpha) { // Create a 3x3 RGBA image with sharp contrasts - std::vector image_data = { + const std::vector image_data = { // Row 1 255, 0, 0, 128, 0, 255, 0, 128, 0, 0, 255, 128, // Row 2 @@ -92,22 +92,22 @@ TEST(GaussianBlurTest, BasicTestRGBAWithoutAlpha) { // Row 3 128, 0, 0, 128, 0, 128, 0, 128, 0, 0, 128, 128}; - ImgGeom image_geom = {3, 3, 4}; // 3x3 image with 4 channels (RGBA) + const ImgGeom image_geom = {3, 3, 4}; // 3x3 image with 4 channels (RGBA) Image image = {image_data, image_geom}; - float sigma = 3.0f; + float sigma = 3.0F; bool apply_to_alpha = false; // Calculate the average color and variance of the original image - float original_mean = calculate_average_color(image.data); - float original_variance = calculate_variance(image.data, original_mean); + const float original_mean = calculate_average_color(image.data); + const float original_variance = calculate_variance(image.data, original_mean); // Apply Gaussian blur gaussianblur::gaussianblur(image, sigma, apply_to_alpha); // Calculate the average color and variance of the blurred image - float blurred_mean = calculate_average_color(image.data); - float blurred_variance = calculate_variance(image.data, blurred_mean); + const float blurred_mean = calculate_average_color(image.data); + const float blurred_variance = calculate_variance(image.data, blurred_mean); // Check that the variance is lower in the blurred image ASSERT_LT(blurred_variance, original_variance); @@ -123,9 +123,9 @@ TEST(GaussianBlurTest, BasicTestRGBAWithoutAlpha) { #ifndef WITH_COVERAGE TEST(GaussianBlurTest, StressTest) { // Create a large image (e.g., 10240x10240 RGBA) with random data - const int width = 10240; - const int height = 10240; - const int channels = 4; + const size_t width = 10240; + const size_t height = 10240; + const size_t channels = 4; std::vector image_data(width * height * channels); // Fill the image with random data @@ -136,11 +136,11 @@ TEST(GaussianBlurTest, StressTest) { pixel = dis(gen); } - ImgGeom image_geom = {width, height, channels}; + const ImgGeom image_geom = {width, height, channels}; Image image = {image_data, image_geom}; - float sigma = 5.0f; - bool apply_to_alpha = true; + const float sigma = 5.0F; + const bool apply_to_alpha = true; // Measure the time taken to apply Gaussian blur auto start = std::chrono::high_resolution_clock::now(); @@ -177,7 +177,7 @@ TEST(GaussianBlurTest, BasicTestRGBAWithAlpha) { ImgGeom image_geom = {3, 3, 4}; // 3x3 image with 4 channels (RGBA) Image image = {image_data, image_geom}; - float sigma = 3.0f; + float sigma = 3.0F; bool apply_to_alpha = true; // Calculate the average color and variance of the original image @@ -207,14 +207,14 @@ TEST(GaussianBlurTest, BasicTestRGBAWithAlpha) { // Test case for flip_block TEST(HelpersTest, FlipBlock) { // Create a simple 2x2 block - std::vector input = {1.0f, 2.0f, 3.0f, 4.0f}; + std::vector input = {1.0F, 2.0F, 3.0F, 4.0F}; std::vector output(4); // Flip the block flip_block<1>(input.data(), output.data(), 2, 2); // Check the results - std::vector expected_output = {1.0f, 3.0f, 2.0f, 4.0f}; + const std::vector expected_output = {1.0F, 3.0F, 2.0F, 4.0F}; ASSERT_EQ(output, expected_output); } @@ -235,9 +235,9 @@ TEST(HelpersTest, DeinterleaveChannels) { deinterleave_channels<3>(interleaved.data(), channels.data(), 9); // Check the results - std::vector expected_red = {255, 0, 0, 0, 255, 128, 128, 0, 0}; - std::vector expected_green = {0, 255, 0, 0, 255, 128, 0, 128, 0}; - std::vector expected_blue = {0, 0, 255, 0, 255, 128, 0, 0, 128}; + const std::vector expected_red = {255, 0, 0, 0, 255, 128, 128, 0, 0}; + const std::vector expected_green = {0, 255, 0, 0, 255, 128, 0, 128, 0}; + const std::vector expected_blue = {0, 0, 255, 0, 255, 128, 0, 0, 128}; ASSERT_EQ(red, expected_red); ASSERT_EQ(green, expected_green); @@ -259,7 +259,7 @@ TEST(HelpersTest, InterleaveChannels) { interleave_channels<3>(channels.data(), interleaved.data(), 9); // Check the results - std::vector expected_interleaved = {// Row 1 + const std::vector expected_interleaved = {// Row 1 255, 0, 0, 0, 255, 0, 0, 0, 255, // Row 2 0, 0, 0, 255, 255, 255, 128, 128,