Skip to content

Commit

Permalink
Refactor CI and enhance code robustness
Browse files Browse the repository at this point in the history
- Removed OpenCppCoverage and Codecov integration from cmake.yml, indicating a shift in coverage analysis strategy.
- Added automated packaging of CZIrepair-binary for Windows Release builds to streamline distribution.
- Deleted GitHub Pages deployment workflow (pages.yml), suggesting a change in documentation hosting strategy.
- Enhanced C++ code documentation and made codebase improvements in `commandlineoptions.h` and `czirepair.cpp`, including adding documentation comments, making variables `const`, standardizing namespace usage, and introducing more detailed logging.
- Improved error handling in `repairutilities.cpp` for better robustness and added documentation comments to `SubBlockDimensionInfoRepairInfo` struct.
- Overhauled Unicode support in `utilities.cpp` for accurate UTF-8 and wide character string conversions, reflecting these changes in `utilities.h` function signatures.
  • Loading branch information
ptahmose committed Jul 5, 2024
1 parent 17d5ae8 commit 02295f4
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 101 deletions.
25 changes: 0 additions & 25 deletions .github/workflows/cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,31 +69,6 @@ jobs:
# Use debug flag to show all exeucted tests
run: ctest --debug -C ${{matrix.build}}

# # Coverage collection based on https://about.codecov.io/blog/how-to-set-up-codecov-with-c-plus-plus-and-github-actions/
# - name: Prepare Coverage
# if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Debug') }}
# run: |
# choco install OpenCppCoverage -y --no-progress
# echo "C:\Program Files\OpenCppCoverage" >> "$env:GITHUB_PATH"

# - name: Get Coverage
# if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Debug') }}
# working-directory: ${{github.workspace}}/build/Src/libCZI_UnitTests/${{matrix.build}}
# shell: cmd
# run: OpenCppCoverage.exe --export_type cobertura:${{github.workspace}}\coverage.xml --config_file "${{github.workspace}}\opencppcoverage.txt" -- libCZI_UnitTests.exe

# - name: Upload Coverage
# uses: codecov/codecov-action@v4
# if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Debug') }}
# with:
# files: ./coverage.xml
# fail_ci_if_error: true
# verbose: true
# # Only one flag to be safe with
# # https://docs.codecov.com/docs/flags#one-to-one-relationship-of-flags-to-uploads
# flags: ${{matrix.OS}}
# token: ${{ secrets.CODECOV_TOKEN }}

# gather the CZIrepair-binary and put it into a folder - it will be zipped with the upload step
- name: Package
if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Release') }}
Expand Down
46 changes: 0 additions & 46 deletions .github/workflows/pages.yml

This file was deleted.

5 changes: 3 additions & 2 deletions Src/czirepair/commandlineoptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

#include <string>

/// Values that represent the command - the operation this program should perform.
enum class Command
{
Invalid = 0,

DryRun,
DryRun, ///< An enum constant representing the "dry run" option - i.e. just determine what would be done.

Patch,
Patch, ///< An enum constant representing the "patch option" - i.e. actually patch the file if necessary.
};

enum class Verbosity
Expand Down
53 changes: 34 additions & 19 deletions Src/czirepair/czirepair.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ namespace
{
void DryRun(const CommandLineOptions& options)
{
shared_ptr<IStream> stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str());
auto reader = libCZI::CreateCZIReader();
const shared_ptr<IStream> stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str());
const auto reader = libCZI::CreateCZIReader();
reader->Open(stream);

vector<RepairUtilities::SubBlockDimensionInfoRepairInfo> repair_info = RepairUtilities::GetRepairInfo(reader.get());

if (repair_info.empty())
{
cout << "No repair needed." << endl;
cout << "No repair needed." << std::endl;
}
else
{
cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << endl;
cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << std::endl;

if (options.IsVerbosityGreaterOrEqual(Verbosity::Verbose))
{
Expand All @@ -38,7 +38,7 @@ namespace
cout << "SubBlockIndex: " << info.sub_block_index << " -> size in 'dimension_info': " <<
sub_block_info.physicalSize.w << "x" << sub_block_info.physicalSize.h << ", JPGXR: " <<
(info.IsFixedSizeXValid() ? info.fixed_size_x : sub_block_info.physicalSize.w) << "x" <<
(info.IsFixedSizeYValid() ? info.fixed_size_y : sub_block_info.physicalSize.h) << endl;
(info.IsFixedSizeYValid() ? info.fixed_size_y : sub_block_info.physicalSize.h) << std::endl;
}
}
}
Expand All @@ -49,8 +49,8 @@ namespace
vector<RepairUtilities::SubBlockDimensionInfoRepairInfo> repair_info;

{
shared_ptr<IStream> stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str());
auto reader = libCZI::CreateCZIReader();
const shared_ptr<IStream> stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str());
const auto reader = libCZI::CreateCZIReader();
reader->Open(stream);

repair_info = RepairUtilities::GetRepairInfo(reader.get());
Expand All @@ -62,34 +62,33 @@ namespace
}
else
{
cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << endl;
cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << std::endl;

if (options.IsVerbosityGreaterOrEqual(Verbosity::Verbose))
{
for (const auto& info : repair_info)
{
cout << "SubBlockIndex: " << info.sub_block_index << " -> size in 'dimension_info': " <<
info.fixed_size_x << "x" << info.fixed_size_y << endl;
cout << endl;
info.fixed_size_x << "x" << info.fixed_size_y << std::endl;
cout << std::endl;
}
}
}
}

cout << "Now opening the file in read-write-mode and will patch the file." << endl;
cout << "Now opening the file in read-write-mode and will patch the file." << std::endl;
shared_ptr<IInputOutputStream> input_output_stream = libCZI::CreateInputOutputStreamForFile(options.GetCZIFilename().c_str());

RepairUtilities::PatchSubBlockDimensionInfoInSubBlockDirectory(input_output_stream.get(), repair_info);
cout << "Patched the subblock-directory." << endl;
cout << "Patched the subblock-directory." << std::endl;

RepairUtilities::PatchSubBlocks(input_output_stream.get(), repair_info);
cout << "Patched the subblocks." << endl;
cout << "Patched the subblocks." << std::endl;

input_output_stream.reset();
}
}


int main(int argc, char** argv)
{
CommandLineOptions options;
Expand All @@ -107,11 +106,28 @@ int main(int argc, char** argv)
}
else if (commandline_parsing_result == CommandLineOptions::ParseResult::Exit)
{
return 0;
return EXIT_SUCCESS;
}

//std::cout << "Command: " << static_cast<int>(options.GetCommand()) << std::endl;
//std::cout << "CZI filename: " << options.GetCZIFilename() << std::endl;
if (options.IsVerbosityGreaterOrEqual(Verbosity::Verbose))
{
const char* command_text;
switch (options.GetCommand())
{
case Command::DryRun:
command_text = "DryRun";
break;
case Command::Patch:
command_text = "Patch";
break;
default:
command_text = "Invalid";
break;
}

std::cout << "Command: " << command_text << std::endl;
std::cout << "Filename: " << Utilities::convertToUtf8(options.GetCZIFilename()) << std::endl << std::endl;
}

if (options.GetCommand() == Command::DryRun)
{
Expand All @@ -122,6 +138,5 @@ int main(int argc, char** argv)
Patch(options);
}


return 0;
return EXIT_SUCCESS;
}
14 changes: 12 additions & 2 deletions Src/czirepair/repairutilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,25 @@ std::vector<RepairUtilities::SubBlockDimensionInfoRepairInfo> RepairUtilities::G
{
std::vector<SubBlockDimensionInfoRepairInfo> result;

// go through all sub-blocks
reader->EnumerateSubBlocks(
[&](int index, const SubBlockInfo& subblock_info)->bool
{
// we are only interested in JPGXR-compressed sub-blocks
if (subblock_info.GetCompressionMode() == CompressionMode::JpgXr)
{
auto sub_block = reader->ReadSubBlock(index);
// read the sub-block
const auto sub_block = reader->ReadSubBlock(index);

// determine the width and height of the JPGXR-compressed bitmap
uint32_t width_from_jpgxr, height_from_jpgxr;
sub_block->TryGetWidthAndHeightOfJpgxrCompressedBitmap(width_from_jpgxr, height_from_jpgxr);
if (!sub_block->TryGetWidthAndHeightOfJpgxrCompressedBitmap(width_from_jpgxr, height_from_jpgxr))
{
throw runtime_error("Failed to determine the width and height of the JPGXR-compressed bitmap.");
}

// Now, compare the width and height of the JPGXR-compressed bitmap with the width and height in the dimension-info.
// If they differ, we need to fix the dimension-info - add the sub-block to the list of sub-blocks that need to be fixed.
SubBlockDimensionInfoRepairInfo repair_info;
repair_info.sub_block_index = index;
if (subblock_info.physicalSize.w != width_from_jpgxr)
Expand All @@ -31,6 +40,7 @@ std::vector<RepairUtilities::SubBlockDimensionInfoRepairInfo> RepairUtilities::G
repair_info.fixed_size_y = height_from_jpgxr;
}

// If the width or height of the sub-block should be fixed, add the sub-block to the list of sub-blocks that need to be fixed.
if (repair_info.IsFixedSizeXValid() || repair_info.IsFixedSizeYValid())
{
result.push_back(repair_info);
Expand Down
15 changes: 12 additions & 3 deletions Src/czirepair/repairutilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,22 @@
class RepairUtilities
{
public:
/// This structure gather information about fixes that need to be applied to a sub-block's dimension-info.
struct SubBlockDimensionInfoRepairInfo
{
int sub_block_index{ -1 };
std::uint32_t fixed_size_x{ std::numeric_limits<std::uint32_t>::max() };
std::uint32_t fixed_size_y{ std::numeric_limits<std::uint32_t>::max() };
int sub_block_index{ -1 }; ///< This index of the sub block.

std::uint32_t fixed_size_x{ std::numeric_limits<std::uint32_t>::max() }; ///< The width the sub-block should have (in the dimension-info).
std::uint32_t fixed_size_y{ std::numeric_limits<std::uint32_t>::max() }; ///< The height the sub-block should have (in the dimension-info).

/// Query if the width of the sub-block should be fixed (and the property fixed_size_x is valid).
///
/// \returns True if the property fixed_size_x is valid, false if not.
bool IsFixedSizeXValid() const { return fixed_size_x != std::numeric_limits<std::uint32_t>::max(); }

/// Query if the width of the sub-block should be fixed (and the property fixed_size_y is valid).
///
/// \returns True if the property fixed_size_y is valid, false if not.
bool IsFixedSizeYValid() const { return fixed_size_y != std::numeric_limits<std::uint32_t>::max(); }
};

Expand Down
38 changes: 36 additions & 2 deletions Src/czirepair/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,52 @@

using namespace std;

std::string Utilities::convertToUtf8(const std::wstring& str)
std::string Utilities::convertToUtf8(const std::wstring& wide_str)
{
#if defined(WIN32ENV)
int byte_count = WideCharToMultiByte(CP_UTF8, 0, wide_str.c_str(), -1, NULL, 0, NULL, NULL);
if (byte_count == 0)
{
// Handle error (e.g., invalid wstring)
return ""; // Empty string on error
}

// Allocate memory for the UTF-8 string (including null terminator)
std::string utf8_str(byte_count, '\0');

// Perform the actual conversion
WideCharToMultiByte(CP_UTF8, 0, wide_str.c_str(), -1, &utf8_str[0], byte_count, NULL, NULL);

return utf8_str;
#else
std::wstring_convert<std::codecvt_utf8<wchar_t>> utf8_conv;
std::string conv = utf8_conv.to_bytes(str);
return conv;
#endif
}

std::wstring Utilities::convertUtf8ToUCS2(const std::string& str)
std::wstring Utilities::convertUtf8ToUCS2(const std::string& utf8_str)
{
#if defined(WIN32ENV)
int wide_char_count = MultiByteToWideChar(CP_UTF8, 0, utf8_str.c_str(), -1, NULL, 0);
if (wide_char_count == 0)
{
// Handle error (e.g., invalid UTF-8)
return L""; // Empty wstring on error
}

// Allocate memory for the wide character string
std::wstring wide_str(wide_char_count, L'\0');

// Perform the actual conversion
MultiByteToWideChar(CP_UTF8, 0, utf8_str.c_str(), -1, &wide_str[0], wide_char_count);

return wide_str;
#else
std::wstring_convert<std::codecvt_utf8<wchar_t>> utf8conv;
std::wstring conv = utf8conv.from_bytes(str);
return conv;
#endif
}

#if defined(WIN32ENV)
Expand Down
4 changes: 2 additions & 2 deletions Src/czirepair/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
class Utilities
{
public:
static std::string convertToUtf8(const std::wstring& str);
static std::wstring convertUtf8ToUCS2(const std::string& str);
static std::string convertToUtf8(const std::wstring& wide_str);
static std::wstring convertUtf8ToUCS2(const std::string& utf8_str);
};

#if defined(WIN32ENV)
Expand Down

0 comments on commit 02295f4

Please sign in to comment.