Skip to content

Commit

Permalink
Jbl/1036970 more lax checking for duplicate subblocks (#70)
Browse files Browse the repository at this point in the history 
* test

* draft...

* cosmetic

* cosmetic

* add unit-tests

* bump version

* fix build

* cosmetic

* review
  • Loading branch information
@ptahmose
ptahmose authored Oct 23, 2023
1 parent 31bf4cc commit 05bc1eb
Show file tree
Hide file tree
Showing 8 changed files with 230 additions and 91 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.15)
cmake_policy(SET CMP0091 NEW) # enable new "MSVC runtime library selection" (https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html)

project(libCZI
VERSION 0.53.1
VERSION 0.53.2
HOMEPAGE_URL "https://github.com/ZEISS/libczi"
DESCRIPTION "libCZI is an Open Source Cross-Platform C++ library to read and write CZI")

Expand Down
139 changes: 79 additions & 60 deletions Src/libCZI/CziSubBlockDirectory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,45 +54,45 @@ void CSbBlkStatisticsUpdater::UpdateStatistics(const CCziSubBlockDirectoryBase::
[&](libCZI::DimensionIndex dim, int value)->bool
{
int start, size;
if (this->statistics.dimBounds.TryGetInterval(dim, &start, &size) == false)
{
this->statistics.dimBounds.Set(dim, value, 1);
}
else
{
bool changed = false;
if (value < start)
{
size += (start - value);
start = value;
changed = true;
}
else if (value >= start + size)
{
size = 1 + value - start;
changed = true;
}
if (this->statistics.dimBounds.TryGetInterval(dim, &start, &size) == false)
{
this->statistics.dimBounds.Set(dim, value, 1);
}
else
{
bool changed = false;
if (value < start)
{
size += (start - value);
start = value;
changed = true;
}
else if (value >= start + size)
{
size = 1 + value - start;
changed = true;
}

if (changed)
{
this->statistics.dimBounds.Set(dim, start, size);
}
}
if (changed)
{
this->statistics.dimBounds.Set(dim, start, size);
}
}

if (entry.IsMIndexValid())
{
if (entry.mIndex < this->statistics.minMindex)
{
this->statistics.minMindex = entry.mIndex;
}
if (entry.IsMIndexValid())
{
if (entry.mIndex < this->statistics.minMindex)
{
this->statistics.minMindex = entry.mIndex;
}

if (entry.mIndex > this->statistics.maxMindex)
{
this->statistics.maxMindex = entry.mIndex;
}
}
if (entry.mIndex > this->statistics.maxMindex)
{
this->statistics.maxMindex = entry.mIndex;
}
}

return true;
return true;
});

// now deal with "enclosing Rect" for the scenes...
Expand Down Expand Up @@ -331,35 +331,35 @@ void CSbBlkStatisticsUpdater::SortPyramidStatistics()
return true;
}

// ...and "not identified" always last
if (a.layerInfo.IsNotIdentifiedAsPyramidLayer())
{
return false;
}
// ...and "not identified" always last
if (a.layerInfo.IsNotIdentifiedAsPyramidLayer())
{
return false;
}

if (b.layerInfo.IsLayer0())
{
return false;
}
if (b.layerInfo.IsLayer0())
{
return false;
}

if (b.layerInfo.IsNotIdentifiedAsPyramidLayer())
{
return true;
}
if (b.layerInfo.IsNotIdentifiedAsPyramidLayer())
{
return true;
}

int minificationFactorA = a.layerInfo.minificationFactor;
for (int i = 0; i < a.layerInfo.pyramidLayerNo - 1; ++i)
{
minificationFactorA *= a.layerInfo.minificationFactor;
}
int minificationFactorA = a.layerInfo.minificationFactor;
for (int i = 0; i < a.layerInfo.pyramidLayerNo - 1; ++i)
{
minificationFactorA *= a.layerInfo.minificationFactor;
}

int minificationFactorB = b.layerInfo.minificationFactor;
for (int i = 0; i < b.layerInfo.pyramidLayerNo - 1; ++i)
{
minificationFactorB *= b.layerInfo.minificationFactor;
}
int minificationFactorB = b.layerInfo.minificationFactor;
for (int i = 0; i < b.layerInfo.pyramidLayerNo - 1; ++i)
{
minificationFactorB *= b.layerInfo.minificationFactor;
}

return minificationFactorA < minificationFactorB;
return minificationFactorA < minificationFactorB;
});
}
}
Expand Down Expand Up @@ -441,6 +441,12 @@ bool PixelTypeForChannelIndexStatistic::TryGetPixelTypeForNoChannelIndex(int* pi

//----------------------------------------------------------------------------------------------

CWriterCziSubBlockDirectory::CWriterCziSubBlockDirectory(bool allow_duplicate_subblocks)
: subBlkEntryComparison{ allow_duplicate_subblocks },
subBlks(subBlkEntryComparison)
{
}

bool CWriterCziSubBlockDirectory::TryAddSubBlock(const SubBlkEntry& entry)
{
auto insert = this->subBlks.insert(entry);
Expand All @@ -463,8 +469,10 @@ bool CWriterCziSubBlockDirectory::SubBlkEntryCompare::operator()(const SubBlkEnt
// 2nd check: coordinate
// 3rd check: m-Index
// 4th check: (only if both subblocks have invalid M-indices) is coordinate
// 5th check: (only if the the property "include_file_position_" of the comparison-object is true)
// the file-position is compared

// subblocks from a lower layer go before subblocks from an upper layer
// 1st check: subblocks from a lower layer go before subblocks from an upper layer
float zoomA = Utils::CalcZoom(IntSize{ (std::uint32_t)a.width,(std::uint32_t)a.height }, IntSize{ (std::uint32_t)a.storedWidth,(std::uint32_t)a.storedHeight });
float zoomB = Utils::CalcZoom(IntSize{ (std::uint32_t)b.width,(std::uint32_t)b.height }, IntSize{ (std::uint32_t)b.storedWidth,(std::uint32_t)b.storedHeight });
if (fabs(zoomA - zoomB) > 0.0001)
Expand All @@ -477,6 +485,7 @@ bool CWriterCziSubBlockDirectory::SubBlkEntryCompare::operator()(const SubBlkEnt
return false;
}

// 2nd check: plane coordinates
int r = Utils::Compare(&a.coordinate, &b.coordinate);
if (r < 0)
{
Expand All @@ -487,6 +496,7 @@ bool CWriterCziSubBlockDirectory::SubBlkEntryCompare::operator()(const SubBlkEnt
return false;
}

// 3rd check: m-index
if (a.IsMIndexValid() == true && b.IsMIndexValid() == false)
{
return true;
Expand All @@ -509,6 +519,7 @@ bool CWriterCziSubBlockDirectory::SubBlkEntryCompare::operator()(const SubBlkEnt
}
}

// 4th check: the x-y-position
if (a.IsMIndexValid() == false && b.IsMIndexValid() == false)
{
int v = a.x - b.x;
Expand All @@ -532,6 +543,14 @@ bool CWriterCziSubBlockDirectory::SubBlkEntryCompare::operator()(const SubBlkEnt
}
}

// 5th check: if we are instructed to include the file-position, then a lower file-position
// shoud go first (otherwise - they are considered "equal" which means that we do not allow
// to add a second one as it would be a duplicate).
if (this->include_file_position_ && a.FilePosition < b.FilePosition)
{
return true;
}

return false;
}

Expand Down
9 changes: 9 additions & 0 deletions Src/libCZI/CziSubBlockDirectory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class CWriterCziSubBlockDirectory : public CCziSubBlockDirectoryBase
std::map<int, int> mapChannelIdxPixelType;
PixelTypeForChannelIndexStatisticCreate pixelTypeForChannel;
public:
CWriterCziSubBlockDirectory() = delete;
CWriterCziSubBlockDirectory(bool allow_duplicate_subblocks);
bool TryAddSubBlock(const SubBlkEntry& entry);

bool EnumEntries(const std::function<bool(size_t index, const SubBlkEntry&)>& func) const;
Expand All @@ -144,11 +146,18 @@ class CWriterCziSubBlockDirectory : public CCziSubBlockDirectoryBase
const libCZI::PyramidStatistics& GetPyramidStatistics() const;
const PixelTypeForChannelIndexStatistic& GetPixelTypeForChannel() const;
private:
/// Implementation of a "less-comparison" for SubBlkEntry objects, which can
/// be parametrized.
struct SubBlkEntryCompare
{
SubBlkEntryCompare(bool include_file_position) : include_file_position_(include_file_position) {}
bool include_file_position_{ false };
bool operator() (const SubBlkEntry& a, const SubBlkEntry& b) const;
};

/// This object is used to implement the "less-comparison" for the set.
SubBlkEntryCompare subBlkEntryComparison;

std::set<SubBlkEntry, SubBlkEntryCompare> subBlks;
};

Expand Down
8 changes: 6 additions & 2 deletions Src/libCZI/CziWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,11 @@ CCziWriter::CziWriterInfoWrapper::CziWriterInfoWrapper(std::shared_ptr<libCZI::I

//--------------------------------------------------------------------------------------------------

CCziWriter::CCziWriter() : nextSegmentPos(0)
CCziWriter::CCziWriter() : CCziWriter(CZIWriterOptions{})
{}

CCziWriter::CCziWriter(const libCZI::CZIWriterOptions& options)
: cziWriterOptions(options), sbBlkDirectory{options.allow_duplicate_subblocks}, nextSegmentPos(0)
{
}

Expand Down Expand Up @@ -1003,7 +1007,7 @@ CCziWriter::~CCziWriter()
this->ThrowIfNotOperational();
this->Finish();
this->nextSegmentPos = 0;
this->sbBlkDirectory = CWriterCziSubBlockDirectory();
this->sbBlkDirectory = CWriterCziSubBlockDirectory{ this->cziWriterOptions.allow_duplicate_subblocks };
this->attachmentDirectory = CWriterCziAttachmentsDirectory();
this->metadataSegment.Invalidate();
this->subBlockDirectorySegment.Invalidate();
Expand Down
2 changes: 2 additions & 0 deletions Src/libCZI/CziWriter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ class CWriterUtils
class CCziWriter : public libCZI::ICziWriter
{
private:
libCZI::CZIWriterOptions cziWriterOptions;
CWriterCziSubBlockDirectory sbBlkDirectory;
CWriterCziAttachmentsDirectory attachmentDirectory;
std::shared_ptr<libCZI::IOutputStream> stream;
Expand All @@ -247,6 +248,7 @@ class CCziWriter : public libCZI::ICziWriter
};
public:
CCziWriter();
CCziWriter(const libCZI::CZIWriterOptions& options);

void Create(std::shared_ptr<libCZI::IOutputStream> stream, std::shared_ptr<libCZI::ICziWriterInfo> info) override;
~CCziWriter() override;
Expand Down
15 changes: 13 additions & 2 deletions Src/libCZI/libCZI.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,20 @@ namespace libCZI
/// \return The newly created CZI-reader.
LIBCZI_API std::shared_ptr<ICZIReader> CreateCZIReader();

/// Options controlling the operation of a CZI-writer object. Those options are set at construction
/// time and cannot be mutated afterwards.
struct CZIWriterOptions
{
/// True if the writer should allow that duplicate subblocks are added. In general, it is
/// not recommended to bypass the check for duplicate subblocks.
bool allow_duplicate_subblocks{ false };
};

/// Creates a new instance of the CZI-writer class.
/// \return The newly created CZI-writer.
LIBCZI_API std::shared_ptr<ICziWriter> CreateCZIWriter();
/// \param options (Optional) Options for controlling the operation. This argument may
/// be null, in which case default options are used.
/// \returns The newly created CZI-writer.
LIBCZI_API std::shared_ptr<ICziWriter> CreateCZIWriter(const CZIWriterOptions* options = nullptr);

/// Creates a new instance of the CZI-reader-writer class.
/// \return The newly created CZI-reader-writer.
Expand Down
9 changes: 7 additions & 2 deletions Src/libCZI/libCZI_Lib.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,14 @@ std::shared_ptr<ICZIReader> libCZI::CreateCZIReader()
return std::make_shared<CCZIReader>();
}

std::shared_ptr<ICziWriter> libCZI::CreateCZIWriter()
std::shared_ptr<ICziWriter> libCZI::CreateCZIWriter(const CZIWriterOptions* options)
{
return std::make_shared<CCziWriter>();
if (options == nullptr)
{
return std::make_shared<CCziWriter>();
}

return std::make_shared<CCziWriter>(*options);
}

std::shared_ptr<ICziReaderWriter> libCZI::CreateCZIReaderWriter()
Expand Down
Loading

0 comments on commit 05bc1eb

Please sign in to comment.