Refactor and test clipping code (#673)

And allow using Gosu.clip_to within Gosu.render.

.github/workflows/ruby.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ${{ matrix.platform }}
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Install dependencies (Ubuntu)
         if: startsWith(matrix.platform, 'ubuntu-')

CHANGELOG.md

@@ -7,6 +7,7 @@
 * Fix a bug where invalid UTF-8 strings could trigger deadlocks or memory errors. (#652)  
 * Gosu now uses and requires C++20. Unfortunately, that means Ubuntu 20.04 has been dropped earlier than expected. (#647)
 * Make `Gosu::Window.sdl_window` available in Ruby. (#637)
+* `Gosu.clip_to` now works within `Gosu.render`. (#673) 
 
 ## [1.4.6] - 2023-05-20
 * When using SDL 2.0.12 or later, the LED indicators on gamepads will now be set to match the gamepad index that Gosu has allocated for them. (#639)

examples/Tutorial-Touch/Podfile.lock

@@ -1,8 +1,8 @@
 PODS:
-  - Gosu/Dependencies (1.4.6)
-  - Gosu/Gosu (1.4.6):
+  - Gosu/Dependencies (2.0.0)
+  - Gosu/Gosu (2.0.0):
     - Gosu/Dependencies
-  - Gosu/GosuAppDelegateMain (1.4.6):
+  - Gosu/GosuAppDelegateMain (2.0.0):
     - Gosu/Gosu
 
 DEPENDENCIES:
@@ -13,7 +13,7 @@ EXTERNAL SOURCES:
     :path: "../.."
 
 SPEC CHECKSUMS:
-  Gosu: 992a2da502347876945f8f20a3932dc535756c49
+  Gosu: 279b6cbd7a4a6330aabe2b493fdfd883937c0f07
 
 PODFILE CHECKSUM: 1419de9bd0dc158f93d2a5639d546afb03b24668
 

examples/Tutorial/Podfile.lock

@@ -1,8 +1,8 @@
 PODS:
-  - Gosu (1.4.6):
-    - Gosu/Gosu (= 1.4.6)
-  - Gosu/Dependencies (1.4.6)
-  - Gosu/Gosu (1.4.6):
+  - Gosu (2.0.0):
+    - Gosu/Gosu (= 2.0.0)
+  - Gosu/Dependencies (2.0.0)
+  - Gosu/Gosu (2.0.0):
     - Gosu/Dependencies
 
 DEPENDENCIES:
@@ -13,7 +13,7 @@ EXTERNAL SOURCES:
     :path: "../.."
 
 SPEC CHECKSUMS:
-  Gosu: 992a2da502347876945f8f20a3932dc535756c49
+  Gosu: 279b6cbd7a4a6330aabe2b493fdfd883937c0f07
 
 PODFILE CHECKSUM: f42ffeb4ade4182085f7b3b553af6c38c355f08d
 

src/BinPacker.cpp

@@ -136,7 +136,7 @@ void Gosu::BinPacker::merge_neighbors(int index)
     //   ┃ ┣━━┻━┫
     //   ┗━┻━━━━┛
     // In this case, the texture gets stuck in this fragmented state. We assume that this is not an
-    // issue in practice, just like memory
+    // issue in practice, just like RAM fragmentation has never been problematic for us.
 
     // Merge any of the other rectangles in the list into the one with the given index if they share
     // any of their four sides.

src/ClipRectStack.cpp

@@ -0,0 +1,38 @@
+#include "ClipRectStack.hpp"
+#include <stdexcept>
+
+void Gosu::ClipRectStack::clear()
+{
+    m_stack.clear();
+    m_effective_rect = std::nullopt;
+}
+
+void Gosu::ClipRectStack::push(const Rect& rect)
+{
+    m_stack.push_back(rect);
+    if (m_effective_rect) {
+        m_effective_rect->clip_to(rect);
+    }
+    else {
+        m_effective_rect = rect;
+    }
+}
+
+void Gosu::ClipRectStack::pop()
+{
+    if (m_stack.empty()) {
+        throw std::logic_error("ClipRectStack is empty");
+    }
+    m_stack.pop_back();
+
+    // The clip rect is the intersection of all active clip rects (if any).
+    m_effective_rect = std::nullopt;
+    for (const auto& rect : m_stack) {
+        if (m_effective_rect) {
+            m_effective_rect->clip_to(rect);
+        }
+        else {
+            m_effective_rect = rect;
+        }
+    }
+}

src/ClipRectStack.hpp

@@ -1,90 +1,24 @@
 #pragma once
 
+#include <Gosu/Utility.hpp>
 #include "GraphicsImpl.hpp"
-#include <cassert>
+#include <limits>
+#include <optional>
+#include <stdexcept>
 #include <vector>
 
-class Gosu::ClipRectStack
+namespace Gosu
 {
-    std::vector<ClipRect> stack;
-    bool has_effective_rect; // is effective_rect valid?
-    ClipRect effective_rect;
-
-    void update_effective_rect()
+    class ClipRectStack
     {
-        // Nothing to do, no clipping in place.
-        if (stack.empty()) {
-            has_effective_rect = false;
-            return;
-        }
-
-        ClipRect result = { 0.0, 0.0, 1e10, 1e10 };
-        for (std::size_t i = 0, end = stack.size(); i < end; ++i) {
-            const ClipRect& rect = stack[i];
-            int result_right     = std::min(result.x + result.width, rect.x + rect.width);
-            int result_bottom    = std::min(result.y + result.height, rect.y + rect.height);
-            result.x             = std::max(result.x, rect.x);
-            result.y             = std::max(result.y, rect.y);
-
-            if (result.x >= result_right || result.y >= result_bottom) {
-                // We have clipped the world away!
-                has_effective_rect = false;
-                return;
-            }
-
-            result.width  = result_right - result.x;
-            result.height = result_bottom - result.y;
-        }
-
-        // On the iPhone, we may have to multiply everything by 2 for Retina displays.
-        // TODO: Doesn't this affect Retina Macs as well?
-        // TODO: This should be handled by a global transform.
-        int fac = clip_rect_base_factor();
-        result.x *= fac;
-        result.y *= fac;
-        result.width *= fac;
-        result.height *= fac;
-
-        // Normal clipping.
-        effective_rect     = result;
-        has_effective_rect = true;
-    }
-
-public:
-    ClipRectStack()
-    : has_effective_rect(false)
-    {
-    }
-
-    void clear()
-    {
-        stack.clear();
-        has_effective_rect = false;
-    }
-
-    void begin_clipping(double x, double y, double width, double height)
-    {
-        ClipRect rect = { x, y, width, height };
-        stack.push_back(rect);
-        update_effective_rect();
-    }
-
-    void end_clipping()
-    {
-        assert (!stack.empty());
-        stack.pop_back();
-        update_effective_rect();
-    }
-
-    const ClipRect* maybe_effective_rect() const
-    {
-        return has_effective_rect ? &effective_rect : 0;
-    }
-
-    bool clipped_world_away() const
-    {
-        // When we have no effective rect but the stack is not empty, we have clipped
-        // the whole world away and don't need to render things.
-        return !has_effective_rect && !stack.empty();
-    }
-};
+        std::vector<Rect> m_stack;
+        std::optional<Rect> m_effective_rect;
+
+    public:
+        void clear();
+        void push(const Rect& rect);
+        void pop();
+
+        const std::optional<Rect>& effective_rect() const { return m_effective_rect; }
+    };
+}

src/DrawOp.hpp

@@ -168,10 +168,5 @@ namespace Gosu
 
             vas.back().vertices.insert(vas.back().vertices.end(), result, result + 4);
         }
-
-        bool operator<(const DrawOp& other) const
-        {
-            return z < other.z;
-        }
     };
 }

src/DrawOpQueue.hpp

@@ -9,6 +9,7 @@
 #include <cmath>
 #include <functional>
 #include <map>
+#include <utility>
 #include <vector>
 
 class Gosu::DrawOpQueue
@@ -22,7 +23,7 @@ class Gosu::DrawOpQueue
     std::vector<std::function<void ()>> gl_blocks;
 
 public:
-    DrawOpQueue(QueueMode mode)
+    explicit DrawOpQueue(QueueMode mode)
     : queue_mode(mode)
     {
     }
@@ -34,39 +35,31 @@ class Gosu::DrawOpQueue
 
     void schedule_draw_op(DrawOp op)
     {
-        if (clip_rect_stack.clipped_world_away()) return;
-
-    #ifdef GOSU_IS_OPENGLES
+#ifdef GOSU_IS_OPENGLES
         // No triangles, no lines supported
-        assert (op.vertices_or_block_index == 4);
-    #endif
+        assert(op.vertices_or_block_index == 4);
+#endif
 
         op.render_state.transform = &transform_stack.current();
-        if (const ClipRect* cr = clip_rect_stack.maybe_effective_rect()) {
-            op.render_state.clip_rect = *cr;
-        }
+        op.render_state.clip_rect = clip_rect_stack.effective_rect();
         ops.push_back(op);
     }
 
     void gl(std::function<void ()> gl_block, ZPos z)
     {
-        // TODO: Document this case: Clipped-away GL blocks are *not* being run.
-        if (clip_rect_stack.clipped_world_away()) return;
-
         int complement_of_block_index = ~(int)gl_blocks.size();
-        gl_blocks.push_back(gl_block);
+        gl_blocks.push_back(std::move(gl_block));
 
         DrawOp op;
         op.vertices_or_block_index = complement_of_block_index;
         op.render_state.transform = &transform_stack.current();
-        if (const ClipRect* cr = clip_rect_stack.maybe_effective_rect()) {
-            op.render_state.clip_rect = *cr;
-        }
+        op.render_state.clip_rect = clip_rect_stack.effective_rect();
         op.z = z;
         ops.push_back(op);
     }
 
-    void begin_clipping(double x, double y, double width, double height, double screen_height)
+    void begin_clipping(double x, double y, double width, double height,
+                        std::optional<int> viewport_height)
     {
         if (mode() == QM_RECORD_MACRO) {
             throw std::logic_error("Clipping is not allowed while creating a macro");
@@ -80,21 +73,21 @@ class Gosu::DrawOpQueue
         transform_stack.current().apply(left, top);
         transform_stack.current().apply(right, bottom);
 
-        double phys_x      = std::min(left, right);
-        double phys_y      = std::min(top, bottom);
-        double phys_width  = std::abs(left - right);
-        double phys_height = std::abs(top - bottom);
-
+        Rect clip_rect {
+            .x = static_cast<int>(std::min(left, right)),
+            .y = static_cast<int>(std::min(top, bottom)),
+            .width = static_cast<int>(std::abs(left - right)),
+            .height = static_cast<int>(std::abs(top - bottom)),
+        };
         // Adjust for OpenGL having the wrong idea of where y=0 is.
-        phys_y = screen_height - phys_y - phys_height;
+        if (viewport_height) {
+            clip_rect.y = *viewport_height - clip_rect.y - clip_rect.height;
+        }
 
-        clip_rect_stack.begin_clipping(phys_x, phys_y, phys_width, phys_height);
+        clip_rect_stack.push(clip_rect);
     }
 
-    void end_clipping()
-    {
-        clip_rect_stack.end_clipping();
-    }
+    void end_clipping() { clip_rect_stack.pop(); }
 
     void set_base_transform(const Transform& base_transform)
     {
@@ -118,7 +111,8 @@ class Gosu::DrawOpQueue
         }
 
         // Apply Z-Ordering.
-        std::stable_sort(ops.begin(), ops.end());
+        std::stable_sort(ops.begin(), ops.end(),
+                         [](const DrawOp& lhs, const DrawOp& rhs) { return lhs.z < rhs.z; });
 
         RenderStateManager manager;
 
@@ -156,7 +150,7 @@ class Gosu::DrawOpQueue
             throw std::logic_error("Custom OpenGL code cannot be recorded as a macro");
         }
 
-        std::stable_sort(ops.begin(), ops.end());
+        std::stable_sort(ops.begin(), ops.end(), [](const DrawOp& lhs, const DrawOp& rhs) { return lhs.z < rhs.z; });
         for (const auto& op : ops) {
             op.compile_to(vas);
         }

src/GosuGLView.cpp

@@ -28,12 +28,6 @@ namespace Gosu
     {
         // Don't bother unsetting anything.
     }
-
-    int clip_rect_base_factor()
-    {
-        static int result = [UIScreen mainScreen].scale;
-        return result;
-    }
 }
 
 @implementation GosuGLView
@@ -97,7 +91,7 @@ namespace Gosu
     [EAGLContext setCurrentContext:_context];
     [self destroyFramebuffer];
     [self createFramebuffer];
-    self.contentScaleFactor = Gosu::clip_rect_base_factor();
+    self.contentScaleFactor = [UIScreen mainScreen].scale;
 }
 
 - (BOOL)createFramebuffer

src/Graphics.cpp

@@ -247,17 +247,20 @@ void Gosu::gl(Gosu::ZPos z, const std::function<void()>& f)
 #endif
 }
 
-void Gosu::clip_to(double x, double y, double width, double height,
-                             const std::function<void()>& f)
+void Gosu::clip_to(double x, double y, double width, double height, const std::function<void()>& f)
 {
-    double screen_height = current_viewport().m_impl->phys_height;
-    current_queue().begin_clipping(x, y, width, height, screen_height);
+    std::optional<int> viewport_height;
+    if (current_viewport_pointer) {
+        viewport_height = current_viewport_pointer->m_impl->phys_height;
+    }
+
+    current_queue().begin_clipping(x, y, width, height, viewport_height);
     f();
     current_queue().end_clipping();
 }
 
 Gosu::Image Gosu::render(int width, int height, const std::function<void()>& f,
-                                   unsigned image_flags)
+                         unsigned image_flags)
 {
     const OpenGLContext current_context;