GPU: Implement missing UBO/SSBO bind tracking

This PR adds a context function to consider all
buffer bindings obsolete. This is in order to
track missing binds and invalid lingering states
accross `draw::Pass`es.

The functions `GPU_storagebuf_debug_unbind_all`
and `GPU_uniformbuf_debug_unbind_all` do nothing
more than resetting the internal debug slot bits
to zero. This is what OpenGL backend does as it
doesn't track the bindings themselves.

Other backends might have other way to detect
missing bindings. If not they should be
implemented separately anyway.

I renamed the function to `debug_unbind_all` to
denote that it actually does something related to
debugging.

This also add SSBO binding check for OpenGL as it
was also missing.

#### Future

This error checking logic is pretty much backend
agnostic. While it would be nice to move it at
`gpu::Context` level, we don't have the resources
for that now.

Pull Request: https://projects.blender.org/blender/blender/pulls/120716
This commit is contained in:
Clément Foucault
2024-04-17 11:06:39 +02:00
committed by Clément Foucault
parent 2154cf6dcb
commit f2ae04db10
15 changed files with 70 additions and 18 deletions

View File

@@ -73,7 +73,7 @@ void ShaderOperation::execute()
GPU_texture_unbind_all();
GPU_texture_image_unbind_all();
GPU_uniformbuf_unbind_all();
GPU_uniformbuf_debug_unbind_all();
GPU_shader_unbind();
}

View File

@@ -67,10 +67,10 @@ struct RecordingState {
}
if (G.debug & G_DEBUG_GPU) {
GPU_storagebuf_unbind_all();
GPU_storagebuf_debug_unbind_all();
GPU_texture_image_unbind_all();
GPU_texture_unbind_all();
GPU_uniformbuf_unbind_all();
GPU_uniformbuf_debug_unbind_all();
}
}
};

View File

@@ -320,8 +320,8 @@ void DRW_state_reset()
GPU_texture_unbind_all();
GPU_texture_image_unbind_all();
GPU_uniformbuf_unbind_all();
GPU_storagebuf_unbind_all();
GPU_uniformbuf_debug_unbind_all();
GPU_storagebuf_debug_unbind_all();
/* Should stay constant during the whole rendering. */
GPU_point_size(5);
@@ -1011,8 +1011,8 @@ static void draw_shgroup(DRWShadingGroup *shgroup, DRWState pass_state)
if (G.debug & G_DEBUG_GPU) {
GPU_texture_unbind_all();
GPU_texture_image_unbind_all();
GPU_uniformbuf_unbind_all();
GPU_storagebuf_unbind_all();
GPU_uniformbuf_debug_unbind_all();
GPU_storagebuf_debug_unbind_all();
}
}
GPU_shader_bind(shgroup->shader);

View File

@@ -34,7 +34,11 @@ void GPU_storagebuf_update(GPUStorageBuf *ssbo, const void *data);
void GPU_storagebuf_bind(GPUStorageBuf *ssbo, int slot);
void GPU_storagebuf_unbind(GPUStorageBuf *ssbo);
void GPU_storagebuf_unbind_all();
/**
* Resets the internal slot usage tracking. But there is no guarantee that
* this actually undo the bindings for the next draw call. Only has effect when G_DEBUG_GPU is set.
*/
void GPU_storagebuf_debug_unbind_all();
void GPU_storagebuf_clear_to_zero(GPUStorageBuf *ssbo);

View File

@@ -38,7 +38,11 @@ void GPU_uniformbuf_update(GPUUniformBuf *ubo, const void *data);
void GPU_uniformbuf_bind(GPUUniformBuf *ubo, int slot);
void GPU_uniformbuf_bind_as_ssbo(GPUUniformBuf *ubo, int slot);
void GPU_uniformbuf_unbind(GPUUniformBuf *ubo);
void GPU_uniformbuf_unbind_all();
/**
* Resets the internal slot usage tracking. But there is no guarantee that
* this actually undo the bindings for the next draw call. Only has effect when G_DEBUG_GPU is set.
*/
void GPU_uniformbuf_debug_unbind_all();
void GPU_uniformbuf_clear_to_zero(GPUUniformBuf *ubo);

View File

@@ -46,6 +46,9 @@ class DummyContext : public Context {
return false;
}
void debug_capture_scope_end(void * /*scope*/) override {}
void debug_unbind_all_ubo() override {}
void debug_unbind_all_ssbo() override {}
};
} // namespace blender::gpu

View File

@@ -93,6 +93,11 @@ class Context {
virtual bool debug_capture_scope_begin(void *scope) = 0;
virtual void debug_capture_scope_end(void *scope) = 0;
/* Consider all buffers slot empty after these call for error checking.
* But doesn't really free them. */
virtual void debug_unbind_all_ubo() = 0;
virtual void debug_unbind_all_ssbo() = 0;
bool is_active_on_thread();
};

View File

@@ -19,6 +19,7 @@
#include "GPU_storage_buffer.hh"
#include "GPU_vertex_buffer.hh"
#include "gpu_context_private.hh"
#include "gpu_storage_buffer_private.hh"
/* -------------------------------------------------------------------- */
@@ -85,9 +86,9 @@ void GPU_storagebuf_unbind(GPUStorageBuf *ssbo)
unwrap(ssbo)->unbind();
}
void GPU_storagebuf_unbind_all()
void GPU_storagebuf_debug_unbind_all()
{
/* FIXME */
Context::get()->debug_unbind_all_ssbo();
}
void GPU_storagebuf_clear_to_zero(GPUStorageBuf *ssbo)

View File

@@ -19,6 +19,7 @@
#include "GPU_material.hh"
#include "GPU_uniform_buffer.hh"
#include "gpu_context_private.hh"
#include "gpu_uniform_buffer_private.hh"
/* -------------------------------------------------------------------- */
@@ -240,9 +241,9 @@ void GPU_uniformbuf_unbind(GPUUniformBuf *ubo)
unwrap(ubo)->unbind();
}
void GPU_uniformbuf_unbind_all()
void GPU_uniformbuf_debug_unbind_all()
{
/* FIXME */
Context::get()->debug_unbind_all_ubo();
}
void GPU_uniformbuf_clear_to_zero(GPUUniformBuf *ubo)

View File

@@ -786,6 +786,9 @@ class MTLContext : public Context {
bool debug_capture_scope_begin(void *scope) override;
void debug_capture_scope_end(void *scope) override;
void debug_unbind_all_ubo() override{};
void debug_unbind_all_ssbo() override{};
/*** MTLContext Utility functions. */
/*
* All below functions modify the global state for the context, controlling the flow of

View File

@@ -140,6 +140,7 @@ void GLContext::activate()
/* Not really following the state but we should consider
* no ubo bound when activating a context. */
bound_ubo_slots = 0;
bound_ssbo_slots = 0;
immActivate();
}

View File

@@ -74,6 +74,7 @@ class GLContext : public Context {
/** Used for debugging purpose. Bitflags of all bound slots. */
uint16_t bound_ubo_slots;
uint16_t bound_ssbo_slots;
private:
/**
@@ -135,6 +136,9 @@ class GLContext : public Context {
bool debug_capture_scope_begin(void *scope) override;
void debug_capture_scope_end(void *scope) override;
void debug_unbind_all_ubo() override;
void debug_unbind_all_ssbo() override;
private:
static void orphans_add(Vector<GLuint> &orphan_list, std::mutex &list_mutex, GLuint id);
void orphans_clear();

View File

@@ -224,6 +224,10 @@ void check_gl_resources(const char *info)
* 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;
/* NOTE: This only check binding. To be valid, the bound ssbo needs to
* be big enough to feed the data range the shader awaits. */
uint16_t ssbo_needed = interface->enabled_ssbo_mask_;
ssbo_needed &= ~ctx->bound_ssbo_slots;
/* NOTE: This only check binding. To be valid, the bound texture needs to
* be the same format/target the shader expects. */
uint64_t tex_needed = interface->enabled_tex_mask_;
@@ -233,7 +237,7 @@ void check_gl_resources(const char *info)
uint8_t ima_needed = interface->enabled_ima_mask_;
ima_needed &= ~GLContext::state_manager_active_get()->bound_image_slots();
if (ubo_needed == 0 && tex_needed == 0 && ima_needed == 0) {
if (ubo_needed == 0 && tex_needed == 0 && ima_needed == 0 && ssbo_needed == 0) {
return;
}
@@ -248,6 +252,17 @@ void check_gl_resources(const char *info)
}
}
for (int i = 0; ssbo_needed != 0; i++, ssbo_needed >>= 1) {
if ((ssbo_needed & 1) != 0) {
const ShaderInput *ssbo_input = interface->ssbo_get(i);
const char *ssbo_name = interface->input_name_get(ssbo_input);
const char *sh_name = ctx->shader->name_get();
char msg[256];
SNPRINTF(msg, "Missing SSBO bind at slot %d : %s > %s : %s", i, sh_name, ssbo_name, info);
debug_callback(0, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH, 0, msg, nullptr);
}
}
for (int i = 0; tex_needed != 0; i++, tex_needed >>= 1) {
if ((tex_needed & 1) != 0) {
/* FIXME: texture_get might return an image input instead. */
@@ -432,6 +447,16 @@ bool GLContext::debug_capture_scope_begin(void * /*scope*/)
void GLContext::debug_capture_scope_end(void * /*scope*/) {}
void GLContext::debug_unbind_all_ubo()
{
this->bound_ubo_slots = 0u;
}
void GLContext::debug_unbind_all_ssbo()
{
this->bound_ssbo_slots = 0u;
}
/** \} */
} // namespace blender::gpu

View File

@@ -115,8 +115,7 @@ void GLStorageBuf::bind(int slot)
#ifndef NDEBUG
BLI_assert(slot < 16);
/* TODO */
// GLContext::get()->bound_ssbo_slots |= 1 << slot;
GLContext::get()->bound_ssbo_slots |= 1 << slot;
#endif
}
@@ -133,8 +132,7 @@ void GLStorageBuf::unbind()
/* NOTE: This only unbinds the last bound slot. */
glBindBufferBase(GL_SHADER_STORAGE_BUFFER, slot_, 0);
/* Hope that the context did not change. */
/* TODO */
// GLContext::get()->bound_ssbo_slots &= ~(1 << slot_);
GLContext::get()->bound_ssbo_slots &= ~(1 << slot_);
#endif
slot_ = 0;
}

View File

@@ -56,6 +56,9 @@ class VKContext : public Context, NonCopyable {
bool debug_capture_scope_begin(void *scope) override;
void debug_capture_scope_end(void *scope) override;
void debug_unbind_all_ubo() override{};
void debug_unbind_all_ssbo() override{};
bool has_active_framebuffer() const;
void activate_framebuffer(VKFrameBuffer &framebuffer);
void deactivate_framebuffer();