fix: add default componentbitcount when writing czi#159
fix: add default componentbitcount when writing czi#159PintoCraig wants to merge 4 commits intoZEISS:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #159 +/- ##
==========================================
- Coverage 66.38% 66.34% -0.04%
==========================================
Files 96 96
Lines 12365 12395 +30
==========================================
+ Hits 8209 8224 +15
- Misses 4156 4171 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where the ComponentBitCount XML element was missing from CZI metadata when writing CZI files. The fix adds a utility function to automatically infer and populate this field based on the pixel type.
Key changes:
- Added
TryGetComponentBitCountFromPixelType()helper method that maps pixel types to their component bit counts - Updated metadata generation to automatically include
ComponentBitCountfor each channel - Incremented library version from 0.67.3 to 0.67.4 (patch version)
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Src/libCZI/CziMetadataBuilder.h | Added declaration for new helper method TryGetComponentBitCountFromPixelType |
| Src/libCZI/CziMetadataBuilder.cpp | Implemented component bit count inference logic and integrated it into metadata generation |
| Src/libCZI_UnitTests/test_writer.cpp | Added comprehensive unit tests verifying ComponentBitCount is correctly written for Gray8, Gray16, and Bgr24 pixel types |
| CMakeLists.txt | Bumped patch version from 0.67.3 to 0.67.4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| componentBitCount = 32; // Complex float uses 32 bits per real/imaginary component | ||
| return true; | ||
| case PixelType::Bgr192ComplexFloat: | ||
| componentBitCount = 32; // Complex float uses 32 bits per component |
There was a problem hiding this comment.
The comment states 'real/imaginary component' but ComponentBitCount typically represents bits per channel component, not per real/imaginary part of a complex number. For Gray64ComplexFloat (64 bits total for one complex value), this should clarify whether it's 32 bits for the entire component or 64 bits with 32 per real/imaginary part. The comment should be more precise about what 'component' means in this context.
| componentBitCount = 32; // Complex float uses 32 bits per real/imaginary component | |
| return true; | |
| case PixelType::Bgr192ComplexFloat: | |
| componentBitCount = 32; // Complex float uses 32 bits per component | |
| componentBitCount = 32; // For complex float, this is 32 bits per channel component (i.e., per real or imaginary part) | |
| return true; | |
| case PixelType::Bgr192ComplexFloat: | |
| componentBitCount = 32; // For complex float, this is 32 bits per channel component (i.e., per real or imaginary part) |
| componentBitCount = 32; // Complex float uses 32 bits per real/imaginary component | ||
| return true; | ||
| case PixelType::Bgr192ComplexFloat: | ||
| componentBitCount = 32; // Complex float uses 32 bits per component |
There was a problem hiding this comment.
For Bgr192ComplexFloat (192 bits total = 3 channels × 64 bits per complex channel), returning 32 as ComponentBitCount is ambiguous. The comment should clarify whether this represents bits per real/imaginary part (32), bits per complex channel (64), or bits per RGB component. This needs to be consistent with the CZI specification's definition of ComponentBitCount for complex pixel types.
| componentBitCount = 32; // Complex float uses 32 bits per component | |
| componentBitCount = 32; // For Bgr192ComplexFloat: 32 bits per real/imaginary part of each complex channel (each RGB channel is a complex float: 64 bits = 32 bits real + 32 bits imaginary), per CZI specification. |
Description
add default componentbitcount when writing czi
Fixes #156
Type of change
How Has This Been Tested?
Added unit tests
Checklist: