From 07c34ee833cf6ad99d84d4ea5b6aebe7961a4da4 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 9 Aug 2018 17:30:17 -0700 Subject: [PATCH] Simplify Image by flipping RADIANCE images. (#63) * Simplify Image by flipping RADIANCE images. Unit test for the new flip() method is forthcoming. * Optimize image flipping. --- libs/image/include/image/Image.h | 16 +-------------- libs/image/src/Image.cpp | 33 ++++++++++++++++++++++++++----- libs/imageio/src/ImageDecoder.cpp | 12 +++++------ libs/imageio/src/ImageEncoder.cpp | 8 ++------ tools/cmgen/src/Cubemap.cpp | 8 ++++---- tools/cmgen/src/Cubemap.h | 9 +-------- tools/cmgen/src/CubemapUtils.cpp | 4 ++-- tools/cmgen/src/cmgen.cpp | 5 ++++- 8 files changed, 47 insertions(+), 48 deletions(-) diff --git a/libs/image/include/image/Image.h b/libs/image/include/image/Image.h index e718d3f3aa..d6609dc5c3 100644 --- a/libs/image/include/image/Image.h +++ b/libs/image/include/image/Image.h @@ -44,7 +44,7 @@ public: void subset(Image const& image, size_t x, size_t y, size_t w, size_t h, uint32_t flags = 0); - void setFlags(uint32_t flags); + void flip(uint32_t flags); bool isValid() const { return mData != nullptr; } size_t getWidth() const { return mWidth; } @@ -52,13 +52,10 @@ public: size_t getBytesPerRow() const { return mBpr; } size_t getBytesPerPixel() const { return mBpp; } size_t getChannelsCount() const { return mChannels; } - uint32_t getFlags() const { return mFlags; } void* getData() const { return mData; } void* getPixelRef(size_t x, size_t y) const; - void* getSampleRef(size_t x, size_t y) const; - private: std::unique_ptr mOwnedData; void* mData = nullptr; @@ -67,23 +64,12 @@ private: size_t mBpr = 0; size_t mBpp = 0; size_t mChannels = 0; - uint32_t mFlags = 0; }; inline void* Image::getPixelRef(size_t x, size_t y) const { return static_cast(mData) + y*mBpr + x*mBpp; } -inline void* Image::getSampleRef(size_t x, size_t y) const { - if (mFlags & FLIP_X) { - x = (mWidth-1)-x; - } - if (mFlags & FLIP_Y) { - y = (mHeight-1)-y; - } - return getPixelRef(x, y); -} - } // namespace image #endif /* IMAGE_IMAGE_H_ */ diff --git a/libs/image/src/Image.cpp b/libs/image/src/Image.cpp index e4d6b7e4b3..9ad7bfda12 100644 --- a/libs/image/src/Image.cpp +++ b/libs/image/src/Image.cpp @@ -42,7 +42,6 @@ void Image::reset() { mBpr = 0; mBpp = 0; mChannels = 0; - mFlags = 0; mData = nullptr; } @@ -53,7 +52,6 @@ void Image::set(Image const& image) { mBpr = image.mBpr; mBpp = image.mBpp; mChannels = image.mChannels; - mFlags ^= image.mFlags & FLIP_XY; mData = image.mData; } @@ -62,15 +60,40 @@ void Image::subset(Image const& image, mOwnedData.release(); mWidth = w; mHeight = h; - mFlags ^= flags & FLIP_XY; mBpr = image.mBpr; mBpp = image.mBpp; mChannels = image.mChannels; mData = static_cast(image.getPixelRef(x, y)); } -void Image::setFlags(uint32_t flags) { - mFlags = flags; +void Image::flip(uint32_t flags) { + // We verified that these lambdas get inlined by clang in release builds. + auto getptr = [this](size_t x, size_t y) -> uint8_t* { + return static_cast(mData) + y*mBpr + x*mBpp; + }; + auto getref = [this, getptr](size_t x, size_t y) -> uint8_t& { + return *(getptr(x, y)); + }; + if (flags & Image::FLIP_Y) { + std::unique_ptr tmp(new uint8_t[mBpr]); + uint8_t* ptmp = tmp.get(); + for (size_t row = 0, nrows = mHeight / 2; row < nrows; ++row) { + uint8_t* a = getptr(0, row); + uint8_t* b = getptr(0, mHeight - 1 - row); + memcpy(ptmp, a, mBpr); + memcpy(a, b, mBpr); + memcpy(b, ptmp, mBpr); + } + } + // Our hflip implementation is inefficient, but it's never invoked in the renderer. + if (flags & Image::FLIP_X) { + for (size_t row = 0, nrows = mHeight; row < nrows; ++row) { + for (size_t src = 0, ncols = mWidth / 2; src < ncols; ++src) { + size_t dst = mWidth - 1 - src; + std::swap(getref(src, row), getref(dst, row)); + } + } + } } } // namespace image diff --git a/libs/imageio/src/ImageDecoder.cpp b/libs/imageio/src/ImageDecoder.cpp index b8d6bd6e1b..e7265300ac 100644 --- a/libs/imageio/src/ImageDecoder.cpp +++ b/libs/imageio/src/ImageDecoder.cpp @@ -364,15 +364,13 @@ Image HDRDecoder::decode() { } while (true); } - uint32_t flags = 0; - if (sx == '-') flags |= Image::FLIP_X; - - // Experimentally, "-Y" means vertical origin at the top, "+Y" at the bottom - if (sy == '+') flags |= Image::FLIP_Y; - std::unique_ptr data(new uint8_t[width * height * sizeof(math::float3)]); Image image(std::move(data), width, height, width*sizeof(math::float3), sizeof(math::float3)); - image.setFlags(flags); + + uint32_t flags = 0; + if (sx == '-') flags |= Image::FLIP_X; + if (sy == '+') flags |= Image::FLIP_Y; + image.flip(flags); uint16_t w; uint16_t magic; diff --git a/libs/imageio/src/ImageEncoder.cpp b/libs/imageio/src/ImageEncoder.cpp index 752992cd5f..0937dff6bb 100644 --- a/libs/imageio/src/ImageEncoder.cpp +++ b/libs/imageio/src/ImageEncoder.cpp @@ -475,18 +475,14 @@ void HDREncoder::encode(const Image& image) { size_t width = image.getWidth(); size_t height = image.getHeight(); - char sx = (image.getFlags() & Image::FLIP_X) ? '-' : '+'; - // Experimentally, "-Y" means vertical origin at the top, "+Y" at the bottom - char sy = (image.getFlags() & Image::FLIP_Y) ? '+' : '-'; - mStream << "#?RADIANCE" << std::endl; mStream << "# cmgen" << std::endl; mStream << "FORMAT=32-bit_rle_rgbe" << std::endl; mStream << "GAMMA=" << std::to_string(1) << std::endl; mStream << "EXPOSURE=" << std::to_string(0) << std::endl; mStream << std::endl; - mStream << sy << "Y " << std::to_string(height) << " " - << sx << "X " << std::to_string(width) << std::endl; + mStream << "-Y " << std::to_string(height) << " " + << "+X " << std::to_string(width) << std::endl; std::unique_ptr rgbe(new uint8_t[width*4]); uint8_t* const r = &rgbe[0]; diff --git a/tools/cmgen/src/Cubemap.cpp b/tools/cmgen/src/Cubemap.cpp index bf2d396291..88129ece56 100644 --- a/tools/cmgen/src/Cubemap.cpp +++ b/tools/cmgen/src/Cubemap.cpp @@ -156,10 +156,10 @@ Cubemap::Texel Cubemap::filterAt(const image::Image& image, double x, double y) const float v = float(y - y0); const float one_minus_u = 1 - u; const float one_minus_v = 1 - v; - const Texel& c0 = sampleAt(image.getSampleRef(x0, y0)); - const Texel& c1 = sampleAt(image.getSampleRef(x1, y0)); - const Texel& c2 = sampleAt(image.getSampleRef(x0, y1)); - const Texel& c3 = sampleAt(image.getSampleRef(x1, y1)); + const Texel& c0 = sampleAt(image.getPixelRef(x0, y0)); + const Texel& c1 = sampleAt(image.getPixelRef(x1, y0)); + const Texel& c2 = sampleAt(image.getPixelRef(x0, y1)); + const Texel& c3 = sampleAt(image.getPixelRef(x1, y1)); return (one_minus_u*one_minus_v)*c0 + (u*one_minus_v)*c1 + (one_minus_u*v)*c2 + (u*v)*c3; } diff --git a/tools/cmgen/src/Cubemap.h b/tools/cmgen/src/Cubemap.h index d1d741b777..5a0c465458 100644 --- a/tools/cmgen/src/Cubemap.h +++ b/tools/cmgen/src/Cubemap.h @@ -123,13 +123,6 @@ inline math::double3 Cubemap::getDirectionFor(Face face, double x, double y) con double cx = (x * mScale) - 1; double cy = 1 - (y * mScale); - uint32_t flags = getImageForFace(face).getFlags(); - if (flags & image::Image::FLIP_X) { - cx = -cx; - } - if (flags & image::Image::FLIP_Y) { - cy = -cy; - } math::double3 dir; const double l = std::sqrt(cx*cx + cy*cy + 1); switch (face) { @@ -147,7 +140,7 @@ inline Cubemap::Texel const& Cubemap::sampleAt(const math::double3& direction) c Cubemap::Address addr(getAddressFor(direction)); const size_t x = std::min(size_t(addr.s * mDimensions), mDimensions-1); const size_t y = std::min(size_t(addr.t * mDimensions), mDimensions-1); - return sampleAt(getImageForFace(addr.face).getSampleRef(x, y)); + return sampleAt(getImageForFace(addr.face).getPixelRef(x, y)); } inline Cubemap::Texel Cubemap::filterAt(const math::double3& direction) const { diff --git a/tools/cmgen/src/CubemapUtils.cpp b/tools/cmgen/src/CubemapUtils.cpp index fe376fda31..0f1bd6ec12 100644 --- a/tools/cmgen/src/CubemapUtils.cpp +++ b/tools/cmgen/src/CubemapUtils.cpp @@ -34,7 +34,7 @@ void CubemapUtils::clamp(image::Image& src) { const size_t height = src.getHeight(); for (size_t y=0 ; y(src.getSampleRef(x, y)); + float3& c = *static_cast(src.getPixelRef(x, y)); c.x = std::min(c.x, 256.0f); c.y = std::min(c.y, 256.0f); c.z = std::min(c.z, 256.0f); @@ -78,7 +78,7 @@ void CubemapUtils::equirectangularToCubemap(Cubemap& dst, const image::Image& sr yf = (yf + 1) * 0.5f * (height-1); // range [0, height[ // we can't use filterAt() here because it reads past the width/height // which is okay for cubmaps but not for square images - c += Cubemap::sampleAt(src.getSampleRef((uint32_t)xf, (uint32_t)yf)); + c += Cubemap::sampleAt(src.getPixelRef((uint32_t)xf, (uint32_t)yf)); } c *= iNumSamples; Cubemap::writeAt(data, c); diff --git a/tools/cmgen/src/cmgen.cpp b/tools/cmgen/src/cmgen.cpp index 41f3295c46..2c30a1763b 100644 --- a/tools/cmgen/src/cmgen.cpp +++ b/tools/cmgen/src/cmgen.cpp @@ -98,7 +98,10 @@ void generateUVGrid(Cubemap const& cml, size_t gridFrequency, size_t dim); static void printUsage(char* name) { std::string exec_name(utils::Path(name).getName()); std::string usage( - "CMGEN is a command-line tool for generating SH and mipmap levels from a cubemap\n" + "CMGEN is a command-line tool for generating SH and mipmap levels from an env map.\n" + "Cubemaps and equirectangular formats are both supported, automatically detected \n" + "according to the aspect ratio of the source image.\n" + "\n" "Usages:\n" " CMGEN [options] \n" " CMGEN [options] \n"