Set re-tiling id in sub-block metadata (#6)

* cleanup * create new guids for each z layer * use guid only where necessary * only activate acquisition tiles if command line parameter is set * suggestions from code review * cleanup * cleanup * fix linux build, more suggestions from code review * missing files from previous commit * suggestions from code review * version number, documentation * add changes * cosmetic - remove some commented out code * fix linter-issues --------- Co-authored-by: ptahmose <jbohl@h-quer.de>
ZEISS · Jul 1, 2024 · 55462e6 · 55462e6
1 parent 31b2f65
commit 55462e6
Show file tree

Hide file tree

Showing 12 changed files with 164 additions and 15 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -19,7 +19,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set (CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/modules" ${CMAKE_MODULE_PATH})
 
 project ("warpaffine"
-          VERSION 0.3.2
+          VERSION 0.4.0
           DESCRIPTION "experimental Deskew operation")
 
 option(WARPAFFINE_BUILD_CLANGTIDY "Build with Clang-Tidy" OFF)

diff --git a/documentation/usage.md b/documentation/usage.md
@@ -78,6 +78,10 @@ Options:
                     Override check of source-document whether it is marked as
                     containing 'skewed z-stacks'.
 
+  --use-acquisition-tiles
+                    Adds metadata to identify which sub-blocks were split during
+                    processing, but can be treated as one contiguous area.
+
 
 
 libCZI version: 0.50.0
@@ -129,6 +133,9 @@ IPP version: 2021.8 (r0xba45569b) -  ippIP AVX-512F/CD/BW/DQ/VL (k0)
 * The source document is examined whether it is suitable to be processed by the application. If it is not, the application will exit with an error. One of those checks is
   examining the XML-metadata of the document, in order to determine that the document is tagged as containing a skewed z-stack. This check can be
   overwritten with the flag `--override-check-for-skewed-source`.
+* The option `--use-acquisition-tiles` can be used to add metadata to all sub-blocks which allows to identify which of them belong to the same contiguous 2d area. This information can be 
+  used to differentiate between sub-blocks that need to be stitched (because they were acquired at different times and positions) and those that were only split for technical reasons but should
+  be treated as one.
 
 The exit code of the application is 0 (EXIT_SUCCESS) only if it ran to completion without any errors. In case of an error (of any kind) it will be <>0.  
 In case of circumstances which lead to an abnormal termination, information may be written to `stderr` (and this is not controlled by the `--verbosity` argument); output to `stderr` will
@@ -148,7 +155,7 @@ For builds where IPP is available, the IPP-based warp-enigne will be selected by
 So, a complete commandine for a basic operation could look like this:
 
 ```
-./warpaffine --source source_document.czi --destination destination_document.czi --interpolation linear --operation CoverGlassTransform 
+./warpaffine --source source_document.czi --destination destination_document.czi --interpolation linear --operation CoverGlassTransform --use-acquisition-tiles
 ```
 
 [Here](https://asciinema.org/a/595898) is a screencast of the application in action.

diff --git a/documentation/version-history.md b/documentation/version-history.md
@@ -4,4 +4,6 @@ version history                 {#version_history}
  version            |  PR                                                  | comment
  ------------------ | ---------------------------------------------------- | ---------------------------------------------------
  0.3.0              |  N/A                                                 | initial release
- 0.3.1              |  [3](https://github.com/ZEISS/warpaffine/pull/3)     | bugfix for a crash for "CZIs containing a single brick but have an S-index"
+ 0.3.1              |  [3](https://github.com/ZEISS/warpaffine/pull/3)     | bugfix for a crash for "CZIs containing a single brick but have an S-index"
+ 0.3.2              |  [5](https://github.com/ZEISS/warpaffine/pull/5)     | bugfix for a deadlock in rare case
+ 0.4.0              |  [6](https://github.com/ZEISS/warpaffine/pull/6)     | set re-tiling id of sub-blocks to allow for more sensible stitching of resulting CZI
diff --git a/libwarpaffine/cmdlineoptions.cpp b/libwarpaffine/cmdlineoptions.cpp
@@ -271,6 +271,7 @@ CCmdLineOptions::ParseResult CCmdLineOptions::Parse(int argc, char** argv)
     int max_tile_extent;
     string override_ram_size_parameter;
     bool override_check_for_skewed_source = false;
+    bool use_acquisition_tiles = false;
     app.add_option("-s,--source", source_filename, "The source CZI-file to be processed.")
         ->option_text("SOURCE_FILE")
         ->required();
@@ -342,6 +343,8 @@ CCmdLineOptions::ParseResult CCmdLineOptions::Parse(int argc, char** argv)
         ->check(memory_size_validator);
     app.add_flag("--override-check-for-skewed-source", override_check_for_skewed_source,
         "Override check of source-document whether it is marked as containing 'skewed z-stacks'.");
+    app.add_flag("--use-acquisition-tiles", use_acquisition_tiles,
+        "Adds metadata to identify which subblocks were split during processing, but can be treated as one contiguous area.");
 
     auto formatter = make_shared<CustomFormatter>();
     app.formatter(formatter);
@@ -376,6 +379,7 @@ CCmdLineOptions::ParseResult CCmdLineOptions::Parse(int argc, char** argv)
     this->hash_result_ = hash_result;
     this->max_tile_extent_ = max_tile_extent;
     this->override_check_for_skewed_source_ = override_check_for_skewed_source;
+    this->use_acquisition_tiles_ = use_acquisition_tiles;
     this->source_stream_class_ = argument_source_stream_class;
 
     if (!override_ram_size_parameter.empty())

diff --git a/libwarpaffine/cmdlineoptions.h b/libwarpaffine/cmdlineoptions.h
@@ -39,6 +39,7 @@ class CCmdLineOptions
     std::uint32_t max_tile_extent_{ 2048 };
     std::uint64_t override_main_memory_size_{ 0 };
     bool override_check_for_skewed_source_{ false };
+    bool use_acquisition_tiles_{ false };
     std::string source_stream_class_;
     std::map<int, libCZI::StreamsFactory::Property> property_bag_for_stream_class;
 public:
@@ -86,6 +87,7 @@ class CCmdLineOptions
     [[nodiscard]] bool GetIsMainMemorySizeOverrideValid() const { return this->override_main_memory_size_ != 0; }
     [[nodiscard]] std::uint64_t GetMainMemorySizeOverride() const { return this->override_main_memory_size_; }
     [[nodiscard]] bool GetOverrideCheckForSkewedSourceDocument() const { return this->override_check_for_skewed_source_; }
+    [[nodiscard]] bool GetUseAcquisitionTiles() const { return this->use_acquisition_tiles_; }
 private:
     bool TryParseInputStreamCreationPropertyBag(const std::string& s, std::map<int, libCZI::StreamsFactory::Property>* property_bag);
 };
diff --git a/libwarpaffine/dowarp.cpp b/libwarpaffine/dowarp.cpp
@@ -40,6 +40,7 @@ class MemoryBlockWrapper : public libCZI::IMemoryBlock
     }
 };
 
+
 DoWarp::OutputBrickInfoRepository::OutputBrickInfoRepository(const AppContext& context, const DeskewDocumentInfo& document_info, const Eigen::Matrix4d& transformation_matrix)
 {
     // Here we create a tiling of the output brick - in other words, we ensure that the output-tiles are of a size
@@ -55,12 +56,15 @@ DoWarp::OutputBrickInfoRepository::OutputBrickInfoRepository(const AppContext& c
     //  as the key.
     map<int, int> scene_index_to_m_index;
 
+    uint32_t brick_id = 0;
     for (const auto& item : document_info.map_brickid_position)
-    { 
+    {
         Eigen::Vector3d edge_point;
         Eigen::Vector3d extent;
         DeskewHelpers::CalculateAxisAlignedBoundingBox(item.second.width, item.second.height, document_info.depth, transformation_matrix, edge_point, extent);
         DestinationBrickInfo destination_brick_info;
+
+        destination_brick_info.brick_id = brick_id++;
         destination_brick_info.cuboid.x_position = 0;
         destination_brick_info.cuboid.y_position = 0;
         destination_brick_info.cuboid.z_position = 0;
@@ -75,11 +79,18 @@ DoWarp::OutputBrickInfoRepository::OutputBrickInfoRepository(const AppContext& c
             s_index = item.first.s_index;
         }
 
+        // Keep track if there has been a retiling on any destination brick - we need this in order to decide whether the
+        // retiling-id should be used in the output-slices. We only want to write a retiling-id into the CZI if a retiling took place (on any brick).
+        if (tiling.size() > 1)
+        {
+            this->has_there_been_a_retiling_on_any_destination_brick_ = true;
+        }
+
         for (const auto& tile : tiling)
         {
             TilingRectAndMandSceneIndex tiling_rect_and_mindex{};
             tiling_rect_and_mindex.s_index = s_index.value_or(numeric_limits<int>::min());
-            
+
             // this will nicely either read and increment an existing m-index from the map, or insert a new one with value zero
             tiling_rect_and_mindex.m_index = scene_index_to_m_index[tiling_rect_and_mindex.s_index]++;
             tiling_rect_and_mindex.rectangle = tile;
@@ -323,16 +334,17 @@ void DoWarp::InputBrick(const Brick& brick, const BrickCoordinateInfo& coordinat
                     coordinate_info_captured = coordinate_info,
                     tile_captured = destination_brick_info.tiling[n],
                     source_cuboid_depth = destination_brick_info.cuboid.depth,
-                    destination_brick_captured = destination_brick
+                    destination_brick_captured = destination_brick,
+                    brick_id_captured = destination_brick_info.brick_id
             ]()->void
             {
-                this->ProcessBrickCommon2(brick_captured, destination_brick_captured, coordinate_info_captured, source_cuboid_depth, tile_captured/*tile_captured.rectangle, tile_captured.m_index*/);
+                this->ProcessBrickCommon2(brick_captured, brick_id_captured, destination_brick_captured, coordinate_info_captured, source_cuboid_depth, tile_captured);
                 this->DecWarpTasksInFlight();
             });
     }
 }
 
-void DoWarp::ProcessBrickCommon2(const Brick& brick, const Brick& destination_brick, const BrickCoordinateInfo& coordinate_info, uint32_t source_depth, const OutputBrickInfoRepository::TilingRectAndMandSceneIndex& rect_and_tile_identifier/*const IntRect& roi, int m_index*/)
+void DoWarp::ProcessBrickCommon2(const Brick& brick, uint32_t brick_id, const Brick& destination_brick, const BrickCoordinateInfo& coordinate_info, uint32_t source_depth, const OutputBrickInfoRepository::TilingRectAndMandSceneIndex& rect_and_tile_identifier)
 {
     this->warp_affine_engine_->Execute(
         this->transformation_matrix_,
@@ -380,13 +392,13 @@ void DoWarp::ProcessBrickCommon2(const Brick& brick, const Brick& destination_br
                     xym.scene_index = coordinate_info.scene_index;
                 }
 
-                this->ProcessOutputSlice(slice_to_compress_task_info, coord, xym);
+                this->ProcessOutputSlice(slice_to_compress_task_info, coord, xym, brick_id);
                 this->DecCompressionTasksInFlight();
             });
     }
 }
 
-void DoWarp::ProcessOutputSlice(OutputSliceToCompressTaskInfo* output_slice_task_info, const libCZI::CDimCoordinate& coordinate, const SubblockXYM& xym)
+void DoWarp::ProcessOutputSlice(OutputSliceToCompressTaskInfo* output_slice_task_info, const libCZI::CDimCoordinate& coordinate, const SubblockXYM& xym, uint32_t source_brick_id)
 {
     auto compression_mode_and_memblk = this->Compress(*output_slice_task_info);
 
@@ -406,6 +418,15 @@ void DoWarp::ProcessOutputSlice(OutputSliceToCompressTaskInfo* output_slice_task
     add_slice_info.scene_index = xym.scene_index;
     add_slice_info.x_position = xym.x_position;
     add_slice_info.y_position = xym.y_position;
+
+    // Only specify a brick_id if there has been a retiling on any destination brick. If it is not necessary,
+    // then adding a retiling-id with the output-document is strictly superfluous - so we better don't want to put
+    // it in.
+    if (this->output_brick_info_repository_.HasThereBeenRetilingOnAnyDestinationBrick())
+    {
+        add_slice_info.brick_id = source_brick_id;
+    }
+
     this->writer_->AddSlice(add_slice_info);
     ++this->number_of_subblocks_added_to_writer_;
 
@@ -500,6 +521,7 @@ Brick DoWarp::CreateBrickAndWaitUntilAvailable(libCZI::PixelType pixel_type, std
     brick.info.depth = depth;
     brick.info.stride_line = width * Utils::GetBytesPerPixel(pixel_type);
     brick.info.stride_plane = brick.info.stride_line * brick.info.height;
+
     const uint64_t size_of_brick = brick.info.stride_plane * static_cast<uint64_t>(brick.info.depth);
     for (;;)
     {

diff --git a/libwarpaffine/dowarp.h b/libwarpaffine/dowarp.h
@@ -68,12 +68,15 @@ class DoWarp
         /// index.
         struct DestinationBrickInfo
         {
-            IntCuboid   cuboid;
+            std::uint32_t brick_id; ///< This id uniquely identifies the destination brick, and since there is a one-to-one correspondence between source and destination bricks, 
+                                    ///< it also uniquely identifies the source brick.
+            IntCuboid cuboid;
             std::vector<TilingRectAndMandSceneIndex> tiling;
         };
     private:
         std::map<BrickInPlaneIdentifier, DestinationBrickInfo> map_brickid_destinationbrickinfo_;
         std::uint32_t number_of_subblocks_to_output_{ 0 };
+        bool has_there_been_a_retiling_on_any_destination_brick_{ false };
     public:
         OutputBrickInfoRepository(const AppContext& context, const DeskewDocumentInfo& document_info, const Eigen::Matrix4d& transformation_matrix);
 
@@ -90,6 +93,12 @@ class DoWarp
         ///
         /// \returns The total number of subblocks to output.
         [[nodiscard]] std::uint32_t GetTotalNumberOfSubblocksToOutput() const;
+
+        /// Query whether there has been a retiling on any destination brick. This is intended to be used in order to
+        /// decide whether it is necessary to have a retiling-id in the output document.
+        ///
+        /// \returns    True if there been retiling on any destination brick, false if not.
+        [[nodiscard]] bool HasThereBeenRetilingOnAnyDestinationBrick() const { return this->has_there_been_a_retiling_on_any_destination_brick_; }
     private:
         static std::vector<libCZI::IntRect> Create2dTiling(std::uint32_t max_extent, const libCZI::IntRect& rectangle);
     };
@@ -161,7 +170,6 @@ class DoWarp
     bool TryGetHash(std::array<uint8_t, 16>* hash_code) const;
 private:
     void InputBrick(const Brick& brick, const BrickCoordinateInfo& coordinate_info);
-private:
     std::vector<ITaskArena::SuspendHandle> resume_handles_;
     std::mutex mutex_resume_handles_;
 
@@ -171,7 +179,7 @@ class DoWarp
         int z_slice;
     };
 
-    void ProcessOutputSlice(OutputSliceToCompressTaskInfo* output_slice_task_info, const libCZI::CDimCoordinate& coordinate, const SubblockXYM& xym);
+    void ProcessOutputSlice(OutputSliceToCompressTaskInfo* output_slice_task_info, const libCZI::CDimCoordinate& coordinate, const SubblockXYM& xym, std::uint32_t source_brick_id);
 
     void IncWarpTasksInFlight();
     void DecWarpTasksInFlight();
@@ -185,7 +193,7 @@ class DoWarp
     Brick CreateBrick(libCZI::PixelType pixel_type, std::uint32_t width, std::uint32_t height, std::uint32_t depth);
     Brick CreateBrickAndWaitUntilAvailable(libCZI::PixelType pixel_type, std::uint32_t width, std::uint32_t height, std::uint32_t depth);
 
-    void ProcessBrickCommon2(const Brick& brick, const Brick& destination_brick, const BrickCoordinateInfo& coordinate_info, std::uint32_t source_depth, const OutputBrickInfoRepository::TilingRectAndMandSceneIndex& rect_and_tile_identifier /*const libCZI::IntRect& roi,  int m_index*/);
+    void ProcessBrickCommon2(const Brick& brick, std::uint32_t brick_id, const Brick& destination_brick, const BrickCoordinateInfo& coordinate_info, std::uint32_t source_depth, const OutputBrickInfoRepository::TilingRectAndMandSceneIndex& rect_and_tile_identifier);
 
     std::tuple<libCZI::CompressionMode, std::shared_ptr<libCZI::IMemoryBlock>> Compress(const OutputSliceToCompressTaskInfo& output_slice_task_info);
 

diff --git a/libwarpaffine/sliceswriter/ISlicesWriter.h b/libwarpaffine/sliceswriter/ISlicesWriter.h
@@ -49,6 +49,10 @@ class ICziSlicesWriter
 
         /// The Y-position of the subblock.
         int y_position{ 0 };
+
+        /// The brick-id this slice belongs to. If present AND the writer supports it, this id
+        /// together with the z-index will be used to construct a retiling-id for the subblock.
+        std::optional<std::uint32_t> brick_id;
     };
 
     /// Gets number of currently pending slice write operations.

diff --git a/libwarpaffine/sliceswriter/SlicesWriterTbb.cpp b/libwarpaffine/sliceswriter/SlicesWriterTbb.cpp
@@ -3,11 +3,13 @@
 // SPDX-License-Identifier: MIT
 
 #include "SlicesWriterTbb.h"
+#include <iomanip>
 #include <array>
 #include <memory>
 #include <string>
 #include <tuple>
 #include <utility>
+#include <iostream>
 
 using namespace std;
 using namespace libCZI;
@@ -16,6 +18,8 @@ CziSlicesWriterTbb::CziSlicesWriterTbb(AppContext& context, const std::wstring&
     : context_(context)
 {
     this->queue_.set_capacity(500 * 10);
+    this->use_acquisition_tiles_ = context.GetCommandLineOptions().GetUseAcquisitionTiles();
+    this->retilingBaseId_ = Utilities::GenerateGuid();
 
     // Create an "output-stream-object"
     const auto output_stream = libCZI::CreateOutputStreamForFile(filename.c_str(), true);
@@ -75,7 +79,40 @@ void CziSlicesWriterTbb::WriteWorker()
 
             add_subblock_info.ptrData = sub_block_write_info.add_slice_info.subblock_raw_data->GetPtr();
             add_subblock_info.dataSize = sub_block_write_info.add_slice_info.subblock_raw_data->GetSizeOfData();
-            this->writer_->SyncAddSubBlock(add_subblock_info);
+
+            if (sub_block_write_info.add_slice_info.brick_id.has_value() && this->use_acquisition_tiles_)
+            {
+                int z;
+                sub_block_write_info.add_slice_info.coordinate.TryGetPosition(libCZI::DimensionIndex::Z, &z);
+                auto guid = this->CreateRetilingIdWithZandSlice(z, sub_block_write_info.add_slice_info.brick_id.value());
+
+                std::ostringstream oss;
+                oss << "<METADATA><Tags><RetilingId>"
+                    << std::hex << std::uppercase
+                    << std::setw(8) << std::setfill('0') << guid.Data1 << '-'
+                    << std::setw(4) << std::setfill('0') << guid.Data2 << '-'
+                    << std::setw(4) << std::setfill('0') << guid.Data3 << '-'
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[0])
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[1]) << '-'
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[2])
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[3])
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[4])
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[5])
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[6])
+                    << std::setw(2) << static_cast<uint32_t>(guid.Data4[7])
+                    << std::dec
+                    <<"</RetilingId></Tags></METADATA>";
+
+                const string metadata_xml = oss.str();
+                add_subblock_info.ptrSbBlkMetadata = metadata_xml.c_str();
+                add_subblock_info.sbBlkMetadataSize = static_cast<uint32_t>(metadata_xml.size());
+
+                this->writer_->SyncAddSubBlock(add_subblock_info);
+            }
+            else
+            {
+                this->writer_->SyncAddSubBlock(add_subblock_info);
+            }
 
             --this->number_of_slicewrite_operations_in_flight_;
         }
@@ -106,6 +143,22 @@ void CziSlicesWriterTbb::WriteWorker()
     }
 }
 
+libCZI::GUID CziSlicesWriterTbb::CreateRetilingIdWithZandSlice(int z, uint32_t slice) const
+{
+    libCZI::GUID guid = this-> retilingBaseId_;
+    guid.Data4[0] = static_cast<uint8_t>(z >> 24);
+    guid.Data4[1] = static_cast<uint8_t>(z >> 16);
+    guid.Data4[2] = static_cast<uint8_t>(z >> 8);
+    guid.Data4[3] = static_cast<uint8_t>(z);
+
+    guid.Data4[4] = static_cast<uint8_t>(slice >> 24);
+    guid.Data4[5] = static_cast<uint8_t>(slice >> 16);
+    guid.Data4[6] = static_cast<uint8_t>(slice >> 8);
+    guid.Data4[7] = static_cast<uint8_t>(slice);
+
+    return guid;
+}
+
 void CziSlicesWriterTbb::Close(const std::shared_ptr<libCZI::ICziMetadata>& source_metadata,
                                 const libCZI::ScalingInfo* new_scaling_info,
                                 const std::function<void(libCZI::IXmlNodeRw*)>& tweak_metadata_hook)

diff --git a/libwarpaffine/sliceswriter/SlicesWriterTbb.h b/libwarpaffine/sliceswriter/SlicesWriterTbb.h
@@ -33,6 +33,8 @@ class CziSlicesWriterTbb : public ICziSlicesWriter
     };
 
     tbb::concurrent_bounded_queue<SubBlockWriteInfo2> queue_;
+    libCZI::GUID retilingBaseId_;
+    bool use_acquisition_tiles_;
 public:
     CziSlicesWriterTbb(AppContext& context, const std::wstring& filename);
 
@@ -47,4 +49,5 @@ class CziSlicesWriterTbb : public ICziSlicesWriter
 private:
     void WriteWorker();
     void CopyMetadata(libCZI::IXmlNodeRead* rootSource, libCZI::IXmlNodeRw* rootDestination);
+    libCZI::GUID CreateRetilingIdWithZandSlice(int z, std::uint32_t slice) const;
 };