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.

source/blender/draw/engines/eevee/eevee_lightprobes.c

@@ -1080,10 +1080,12 @@ void EEVEE_lightbake_filter_glossy(EEVEE_ViewLayerData *sldata,
                            log(2);
     pinfo->firefly_fac = (firefly_fac > 0.0) ? firefly_fac : 1e16;
 
-    GPU_framebuffer_ensure_config(
-        &fb, {GPU_ATTACHMENT_NONE, GPU_ATTACHMENT_TEXTURE_MIP(light_cache->cube_tx.tex, i)});
+    GPU_framebuffer_ensure_config(&fb,
+                                  {
+                                      GPU_ATTACHMENT_NONE,
+                                      GPU_ATTACHMENT_TEXTURE_MIP(light_cache->cube_tx.tex, i),
+                                  });
     GPU_framebuffer_bind(fb);
-    GPU_framebuffer_viewport_set(fb, 0, 0, mipsize, mipsize);
     DRW_draw_pass(psl->probe_glossy_compute);
 
     mipsize /= 2;
@@ -1144,6 +1146,7 @@ void EEVEE_lightbake_filter_diffuse(EEVEE_ViewLayerData *sldata,
   GPU_framebuffer_bind(fb);
   GPU_framebuffer_viewport_set(fb, x, y, size[0], size[1]);
   DRW_draw_pass(psl->probe_diffuse_compute);
+  GPU_framebuffer_viewport_reset(fb);
 }
 
 /* Filter rt_depth to light_cache->grid_tx.tex at index grid_offset */
@@ -1182,6 +1185,7 @@ void EEVEE_lightbake_filter_visibility(EEVEE_ViewLayerData *sldata,
   GPU_framebuffer_bind(fb);
   GPU_framebuffer_viewport_set(fb, x, y, vis_size, vis_size);
   DRW_draw_pass(psl->probe_visibility_compute);
+  GPU_framebuffer_viewport_reset(fb);
 }
 
 /* Actually a simple down-sampling. */

source/blender/draw/engines/eevee/eevee_lookdev.c

@@ -331,6 +331,8 @@ void EEVEE_lookdev_draw(EEVEE_Data *vedata)
 
     DRW_draw_pass(psl->lookdev_glossy_pass);
 
+    GPU_framebuffer_viewport_reset(fb);
+
     DRW_stats_group_end();
 
     DRW_view_set_active(NULL);

source/blender/draw/engines/eevee/eevee_motion_blur.c

@@ -616,6 +616,8 @@ void EEVEE_motion_blur_draw(EEVEE_Data *vedata)
 
       DRW_draw_pass(psl->velocity_tiles_expand[buf]);
 
+      GPU_framebuffer_viewport_reset(fbl->velocity_tiles_fb[buf]);
+
       buf = buf ? 0 : 1;
     }
 

source/blender/gpu/GPU_framebuffer.h

@@ -163,6 +163,8 @@ void GPU_framebuffer_config_array(GPUFrameBuffer *fb, const GPUAttachment *confi
 /* Framebuffer operations */
 
 void GPU_framebuffer_viewport_set(GPUFrameBuffer *fb, int x, int y, int w, int h);
+void GPU_framebuffer_viewport_get(GPUFrameBuffer *fb, int r_viewport[4]);
+void GPU_framebuffer_viewport_reset(GPUFrameBuffer *fb);
 
 void GPU_framebuffer_clear(GPUFrameBuffer *fb,
                            eGPUFrameBufferBits buffers,

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)
-
-/* Needs to be done after binding. */
-void GPU_framebuffer_viewport_set(GPUFrameBuffer *gpu_fb, int x, int y, int w, int h)
+/* 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)
 {
-  CHECK_FRAMEBUFFER_IS_BOUND(gpu_fb);
-
-  GPU_viewport(x, y, w, h);
+  int viewport_rect[4] = {x, y, width, height};
+  reinterpret_cast<FrameBuffer *>(gpu_fb)->viewport_set(viewport_rect);
 }
 
+void GPU_framebuffer_viewport_get(GPUFrameBuffer *gpu_fb, int r_viewport[4])
+{
+  reinterpret_cast<FrameBuffer *>(gpu_fb)->viewport_get(r_viewport);
+}
+
+/* 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;
 }
 

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

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;

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

source/blender/gpu/opengl/gl_context.cc

@@ -85,6 +85,8 @@ GLContext::GLContext(void *ghost_window, GLSharedOrphanLists &shared_orphan_list
   }
 
   active_fb = back_left;
+  static_cast<GLStateManager *>(state_manager)->active_fb = static_cast<GLFrameBuffer *>(
+      back_left);
 }
 
 GLContext::~GLContext()

source/blender/gpu/opengl/gl_framebuffer.cc

@@ -27,6 +27,7 @@
 
 #include "gl_backend.hh"
 #include "gl_framebuffer.hh"
+#include "gl_state.hh"
 #include "gl_texture.hh"
 
 namespace blender::gpu {
@@ -47,6 +48,7 @@ GLFrameBuffer::GLFrameBuffer(
     : FrameBuffer(name)
 {
   context_ = ctx;
+  state_manager_ = static_cast<GLStateManager *>(ctx->state_manager);
   immutable_ = true;
   fbo_id_ = fbo;
   gl_attachments_[0] = target;
@@ -56,6 +58,11 @@ GLFrameBuffer::GLFrameBuffer(
   height_ = h;
   srgb_ = false;
 
+  viewport_[0] = scissor_[0] = 0;
+  viewport_[1] = scissor_[1] = 0;
+  viewport_[2] = scissor_[2] = w;
+  viewport_[3] = scissor_[3] = h;
+
 #ifndef __APPLE__
   if (fbo_id_ && (G.debug & G_DEBUG_GPU) && (GLEW_VERSION_4_3 || GLEW_KHR_debug)) {
     char sh_name[32];
@@ -82,6 +89,7 @@ GLFrameBuffer::~GLFrameBuffer()
 void GLFrameBuffer::init(void)
 {
   context_ = static_cast<GLContext *>(GPU_context_active_get());
+  state_manager_ = static_cast<GLStateManager *>(context_->state_manager);
   glGenFramebuffers(1, &fbo_id_);
 
 #ifndef __APPLE__
@@ -227,6 +235,25 @@ void GLFrameBuffer::update_attachments(void)
   }
 }
 
+void GLFrameBuffer::apply_state(void)
+{
+  if (dirty_state_ == false) {
+    return;
+  }
+
+  glViewport(UNPACK4(viewport_));
+  glScissor(UNPACK4(scissor_));
+
+  if (scissor_test_) {
+    glEnable(GL_SCISSOR_TEST);
+  }
+  else {
+    glDisable(GL_SCISSOR_TEST);
+  }
+
+  dirty_state_ = false;
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -235,18 +262,16 @@ void GLFrameBuffer::update_attachments(void)
 
 void GLFrameBuffer::bind(bool enabled_srgb)
 {
-  GPUContext *ctx = GPU_context_active_get();
-  BLI_assert(ctx);
-
-  if (context_ != NULL && context_ != ctx) {
-    BLI_assert(!"Trying to use the same framebuffer in multiple context");
-  }
-
   if (!immutable_ && fbo_id_ == 0) {
     this->init();
   }
 
-  if (ctx->active_fb != this) {
+  if (context_ != GPU_context_active_get()) {
+    BLI_assert(!"Trying to use the same framebuffer in multiple context");
+    return;
+  }
+
+  if (context_->active_fb != this) {
     glBindFramebuffer(GL_FRAMEBUFFER, fbo_id_);
     /* Internal framebuffers have only one color output and needs to be set everytime. */
     if (immutable_ && fbo_id_ == 0) {
@@ -256,10 +281,14 @@ void GLFrameBuffer::bind(bool enabled_srgb)
 
   if (dirty_attachments_) {
     this->update_attachments();
+    this->viewport_reset();
+    this->scissor_reset();
   }
 
-  if (ctx->active_fb != this) {
-    ctx->active_fb = this;
+  if (context_->active_fb != this) {
+    context_->active_fb = this;
+    state_manager_->active_fb = this;
+    dirty_state_ = true;
 
     if (enabled_srgb) {
       glEnable(GL_FRAMEBUFFER_SRGB);
@@ -270,8 +299,6 @@ void GLFrameBuffer::bind(bool enabled_srgb)
 
     GPU_shader_set_framebuffer_srgb_target(enabled_srgb && srgb_);
   }
-
-  GPU_viewport(0, 0, width_, height_);
 }
 
 /** \} */
@@ -285,6 +312,9 @@ void GLFrameBuffer::clear(eGPUFrameBufferBits buffers,
                           float clear_depth,
                           uint clear_stencil)
 {
+  BLI_assert(GPU_context_active_get() == context_);
+  BLI_assert(context_->active_fb == this);
+
   /* Save and restore the state. */
   eGPUWriteMask write_mask = GPU_write_mask_get();
   uint stencil_mask = GPU_stencil_mask_get();
@@ -320,6 +350,9 @@ void GLFrameBuffer::clear(eGPUFrameBufferBits buffers,
 
 void GLFrameBuffer::clear_multi(const float (*clear_cols)[4])
 {
+  BLI_assert(GPU_context_active_get() == context_);
+  BLI_assert(context_->active_fb == this);
+
   /* Save and restore the state. */
   eGPUWriteMask write_mask = GPU_write_mask_get();
   GPU_color_mask(true, true, true, true);
@@ -401,7 +434,7 @@ void GLFrameBuffer::blit_to(
     glDrawBuffer(dst->gl_attachments_[dst_slot]);
   }
 
-  GPU_context_active_get()->state_manager->apply_state();
+  context_->state_manager->apply_state();
 
   int w = src->width_;
   int h = src->height_;

source/blender/gpu/opengl/gl_framebuffer.hh

@@ -33,6 +33,8 @@
 
 namespace blender::gpu {
 
+class GLStateManager;
+
 /**
  * Implementation of FrameBuffer object using OpenGL.
  **/
@@ -42,6 +44,8 @@ class GLFrameBuffer : public FrameBuffer {
   GLuint fbo_id_ = 0;
   /** Context the handle is from. Framebuffers are not shared accros contexts. */
   GLContext *context_ = NULL;
+  /** State Manager of the same contexts. */
+  GLStateManager *state_manager_ = NULL;
   /** Copy of the GL state. Contains ONLY color attachments enums for slot binding. */
   GLenum gl_attachments_[GPU_FB_MAX_COLOR_ATTACHMENT];
   /** Internal framebuffers are immutable. */
@@ -92,6 +96,8 @@ class GLFrameBuffer : public FrameBuffer {
                int dst_offset_x,
                int dst_offset_y) override;
 
+  void apply_state(void);
+
  private:
   void init(void);
   void update_attachments(void);

source/blender/gpu/opengl/gl_state.cc

@@ -27,6 +27,7 @@
 #include "glew-mx.h"
 
 #include "gl_context.hh"
+#include "gl_framebuffer.hh"
 #include "gl_state.hh"
 
 using namespace blender::gpu;
@@ -64,6 +65,13 @@ GLStateManager::GLStateManager(void) : GPUStateManager()
   set_mutable_state(mutable_state);
 }
 
+void GLStateManager::apply_state(void)
+{
+  this->set_state(this->state);
+  this->set_mutable_state(this->mutable_state);
+  active_fb->apply_state();
+};
+
 void GLStateManager::set_state(const GPUState &state)
 {
   GPUState changed = state ^ current_;
@@ -125,22 +133,6 @@ void GLStateManager::set_mutable_state(const GPUStateMutable &state)
 {
   GPUStateMutable changed = state ^ current_mutable_;
 
-  if ((changed.viewport_rect[0] != 0) || (changed.viewport_rect[1] != 0) ||
-      (changed.viewport_rect[2] != 0) || (changed.viewport_rect[3] != 0)) {
-    glViewport(UNPACK4(state.viewport_rect));
-  }
-
-  if ((changed.scissor_rect[0] != 0) || (changed.scissor_rect[1] != 0) ||
-      (changed.scissor_rect[2] != 0) || (changed.scissor_rect[3] != 0)) {
-    if ((state.scissor_rect[2] > 0)) {
-      glScissor(UNPACK4(state.scissor_rect));
-      glEnable(GL_SCISSOR_TEST);
-    }
-    else {
-      glDisable(GL_SCISSOR_TEST);
-    }
-  }
-
   /* TODO remove, should be uniform. */
   if (changed.point_size != 0) {
     if (state.point_size > 0.0f) {

source/blender/gpu/opengl/gl_state.hh

@@ -31,11 +31,17 @@
 namespace blender {
 namespace gpu {
 
+class GLFrameBuffer;
+
 /**
  * State manager keeping track of the draw state and applying it before drawing.
  * Opengl Implementation.
  **/
 class GLStateManager : public GPUStateManager {
+ public:
+  /** Anothter reference to tje active framebuffer. */
+  GLFrameBuffer *active_fb;
+
  private:
   /** Current state of the GL implementation. Avoids resetting the whole state for every change. */
   GPUState current_;
@@ -46,8 +52,7 @@ class GLStateManager : public GPUStateManager {
  public:
   GLStateManager();
 
-  void set_state(const GPUState &state) override;
-  void set_mutable_state(const GPUStateMutable &state) override;
+  void apply_state(void) override;
 
  private:
   static void set_write_mask(const eGPUWriteMask value);
@@ -62,6 +67,9 @@ class GLStateManager : public GPUStateManager {
   static void set_shadow_bias(const bool enable);
   static void set_blend(const eGPUBlend value);
 
+  void set_state(const GPUState &state);
+  void set_mutable_state(const GPUStateMutable &state);
+
   MEM_CXX_CLASS_ALLOC_FUNCS("GLStateManager")
 };