GPU: Move UBO binding validation to GL backend
This also make the validation quicker by tracking the currently bound slots.
This commit is contained in:
parent
5ec0250df9
commit
53a806f6df
@ -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,
|
||||
|
@ -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
|
||||
|
@ -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);
|
||||
};
|
||||
|
@ -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);
|
||||
|
@ -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)
|
||||
|
@ -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. */
|
||||
|
@ -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
|
@ -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
|
||||
|
@ -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());
|
||||
|
@ -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_mask_ is uint16_t");
|
||||
|
||||
/* Work around driver bug with Intel HD 4600 on Windows 7/8, where
|
||||
* GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH does not work. */
|
||||
if (attr_len > 0 && max_attr_name_len == 0) {
|
||||
|
@ -55,8 +55,6 @@ class GLShaderInterface : public ShaderInterface {
|
||||
void ref_add(GLVaoCache *ref);
|
||||
void ref_remove(GLVaoCache *ref);
|
||||
|
||||
// bool resource_binding_validate();
|
||||
|
||||
MEM_CXX_CLASS_ALLOC_FUNCS("GLShaderInterface");
|
||||
};
|
||||
|
||||
|
@ -110,6 +110,11 @@ void GLUniformBuf::bind(int slot)
|
||||
|
||||
slot_ = slot;
|
||||
glBindBufferBase(GL_UNIFORM_BUFFER, slot_, ubo_id_);
|
||||
|
||||
#ifdef DEBUG
|
||||
BLI_assert(slot < 16);
|
||||
static_cast<GLContext *>(GPU_context_active_get())->bound_ubo_slots |= 1 << slot;
|
||||
#endif
|
||||
}
|
||||
|
||||
void GLUniformBuf::unbind(void)
|
||||
@ -117,6 +122,8 @@ void GLUniformBuf::unbind(void)
|
||||
#ifdef DEBUG
|
||||
/* NOTE: This only unbinds the last bound slot. */
|
||||
glBindBufferBase(GL_UNIFORM_BUFFER, slot_, 0);
|
||||
/* Hope that the context did not change. */
|
||||
static_cast<GLContext *>(GPU_context_active_get())->bound_ubo_slots &= ~(1 << slot_);
|
||||
#endif
|
||||
slot_ = 0;
|
||||
}
|
||||
|
@ -37,7 +37,9 @@ namespace gpu {
|
||||
**/
|
||||
class GLUniformBuf : public UniformBuf {
|
||||
private:
|
||||
/** Slot to which this UBO is currently bound. -1 if not bound. */
|
||||
int slot_ = -1;
|
||||
/** OpenGL Object handle. */
|
||||
GLuint ubo_id_ = 0;
|
||||
|
||||
public:
|
||||
|
Loading…
Reference in New Issue
Block a user