From f767c55733b6cc224185246ef6cc13de0e75257b Mon Sep 17 00:00:00 2001 From: Stephen Parker Date: Sun, 17 Jun 2012 13:18:34 -0700 Subject: [PATCH 1/3] colorspace conversion to linear prior to filtering --- .gitignore | 10 ++ src/maketx/maketx.cpp | 269 +++++++++++++++++++++++++++--------------- 2 files changed, 182 insertions(+), 97 deletions(-) diff --git a/.gitignore b/.gitignore index b8d83a3ec0..9333c627c0 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,13 @@ branches/ build/ dist/ testsuite/runtest.pyc + +Makefile.mine + +test.tif + +test.tx + +test2.tx + +test.exr diff --git a/src/maketx/maketx.cpp b/src/maketx/maketx.cpp index b643d3a77f..672ebcfadc 100644 --- a/src/maketx/maketx.cpp +++ b/src/maketx/maketx.cpp @@ -283,6 +283,8 @@ getargs (int argc, char *argv[]) "", colortitle_help_string().c_str(), "--colorconvert %s %s", &incolorspace, &outcolorspace, colorconvert_help_string().c_str(), + "--colorspace %s", &incolorspace, "Set input colorspace. " + "This is used for internal processing and does not affect the output colorspace.", "--unpremult", &unpremult, "Unpremultiply before color conversion, then premultiply " "after the color conversion. You'll probably want to use this flag " "if your image contains an alpha channel.", @@ -658,6 +660,105 @@ formatres (const ImageSpec &spec, bool extended=false) } +static void +convert_color (ImageBuf &src, const std::string &inspace, + ImageBuf &dst, const std::string &outspace) +{ + Timer colorconverttimer; + if (verbose) { + std::cout << " Converting from colorspace " << inspace + << " to colorspace " << outspace << std::endl; + } + + if (colorconfig.error()) { + std::cerr << "Error Creating ColorConfig\n"; + std::cerr << colorconfig.geterror() << std::endl; + exit (EXIT_FAILURE); + } + + ColorProcessor * processor = colorconfig.createColorProcessor ( + inspace.c_str(), outspace.c_str()); + + if (!processor || colorconfig.error()) { + std::cerr << "Error Creating Color Processor." << std::endl; + std::cerr << colorconfig.geterror() << std::endl; + exit (EXIT_FAILURE); + } + + if (unpremult && verbose) + std::cout << " Unpremulting image..." << std::endl; + + if (!ImageBufAlgo::colorconvert (dst, src, processor, unpremult)) { + std::cerr << "Error applying color conversion to image.\n"; + exit (EXIT_FAILURE); + } + + ColorConfig::deleteColorProcessor(processor); + processor = NULL; + stat_colorconverttime += colorconverttimer(); +} + + +static void +convert_color (std::vector &color, + const std::string &inspace, const std::string &outspace) +{ + Timer colorconverttimer; + if (verbose) { + std::cout << " Converting from colorspace " << inspace + << " to colorspace " << outspace << std::endl; + } + + if (colorconfig.error()) { + std::cerr << "Error Creating ColorConfig\n"; + std::cerr << colorconfig.geterror() << std::endl; + exit (EXIT_FAILURE); + } + + ColorProcessor * processor = colorconfig.createColorProcessor ( + inspace.c_str(), outspace.c_str()); + + if (!processor || colorconfig.error()) { + std::cerr << "Error Creating Color Processor." << std::endl; + std::cerr << colorconfig.geterror() << std::endl; + exit (EXIT_FAILURE); + } + + if (unpremult && verbose) + std::cout << " Unpremulting image..." << std::endl; + + if (!ImageBufAlgo::colorconvert (&color[0], + static_cast(color.size()), processor, unpremult)) { + std::cerr << "Error applying color conversion to constant color.\n"; + exit (EXIT_FAILURE); + } + + ColorConfig::deleteColorProcessor(processor); + processor = NULL; + stat_colorconverttime += colorconverttimer(); +} + + +static void +append_desc (std::string &desc, const std::string &key, + const std::string &value) +{ + size_t key_pos = desc.find (key); + if (key_pos != std::string::npos) { //found, replace + size_t val_pos = key_pos + key.length() + 1; + size_t end_pos = desc.find_first_of (' ', val_pos); + desc.replace (val_pos, end_pos, value); + } + else { // not found, append + if (desc.length()) + desc += " "; + desc += key + "="; + desc += value; + } + if (verbose) + std::cout << " " << key << ": " << value << std::endl; +} + static void make_texturemap (const char *maptypename = "texture map") @@ -987,54 +1088,24 @@ make_texturemap (const char *maptypename = "texture map") // independently color convert the constant color metadata ImageBuf * ccSrc = &src; // Ptr to cc'd src image ImageBuf colorBuffer; - if (!incolorspace.empty() && !outcolorspace.empty() && incolorspace != outcolorspace) { - if (src.spec().format != TypeDesc::FLOAT) { - ImageSpec floatSpec = src.spec(); - floatSpec.set_format(TypeDesc::FLOAT); - colorBuffer.reset("bitdepth promoted", floatSpec); - ccSrc = &colorBuffer; - } - - Timer colorconverttimer; - if (verbose) { - std::cout << " Converting from colorspace " << incolorspace - << " to colorspace " << outcolorspace << std::endl; - } - - if (colorconfig.error()) { - std::cerr << "Error Creating ColorConfig\n"; - std::cerr << colorconfig.geterror() << std::endl; - exit (EXIT_FAILURE); - } - - ColorProcessor * processor = colorconfig.createColorProcessor ( - incolorspace.c_str(), outcolorspace.c_str()); - - if (!processor || colorconfig.error()) { - std::cerr << "Error Creating Color Processor." << std::endl; - std::cerr << colorconfig.geterror() << std::endl; - exit (EXIT_FAILURE); - } - - if (unpremult && verbose) - std::cout << " Unpremulting image..." << std::endl; - - if (!ImageBufAlgo::colorconvert (*ccSrc, src, processor, unpremult)) { - std::cerr << "Error applying color conversion to image.\n"; - exit (EXIT_FAILURE); + + if (!incolorspace.empty()) { + if (outcolorspace.empty()) { + // If outcolorspace is empty, assume that --colorspace flag was + // issued and that the in/out colorspace should be the same. + outcolorspace = incolorspace; } - - if (isConstantColor) { - if (!ImageBufAlgo::colorconvert (&constantColor[0], - static_cast(constantColor.size()), processor, unpremult)) { - std::cerr << "Error applying color conversion to constant color.\n"; - exit (EXIT_FAILURE); + if (incolorspace != "linear") { + // We're only interested in getting from incolorspace -> linear, + // until after processing. + if (src.spec().format != TypeDesc::FLOAT) { + ImageSpec floatSpec = src.spec(); + floatSpec.set_format(TypeDesc::FLOAT); + colorBuffer.reset("bitdepth promoted", floatSpec); + ccSrc = &colorBuffer; } + convert_color (src, incolorspace, *ccSrc, "linear"); } - - ColorConfig::deleteColorProcessor(processor); - processor = NULL; - stat_colorconverttime += colorconverttimer(); } // Force float for the sake of the ImageBuf math @@ -1101,43 +1172,13 @@ make_texturemap (const char *maptypename = "texture map") } stat_resizetime += resizetimer(); - - // Update the toplevel ImageDescription with the sha1 pixel hash and constant color - std::string desc = dstspec.get_string_attribute ("ImageDescription"); - bool updatedDesc = false; - - // FIXME: We need to do real dictionary style partial updates on the - // ImageDescription. I.e., set one key without affecting the - // other keys. But in the meantime, just clear it out if - // it appears the incoming image was a maketx style texture. - - if ((desc.find ("SHA-1=") != std::string::npos) || - (desc.find ("ConstantColor=") != std::string::npos)) { - desc = ""; - } - - // The hash is only computed for the top mipmap level of pixel data. - // Thus, any additional information that will effect the lower levels - // (such as filtering information) needs to be manually added into the - // hash. - std::ostringstream addlHashData; - addlHashData << filter->name() << " "; - addlHashData << filter->width() << " "; - - std::string hash_digest = ImageBufAlgo::computePixelHashSHA1 (*toplevel, - addlHashData.str()); - if (hash_digest.length()) { - if (desc.length()) - desc += " "; - desc += "SHA-1="; - desc += hash_digest; - if (verbose) - std::cout << " SHA-1: " << hash_digest << std::endl; - updatedDesc = true; - dstspec.attribute ("oiio:SHA-1", hash_digest); - } - if (isConstantColor) { + if (incolorspace != outcolorspace) { + // do the full colorspace conversion for metadata, as there will + // be no further processing of this data. + convert_color (constantColor, incolorspace, outcolorspace); + } + std::ostringstream os; // Emulate a JSON array os << "["; for (unsigned int i=0; iname() << " "; + addlHashData << filter->width() << " "; + + std::string hash_digest = ImageBufAlgo::computePixelHashSHA1 (*toplevel, + addlHashData.str()); + if (hash_digest.length()) { + std::string desc = outspec.get_string_attribute ("ImageDescription"); + append_desc (desc, "SHA-1", hash_digest); + outspec.attribute ("ImageDescription", desc); + } + + // Write out the image Timer writetimer; if (! out->open (outputfilename.c_str(), outspec)) { std::cerr << "maketx ERROR: Could not open \"" << outputfilename @@ -1213,7 +1275,6 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, exit (EXIT_FAILURE); } - // Write out the image if (verbose) { std::cout << " Writing file: " << outputfilename << std::endl; std::cout << " Filter \"" << filter->name() << "\" width = " @@ -1222,7 +1283,7 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, } bool ok = true; - ok &= img.write (out); + ok &= toplevel->write (out); stat_writetime += writetimer(); if (mipmap) { // Mipmap levels: @@ -1278,10 +1339,13 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, smallspec.y = 0; smallspec.full_x = 0; smallspec.full_y = 0; - small->alloc (smallspec); // Realocate with new size + small->alloc (smallspec); // Reallocate with new size big->set_full (big->xbegin(), big->xend(), big->ybegin(), big->yend(), big->zbegin(), big->zend()); + if (verbose) + std::cout << " Resizing...\n" << std::flush; + if (filtername == "box" && filter->width() == 1.0f) parallel_image (resize_block, small, big, small->xbegin(), small->xend(), @@ -1300,6 +1364,15 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, if (envlatlmode && src_samples_border) fix_latl_edges (*small); + if (!incolorspace.empty() && !outcolorspace.empty()) { + if (outcolorspace != "linear") { + big->reset ("colorspace conversion", small->spec()); + convert_color (*small, "linear", *big, outcolorspace); + // big now points to linear small, and small points to cc'd small + std::swap (small, big); + } + } + Timer writetimer; // If the format explicitly supports MIP-maps, use that, // otherwise try to simulate MIP-mapping with multi-image. @@ -1316,7 +1389,9 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, if (verbose) { std::cout << " " << formatres(smallspec) << std::endl; } - std::swap (big, small); + + if (incolorspace.empty() && outcolorspace.empty()) + std::swap (big, small); } } From 485a1714049c041f7f043363bbd916e32b9e2684 Mon Sep 17 00:00:00 2001 From: parker Date: Sun, 17 Jun 2012 17:21:37 -0700 Subject: [PATCH 2/3] works on linux --- src/maketx/maketx.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/maketx/maketx.cpp b/src/maketx/maketx.cpp index 672ebcfadc..c5be856961 100644 --- a/src/maketx/maketx.cpp +++ b/src/maketx/maketx.cpp @@ -745,9 +745,9 @@ append_desc (std::string &desc, const std::string &key, { size_t key_pos = desc.find (key); if (key_pos != std::string::npos) { //found, replace - size_t val_pos = key_pos + key.length() + 1; - size_t end_pos = desc.find_first_of (' ', val_pos); - desc.replace (val_pos, end_pos, value); + size_t val_pos = key_pos + key.length() + 1; // + 1 -> '=' + size_t val_end = desc.find_first_of (' ', val_pos); + desc.replace (val_pos, val_end, value); } else { // not found, append if (desc.length()) @@ -1172,7 +1172,7 @@ make_texturemap (const char *maptypename = "texture map") } stat_resizetime += resizetimer(); - if (isConstantColor) { + if (isConstantColor && constant_color_detect) { if (incolorspace != outcolorspace) { // do the full colorspace conversion for metadata, as there will // be no further processing of this data. @@ -1241,7 +1241,7 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, ImageBuf *toplevel = &img; // Ptr to top level of mipmap - // convert to outcolorspace prior to hash computation + // Convert to outcolorspace prior to hash computation. ImageBuf ccDst; if (!incolorspace.empty() && !outcolorspace.empty()) { if (outcolorspace != "linear") { @@ -1366,6 +1366,7 @@ write_mipmap (ImageBuf &img, const ImageSpec &outspec_template, if (!incolorspace.empty() && !outcolorspace.empty()) { if (outcolorspace != "linear") { + // we're done with big ptr, so let's reuse it big->reset ("colorspace conversion", small->spec()); convert_color (*small, "linear", *big, outcolorspace); // big now points to linear small, and small points to cc'd small From 9a61924962f92644ca0b9a9c0ef4d7b47e7517b3 Mon Sep 17 00:00:00 2001 From: Stephen Parker Date: Sun, 17 Jun 2012 17:29:01 -0700 Subject: [PATCH 3/3] changed help for colorspace flag --- src/maketx/maketx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/maketx/maketx.cpp b/src/maketx/maketx.cpp index c5be856961..02e25a9c4f 100644 --- a/src/maketx/maketx.cpp +++ b/src/maketx/maketx.cpp @@ -284,7 +284,7 @@ getargs (int argc, char *argv[]) "--colorconvert %s %s", &incolorspace, &outcolorspace, colorconvert_help_string().c_str(), "--colorspace %s", &incolorspace, "Set input colorspace. " - "This is used for internal processing and does not affect the output colorspace.", + "Used for internal image processing and does not affect the output colorspace.", "--unpremult", &unpremult, "Unpremultiply before color conversion, then premultiply " "after the color conversion. You'll probably want to use this flag " "if your image contains an alpha channel.",