diff --git a/BREAKING_CHANGES.md b/BREAKING_CHANGES.md index 226fef537b..d438458986 100644 --- a/BREAKING_CHANGES.md +++ b/BREAKING_CHANGES.md @@ -2,6 +2,34 @@ # develop +## Change + +The signatures of OpenGLFrameBuffer::readPixels() and +OpenGLFrameBuffer::writePixels() have changed, adding a new RowOrder parameter. + +**Possible Issues** + +Code that does not specify this parameter will not compile. + +**Workaround** + +Pass the extra parameter to specify whether the pixel data should be ordered +with the top-most or bottom-most row first. + +**Rationale** + +The previous function calls did not allow the pixel order to be configured. +readPixels() would return pixel data with the bottom-most row first (this is +convention for the OpenGL API), but writePixels() would expect the top-most row +first. This meant that reading and then immediately writing the same data would +have the unexpected effect of flipping the image. Changing readPixels() to +order pixels from top to bottom would be slightly dangerous, as it would +introduce a change of behaviour with no accompanying compiler warning. +Additionally, flipping the pixel storage introduces additional work that can be +safely skipped when the pixel data is going to be written back to the +framebuffer later. + + ## Change The behaviour of the default constructed FocusTraverser objects has changed, and diff --git a/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.cpp b/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.cpp index 94b6d7c33f..b824a6b164 100644 --- a/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.cpp +++ b/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.cpp @@ -58,6 +58,8 @@ public: { int width = 0, height = 0; std::vector data; + + static constexpr auto order = RowOrder::fromBottomUp; }; ~Pimpl() override @@ -97,7 +99,7 @@ public: Image::BitmapData bitmap (image, Image::BitmapData::readOnly); return initialise (context, bitmap.width, bitmap.height) - && writePixels ((const PixelARGB*) bitmap.data, image.getBounds()); + && writePixels ((const PixelARGB*) bitmap.data, image.getBounds(), RowOrder::fromTopDown); } bool initialise (OpenGLFrameBuffer& other) @@ -233,7 +235,7 @@ public: glClear (GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT); } - bool readPixels (PixelARGB* target, const Rectangle& area) + bool readPixels (PixelARGB* target, const Rectangle& area, RowOrder order) { auto* transientState = makeAndGetCurrentRenderingTarget(); @@ -246,10 +248,25 @@ public: glReadPixels (area.getX(), area.getY(), area.getWidth(), area.getHeight(), JUCE_RGBA_FORMAT, GL_UNSIGNED_BYTE, target); + if (order == RowOrder::fromTopDown) + { + auto* end = target + area.getWidth() * area.getHeight(); + + for (auto y = 0; y < area.getHeight() / 2; ++y) + { + const auto offset = area.getWidth() * y; + auto* rowA = target + offset; + auto* rowB = end - offset - area.getWidth(); + + for (auto x = 0; x < area.getWidth(); ++x) + std::swap (rowA[x], rowB[x]); + } + } + return true; } - bool writePixels (const PixelARGB* data, const Rectangle& area) + bool writePixels (const PixelARGB* data, const Rectangle& area, RowOrder order) { if (associatedContext == nullptr) return false; @@ -275,7 +292,7 @@ public: tex.getHeight()), transientState->width, transientState->height, - true, + order == RowOrder::fromTopDown, false); JUCE_CHECK_OPENGL_ERROR @@ -422,7 +439,9 @@ private: if (! initialise (context, savedState.width, savedState.height)) return false; - writePixels (savedState.data.data(), Rectangle (savedState.width, savedState.height)); + writePixels (savedState.data.data(), + Rectangle (savedState.width, savedState.height), + SavedState::order); return true; } @@ -432,7 +451,7 @@ private: area.getHeight(), std::vector ((size_t) area.getWidth() * (size_t) area.getHeight()) }; - if (! readPixels (result.data.data(), area)) + if (! readPixels (result.data.data(), area, SavedState::order)) return {}; return result; @@ -551,14 +570,14 @@ void OpenGLFrameBuffer::makeCurrentAndClear() pimpl->makeCurrentAndClear(); } -bool OpenGLFrameBuffer::readPixels (PixelARGB* targetData, const Rectangle& sourceArea) +bool OpenGLFrameBuffer::readPixels (PixelARGB* targetData, const Rectangle& sourceArea, RowOrder order) { - return pimpl->readPixels (targetData, sourceArea); + return pimpl->readPixels (targetData, sourceArea, order); } -bool OpenGLFrameBuffer::writePixels (const PixelARGB* srcData, const Rectangle& targetArea) +bool OpenGLFrameBuffer::writePixels (const PixelARGB* srcData, const Rectangle& targetArea, RowOrder order) { - return pimpl->writePixels (srcData, targetArea); + return pimpl->writePixels (srcData, targetArea, order); } GLuint OpenGLFrameBuffer::getCurrentFrameBufferTarget() noexcept diff --git a/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.h b/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.h index bab37b2ec9..7ad02c41f3 100644 --- a/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.h +++ b/modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.h @@ -116,17 +116,24 @@ public: /** Selects the framebuffer as the current target, and clears it to transparent. */ void makeCurrentAndClear(); + enum class RowOrder + { + fromBottomUp, //< Standard order for OpenGL, the bottom-most row of pixels is first. + //< Using this pixel ordering may be faster, but may be incompatible + //< with other JUCE functions that operate on image pixel data, as these + //< generally expect the rows to be ordered top-down. + fromTopDown, //< Standard order for JUCE images, the top-most row of pixels is first. + }; + /** Reads an area of pixels from the framebuffer into a 32-bit ARGB pixel array. - The lineStride is measured as a number of pixels, not bytes - pass a stride - of 0 to indicate a packed array. + The RowOrder parameter specifies the order of rows in the resulting array. */ - bool readPixels (PixelARGB* targetData, const Rectangle& sourceArea); + bool readPixels (PixelARGB* targetData, const Rectangle& sourceArea, RowOrder); /** Writes an area of pixels into the framebuffer from a specified pixel array. - The lineStride is measured as a number of pixels, not bytes - pass a stride - of 0 to indicate a packed array. + The RowOrder parameter specifies the order of rows in srcData. */ - bool writePixels (const PixelARGB* srcData, const Rectangle& targetArea); + bool writePixels (const PixelARGB* srcData, const Rectangle& targetArea, RowOrder); private: class Pimpl; diff --git a/modules/juce_opengl/opengl/juce_OpenGLImage.cpp b/modules/juce_opengl/opengl/juce_OpenGLImage.cpp index e36d77c5ca..f43af5b468 100644 --- a/modules/juce_opengl/opengl/juce_OpenGLImage.cpp +++ b/modules/juce_opengl/opengl/juce_OpenGLImage.cpp @@ -116,13 +116,13 @@ private: mode (modeIn) { if (mode != Image::BitmapData::writeOnly) - self->frameBuffer.readPixels (data.get(), getArea()); + self->frameBuffer.readPixels (data.get(), getArea(), order); } ~DataReleaser() override { if (mode != Image::BitmapData::readOnly) - self->frameBuffer.writePixels (data, getArea()); + self->frameBuffer.writePixels (data, getArea(), order); } Rectangle getArea() const @@ -134,6 +134,8 @@ private: HeapBlock data; Rectangle area; Image::BitmapData::ReadWriteMode mode; + + static constexpr auto order = OpenGLFrameBuffer::RowOrder::fromBottomUp; }; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (OpenGLFrameBufferImage)