GPUState: Remove gpuPushAttr/gpuPopAttr

And use manual save/restore mechanism.

The stack method is not used so much to be considered useful.

7 files changed, 86 insertions, 160 deletions

diff --git a/source/blender/editors/sculpt_paint/paint_cursor.c b/source/blender/editors/sculpt_paint/paint_cursor.c
index 4115b5e51d6..0e38340d3bc 100644
--- a/source/blender/editors/sculpt_paint/paint_cursor.c
+++ b/source/blender/editors/sculpt_paint/paint_cursor.c
@@ -775,7 +775,8 @@ static bool paint_draw_alpha_overlay(UnifiedPaintSettings *ups,
   bool alpha_overlay_active = false;
 
   ePaintOverlayControlFlags flags = BKE_paint_get_overlay_flags();
-  gpuPushAttr(GPU_DEPTH_BUFFER_BIT | GPU_BLEND_BIT);
+  eGPUBlend blend_state = GPU_blend_get();
+  bool depth_test = GPU_depth_test_enabled();
 
   /* Translate to region. */
   GPU_matrix_push();
@@ -805,7 +806,8 @@ static bool paint_draw_alpha_overlay(UnifiedPaintSettings *ups,
   }
 
   GPU_matrix_pop();
-  gpuPopAttr();
+  GPU_blend(blend_state);
+  GPU_depth_test(depth_test);
 
   return alpha_overlay_active;
 }
diff --git a/source/blender/editors/space_node/node_draw.c b/source/blender/editors/space_node/node_draw.c
index 5553928b012..696fe666f41 100644
--- a/source/blender/editors/space_node/node_draw.c
+++ b/source/blender/editors/space_node/node_draw.c
@@ -835,7 +835,7 @@ void ED_node_socket_draw(bNodeSocket *sock, const rcti *rect, const float color[
   uint outline_col_id = GPU_vertformat_attr_add(
       format, "outlineColor", GPU_COMP_F32, 4, GPU_FETCH_FLOAT);
 
-  gpuPushAttr(GPU_BLEND_BIT);
+  eGPUBlend state = GPU_blend_get();
   GPU_blend(GPU_BLEND_ALPHA);
   GPU_program_point_size(true);
 
@@ -860,7 +860,9 @@ void ED_node_socket_draw(bNodeSocket *sock, const rcti *rect, const float color[
 
   immUnbindProgram();
   GPU_program_point_size(false);
-  gpuPopAttr();
+
+  /* Restore. */
+  GPU_blend(state);
 }
 
 /* **************  Socket callbacks *********** */
diff --git a/source/blender/gpu/GPU_state.h b/source/blender/gpu/GPU_state.h
index 39b52567a46..1e2b772224c 100644
--- a/source/blender/gpu/GPU_state.h
+++ b/source/blender/gpu/GPU_state.h
@@ -22,10 +22,6 @@
 
 #include "BLI_utildefines.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 typedef enum eGPUWriteMask {
   GPU_WRITE_RED = (1 << 0),
   GPU_WRITE_GREEN = (1 << 1),
@@ -103,6 +99,10 @@ typedef enum eGPUProvokingVertex {
   GPU_VERTEX_FIRST = 1, /* Follow Blender loop order. */
 } eGPUProvokingVertex;
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /* Initialize
  * - sets the default Blender opengl state, if in doubt, check
  *   the contents of this function
@@ -119,6 +119,7 @@ bool GPU_depth_test_enabled(void);
 void GPU_scissor_test(bool enable);
 void GPU_line_smooth(bool enable);
 void GPU_line_width(float width);
+void GPU_logic_op_xor_set(bool enable);
 void GPU_point_size(float size);
 void GPU_polygon_smooth(bool enable);
 void GPU_program_point_size(bool enable);
@@ -127,6 +128,7 @@ void GPU_scissor_get(int coords[4]);
 void GPU_viewport(int x, int y, int width, int height);
 void GPU_viewport_size_get_f(float coords[4]);
 void GPU_viewport_size_get_i(int coords[4]);
+void GPU_write_mask(eGPUWriteMask mask);
 void GPU_color_mask(bool r, bool g, bool b, bool a);
 void GPU_depth_mask(bool depth);
 bool GPU_depth_mask_get(void);
@@ -134,23 +136,12 @@ void GPU_unpack_row_length_set(uint len);
 void GPU_clip_distances(int enabled_len);
 bool GPU_mipmap_enabled(void);
 
+eGPUBlend GPU_blend_get(void);
+eGPUWriteMask GPU_write_mask_get(void);
+
 void GPU_flush(void);
 void GPU_finish(void);
 
-void GPU_logic_op_xor_set(bool enable);
-
-/* Attribute push & pop. */
-typedef enum eGPUAttrMask {
-  GPU_DEPTH_BUFFER_BIT = (1 << 0),
-  GPU_ENABLE_BIT = (1 << 1),
-  GPU_SCISSOR_BIT = (1 << 2),
-  GPU_VIEWPORT_BIT = (1 << 3),
-  GPU_BLEND_BIT = (1 << 4),
-} eGPUAttrMask;
-
-void gpuPushAttr(eGPUAttrMask mask);
-void gpuPopAttr(void);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/source/blender/gpu/intern/gpu_framebuffer.cc b/source/blender/gpu/intern/gpu_framebuffer.cc
index 627f417e0c3..4c7f13a58eb 100644
--- a/source/blender/gpu/intern/gpu_framebuffer.cc
+++ b/source/blender/gpu/intern/gpu_framebuffer.cc
@@ -901,6 +901,13 @@ struct GPUOffScreen {
 
   GPUTexture *color;
   GPUTexture *depth;
+
+  /** Saved state of the previously bound framebuffer. */
+  /* TODO(fclem) This is quite hacky and a proper fix would be to
+   * put these states directly inside the GPUFrambuffer.
+   * But we don't have a GPUFramebuffer for the default framebuffer yet. */
+  int saved_viewport[4];
+  int saved_scissor[4];
 };
 
 /* Returns the correct framebuffer for the current context. */
@@ -964,21 +971,19 @@ GPUOffScreen *GPU_offscreen_create(
     return NULL;
   }
 
-  gpuPushAttr(GPU_VIEWPORT_BIT);
+  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);
-    gpuPopAttr();
+    GPU_viewport(UNPACK4(viewport));
     return NULL;
   }
-
   GPU_framebuffer_restore();
-
-  gpuPopAttr();
-
+  GPU_viewport(UNPACK4(viewport));
   return ofs;
 }
 
@@ -1002,7 +1007,9 @@ void GPU_offscreen_free(GPUOffScreen *ofs)
 void GPU_offscreen_bind(GPUOffScreen *ofs, bool save)
 {
   if (save) {
-    gpuPushAttr((eGPUAttrMask)(GPU_SCISSOR_BIT | GPU_VIEWPORT_BIT));
+    GPU_scissor_get(ofs->saved_scissor);
+    GPU_viewport_size_get_i(ofs->saved_viewport);
+
     GPUFrameBuffer *fb = GPU_framebuffer_active_get();
     gpuPushFrameBuffer(fb);
   }
@@ -1013,12 +1020,13 @@ void GPU_offscreen_bind(GPUOffScreen *ofs, bool save)
   GPU_shader_set_framebuffer_srgb_target(false);
 }
 
-void GPU_offscreen_unbind(GPUOffScreen *UNUSED(ofs), bool restore)
+void GPU_offscreen_unbind(GPUOffScreen *ofs, bool restore)
 {
   GPUFrameBuffer *fb = NULL;
 
   if (restore) {
-    gpuPopAttr();
+    GPU_scissor(UNPACK4(ofs->saved_scissor));
+    GPU_viewport(UNPACK4(ofs->saved_viewport));
     fb = gpuPopFrameBuffer();
   }
 
diff --git a/source/blender/gpu/intern/gpu_select_pick.c b/source/blender/gpu/intern/gpu_select_pick.c
index 0f6f29fab40..29e2615345c 100644
--- a/source/blender/gpu/intern/gpu_select_pick.c
+++ b/source/blender/gpu/intern/gpu_select_pick.c
@@ -282,6 +282,12 @@ typedef struct GPUPickState {
       uint *rect_id;
     } nearest;
   };
+
+  /* Previous state to restore after drawing. */
+  int viewport[4];
+  int scissor[4];
+  eGPUWriteMask write_mask;
+  bool depth_test;
 } GPUPickState;
 
 static GPUPickState g_pick_state = {0};
@@ -304,7 +310,9 @@ void gpu_select_pick_begin(uint (*buffer)[4], uint bufsize, const rcti *input, c
 
   /* Restrict OpenGL operations for when we don't have cache */
   if (ps->is_cached == false) {
-    gpuPushAttr(GPU_DEPTH_BUFFER_BIT | GPU_VIEWPORT_BIT);
+    ps->write_mask = GPU_write_mask_get();
+    ps->depth_test = GPU_depth_test_enabled();
+    GPU_scissor_get(ps->scissor);
 
     /* disable writing to the framebuffer */
     GPU_color_mask(false, false, false, false);
@@ -535,8 +543,9 @@ uint gpu_select_pick_end(void)
       /* force finishing last pass */
       gpu_select_pick_load_id(ps->gl.prev_id, true);
     }
-    gpuPopAttr();
-    GPU_color_mask(true, true, true, true);
+    GPU_write_mask(ps->write_mask);
+    GPU_depth_test(ps->depth_test);
+    GPU_viewport(UNPACK4(ps->viewport));
   }
 
   /* assign but never free directly since it may be in cache */
diff --git a/source/blender/gpu/intern/gpu_select_sample_query.c b/source/blender/gpu/intern/gpu_select_sample_query.c
index f67c9c36a6b..62414febb44 100644
--- a/source/blender/gpu/intern/gpu_select_sample_query.c
+++ b/source/blender/gpu/intern/gpu_select_sample_query.c
@@ -60,6 +60,12 @@ typedef struct GPUQueryState {
   char mode;
   uint index;
   int oldhits;
+
+  /* Previous state to restore after drawing. */
+  int viewport[4];
+  int scissor[4];
+  eGPUWriteMask write_mask;
+  bool depth_test;
 } GPUQueryState;
 
 static GPUQueryState g_query_state = {0};
@@ -67,8 +73,6 @@ static GPUQueryState g_query_state = {0};
 void gpu_select_query_begin(
     uint (*buffer)[4], uint bufsize, const rcti *input, char mode, int oldhits)
 {
-  float viewport[4];
-
   g_query_state.query_issued = false;
   g_query_state.active_query = 0;
   g_query_state.num_of_queries = 0;
@@ -86,7 +90,10 @@ void gpu_select_query_begin(
                                  "gpu selection ids");
   glGenQueries(g_query_state.num_of_queries, g_query_state.queries);
 
-  gpuPushAttr(GPU_DEPTH_BUFFER_BIT | GPU_VIEWPORT_BIT | GPU_SCISSOR_BIT);
+  g_query_state.write_mask = GPU_write_mask_get();
+  g_query_state.depth_test = GPU_depth_test_enabled();
+  GPU_scissor_get(g_query_state.scissor);
+
   /* disable writing to the framebuffer */
   GPU_color_mask(false, false, false, false);
 
@@ -94,8 +101,11 @@ void gpu_select_query_begin(
    * We need to get the region of the viewport so that our geometry doesn't
    * get rejected before the depth test. Should probably cull rect against
    * the viewport but this is a rare case I think */
-  GPU_viewport_size_get_f(viewport);
-  GPU_viewport(viewport[0], viewport[1], BLI_rcti_size_x(input), BLI_rcti_size_y(input));
+  GPU_viewport_size_get_i(g_query_state.viewport);
+  GPU_viewport(g_query_state.viewport[0],
+               g_query_state.viewport[1],
+               BLI_rcti_size_x(input),
+               BLI_rcti_size_y(input));
 
   /* occlusion queries operates on fragments that pass tests and since we are interested on all
    * objects in the view frustum independently of their order, we need to disable the depth test */
@@ -204,8 +214,10 @@ uint gpu_select_query_end(void)
   glDeleteQueries(g_query_state.num_of_queries, g_query_state.queries);
   MEM_freeN(g_query_state.queries);
   MEM_freeN(g_query_state.id);
-  gpuPopAttr();
-  GPU_color_mask(true, true, true, true);
+
+  GPU_write_mask(g_query_state.write_mask);
+  GPU_depth_test(g_query_state.depth_test);
+  GPU_viewport(UNPACK4(g_query_state.viewport));
 
   return hits;
 }
diff --git a/source/blender/gpu/intern/gpu_state.cc b/source/blender/gpu/intern/gpu_state.cc
index 6d3d74e60f3..8a9724e967f 100644
--- a/source/blender/gpu/intern/gpu_state.cc
+++ b/source/blender/gpu/intern/gpu_state.cc
@@ -82,7 +82,7 @@ void GPU_provoking_vertex(eGPUProvokingVertex vert)
 /* TODO explicit depth test. */
 void GPU_depth_test(bool enable)
 {
-  SET_IMMUTABLE_STATE(depth_test, (enable) ? GPU_DEPTH_LESS : GPU_DEPTH_NONE);
+  SET_IMMUTABLE_STATE(depth_test, (enable) ? GPU_DEPTH_LESS_EQUAL : GPU_DEPTH_NONE);
 }
 
 void GPU_line_smooth(bool enable)
@@ -100,6 +100,11 @@ void GPU_logic_op_xor_set(bool enable)
   SET_IMMUTABLE_STATE(logic_op_xor, enable);
 }
 
+void GPU_write_mask(eGPUWriteMask mask)
+{
+  SET_IMMUTABLE_STATE(write_mask, mask);
+}
+
 void GPU_color_mask(bool r, bool g, bool b, bool a)
 {
   GPUStateStack *stack = GPU_context_active_get()->state_stack;
@@ -205,6 +210,18 @@ void GPU_viewport(int x, int y, int width, int height)
 /** \name State Getters
  * \{ */
 
+eGPUBlend GPU_blend_get()
+{
+  GPUState &state = GPU_context_active_get()->state_stack->stack_top_get();
+  return state.blend;
+}
+
+eGPUWriteMask GPU_write_mask_get()
+{
+  GPUState &state = GPU_context_active_get()->state_stack->stack_top_get();
+  return state.write_mask;
+}
+
 bool GPU_depth_test_enabled()
 {
   GPUState &state = GPU_context_active_get()->state_stack->stack_top_get();
@@ -295,128 +312,13 @@ void GPU_unpack_row_length_set(uint len)
 /** \} */
 
 /* -------------------------------------------------------------------- */
-/** \name GPU Push/Pop State
- * \{ */
-
-#define STATE_STACK_DEPTH 16
-
-typedef struct {
-  eGPUAttrMask mask;
-
-  /* GL_BLEND_BIT */
-  uint is_blend : 1;
-
-  /* GL_DEPTH_BUFFER_BIT */
-  uint is_depth_test : 1;
-  int depth_func;
-  double depth_clear_value;
-  bool depth_write_mask;
-
-  /* GL_SCISSOR_BIT */
-  int scissor_box[4];
-  uint is_scissor_test : 1;
-
-  /* GL_VIEWPORT_BIT */
-  int viewport[4];
-  double near_far[2];
-} GPUAttrValues;
-
-typedef struct {
-  GPUAttrValues attr_stack[STATE_STACK_DEPTH];
-  uint top;
-} GPUAttrStack;
-
-static GPUAttrStack state = {
-    {},
-    0,
-};
-
-#define AttrStack state
-#define Attr state.attr_stack[state.top]
-
-/**
- * Replacement for glPush/PopAttributes
- *
- * We don't need to cover all the options of legacy OpenGL
- * but simply the ones used by Blender.
- */
-void gpuPushAttr(eGPUAttrMask mask)
-{
-  Attr.mask = mask;
-
-  if ((mask & GPU_DEPTH_BUFFER_BIT) != 0) {
-    Attr.is_depth_test = glIsEnabled(GL_DEPTH_TEST);
-    glGetIntegerv(GL_DEPTH_FUNC, &Attr.depth_func);
-    glGetDoublev(GL_DEPTH_CLEAR_VALUE, &Attr.depth_clear_value);
-    glGetBooleanv(GL_DEPTH_WRITEMASK, (GLboolean *)&Attr.depth_write_mask);
-  }
-
-  if ((mask & GPU_SCISSOR_BIT) != 0) {
-    Attr.is_scissor_test = glIsEnabled(GL_SCISSOR_TEST);
-    glGetIntegerv(GL_SCISSOR_BOX, (GLint *)&Attr.scissor_box);
-  }
-
-  if ((mask & GPU_VIEWPORT_BIT) != 0) {
-    glGetDoublev(GL_DEPTH_RANGE, (GLdouble *)&Attr.near_far);
-    glGetIntegerv(GL_VIEWPORT, (GLint *)&Attr.viewport);
-  }
-
-  if ((mask & GPU_BLEND_BIT) != 0) {
-    Attr.is_blend = glIsEnabled(GL_BLEND);
-  }
-
-  BLI_assert(AttrStack.top < STATE_STACK_DEPTH);
-  AttrStack.top++;
-}
-
-static void restore_mask(GLenum cap, const bool value)
-{
-  if (value) {
-    glEnable(cap);
-  }
-  else {
-    glDisable(cap);
-  }
-}
-
-void gpuPopAttr(void)
-{
-  BLI_assert(AttrStack.top > 0);
-  AttrStack.top--;
-
-  GLint mask = Attr.mask;
-
-  if ((mask & GPU_DEPTH_BUFFER_BIT) != 0) {
-    restore_mask(GL_DEPTH_TEST, Attr.is_depth_test);
-    glDepthFunc(Attr.depth_func);
-    glClearDepth(Attr.depth_clear_value);
-    glDepthMask(Attr.depth_write_mask);
-  }
-
-  if ((mask & GPU_VIEWPORT_BIT) != 0) {
-    glViewport(Attr.viewport[0], Attr.viewport[1], Attr.viewport[2], Attr.viewport[3]);
-    glDepthRange(Attr.near_far[0], Attr.near_far[1]);
-  }
-
-  if ((mask & GPU_SCISSOR_BIT) != 0) {
-    restore_mask(GL_SCISSOR_TEST, Attr.is_scissor_test);
-    glScissor(Attr.scissor_box[0], Attr.scissor_box[1], Attr.scissor_box[2], Attr.scissor_box[3]);
-  }
-
-  if ((mask & GPU_BLEND_BIT) != 0) {
-    restore_mask(GL_BLEND, Attr.is_blend);
-  }
-}
-
-#undef Attr
-#undef AttrStack
-
-/* Default OpenGL State
+/** \name Default OpenGL State
  *
  * This is called on startup, for opengl offscreen render.
  * Generally we should always return to this state when
  * temporarily modifying the state for drawing, though that are (undocumented)
- * exceptions that we should try to get rid of. */
+ * exceptions that we should try to get rid of.
+ * \{ */
 
 void GPU_state_init(void)
 {