From 42dbc966df9cab91662e766287852b322cf567a3 Mon Sep 17 00:00:00 2001 From: Patrick Hodoul Date: Mon, 1 Jun 2020 14:20:51 -0400 Subject: [PATCH] Adsk Contrib - Various CLF writer improvements (#1020) Signed-off-by: Patrick Hodoul --- .../fileformats/ctf/CTFTransform.cpp | 214 +++++++++++------- .../fileformats/xmlutils/XMLWriterUtils.cpp | 10 +- src/apps/ociomakeclf/main.cpp | 25 +- src/apps/ocioperf/main.cpp | 71 +----- src/apputils/measure.h | 90 ++++++++ tests/cpu/fileformats/FileFormatCTF_tests.cpp | 12 +- 6 files changed, 257 insertions(+), 165 deletions(-) create mode 100644 src/apputils/measure.h diff --git a/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp b/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp index c221776aea..8b78d3e02a 100644 --- a/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp +++ b/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp @@ -26,6 +26,12 @@ namespace OCIO_NAMESPACE { +// Note: 17 would allow exactly restoring most doubles but anything higher than 15, +// introduces some serialization issues such as: 81.9 -> 81.90000000000001. +// This results in less pretty output and also causes problems for some unit tests. +static constexpr unsigned DOUBLE_PRECISION = 15; + + void CTFVersion::ReadVersion(const std::string & versionString, CTFVersion & versionOut) { unsigned int numDot = 0; @@ -495,7 +501,7 @@ template <> void SetOStream(double, std::ostream & xml) { xml.width(19); - xml.precision(15); + xml.precision(DOUBLE_PRECISION); } template @@ -507,73 +513,101 @@ void WriteValues(XmlFormatter & formatter, unsigned iterStep, scaleType scale) { - std::ostream& xml = formatter.getStream(); + // Method used to write an array of values of the same type. - for (Iter it(valuesBegin); it != valuesEnd; it += iterStep) + std::ostream & xml = formatter.getStream(); + std::ostringstream oss; + + // The numbers in a CLF/CTF file may always contain fractional values, regardless of the + // bit-depth attributes. E.g., even if the bit-depth is 8i, the array could contain values + // such as [-0.1, 8, 100.234, 305]. However, we do use the bit-depth to initialize the most + // likely formatting to use when printing arrays with large numbers of values such as LUTs. + // And if the array really does only have integers, it is nicer to print those without + // decimal points. + + switch (bitDepth) { - switch (bitDepth) - { - case BIT_DEPTH_UINT8: - { - xml.width(3); - xml << (*it) * scale; - break; - } - case BIT_DEPTH_UINT10: - { - xml.width(4); - xml << (*it) * scale; - break; - } + case BIT_DEPTH_UINT8: + { + oss.width(3); + break; + } + case BIT_DEPTH_UINT10: + { + oss.width(4); + break; + } - case BIT_DEPTH_UINT12: - { - xml.width(4); - xml << (*it) * scale; - break; - } + case BIT_DEPTH_UINT12: + { + oss.width(4); + break; + } - case BIT_DEPTH_UINT16: - { - xml.width(5); - xml << (*it) * scale; - break; - } + case BIT_DEPTH_UINT16: + { + oss.width(5); + break; + } - case BIT_DEPTH_F16: - { - xml.width(11); - xml.precision(5); - WriteValue((*it) * scale, xml); - break; - } + case BIT_DEPTH_F16: + { + oss.width(11); + oss.precision(5); + break; + } + + case BIT_DEPTH_F32: + { + SetOStream(*valuesBegin, oss); + break; + } + + case BIT_DEPTH_UINT14: + case BIT_DEPTH_UINT32: + { + throw Exception("Unsupported bitdepth."); + break; + } + + case BIT_DEPTH_UNKNOWN: + { + throw Exception("Unknown bitdepth."); + break; + } + } + + const bool floatValues = (bitDepth == BIT_DEPTH_F16) || (bitDepth == BIT_DEPTH_F32); + + for (Iter it(valuesBegin); it != valuesEnd; it += iterStep) + { + oss.str(""); - case BIT_DEPTH_F32: + if (floatValues) { - SetOStream(*it, xml); - WriteValue((*it) * scale, xml); - break; + WriteValue((*it) * scale, oss); } - - case BIT_DEPTH_UINT14: - case BIT_DEPTH_UINT32: + else { - throw Exception("Unsupported bitdepth."); - break; + oss << (*it) * scale; } - case BIT_DEPTH_UNKNOWN: + const std::string value = oss.str(); + if (value.length() > (size_t)oss.width()) { - throw Exception("Unknown bitdepth."); - break; + // The imposed precision requires more characters so the code + // recomputes the width to better align the values for the next lines. + oss.width(value.length()); } - } + xml << value; - if (std::distance(valuesBegin, it) % valuesPerLine - == valuesPerLine - 1) + if (std::distance(valuesBegin, it) % valuesPerLine == valuesPerLine - 1) { - xml << std::endl; + // std::endln writes a newline and flushes the output buffer where '\n' only + // writes a newline. Flushing the buffer for all floats of large LUTs + // introduces a huge performance hit so only use '\n'. + xml << "\n"; } else { @@ -775,7 +809,14 @@ void CDLWriter::getAttributes(XmlFormatter::Attributes & attributes) const void CDLWriter::writeContent() const { XmlFormatter::Attributes attributes; + auto op = getOp(); + + std::ostringstream oss; + oss.precision(DOUBLE_PRECISION); + + CDLOpData::ChannelParams params; + // SOPNode. m_formatter.writeStartTag(TAG_SOPNODE, attributes); { @@ -786,9 +827,20 @@ void CDLWriter::writeContent() const METADATA_SOP_DESCRIPTION, desc); WriteDescriptions(m_formatter, TAG_DESCRIPTION, desc); - m_formatter.writeContentTag(TAG_SLOPE, m_cdl->getSlopeString()); - m_formatter.writeContentTag(TAG_OFFSET, m_cdl->getOffsetString()); - m_formatter.writeContentTag(TAG_POWER, m_cdl->getPowerString()); + oss.str(""); + params = m_cdl->getSlopeParams(); + oss << params[0] << ", " << params[1] << ", " << params[2]; + m_formatter.writeContentTag(TAG_SLOPE, oss.str()); + + oss.str(""); + params = m_cdl->getOffsetParams(); + oss << params[0] << ", " << params[1] << ", " << params[2]; + m_formatter.writeContentTag(TAG_OFFSET, oss.str()); + + oss.str(""); + params = m_cdl->getPowerParams(); + oss << params[0] << ", " << params[1] << ", " << params[2]; + m_formatter.writeContentTag(TAG_POWER, oss.str()); } m_formatter.writeEndTag(TAG_SOPNODE); @@ -802,7 +854,9 @@ void CDLWriter::writeContent() const METADATA_SAT_DESCRIPTION, desc); WriteDescriptions(m_formatter, TAG_DESCRIPTION, desc); - m_formatter.writeContentTag(TAG_SATURATION, m_cdl->getSaturationString()); + oss.str(""); + oss << m_cdl->getSaturation(); + m_formatter.writeContentTag(TAG_SATURATION, oss.str()); } m_formatter.writeEndTag(TAG_SATNODE); } @@ -877,41 +931,41 @@ void ExposureContrastWriter::getAttributes(XmlFormatter::Attributes& attributes) void ExposureContrastWriter::writeContent() const { + std::ostringstream oss; + oss.precision(DOUBLE_PRECISION); + XmlFormatter::Attributes attributes; { - std::stringstream expAttr; - WriteValue(m_ec->getExposure(), expAttr); - attributes.push_back(XmlFormatter::Attribute(ATTR_EXPOSURE, - expAttr.str())); + oss.str(""); + WriteValue(m_ec->getExposure(), oss); + attributes.push_back(XmlFormatter::Attribute(ATTR_EXPOSURE, oss.str())); } { - std::stringstream contAttr; - WriteValue(m_ec->getContrast(), contAttr); - attributes.push_back(XmlFormatter::Attribute(ATTR_CONTRAST, - contAttr.str())); + oss.str(""); + WriteValue(m_ec->getContrast(), oss); + attributes.push_back(XmlFormatter::Attribute(ATTR_CONTRAST, oss.str())); } { - std::stringstream gammaAttr; - WriteValue(m_ec->getGamma(), gammaAttr); - attributes.push_back(XmlFormatter::Attribute(ATTR_GAMMA, - gammaAttr.str())); + oss.str(""); + WriteValue(m_ec->getGamma(), oss); + attributes.push_back(XmlFormatter::Attribute(ATTR_GAMMA, oss.str())); } { - std::ostringstream oss; + oss.str(""); WriteValue(m_ec->getPivot(), oss); attributes.push_back(XmlFormatter::Attribute(ATTR_PIVOT, oss.str())); } if (m_ec->getLogExposureStep() != ExposureContrastOpData::LOGEXPOSURESTEP_DEFAULT) { - std::ostringstream oss; + oss.str(""); WriteValue(m_ec->getLogExposureStep(), oss); attributes.push_back(XmlFormatter::Attribute(ATTR_LOGEXPOSURESTEP, oss.str())); } if (m_ec->getLogMidGray() != ExposureContrastOpData::LOGMIDGRAY_DEFAULT) { - std::ostringstream oss; + oss.str(""); WriteValue(m_ec->getLogMidGray(), oss); attributes.push_back(XmlFormatter::Attribute(ATTR_LOGMIDGRAY, oss.str())); } @@ -995,6 +1049,7 @@ void FixedFunctionWriter::getAttributes(XmlFormatter::Attributes& attributes) co { size_t i = 0; std::stringstream ffParams; + ffParams.precision(DOUBLE_PRECISION); WriteValue(params[i], ffParams); while (++i < numParams) { @@ -1074,11 +1129,12 @@ void AddGammaParams(XmlFormatter::Attributes & attributes, const GammaOpData::Style style, bool useGamma) { - std::stringstream gammaAttr; - gammaAttr << params[0]; + std::stringstream oss; + oss.precision(DOUBLE_PRECISION); + oss << params[0]; attributes.push_back(XmlFormatter::Attribute(useGamma ? ATTR_GAMMA : ATTR_EXPONENT, - gammaAttr.str())); + oss.str())); switch (style) { @@ -1087,10 +1143,9 @@ void AddGammaParams(XmlFormatter::Attributes & attributes, case GammaOpData::MONCURVE_MIRROR_FWD: case GammaOpData::MONCURVE_MIRROR_REV: { - std::stringstream offsetAttr; - offsetAttr << params[1]; - attributes.push_back(XmlFormatter::Attribute(ATTR_OFFSET, - offsetAttr.str())); + oss.str(""); + oss << params[1]; + attributes.push_back(XmlFormatter::Attribute(ATTR_OFFSET, oss.str())); break; } case GammaOpData::BASIC_FWD: @@ -1244,6 +1299,7 @@ void AddLogParam(XmlFormatter::Attributes & attributes, double attrValue) { std::stringstream stream; + stream.precision(DOUBLE_PRECISION); stream << attrValue; attributes.push_back(XmlFormatter::Attribute(attrName, stream.str())); } @@ -1743,7 +1799,7 @@ const char * RangeWriter::getTagName() const void WriteTag(XmlFormatter & fmt, const char * tag, double value) { std::ostringstream o; - o.precision(15); + o.precision(DOUBLE_PRECISION); o << value; fmt.writeContentTag(tag, ' ' + o.str() + ' '); } diff --git a/src/OpenColorIO/fileformats/xmlutils/XMLWriterUtils.cpp b/src/OpenColorIO/fileformats/xmlutils/XMLWriterUtils.cpp index 2747521d00..30fa0a8266 100644 --- a/src/OpenColorIO/fileformats/xmlutils/XMLWriterUtils.cpp +++ b/src/OpenColorIO/fileformats/xmlutils/XMLWriterUtils.cpp @@ -43,7 +43,7 @@ void XmlFormatter::writeStartTag(const std::string & tagName, m_stream << "\""; } - m_stream << ">" << std::endl; + m_stream << ">\n"; } void XmlFormatter::writeStartTag(const std::string & tagName) @@ -55,7 +55,7 @@ void XmlFormatter::writeStartTag(const std::string & tagName) void XmlFormatter::writeEndTag(const std::string & tagName) { writeIndent(); - m_stream << "" << std::endl; + m_stream << "\n"; } void XmlFormatter::writeContentTag(const std::string & tagName, @@ -79,7 +79,7 @@ void XmlFormatter::writeContentTag(const std::string & tagName, } m_stream << ">"; writeString(content); - m_stream << "" << std::endl; + m_stream << "\n"; } // Write the content using escaped characters if needed. @@ -87,7 +87,7 @@ void XmlFormatter::writeContent(const std::string & content) { writeIndent(); writeString(content); - m_stream << std::endl; + m_stream << "\n"; } void XmlFormatter::writeEmptyTag(const std::string & tagName, @@ -105,7 +105,7 @@ void XmlFormatter::writeEmptyTag(const std::string & tagName, } // Note we close the tag, no end tag is needed. - m_stream << " />" << std::endl; + m_stream << " />\n"; } std::ostream & XmlFormatter::getStream() diff --git a/src/apps/ociomakeclf/main.cpp b/src/apps/ociomakeclf/main.cpp index 080806e823..d9458cab8f 100644 --- a/src/apps/ociomakeclf/main.cpp +++ b/src/apps/ociomakeclf/main.cpp @@ -11,6 +11,7 @@ namespace OCIO = OCIO_NAMESPACE; #include "apputils/argparse.h" +#include "apputils/measure.h" #include "utils/StringUtils.h" @@ -78,7 +79,7 @@ void CreateOutputLutFile(const std::string & outLutFilepath, OCIO::ConstGroupTra int main(int argc, const char ** argv) { - bool help = false, verbose = false, listCSCColorSpaces = false; + bool help = false, verbose = false, measure = false, listCSCColorSpaces = false; std::string cscColorSpace; ArgParse ap; @@ -92,6 +93,7 @@ int main(int argc, const char ** argv) "", "Options:", "--help", &help, "Print help message", "--verbose", &verbose, "Display general information", + "--measure", &measure, "Measure (in ms) the CLF write", "--list", &listCSCColorSpaces, "List of the supported CSC color spaces", "--csc %s", &cscColorSpace, "The color space that the input LUT expects and produces", nullptr); @@ -251,13 +253,26 @@ int main(int argc, const char ** argv) grp->appendTransform(outBuiltin); } - if (verbose) + static constexpr char Msg[] = "Creating the CLF lut file"; + + if (verbose && !measure) { - std::cout << "Creating the CLF lut file." << std::endl; + std::cout << Msg << "." << std::endl; } - // Create the CLF file. - CreateOutputLutFile(outLutFilepath, grp); + if (measure) + { + Measure m(Msg); + m.resume(); + + // Create the CLF file. + CreateOutputLutFile(outLutFilepath, grp); + } + else + { + // Create the CLF file. + CreateOutputLutFile(outLutFilepath, grp); + } } catch (OCIO::Exception & exception) { diff --git a/src/apps/ocioperf/main.cpp b/src/apps/ocioperf/main.cpp index c505774b4b..29644f5906 100644 --- a/src/apps/ocioperf/main.cpp +++ b/src/apps/ocioperf/main.cpp @@ -1,7 +1,6 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright Contributors to the OpenColorIO Project. -#include #include @@ -12,6 +11,7 @@ namespace OIIO = OIIO_NAMESPACE; #endif #include "apputils/argparse.h" +#include "apputils/measure.h" #include "OpenEXR/half.h" #include "oiiohelpers.h" #include "utils/StringUtils.h" @@ -21,75 +21,6 @@ namespace OCIO = OCIO_NAMESPACE; -// Utility to measure time in ms. -class Measure -{ -public: - Measure() = delete; - Measure(const Measure &) = delete; - - explicit Measure(const char * explanation, unsigned iterations) - : m_explanations(explanation) - , m_iterations(iterations) - , m_started(false) - , m_duration(0) - { - } - - ~Measure() - { - if(m_started) - { - pause(); - } - - std::cout << std::endl; - std::cout << m_explanations << std::endl; - std::cout << " CPU processing took: " - << (m_duration.count()/float(m_iterations)) - << " ms" << std::endl; - } - - void resume() - { - if(m_started) - { - throw OCIO::Exception("Measure already started."); - } - - m_started = true; - m_start = std::chrono::high_resolution_clock::now(); - } - - void pause() - { - std::chrono::high_resolution_clock::time_point end - = std::chrono::high_resolution_clock::now(); - - if(m_started) - { - std::chrono::duration duration = end - m_start; - - m_duration += duration; - } - else - { - throw OCIO::Exception("Measure already stopped."); - } - - m_started = false; - } - -private: - const std::string m_explanations; - const unsigned m_iterations; - - bool m_started; - std::chrono::high_resolution_clock::time_point m_start; - - std::chrono::duration m_duration; -}; - // Load in memory an image from disk. void LoadImage(const std::string & filepath, bool verbose, diff --git a/src/apputils/measure.h b/src/apputils/measure.h new file mode 100644 index 0000000000..2a35b58162 --- /dev/null +++ b/src/apputils/measure.h @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright Contributors to the OpenColorIO Project. + +#ifndef INCLUDED_OCIO_MEASURE_H +#define INCLUDED_OCIO_MEASURE_H + + +#include +#include +#include + + +// Utility to measure time in ms. +class Measure +{ +public: + Measure() = delete; + Measure(const Measure &) = delete; + + explicit Measure(const char * explanation) + : m_explanations(explanation) + { + } + + explicit Measure(const char * explanation, unsigned iterations) + : m_explanations(explanation) + , m_iterations(iterations) + { + } + + ~Measure() + { + if(m_started) + { + pause(); + } + print(); + } + + void resume() + { + if(m_started) + { + throw std::runtime_error("Measure already started."); + } + + m_started = true; + m_start = std::chrono::high_resolution_clock::now(); + } + + void pause() + { + std::chrono::high_resolution_clock::time_point end + = std::chrono::high_resolution_clock::now(); + + if(m_started) + { + const std::chrono::duration duration = end - m_start; + + m_duration += duration; + } + else + { + throw std::runtime_error("Measure already stopped."); + } + + m_started = false; + } + + void print() const noexcept + { + std::cout << "\n" + << m_explanations << "\n" + << " Processing took: " + << (m_duration.count() / float(m_iterations)) + << " ms" << std::endl; + } + +private: + const std::string m_explanations; + const unsigned m_iterations = 1; + + bool m_started = false; + std::chrono::high_resolution_clock::time_point m_start; + + std::chrono::duration m_duration { 0 }; +}; + + +#endif // INCLUDED_OCIO_MEASURE_H diff --git a/tests/cpu/fileformats/FileFormatCTF_tests.cpp b/tests/cpu/fileformats/FileFormatCTF_tests.cpp index 2547dd5c11..f737894e06 100644 --- a/tests/cpu/fileformats/FileFormatCTF_tests.cpp +++ b/tests/cpu/fileformats/FileFormatCTF_tests.cpp @@ -5405,10 +5405,10 @@ OCIO_ADD_TEST(CTFTransform, gamma5_ctf) const std::string expected{ R"( - - - - + + + + )" }; @@ -6067,7 +6067,7 @@ OCIO_ADD_TEST(CTFTransform, lut1d_10i_ctf) 0 0 0 511 4011.12 -24.103 -1023 1023 1023 + 1023 1023 1023 @@ -6395,7 +6395,7 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf) 0 511.5 -1023 + 1023