GPUState: Move Scissor and Viewport state to framebuffer

This way it is way clearer what each viewport state is. There is
no more save and reset. The scissor test is also saved per
framebuffer.

The only rule to remember is that the viewport state (size and
origin) is reset for both the viewport and scissor when a texture
is attached or detached from an attachment slot.

4 files changed, 84 insertions, 55 deletions

diff --git a/source/blender/gpu/intern/gpu_framebuffer.cc b/source/blender/gpu/intern/gpu_framebuffer.cc
index 600dd129aef..f4b8a4040d4 100644
--- a/source/blender/gpu/intern/gpu_framebuffer.cc
+++ b/source/blender/gpu/intern/gpu_framebuffer.cc
@@ -55,6 +55,7 @@ FrameBuffer::FrameBuffer(const char *name)
   }
   /* Force config on first use. */
   dirty_attachments_ = true;
+  dirty_state_ = true;
 
   for (int i = 0; i < ARRAY_SIZE(attachments_); i++) {
     attachments_[i].tex = NULL;
@@ -341,21 +342,30 @@ void GPU_framebuffer_config_array(GPUFrameBuffer *gpu_fb,
   }
 }
 
-/* ---------- Framebuffer Operations ----------- */
+/* ---------- Viewport & Scissor Region ----------- */
 
-#define CHECK_FRAMEBUFFER_IS_BOUND(_fb) \
-  BLI_assert(GPU_framebuffer_bound(_fb)); \
-  UNUSED_VARS_NDEBUG(_fb); \
-  ((void)0)
+/* Viewport and scissor size is stored per framebuffer.
+ * It is only reset to its original dimensions explicitely OR when binding the framebuffer after
+ * modifiying its attachments. */
+void GPU_framebuffer_viewport_set(GPUFrameBuffer *gpu_fb, int x, int y, int width, int height)
+{
+  int viewport_rect[4] = {x, y, width, height};
+  reinterpret_cast<FrameBuffer *>(gpu_fb)->viewport_set(viewport_rect);
+}
 
-/* Needs to be done after binding. */
-void GPU_framebuffer_viewport_set(GPUFrameBuffer *gpu_fb, int x, int y, int w, int h)
+void GPU_framebuffer_viewport_get(GPUFrameBuffer *gpu_fb, int r_viewport[4])
 {
-  CHECK_FRAMEBUFFER_IS_BOUND(gpu_fb);
+  reinterpret_cast<FrameBuffer *>(gpu_fb)->viewport_get(r_viewport);
+}
 
-  GPU_viewport(x, y, w, h);
+/* Reset to its attachement(s) size. */
+void GPU_framebuffer_viewport_reset(GPUFrameBuffer *gpu_fb)
+{
+  reinterpret_cast<FrameBuffer *>(gpu_fb)->viewport_reset();
 }
 
+/* ---------- Framebuffer Operations ----------- */
+
 void GPU_framebuffer_clear(GPUFrameBuffer *gpu_fb,
                            eGPUFrameBufferBits buffers,
                            const float clear_col[4],
@@ -582,19 +592,14 @@ GPUOffScreen *GPU_offscreen_create(
     return NULL;
   }
 
-  int viewport[4];
-  GPU_viewport_size_get_i(viewport);
-
   GPUFrameBuffer *fb = gpu_offscreen_fb_get(ofs);
 
   /* check validity at the very end! */
   if (!GPU_framebuffer_check_valid(fb, err_out)) {
     GPU_offscreen_free(ofs);
-    GPU_viewport(UNPACK4(viewport));
     return NULL;
   }
   GPU_framebuffer_restore();
-  GPU_viewport(UNPACK4(viewport));
   return ofs;
 }
 
diff --git a/source/blender/gpu/intern/gpu_framebuffer_private.hh b/source/blender/gpu/intern/gpu_framebuffer_private.hh
index a34fe38a267..3fba0c8de92 100644
--- a/source/blender/gpu/intern/gpu_framebuffer_private.hh
+++ b/source/blender/gpu/intern/gpu_framebuffer_private.hh
@@ -30,6 +30,7 @@
 
 #pragma once
 
+#include "BLI_math_vector.h"
 #include "BLI_span.hh"
 
 #include "MEM_guardedalloc.h"
@@ -97,6 +98,11 @@ class FrameBuffer {
   int width_, height_;
   /** Debug name. */
   char name_[DEBUG_NAME_LEN];
+  /** Framebuffer state. */
+  int viewport_[4];
+  int scissor_[4];
+  bool scissor_test_ = false;
+  bool dirty_state_;
 
  public:
   FrameBuffer(const char *name);
@@ -134,6 +140,55 @@ class FrameBuffer {
   {
     width_ = width;
     height_ = height;
+    dirty_state_ = true;
+  }
+
+  inline void viewport_set(const int viewport[4])
+  {
+    if (!equals_v4v4_int(viewport_, viewport)) {
+      copy_v4_v4_int(viewport_, viewport);
+      dirty_state_ = true;
+    }
+  }
+
+  inline void scissor_set(const int scissor[4])
+  {
+    if (!equals_v4v4_int(scissor_, scissor)) {
+      copy_v4_v4_int(scissor_, scissor);
+      dirty_state_ = true;
+    }
+  }
+
+  inline void scissor_test_set(bool test)
+  {
+    scissor_test_ = test;
+  }
+
+  inline void viewport_get(int r_viewport[4]) const
+  {
+    copy_v4_v4_int(r_viewport, viewport_);
+  }
+
+  inline void scissor_get(int r_scissor[4]) const
+  {
+    copy_v4_v4_int(r_scissor, scissor_);
+  }
+
+  inline bool scissor_test_get(void) const
+  {
+    return scissor_test_;
+  }
+
+  inline void viewport_reset(void)
+  {
+    int viewport_rect[4] = {0, 0, width_, height_};
+    viewport_set(viewport_rect);
+  }
+
+  inline void scissor_reset(void)
+  {
+    int scissor_rect[4] = {0, 0, width_, height_};
+    scissor_set(scissor_rect);
   }
 
   inline GPUTexture *depth_tex(void) const
diff --git a/source/blender/gpu/intern/gpu_state.cc b/source/blender/gpu/intern/gpu_state.cc
index fcbaa500e2d..478fd639cdd 100644
--- a/source/blender/gpu/intern/gpu_state.cc
+++ b/source/blender/gpu/intern/gpu_state.cc
@@ -191,27 +191,19 @@ void GPU_program_point_size(bool enable)
 
 void GPU_scissor_test(bool enable)
 {
-  GPUStateManager *stack = GPU_context_active_get()->state_manager;
-  auto &state = stack->mutable_state;
-  /* Set point size sign negative to disable. */
-  state.scissor_rect[2] = abs(state.scissor_rect[2]) * (enable ? 1 : -1);
+  GPU_context_active_get()->active_fb->scissor_test_set(enable);
 }
 
 void GPU_scissor(int x, int y, int width, int height)
 {
-  GPUStateManager *stack = GPU_context_active_get()->state_manager;
-  auto &state = stack->mutable_state;
-  bool enabled = state.scissor_rect[2] > 0;
-  int scissor_rect[4] = {x, y, enabled ? width : -width, height};
-  copy_v4_v4_int(state.scissor_rect, scissor_rect);
+  int scissor_rect[4] = {x, y, width, height};
+  GPU_context_active_get()->active_fb->scissor_set(scissor_rect);
 }
 
 void GPU_viewport(int x, int y, int width, int height)
 {
-  GPUStateManager *stack = GPU_context_active_get()->state_manager;
-  auto &state = stack->mutable_state;
   int viewport_rect[4] = {x, y, width, height};
-  copy_v4_v4_int(state.viewport_rect, viewport_rect);
+  GPU_context_active_get()->active_fb->viewport_set(viewport_rect);
 }
 
 void GPU_stencil_reference_set(uint reference)
@@ -267,22 +259,21 @@ eGPUStencilTest GPU_stencil_test_get()
 
 void GPU_scissor_get(int coords[4])
 {
-  GPUStateMutable &state = GPU_context_active_get()->state_manager->mutable_state;
-  copy_v4_v4_int(coords, state.scissor_rect);
+  GPU_context_active_get()->active_fb->scissor_get(coords);
 }
 
 void GPU_viewport_size_get_f(float coords[4])
 {
-  GPUStateMutable &state = GPU_context_active_get()->state_manager->mutable_state;
+  int viewport[4];
+  GPU_context_active_get()->active_fb->viewport_get(viewport);
   for (int i = 0; i < 4; i++) {
-    coords[i] = state.viewport_rect[i];
+    coords[i] = viewport[i];
   }
 }
 
 void GPU_viewport_size_get_i(int coords[4])
 {
-  GPUStateMutable &state = GPU_context_active_get()->state_manager->mutable_state;
-  copy_v4_v4_int(coords, state.viewport_rect);
+  GPU_context_active_get()->active_fb->viewport_get(coords);
 }
 
 bool GPU_depth_mask_get(void)
@@ -345,16 +336,6 @@ GPUStateManager::GPUStateManager(void)
   state.polygon_smooth = false;
   state.clip_distances = 0;
 
-  /* TODO: We should have better default for viewport and scissors.
-   * For now it's not important since they are overwritten at soon as a framebuffer is bound. */
-  mutable_state.viewport_rect[0] = 0;
-  mutable_state.viewport_rect[1] = 0;
-  mutable_state.viewport_rect[2] = 10;
-  mutable_state.viewport_rect[3] = 10;
-  mutable_state.scissor_rect[0] = 0;
-  mutable_state.scissor_rect[1] = 0;
-  mutable_state.scissor_rect[2] = -10; /* Disable */
-  mutable_state.scissor_rect[3] = 10;
   mutable_state.depth_range[0] = 0.0f;
   mutable_state.depth_range[1] = 1.0f;
   mutable_state.point_size = 1.0f;
diff --git a/source/blender/gpu/intern/gpu_state_private.hh b/source/blender/gpu/intern/gpu_state_private.hh
index a1bfefbaff5..61234c4612c 100644
--- a/source/blender/gpu/intern/gpu_state_private.hh
+++ b/source/blender/gpu/intern/gpu_state_private.hh
@@ -94,11 +94,6 @@ inline GPUState operator~(const GPUState &a)
 union GPUStateMutable {
   struct {
     /* Viewport State */
-    /** TODO put inside GPUFramebuffer. */
-    /** Offset + Extent of the drawable region inside the framebuffer. */
-    int viewport_rect[4];
-    /** Offset + Extent of the scissor region inside the framebuffer. */
-    int scissor_rect[4];
     /** TODO remove */
     float depth_range[2];
     /** TODO remove, use explicit clear calls. */
@@ -164,14 +159,7 @@ class GPUStateManager {
   GPUStateManager();
   virtual ~GPUStateManager(){};
 
-  virtual void set_state(const GPUState &state) = 0;
-  virtual void set_mutable_state(const GPUStateMutable &state) = 0;
-
-  inline void apply_state(void)
-  {
-    this->set_state(this->state);
-    this->set_mutable_state(this->mutable_state);
-  };
+  virtual void apply_state(void) = 0;
 };
 
 }  // namespace gpu