OpenGLFrameBuffer: Add a row order parameter for reading and writing pixels

This also fixes a bug where saving and restoring the framebuffer state
could unexpectedly apply a vertical flip to the buffer content.

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

modules/juce_opengl/opengl/juce_OpenGLFrameBuffer.cpp

@@ -58,6 +58,8 @@ public:
     {
         int width = 0, height = 0;
         std::vector<PixelARGB> 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<int>& area)
+    bool readPixels (PixelARGB* target, const Rectangle<int>& 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<int>& area)
+    bool writePixels (const PixelARGB* data, const Rectangle<int>& 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<PixelARGB> ((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<int>& sourceArea)
+bool OpenGLFrameBuffer::readPixels (PixelARGB* targetData, const Rectangle<int>& sourceArea, RowOrder order)
 {
-    return pimpl->readPixels (targetData, sourceArea);
+    return pimpl->readPixels (targetData, sourceArea, order);
 }
 
-bool OpenGLFrameBuffer::writePixels (const PixelARGB* srcData, const Rectangle<int>& targetArea)
+bool OpenGLFrameBuffer::writePixels (const PixelARGB* srcData, const Rectangle<int>& targetArea, RowOrder order)
 {
-    return pimpl->writePixels (srcData, targetArea);
+    return pimpl->writePixels (srcData, targetArea, order);
 }
 
 GLuint OpenGLFrameBuffer::getCurrentFrameBufferTarget() noexcept

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<int>& sourceArea);
+    bool readPixels (PixelARGB* targetData, const Rectangle<int>& 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<int>& targetArea);
+    bool writePixels (const PixelARGB* srcData, const Rectangle<int>& targetArea, RowOrder);
 
 private:
     class Pimpl;

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<int> getArea() const
@@ -134,6 +134,8 @@ private:
         HeapBlock<PixelARGB> data;
         Rectangle<int> area;
         Image::BitmapData::ReadWriteMode mode;
+
+        static constexpr auto order = OpenGLFrameBuffer::RowOrder::fromBottomUp;
     };
 
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (OpenGLFrameBufferImage)