Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve documentation - "coordinate systems" #122

Merged
merged 9 commits into from
Dec 5, 2024

Conversation

ptahmose
Copy link
Contributor

@ptahmose ptahmose commented Dec 4, 2024

Description

This is adding documentation and clarification about "usage of coordinates in libCZI", defining coordinate systems.

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?

  • No functional change, only documentation added/changed
  • ran doxygen locally and checked result

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.

ptahmose and others added 3 commits December 4, 2024 01:57
Added a new documentation file `coordinate_systems.markdown` to the
Doxyfile's INPUT section. Introduced a "Coordinate Systems" section
in the new file with a header and placeholder description for CZI and
libCZI coordinate systems. Also, added a new diagram in
`coordinate_systems.drawio` named "Page-1" that includes graphical
elements representing a coordinate system with labeled axes.
Rephrased a sentence in `coordinate_systems.markdown` for clarity. Updated comments in `libCZI.h` to clarify bounding boxes and coordinate systems. Enhanced `libCZI_Compositor.h` comments to specify ROI and positions in the 'raw-subblock-coordinate-system'. Removed summary tags and redundant information to streamline documentation. These changes provide clearer and more precise descriptions of coordinate systems and bounding boxes in libCZI.
@ptahmose ptahmose added the cla Contributor License Agreement sent to Admin label Dec 4, 2024
ptahmose and others added 4 commits December 4, 2024 15:20
Updated CMakeLists.txt to change project version from 0.62.6 to 0.62.7.
Added a new entry in version-history.markdown for version 0.62.7, documenting a "documentation update" linked to pull request ZEISS#122.
@ptahmose ptahmose requested a review from a team December 4, 2024 19:56
m-aXimilian
m-aXimilian previously approved these changes Dec 5, 2024
Copy link
Contributor

@m-aXimilian m-aXimilian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Just added some points that might reduce confusion 😄

Src/libCZI/Doc/coordinate_systems.markdown Outdated Show resolved Hide resolved
Src/libCZI/Doc/coordinate_systems.markdown Outdated Show resolved Hide resolved
Src/libCZI/Doc/coordinate_systems.markdown Show resolved Hide resolved
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.49%. Comparing base (d615ba9) to head (6b9013c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #122   +/-   ##
=======================================
  Coverage   65.49%   65.49%           
=======================================
  Files          86       86           
  Lines       10906    10906           
=======================================
  Hits         7143     7143           
  Misses       3763     3763           
Flag Coverage Δ
windows-latest 65.49% <ø> (ø)

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.

@ptahmose ptahmose merged commit ae43e1b into ZEISS:main Dec 5, 2024
15 checks passed
ptahmose added a commit that referenced this pull request Dec 13, 2024
* Add coordinate systems documentation and diagram

Added a new documentation file `coordinate_systems.markdown` to the
Doxyfile's INPUT section. Introduced a "Coordinate Systems" section
in the new file with a header and placeholder description for CZI and
libCZI coordinate systems. Also, added a new diagram in
`coordinate_systems.drawio` named "Page-1" that includes graphical
elements representing a coordinate system with labeled axes.

* Update coordinate systems documentation in libCZI

* Improve documentation clarity for coordinate systems

Rephrased a sentence in `coordinate_systems.markdown` for clarity. Updated comments in `libCZI.h` to clarify bounding boxes and coordinate systems. Enhanced `libCZI_Compositor.h` comments to specify ROI and positions in the 'raw-subblock-coordinate-system'. Removed summary tags and redundant information to streamline documentation. These changes provide clearer and more precise descriptions of coordinate systems and bounding boxes in libCZI.

* Update project to version 0.62.7

Updated CMakeLists.txt to change project version from 0.62.6 to 0.62.7.
Added a new entry in version-history.markdown for version 0.62.7, documenting a "documentation update" linked to pull request #122.

* remove "high-res-bitmaps"

* wip

* fix table

* cosmetic

* Refactor coordinate transformation methods

Added TransformPoint to CCZIReader for point transformations.
Updated CZIReader.h to declare TransformPoint method.
Modified CSingleChannelTileAccessor::Get to use TransformRectangle.
Added TransformRectangle to ISubBlockRepository in libCZI.h.
Introduced IntPoint and IntPointAndFrameOfReference structures.
Removed ConvertToFrameOfReference function from libCZI_Utilities.
Minor formatting changes in libCZI.h and libCZI_Utilities.h.
Improved code design, readability, and maintainability.

* Update TransformPoint to return IntPointAndFrameOfReference

Updated TransformPoint method to return libCZI::IntPointAndFrameOfReference instead of libCZI::IntPoint. This change is reflected across various classes and interfaces, including CCZIReader, CziReaderWriter, and libCZI::ISubBlockRepository.

Added a new member variable default_frame_of_reference to CCZIReader and libCZI::Options to handle cases where the frame of reference is set to Default. Updated TransformRectangle method to handle the new return type of TransformPoint. Implemented changes in test_TileAccessorCoverageOptimization.cpp to reflect the new return type.

* Add frame of reference handling and related tests

- Initialize `default_frame_of_reference` to `CZIFrameOfReference::Invalid` in `CCZIReader` constructor.
- Update `CCZIReader::Open` to use a switch statement for `options->default_frame_of_reference`.
- Modify `ISubBlockRepository::TransformRectangle` to return `IntRectAndFrameOfReference`.
- Update `CSingleChannelTileAccessor::Get` to use the `rectangle` member of `IntRectAndFrameOfReference`.
- Add `test_frame_of_reference_transform.cpp` with tests for point and rectangle transformations.
- Include new test file in `libCZI_UnitTests` target in `CMakeLists.txt`.

* linter

* cosmetic

* Refactor TransformPoint and add new tests

Refactored `CCZIReader::TransformPoint` for robust frame-of-reference transformations. Updated `AccessorType` enum in `libCZI_Compositor.h` to use `std::uint8_t`. Corrected typo in `backGroundColor` comment. Added `Check2x2Gray8Bitmap` function and new tests in `test_frame_of_reference_transform.cpp` for point transformations and tile composite verification.

* Add TransformPoint method and default frame of reference

Implemented TransformPoint in CCziReaderWriter for frame-of-reference transformations. Added GetDefaultFrameOfReference method and updated ICziReaderWriterInfo interface. Fixed formatting issues and improved type safety. Added test case for TransformPoint. Updated documentation and constructor initialization. Handled unsupported transformations with exceptions.

* Update bounding-box and transformation expectations

Updated the comment to reflect the correct bounding-box (0, -1, 1, 1) and transformation result (0, -1) after removing subblocks. Adjusted the test assertion for the y-coordinate of the transformed point to expect -1 instead of 0.

* Refactor and cleanup in SingleChannelTileAccessor

Modified the constructor in SingleChannelTileAccessor.cpp to make
roi_raw_sub_block_cs a const IntRect, ensuring immutability.
Removed a commented-out Get method declaration in
SingleChannelTileAccessor.h for improved readability.

* Refactor to use IntPointAndFrameOfReference type

Updated method signatures in SingleChannelTileAccessor.cpp and .h to use IntPointAndFrameOfReference instead of separate xPos and yPos parameters. Adjusted libCZI_Compositor.h to reflect these changes and added inline methods for backward compatibility. Updated ISingleChannelTileAccessor destructor to use override keyword for modern C++ standards adherence.

* linter

* Improve comment formatting in Z-order description

Added a space after the period in the comment describing the Z-order behavior when `sortByM` is false. This change enhances readability and consistency of the comments in the codebase.

* Refactor Get methods and update documentation

- Updated `Get` methods in `CSingleChannelScalingTileAccessor` and `libCZI::ISingleChannelScalingTileAccessor` to use `IntRectAndFrameOfReference` instead of `IntRect`.
- Added overloads to handle `IntRect` by converting to `IntRectAndFrameOfReference`.
- Modified `InternalCalcSize` to use the transformed rectangle in the raw sub-block coordinate system.
- Corrected typo in `Options` struct documentation ("back ground color" to "background color").
- Updated `SingleChannelScalingTileAccessorHandler` in `test_TileAccessorCoverageOptimization.cpp` to use a shared pointer to `ISingleChannelScalingTileAccessor`.
- Added documentation to `libCZI::ISingleChannelScalingTileAccessor` interface for new `Get` methods and their parameters.

* Refactor coordinate handling to use explicit frames

Updated SingleChannelPyramidLevelTileAccessor to use IntRectAndFrameOfReference and IntPointAndFrameOfReference for coordinates, ensuring explicit frame of reference handling. Transformed input coordinates to RawSubBlockCoordinateSystem in Get methods for consistency. Updated InternalGet method accordingly. Modified libCZI_Compositor.h to reflect new method signatures and retained old methods with internal transformations. Improved clarity and correctness of coordinate handling.

* Refactor ROI coordinate calculation and formatting

Updated ROI calculation to use boundingBoxLayer0Only instead of boundingBox for relative coordinates in execute.cpp and executeBase.cpp. Applied consistent changes across multiple sections. Added spaces after commas in ROI initialization for better readability.

* Update project to version 0.63.0

Updated CMakeLists.txt to change project version from 0.62.7 to 0.63.0.
Added a new entry in version-history.markdown for version 0.63.0, documenting the introduction of "frames-of-reference". The entry for version 0.62.7 remains unchanged.

* add documentation

* fix typos

* Update Src/CZICmd/execute.cpp

Co-authored-by: m-aXimilian <[email protected]>

* Update Src/libCZI/libCZI_ReadWrite.h

Co-authored-by: m-aXimilian <[email protected]>

* Simplify return statements with brace-enclosed initializer

Simplified the return statements in two conditional blocks by removing the explicit type `libCZI::IntPointAndFrameOfReference` and using the brace-enclosed initializer list directly. This change makes the code more concise and leverages the compiler's ability to deduce the return type.

---------

Co-authored-by: m-aXimilian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla Contributor License Agreement sent to Admin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants