Basic Topography Metadata validations (#10)

* add a topographyappliance checker skeleton * check for appliance section * apply comments * guard GetCziMetadataAndReportErrors retreival * extract dimensions from topoitems * simple superfluous checker * make megalinter happy * checker for c index * exctract const strings & remove useless checker * don't creep topo warnings in every images * apply review comments * ref shared_ptr * add tests * add missing testdata to cmake fetch * test output adaptions * using non-local file to generate test output * add docu * consequently use config ref * The most significant changes were made to correct spelling mistakes in class and variable names, and to improve code organization and readability. The class name `CCheckTopgraphyApplianceMetadata` was corrected to `CCheckTopographyApplianceMetadata` in `checkerfactory.cpp` and `checkerTopographyApplianceValidation.cpp` files. The `DimensionView` struct was moved from the global scope to the private scope of the `CCheckTopographyApplianceMetadata` class in `checkerTopographyApplianceValidation.h` for better encapsulation. The variable names `texture_views` and `heightmap_views` were updated to `texture_views_` and `heightmap_views_` to follow the naming convention for private member variables. Here is the list of changes: 1. The class name `CCheckTopgraphyApplianceMetadata` was corrected to `CCheckTopographyApplianceMetadata` in `checkerfactory.cpp` and `checkerTopographyApplianceValidation.cpp` files (spelling correction). 2. The `#include <codecvt>` directive was replaced with `#include <functional>`, `#include <string>`, and `#include <vector>` in `checkerTopographyApplianceValidation.cpp` (library inclusion). 3. The variable names `texture_views` and `heightmap_views` were changed to `texture_views_` and `heightmap_views_` in `checkerTopographyApplianceValidation.cpp` (naming convention). 4. The character `'0'` was replaced with `'\0'` in the `IsValid` method of the `DimensionView` struct in `checkerTopographyApplianceValidation.cpp` (correct representation of null character). 5. The `DimensionView` struct was moved from the global scope to the private scope of the `CCheckTopographyApplianceMetadata` class in `checkerTopographyApplianceValidation.h` (code organization). 6. The comment for the `CCheckTopographyApplianceMetadata` class was updated in `checkerTopographyApplianceValidation.h` (documentation update). 7. The `kHeighMapItemKey` constant was corrected to `kHeightMapItemKey` in `checkerTopographyApplianceValidation.h` (spelling correction). 8. The lambda function `enumChildrenLabmda` was renamed to `enumChildrenLambda` in `checkerTopographyApplianceValidation.cpp` (spelling correction). * update version number and history * whitespace --------- Co-authored-by: ptahmose <jbohl@h-quer.de> Co-authored-by: Maximilian Küffner <keuffnermax@gmail.com>
ZEISS · May 9, 2024 · e9a12ef · e9a12ef
1 parent b00354b
commit e9a12ef
Show file tree

Hide file tree

Showing 24 changed files with 410 additions and 4 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -5,7 +5,7 @@
 cmake_minimum_required(VERSION 3.15)
 
 project(CZICheck 
-      VERSION 0.1.2
+      VERSION 0.2.0
       HOMEPAGE_URL "https://github.com/ZEISS/czicheck"
       DESCRIPTION "CZICheck is a validator for CZI-documents")
 

diff --git a/CZICheck/CMakeLists.txt b/CZICheck/CMakeLists.txt
@@ -70,6 +70,8 @@ set(CZICHECKSRCFILES
 "checkers/checkerOverlappingScenes.cpp"
 "checkers/checkerSubBlkBitmapValid.h"
 "checkers/checkerSubBlkBitmapValid.cpp"
+"checkers/checkerTopographyApplianceValidation.h"
+"checkers/checkerTopographyApplianceValidation.cpp"
 checkerfactory.cpp
 checkerfactory.h
 checks.h
@@ -162,6 +164,8 @@ if (CZICHECK_BUILD_TESTS)
           -r overlapping_scenes.czi=DATA{${CMAKE_CURRENT_SOURCE_DIR}/../Test/CZICheckSamples/overlapping_scenes.czi}
           -r jpgxrcompressed_inconsistent_size.czi=DATA{${CMAKE_CURRENT_SOURCE_DIR}/../Test/CZICheckSamples/jpgxrcompressed_inconsistent_size.czi}
           -r jpgxrcompressed_inconsistent_pixeltype.czi=DATA{${CMAKE_CURRENT_SOURCE_DIR}/../Test/CZICheckSamples/jpgxrcompressed_inconsistent_pixeltype.czi}
+          -r edf-missing-texture.czi=DATA{${CMAKE_CURRENT_SOURCE_DIR}/../Test/CZICheckSamples/edf-missing-texture.czi}
+          -r edf-superfluous.czi=DATA{${CMAKE_CURRENT_SOURCE_DIR}/../Test/CZICheckSamples/edf-superfluous.czi}
     )
 
     # Add a build target to populate the real data.

diff --git a/CZICheck/checkerfactory.cpp b/CZICheck/checkerfactory.cpp
@@ -20,6 +20,7 @@
 #include "checkers/checkerXmlBasicMetadataValidation.h"
 #include "checkers/checkerOverlappingScenes.h"
 #include "checkers/checkerSubBlkBitmapValid.h"
+#include "checkers/checkerTopographyApplianceValidation.h"
 
 using namespace std;
 
@@ -94,6 +95,7 @@ static const classEntry classesList[] =
     MakeEntry<CCheckConsecutivePlaneIndices>(),
     MakeEntry<CCheckMissingMindex>(),
     MakeEntry<CCheckBasicMetadataValidation>(),
+    MakeEntry<CCheckTopographyApplianceMetadata>(),
 #if CZICHECK_XERCESC_AVAILABLE
     MakeEntry<CCheckXmlMetadataXsdValidation>(true),
 #endif

diff --git a/CZICheck/checkers/checkerTopographyApplianceValidation.cpp b/CZICheck/checkers/checkerTopographyApplianceValidation.cpp
@@ -0,0 +1,261 @@
+// SPDX-FileCopyrightText: 2024 Carl Zeiss Microscopy GmbH
+//
+// SPDX-License-Identifier: MIT
+
+#include "checkerTopographyApplianceValidation.h"
+#include <memory>
+#include <unordered_map>
+#include <utility>
+#include <functional>
+#include <string>
+#include <vector>
+
+using namespace std;
+using namespace libCZI;
+
+/*static*/const char* CCheckTopographyApplianceMetadata::kDisplayName = "Basic semantic checks for TopographyDataItems";
+/*static*/const char* CCheckTopographyApplianceMetadata::kShortName = "topographymetadata";
+
+CCheckTopographyApplianceMetadata::CCheckTopographyApplianceMetadata(
+    const std::shared_ptr<libCZI::ICZIReader>& reader,
+    CResultGatherer& result_gatherer,
+    const CheckerCreateInfo& additional_info) :
+    CCheckerBase(reader, result_gatherer, additional_info)
+{
+}
+
+void CCheckTopographyApplianceMetadata::RunCheck()
+{
+    this->result_gatherer_.StartCheck(CCheckTopographyApplianceMetadata::kCheckType);
+
+    const auto czi_metadata = this->GetCziMetadataAndReportErrors(CCheckTopographyApplianceMetadata::kCheckType);
+    if (czi_metadata)
+    {
+        this->CheckValidDimensionInTopographyDataItems(czi_metadata);
+    }
+
+    this->result_gatherer_.FinishCheck(CCheckTopographyApplianceMetadata::kCheckType);
+}
+
+void CCheckTopographyApplianceMetadata::CheckValidDimensionInTopographyDataItems(const std::shared_ptr<libCZI::ICziMetadata>& czi_metadata)
+{
+    if (!this->ExtractMetaDataDimensions(czi_metadata))
+    {
+        // we don't have any topography items and stop here
+        return;
+    }
+
+    if (this->texture_views_.empty() || this->heightmap_views_.empty())
+    {
+        CResultGatherer::Finding finding(CCheckTopographyApplianceMetadata::kCheckType);
+        finding.severity = CResultGatherer::Severity::Warning;
+        finding.information = "The image contains incomplete TopographyDataItems.";
+        this->result_gatherer_.ReportFinding(finding);
+
+        return;
+    }
+
+    // as soon as we have more than StartC specified for a Texutre or a Heightmap node
+    // the node contains superfluous data
+    auto superfluous_elements_check = [](const unordered_map<char, DimensionView>& dim_map) -> bool
+        {
+            if (dim_map.size() != 1)
+            {
+                return false;
+            }
+
+            return true;
+        };
+
+    auto start_c_defined_check = [](const unordered_map<char, DimensionView>& dim_map) -> bool
+        {
+            for (const auto& dim : dim_map)
+            {
+                if (dim.second.DimensionIndex == DimensionIndex::C
+                    && dim.second.IsValid())
+                    return true;
+            }
+
+            return false;
+        };
+
+    bool superfluous_free{ true };
+    bool start_c_defined{ true };
+    for (const auto& txt : this->texture_views_)
+    {
+        superfluous_free &= superfluous_elements_check(txt);
+        start_c_defined &= start_c_defined_check(txt);
+    }
+
+    for (const auto& hmp : this->heightmap_views_)
+    {
+        superfluous_free &= superfluous_elements_check(hmp);
+        start_c_defined &= start_c_defined_check(hmp);
+    }
+
+    if (!superfluous_free)
+    {
+        CResultGatherer::Finding finding(CCheckTopographyApplianceMetadata::kCheckType);
+        finding.severity = CResultGatherer::Severity::Warning;
+        finding.information = "There are superfluous dimensions specified in the TopographyDataItems. This might yield errors.";
+        this->result_gatherer_.ReportFinding(finding);
+    }
+
+    if (!start_c_defined)
+    {
+        CResultGatherer::Finding finding(CCheckTopographyApplianceMetadata::kCheckType);
+        finding.severity = CResultGatherer::Severity::Fatal;
+        finding.information = "The image contains TopographyDataItems that do not define a channel.";
+        this->result_gatherer_.ReportFinding(finding);
+    }
+}
+
+bool CCheckTopographyApplianceMetadata::ExtractMetaDataDimensions(const std::shared_ptr<libCZI::ICziMetadata>& czi_metadata)
+{
+    // within the TopographyData we allow
+    // any number of TopographyDataItem which itself can contain a set of Texutures and a set of heightmaps
+    // within the heightmaps AND Textures, each item reside in its own channel.
+    string topography_path{ this->kImageAppliancePath };
+    topography_path
+        .append("/Appliance[Id=")
+        .append(this->kTopographyItemId)
+        .append("]");
+
+    const auto topo_metadata{ czi_metadata->GetChildNodeReadonly(topography_path.c_str()) };
+
+    if (!topo_metadata)
+    {
+        // there is no topo metadata section, we end here
+        return false;
+    }
+
+    vector<vector<pair<wstring, wstring>>> heightmaps;
+    vector<vector<pair<wstring, wstring>>> textures;
+
+    // we need a "named" lambda here to call it recursively
+    std::function<bool(std::shared_ptr < libCZI::IXmlNodeRead>)> enumChildrenLambda =
+        [this, &heightmaps, &textures, &enumChildrenLambda](const std::shared_ptr<libCZI::IXmlNodeRead>& xmlnode) -> bool
+        {
+            std::vector<std::pair<std::wstring, std::wstring>> current_texture;
+            std::vector<std::pair<std::wstring, std::wstring>> current_heightmap;
+
+            auto textureLambda = [&current_texture](const std::wstring& attr, const std::wstring& val) -> bool
+                {
+                    current_texture.push_back({ attr, val });
+                    return true;
+                };
+
+            auto heighmapLambda = [&current_heightmap](const std::wstring& attr, const std::wstring& val) -> bool
+                {
+                    current_heightmap.push_back({ attr, val });
+                    return true;
+                };
+
+            auto node_name = xmlnode->Name();
+            if (node_name == CCheckTopographyApplianceMetadata::kTextureItemKey)
+            {
+                xmlnode->EnumAttributes(textureLambda);
+
+                if (!current_texture.empty())
+                {
+                    textures.push_back(current_texture);
+                }
+            }
+
+            if (node_name == CCheckTopographyApplianceMetadata::kHeightMapItemKey)
+            {
+                xmlnode->EnumAttributes(heighmapLambda);
+
+                if (!current_heightmap.empty())
+                {
+                    heightmaps.push_back(current_heightmap);
+                }
+            }
+
+            // recursively go through child items 
+            xmlnode->EnumChildren(enumChildrenLambda);
+
+            return true;
+        };
+
+    // call the enumeration lambda
+    topo_metadata->EnumChildren(enumChildrenLambda);
+
+    // parse the dimension vectors
+    for (const auto& hm : heightmaps)
+    {
+        this->SetBoundsFromVector(hm, this->heightmap_views_);
+    }
+
+    for (const auto& tx : textures)
+    {
+        this->SetBoundsFromVector(tx, this->texture_views_);
+    }
+
+    if (!this->heightmap_views_.empty() || !this->texture_views_.empty())
+    {
+        return true;
+    }
+
+    return false;
+}
+
+bool CCheckTopographyApplianceMetadata::SetBoundsFromVector(const std::vector<std::pair<std::wstring, std::wstring>>& vec, std::vector<std::unordered_map<char, DimensionView>>& view)
+{
+    // using a set here to ensure exactly one element per dimension
+    unordered_map<char, DimensionView> configurations;
+
+    const wstring kStart = L"Start";
+    const wstring kSize = L"Size";
+
+    for (const auto& element : vec)
+    {
+        // get the dimension index
+        char dim{ static_cast<char>(element.first.back()) };
+
+        configurations.insert({ dim, DimensionView() });
+        int value{ -1 };
+        try
+        {
+            value = stoi(element.second);
+        }
+        catch (const invalid_argument&)
+        {
+            // this will ensure an "invalid" dimension later
+            value = -1;
+        }
+
+        DimensionView& config = configurations.at(dim);
+        if (config.DimensionIndex == DimensionIndex::invalid)
+        {
+            config.DimensionIndex = Utils::CharToDimension(dim);
+        }
+
+        // -1 means not set
+        if (element.first.find(kStart) != string::npos && config.Start == -1)
+        {
+            config.Start = value;
+        }
+
+        if (element.first.find(kSize) != string::npos && config.Size == -1)
+        {
+            config.Size = value;
+        }
+
+        // '\0' means not set
+        if (config.DimensionName == '\0')
+        {
+            config.DimensionName = dim;
+        }
+    }
+
+    bool all_good{ true };
+    for (const auto& el : configurations)
+    {
+        all_good &= el.second.IsValid();
+    }
+
+    view.push_back(configurations);
+
+    return all_good;
+};
diff --git a/CZICheck/checkers/checkerTopographyApplianceValidation.h b/CZICheck/checkers/checkerTopographyApplianceValidation.h
@@ -0,0 +1,61 @@
+// SPDX-FileCopyrightText: 2024 Carl Zeiss Microscopy GmbH
+//
+// SPDX-License-Identifier: MIT
+
+#pragma once
+
+#include "checkerbase.h"
+#include <memory>
+#include <vector>
+#include <string>
+#include <unordered_map>
+#include <utility>
+
+/// This checker validates the topography-XML-metadata.
+class CCheckTopographyApplianceMetadata : public IChecker, CCheckerBase
+{
+private:
+    struct DimensionView
+    {
+        libCZI::DimensionIndex DimensionIndex{ libCZI::DimensionIndex::invalid };
+        char DimensionName{ '\0' };
+        int Start{ -1 };
+        int Size{ -1 };
+        bool IsValid() const
+        {
+            // this is an object used exclusively for this checker
+            //  a Size (SizeC, SizeX, etc.) is not needed to yield a "valid" dimension for this
+            if (this->Start >= 0 && this->DimensionIndex != libCZI::DimensionIndex::invalid)
+            {
+                return true;
+            }
+
+            return false;
+        }
+    };
+
+    static constexpr const char* kTopographyItemId = "Topography:1";
+    static constexpr const char* kImageAppliancePath = "ImageDocument/Metadata/Appliances";
+    static constexpr const wchar_t* kTextureItemKey = L"Texture";
+    static constexpr const wchar_t* kHeightMapItemKey = L"HeightMap";
+
+    std::vector<std::unordered_map<char, DimensionView>> texture_views_;
+    std::vector<std::unordered_map<char, DimensionView>> heightmap_views_;
+
+public:
+    static const CZIChecks kCheckType = CZIChecks::ApplianceMetadataTopographyItemValid;
+    static const char* kDisplayName;
+    static const char* kShortName;
+
+    CCheckTopographyApplianceMetadata(
+        const std::shared_ptr<libCZI::ICZIReader>& reader,
+        CResultGatherer& result_gatherer,
+        const CheckerCreateInfo& additional_info);
+    void RunCheck() override;
+
+private:
+    void CheckValidDimensionInTopographyDataItems(const std::shared_ptr<libCZI::ICziMetadata>& czi_metadata);
+
+    bool SetBoundsFromVector(const std::vector<std::pair<std::wstring, std::wstring>>&, std::vector<std::unordered_map<char, DimensionView>>&);
+    bool ExtractMetaDataDimensions(const std::shared_ptr<libCZI::ICziMetadata>& czi_metadata);
+};
diff --git a/CZICheck/checks.h b/CZICheck/checks.h
@@ -52,5 +52,8 @@ enum class CZIChecks
 
     ConsistentMIndex,   ///< To be done, not yet implemented.
 
-    AttachmentDirectoryPositionsWithinRange ///< To be done, not yet implemented.
+    AttachmentDirectoryPositionsWithinRange, ///< To be done, not yet implemented.
+
+    /// The Applicance Metadata specified for TopographyDataItem(s) are valid
+    ApplianceMetadataTopographyItemValid,
 };
diff --git a/Test/CZICheckSamples/TestCasesLists.txt b/Test/CZICheckSamples/TestCasesLists.txt
@@ -28,3 +28,5 @@ inaccessible_file.czi,5,*
 overlapping_scenes.czi,2,overlapping_scenes.txt
 jpgxrcompressed_inconsistent_size.czi,2,jpgxrcompressed_inconsistent_size.txt
 jpgxrcompressed_inconsistent_pixeltype.czi,2,jpgxrcompressed_inconsistent_pixeltype.txt
+edf-missing-texture.czi,2,edf-missing-texture.txt
+edf-superfluous.czi,2,edf-superfluous.txt
diff --git a/Test/CZICheckSamples/differentpixeltypeinchannel.txt b/Test/CZICheckSamples/differentpixeltypeinchannel.txt
@@ -17,6 +17,9 @@ Test "validate the XML-metadata against XSD-schema" :
  WARN
 Test "check if subblocks at pyramid-layer 0 of different scenes are overlapping" : OK
 Test "SubBlock-Segments in SubBlockDirectory are valid and valid content" : OK
+Test "Basic semantic checks for TopographyDataItems" :
+  Could not read metadata-segment
+ WARN
 
 
 Result: With Warnings