[Bf-blender-cvs] [53a806f6dff] master: GPU: Move UBO binding validation to GL backend
Clément Foucault
noreply at git.blender.org
Tue Sep 1 12:03:55 CEST 2020
Commit: 53a806f6dffb2a778e383a82c3d0cdb6e9d9d552
Author: Clément Foucault
Date: Tue Sep 1 02:41:29 2020 +0200
Branches: master
https://developer.blender.org/rB53a806f6dffb2a778e383a82c3d0cdb6e9d9d552
GPU: Move UBO binding validation to GL backend
This also make the validation quicker by tracking the currently
bound slots.
===================================================================
M source/blender/draw/intern/draw_manager_exec.c
M source/blender/gpu/intern/gpu_shader_interface.hh
M source/blender/gpu/intern/gpu_shader_private.hh
M source/blender/gpu/opengl/gl_batch.cc
M source/blender/gpu/opengl/gl_context.cc
M source/blender/gpu/opengl/gl_context.hh
M source/blender/gpu/opengl/gl_debug.cc
M source/blender/gpu/opengl/gl_debug.hh
M source/blender/gpu/opengl/gl_immediate.cc
M source/blender/gpu/opengl/gl_shader_interface.cc
M source/blender/gpu/opengl/gl_shader_interface.hh
M source/blender/gpu/opengl/gl_uniform_buffer.cc
M source/blender/gpu/opengl/gl_uniform_buffer.hh
===================================================================
diff --git a/source/blender/draw/intern/draw_manager_exec.c b/source/blender/draw/intern/draw_manager_exec.c
index 5584405c965..1d15548e1ed 100644
--- a/source/blender/draw/intern/draw_manager_exec.c
+++ b/source/blender/draw/intern/draw_manager_exec.c
@@ -567,58 +567,6 @@ BLI_INLINE void draw_indirect_call(DRWShadingGroup *shgroup, DRWCommandsState *s
}
}
-#ifndef NDEBUG
-/**
- * Opengl specification is strict on buffer binding.
- *
- * " If any active uniform block is not backed by a
- * sufficiently large buffer object, the results of shader
- * execution are undefined, and may result in GL interruption or
- * termination. " - Opengl 3.3 Core Specification
- *
- * For now we only check if the binding is correct. Not the size of
- * the bound ubo.
- *
- * See T55475.
- * */
-static bool ubo_bindings_validate(DRWShadingGroup *shgroup)
-{
- bool valid = true;
-# ifdef DEBUG_UBO_BINDING
- /* Check that all active uniform blocks have a non-zero buffer bound. */
- GLint program = 0;
- GLint active_blocks = 0;
-
- glGetIntegerv(GL_CURRENT_PROGRAM, &program);
- glGetProgramiv(program, GL_ACTIVE_UNIFORM_BLOCKS, &active_blocks);
-
- for (uint i = 0; i < active_blocks; i++) {
- int binding = 0;
- int buffer = 0;
-
- glGetActiveUniformBlockiv(program, i, GL_UNIFORM_BLOCK_BINDING, &binding);
- glGetIntegeri_v(GL_UNIFORM_BUFFER_BINDING, binding, &buffer);
-
- if (buffer == 0) {
- char blockname[64];
- glGetActiveUniformBlockName(program, i, sizeof(blockname), NULL, blockname);
-
- if (valid) {
- printf("Trying to draw with missing UBO binding.\n");
- valid = false;
- }
-
- DRWPass *parent_pass = DRW_memblock_elem_from_handle(DST.vmempool->passes,
- &shgroup->pass_handle);
-
- printf("Pass : %s, Block : %s, Binding %d\n", parent_pass->name, blockname, binding);
- }
- }
-# endif
- return valid;
-}
-#endif
-
static void draw_update_uniforms(DRWShadingGroup *shgroup,
DRWCommandsState *state,
bool *use_tfeedback)
@@ -688,8 +636,6 @@ static void draw_update_uniforms(DRWShadingGroup *shgroup,
}
}
}
-
- BLI_assert(ubo_bindings_validate(shgroup));
}
BLI_INLINE void draw_select_buffer(DRWShadingGroup *shgroup,
diff --git a/source/blender/gpu/intern/gpu_shader_interface.hh b/source/blender/gpu/intern/gpu_shader_interface.hh
index 6e1cb342c68..265fe90fc76 100644
--- a/source/blender/gpu/intern/gpu_shader_interface.hh
+++ b/source/blender/gpu/intern/gpu_shader_interface.hh
@@ -83,12 +83,21 @@ class ShaderInterface {
{
return input_lookup(inputs_ + attr_len_, ubo_len_, name);
}
+ inline const ShaderInput *ubo_get(const int binding) const
+ {
+ return input_lookup(inputs_ + attr_len_, ubo_len_, binding);
+ }
inline const ShaderInput *uniform_get(const char *name) const
{
return input_lookup(inputs_ + attr_len_ + ubo_len_, uniform_len_, name);
}
+ inline const char *input_name_get(const ShaderInput *input) const
+ {
+ return name_buffer_ + input->name_offset;
+ }
+
/* Returns uniform location. */
inline int32_t uniform_builtin(const GPUUniformBuiltin builtin) const
{
@@ -116,6 +125,10 @@ class ShaderInterface {
inline const ShaderInput *input_lookup(const ShaderInput *const inputs,
const uint inputs_len,
const char *name) const;
+
+ inline const ShaderInput *input_lookup(const ShaderInput *const inputs,
+ const uint inputs_len,
+ const int binding) const;
};
inline const char *ShaderInterface::builtin_uniform_name(GPUUniformBuiltin u)
@@ -226,4 +239,17 @@ inline const ShaderInput *ShaderInterface::input_lookup(const ShaderInput *const
return NULL; /* not found */
}
+inline const ShaderInput *ShaderInterface::input_lookup(const ShaderInput *const inputs,
+ const uint inputs_len,
+ const int binding) const
+{
+ /* Simple linear search for now. */
+ for (int i = inputs_len - 1; i >= 0; i--) {
+ if (inputs[i].binding == binding) {
+ return inputs + i;
+ }
+ }
+ return NULL; /* not found */
+}
+
} // namespace blender::gpu
diff --git a/source/blender/gpu/intern/gpu_shader_private.hh b/source/blender/gpu/intern/gpu_shader_private.hh
index dc6941a067e..9c9aa835b97 100644
--- a/source/blender/gpu/intern/gpu_shader_private.hh
+++ b/source/blender/gpu/intern/gpu_shader_private.hh
@@ -64,6 +64,11 @@ class Shader {
virtual void vertformat_from_shader(GPUVertFormat *) const = 0;
+ inline const char *const name_get(void) const
+ {
+ return name;
+ };
+
protected:
void print_errors(Span<const char *> sources, char *log, const char *stage);
};
diff --git a/source/blender/gpu/opengl/gl_batch.cc b/source/blender/gpu/opengl/gl_batch.cc
index 63547bf190f..bb7d5654efd 100644
--- a/source/blender/gpu/opengl/gl_batch.cc
+++ b/source/blender/gpu/opengl/gl_batch.cc
@@ -330,6 +330,7 @@ void GLBatch::bind(int i_first)
void GLBatch::draw(int v_first, int v_count, int i_first, int i_count)
{
+ GL_CHECK_RESOURCES("Batch");
GL_CHECK_ERROR("Batch Pre drawing");
this->bind(i_first);
diff --git a/source/blender/gpu/opengl/gl_context.cc b/source/blender/gpu/opengl/gl_context.cc
index 666f2b4756a..a3b060abf0e 100644
--- a/source/blender/gpu/opengl/gl_context.cc
+++ b/source/blender/gpu/opengl/gl_context.cc
@@ -36,6 +36,7 @@
#include "gl_debug.hh"
#include "gl_immediate.hh"
#include "gl_state.hh"
+#include "gl_uniform_buffer.hh"
#include "gl_backend.hh" /* TODO remove */
#include "gl_context.hh"
@@ -146,6 +147,10 @@ void GLContext::activate(void)
back_right->size_set(w, h);
}
}
+
+ /* Not really following the state but we should consider
+ * no ubo bound when activating a context. */
+ bound_ubo_slots = 0;
}
void GLContext::deactivate(void)
diff --git a/source/blender/gpu/opengl/gl_context.hh b/source/blender/gpu/opengl/gl_context.hh
index 99076fa3d1e..fa1c6b9918e 100644
--- a/source/blender/gpu/opengl/gl_context.hh
+++ b/source/blender/gpu/opengl/gl_context.hh
@@ -52,6 +52,10 @@ class GLSharedOrphanLists {
};
class GLContext : public GPUContext {
+ public:
+ /** Used for debugging purpose. Bitflags of all bound slots. */
+ uint16_t bound_ubo_slots;
+
/* TODO(fclem) these needs to become private. */
public:
/** VBO for missing vertex attrib binding. Avoid undefined behavior on some implementation. */
diff --git a/source/blender/gpu/opengl/gl_debug.cc b/source/blender/gpu/opengl/gl_debug.cc
index 2dc0835adfb..fb3c4f52fd0 100644
--- a/source/blender/gpu/opengl/gl_debug.cc
+++ b/source/blender/gpu/opengl/gl_debug.cc
@@ -30,6 +30,9 @@
#include "glew-mx.h"
+#include "gl_context.hh"
+#include "gl_uniform_buffer.hh"
+
#include "gl_debug.hh"
#include <stdio.h>
@@ -140,7 +143,8 @@ void init_gl_callbacks(void)
/* -------------------------------------------------------------------- */
/** \name Error Checking
*
- * This is only useful for implementation that does not support the KHR_debug extension.
+ * This is only useful for implementation that does not support the KHR_debug extension OR when the
+ * implementations do not report any errors even when clearly doing shady things.
* \{ */
void check_gl_error(const char *info)
@@ -173,6 +177,31 @@ void check_gl_error(const char *info)
}
}
+void check_gl_resources(const char *info)
+{
+ GLContext *ctx = static_cast<GLContext *>(GPU_context_active_get());
+ ShaderInterface *interface = ctx->shader->interface;
+ /* NOTE: This only check binding. To be valid, the bound ubo needs to
+ * be big enough to feed the data range the shader awaits. */
+ uint16_t ubo_needed = interface->enabled_ubo_mask_;
+ ubo_needed &= ~ctx->bound_ubo_slots;
+
+ if (ubo_needed == 0) {
+ return;
+ }
+
+ for (int i = 0; ubo_needed != 0; i++, ubo_needed >>= 1) {
+ if ((ubo_needed & 1) != 0) {
+ const ShaderInput *ubo_input = interface->ubo_get(i);
+ const char *ubo_name = interface->input_name_get(ubo_input);
+ const char *sh_name = ctx->shader->name_get();
+ char msg[256];
+ SNPRINTF(msg, "Missing UBO bind at slot %d : %s > %s : %s", i, sh_name, ubo_name, info);
+ debug_callback(0, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH, 0, msg, NULL);
+ }
+ }
+}
+
/** \} */
} // namespace blender::gpu::debug
\ No newline at end of file
diff --git a/source/blender/gpu/opengl/gl_debug.hh b/source/blender/gpu/opengl/gl_debug.hh
index 47c9558f13a..dd98505ebc1 100644
--- a/source/blender/gpu/opengl/gl_debug.hh
+++ b/source/blender/gpu/opengl/gl_debug.hh
@@ -31,7 +31,14 @@ namespace debug {
# define GL_CHECK_ERROR(info)
#endif
+#ifdef DEBUG
+# define GL_CHECK_RESOURCES(info) debug::check_gl_resources(info)
+#else
+# define GL_CHECK_RESOURCES(info)
+#endif
+
void check_gl_error(const char *info);
+void check_gl_resources(const char *info);
void init_gl_callbacks(void);
} // namespace debug
diff --git a/source/blender/gpu/opengl/gl_immediate.cc b/source/blender/gpu/opengl/gl_immediate.cc
index 614b6893347..7f12f41a598 100644
--- a/source/blender/gpu/opengl/gl_immediate.cc
+++ b/source/blender/gpu/opengl/gl_immediate.cc
@@ -90,6 +90,7 @@ uchar *GLImmediate::begin()
/* Does the current buffer have enough room? */
const size_t available_bytes = buffer_size() - buffer_offset();
+ GL_CHECK_RESOURCES("Immediate");
GL_CHECK_ERROR("Immediate Pre-Begin");
glBindBuffer(GL_ARRAY_BUFFER, vbo_id());
diff --git a/source/blender/gpu/opengl/gl_shader_interface.cc b/source/blender/gpu/opengl/gl_shader_interface.cc
index 423db5c8c97..d611efcd975 100644
--- a/source/blender/gpu/opengl/gl_shader_interface.cc
+++ b/source/blender/gpu/opengl/gl_shader_interface.cc
@@ -124,6 +124,8 @@ GLShaderInterface::GLShaderInterface(GLuint program)
glGetProgramiv(program, GL_ACTIVE_UNIFORMS, &active_uniform_len);
uniform_len = active_uniform_len;
+ BLI_assert(ubo_len <= 16 && "enabled_ubo_
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list