improve behavior in case of contradictory data

[ptahmose]

Description

This PR aims to define "expected, correct and canonical behavior" for cases where contradictory information is found in a CZI-document. The goal is to have defined and consistent behavior even in cases where a malformed CZI is processed.
According to this definition, libCZI is modified to conform to this specification.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

locally

Checklist:

• I followed the Contributing Guidelines.
• I did a self-review.
• I commented my code, particularly in hard-to-understand areas.
• I updated the documentation.
• I updated the version of libCZI following Versioning of libCZI depending on the type of change
  • Bug fix -> PATCH
  • New feature -> MINOR
  • Breaking change -> MAJOR
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.

[Copilot]

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

Src/libCZI/CZIReader.cpp:318

• Consider using the overloaded operator& for SubBlockDirectoryInfoPolicy instead of casting to uint8_t. For example, rewrite the condition as 'if (!(this->sub_block_directory_info_policy_ & OpenOptions::SubBlockDirectoryInfoPolicy::IgnoreDiscrepancy))' for improved readability and consistency.

    if (!((uint8_t)this->sub_block_directory_info_policy_ & (uint8_t)OpenOptions::SubBlockDirectoryInfoPolicy::IgnoreDiscrepancy))

[codecov]

Codecov Report

Attention: Patch coverage is 71.69811% with 90 lines in your changes missing coverage. Please review.

Project coverage is 66.57%. Comparing base (11015ae) to head (a753e1f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/libCZI/decoder_zstd.cpp	61.49%	72 Missing ⚠️
Src/libCZI/decoder_wic.cpp	0.00%	11 Missing ⚠️
Src/libCZI/CreateBitmap.cpp	93.84%	4 Missing ⚠️
Src/libCZI/decoder.cpp	66.66%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #130      +/-   ##
==========================================
+ Coverage   65.88%   66.57%   +0.68%     
==========================================
  Files          86       89       +3     
  Lines       11004    11227     +223     
==========================================
+ Hits         7250     7474     +224     
+ Misses       3754     3753       -1     

Flag	Coverage Δ
windows-latest	66.57% <71.69%> (+0.79%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

Src/libCZI/CZIReader.cpp:334

• [nitpick] The use of the magic constant '1' for checking the flag value reduces clarity; consider replacing it with a well-named constant or an enum method to improve readability.

if (((uint8_t)this->sub_block_directory_info_policy_ & 1) == (uint8_t)OpenOptions::SubBlockDirectoryInfoPolicy::SubBlockDirectoryPrecedence)

[Copilot]

Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

Src/libCZI/decoder_zstd.cpp:75

• [nitpick] The function 'DecodeAndHiLoBytePackRequireCorrectSize' uses 'BytePack' in its name, while a related function uses 'ByteUnpack'. Consider renaming it for consistency if the operation performed is actually unpacking.

shared_ptr<libCZI::IBitmapData> DecodeAndHiLoBytePackRequireCorrectSize(const void* ptrData, size_t size, libCZI::PixelType pixelType, uint32_t width, uint32_t height)

Src/libCZI/CZIReader.cpp:317

• The strict mismatch check between sub-block directory and header information may be too rigid. Verify that this condition fully reflects the intended resolution protocol and consider adding more granular logging to aid debugging in case of expected discrepancies.

if (entry.PixelType != subBlkData.pixelType || entry.Compression != subBlkData.compression || ... )

Src/libCZI/CreateBitmap.cpp:130

• Ensure that the logic for handling insufficient uncompressed data correctly calculates the remaining size and zero-fills incomplete rows. A detailed review of the loop logic and the 'remaining_size' updates is recommended to avoid potential data corruption.

if (expected_size <= size) { ... } else { /* zero-fill logic */ }

[Copilot]

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

Src/libCZI/CZIReader.cpp:317

• [nitpick] Consider refactoring the bitwise flag comparison for clarity, such as by introducing a helper function or a boolean variable to improve readability.

if (static_cast<std::underlying_type<OpenOptions::SubBlockDirectoryInfoPolicy>::type>(this->sub_block_directory_info_policy_ & OpenOptions::SubBlockDirectoryInfoPolicy::IgnoreDiscrepancy) == 0)

[Copilot]

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

• CMakeLists.txt: Language not supported
• Src/Doxyfile: Language not supported

Comments suppressed due to low confidence (2)

Src/libCZI/CreateBitmap.cpp:140

• [nitpick] Consider renaming 'remaining_size' to 'data_remaining' to more clearly indicate its purpose.

size_t remaining_size = size;

Src/libCZI/CZIReader.cpp:317

• [nitpick] Consider extracting the bitmask check into a named variable or helper function to improve readability.

if (static_cast<std::underlying_type<OpenOptions::SubBlockDirectoryInfoPolicy>::type>(this->sub_block_directory_info_policy_ & OpenOptions::SubBlockDirectoryInfoPolicy::IgnoreDiscrepancy) == 0)

[DaveyJonesBitPail]

Approved w/ recommendations/questions