From 54525808aa58b0f94892d3f4e5919cb4ae9493cf Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Sat, 21 Mar 2020 23:47:08 -0400 Subject: [PATCH] mesa: don't ever bind NullBufferObj to glBindBuffer(Base,Range) slots Reviewed-by: Pierre-Eric Pelloux-Prayer Part-of: --- .../drivers/dri/i965/brw_wm_surface_state.c | 2 +- src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +- src/mesa/main/bufferobj.c | 124 +++++++++--------- src/mesa/main/bufferobj.h | 3 +- src/mesa/main/get.c | 9 +- src/mesa/main/transformfeedback.c | 28 ++-- src/mesa/main/transformfeedback.h | 4 +- src/mesa/main/varray.c | 10 +- src/mesa/state_tracker/st_atom_constbuf.c | 2 +- src/mesa/state_tracker/st_atom_storagebuf.c | 2 +- 10 files changed, 96 insertions(+), 90 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c index bb2d8043f18..a200468ac2a 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c @@ -1329,7 +1329,7 @@ upload_buffer_surface(struct brw_context *brw, { struct gl_context *ctx = &brw->ctx; - if (binding->BufferObject == ctx->Shared->NullBufferObj) { + if (!binding->BufferObject) { emit_null_surface_state(brw, NULL, out_offset); } else { ptrdiff_t size = binding->BufferObject->Size - binding->Offset; diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index a3738e63511..e9d3b614ee3 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -3125,7 +3125,7 @@ genX(upload_push_constant_packets)(struct brw_context *brw) const struct gl_buffer_binding *binding = &ctx->UniformBufferBindings[block->Binding]; - if (binding->BufferObject == ctx->Shared->NullBufferObj) { + if (!binding->BufferObject) { static unsigned msg_id = 0; _mesa_gl_debugf(ctx, &msg_id, MESA_DEBUG_SOURCE_API, MESA_DEBUG_TYPE_UNDEFINED, diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index 39a620b22e6..4119f3ec92c 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -872,7 +872,7 @@ _mesa_init_buffer_objects( struct gl_context *ctx ) for (i = 0; i < MAX_COMBINED_UNIFORM_BUFFERS; i++) { _mesa_reference_buffer_object(ctx, &ctx->UniformBufferBindings[i].BufferObject, - ctx->Shared->NullBufferObj); + NULL); ctx->UniformBufferBindings[i].Offset = -1; ctx->UniformBufferBindings[i].Size = -1; } @@ -880,7 +880,7 @@ _mesa_init_buffer_objects( struct gl_context *ctx ) for (i = 0; i < MAX_COMBINED_SHADER_STORAGE_BUFFERS; i++) { _mesa_reference_buffer_object(ctx, &ctx->ShaderStorageBufferBindings[i].BufferObject, - ctx->Shared->NullBufferObj); + NULL); ctx->ShaderStorageBufferBindings[i].Offset = -1; ctx->ShaderStorageBufferBindings[i].Size = -1; } @@ -888,7 +888,7 @@ _mesa_init_buffer_objects( struct gl_context *ctx ) for (i = 0; i < MAX_COMBINED_ATOMIC_BUFFERS; i++) { _mesa_reference_buffer_object(ctx, &ctx->AtomicBufferBindings[i].BufferObject, - ctx->Shared->NullBufferObj); + NULL); ctx->AtomicBufferBindings[i].Offset = 0; ctx->AtomicBufferBindings[i].Size = 0; } @@ -1079,10 +1079,9 @@ _mesa_lookup_bufferobj_err(struct gl_context *ctx, GLuint buffer, * of an existing buffer object. * * If the buffer ID refers to an existing buffer object, a pointer - * to the buffer object is returned. If the ID is zero, a pointer - * to the shared NullBufferObj is returned. If the ID is not zero - * and does not refer to a valid buffer object, this function - * returns NULL. + * to the buffer object is returned. If the ID is zero, NULL is returned. + * If the ID is not zero and does not refer to a valid buffer object, this + * function returns NULL. * * This function assumes that the caller has already locked the * hash table mutex by calling @@ -1091,9 +1090,12 @@ _mesa_lookup_bufferobj_err(struct gl_context *ctx, GLuint buffer, struct gl_buffer_object * _mesa_multi_bind_lookup_bufferobj(struct gl_context *ctx, const GLuint *buffers, - GLuint index, const char *caller) + GLuint index, const char *caller, + bool *error) { - struct gl_buffer_object *bufObj; + struct gl_buffer_object *bufObj = NULL; + + *error = false; if (buffers[index] != 0) { bufObj = _mesa_lookup_bufferobj_locked(ctx, buffers[index]); @@ -1102,20 +1104,20 @@ _mesa_multi_bind_lookup_bufferobj(struct gl_context *ctx, when they don't exist. */ if (bufObj == &DummyBufferObject) bufObj = NULL; - } else - bufObj = ctx->Shared->NullBufferObj; - if (!bufObj) { - /* The ARB_multi_bind spec says: - * - * "An INVALID_OPERATION error is generated if any value - * in is not zero or the name of an existing - * buffer object (per binding)." - */ - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(buffers[%u]=%u is not zero or the name " - "of an existing buffer object)", - caller, index, buffers[index]); + if (!bufObj) { + /* The ARB_multi_bind spec says: + * + * "An INVALID_OPERATION error is generated if any value + * in is not zero or the name of an existing + * buffer object (per binding)." + */ + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(buffers[%u]=%u is not zero or the name " + "of an existing buffer object)", + caller, index, buffers[index]); + *error = true; + } } return bufObj; @@ -1255,17 +1257,21 @@ set_buffer_multi_binding(struct gl_context *ctx, gl_buffer_usage usage) { struct gl_buffer_object *bufObj; + if (binding->BufferObject && binding->BufferObject->Name == buffers[idx]) bufObj = binding->BufferObject; - else - bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, idx, caller); - - if (bufObj) { - if (bufObj == ctx->Shared->NullBufferObj) - set_buffer_binding(ctx, binding, bufObj, -1, -1, !range, usage); - else - set_buffer_binding(ctx, binding, bufObj, offset, size, !range, usage); + else { + bool error; + bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, idx, caller, + &error); + if (error) + return; } + + if (!bufObj) + set_buffer_binding(ctx, binding, bufObj, -1, -1, !range, usage); + else + set_buffer_binding(ctx, binding, bufObj, offset, size, !range, usage); } static void @@ -1367,7 +1373,7 @@ bind_buffer_base_uniform_buffer(struct gl_context *ctx, _mesa_reference_buffer_object(ctx, &ctx->UniformBuffer, bufObj); - if (bufObj == ctx->Shared->NullBufferObj) + if (!bufObj) bind_uniform_buffer(ctx, index, bufObj, -1, -1, GL_TRUE); else bind_uniform_buffer(ctx, index, bufObj, 0, 0, GL_TRUE); @@ -1389,7 +1395,7 @@ bind_buffer_base_shader_storage_buffer(struct gl_context *ctx, _mesa_reference_buffer_object(ctx, &ctx->ShaderStorageBuffer, bufObj); - if (bufObj == ctx->Shared->NullBufferObj) + if (!bufObj) bind_shader_storage_buffer(ctx, index, bufObj, -1, -1, GL_TRUE); else bind_shader_storage_buffer(ctx, index, bufObj, 0, 0, GL_TRUE); @@ -1411,7 +1417,7 @@ bind_buffer_base_atomic_buffer(struct gl_context *ctx, _mesa_reference_buffer_object(ctx, &ctx->AtomicBuffer, bufObj); - if (bufObj == ctx->Shared->NullBufferObj) + if (!bufObj) bind_atomic_buffer(ctx, index, bufObj, -1, -1, GL_TRUE); else bind_atomic_buffer(ctx, index, bufObj, 0, 0, GL_TRUE); @@ -1484,16 +1490,14 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids) if (ctx->TransformFeedback.CurrentObject->Buffers[j] == bufObj) { _mesa_bind_buffer_base_transform_feedback(ctx, ctx->TransformFeedback.CurrentObject, - j, ctx->Shared->NullBufferObj, - false); + j, NULL, false); } } /* unbind UBO binding points */ for (j = 0; j < ctx->Const.MaxUniformBufferBindings; j++) { if (ctx->UniformBufferBindings[j].BufferObject == bufObj) { - bind_buffer_base_uniform_buffer(ctx, j, - ctx->Shared->NullBufferObj); + bind_buffer_base_uniform_buffer(ctx, j, NULL); } } @@ -1504,8 +1508,7 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids) /* unbind SSBO binding points */ for (j = 0; j < ctx->Const.MaxShaderStorageBufferBindings; j++) { if (ctx->ShaderStorageBufferBindings[j].BufferObject == bufObj) { - bind_buffer_base_shader_storage_buffer(ctx, j, - ctx->Shared->NullBufferObj); + bind_buffer_base_shader_storage_buffer(ctx, j, NULL); } } @@ -1516,8 +1519,7 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids) /* unbind Atomci Buffer binding points */ for (j = 0; j < ctx->Const.MaxAtomicBufferBindings; j++) { if (ctx->AtomicBufferBindings[j].BufferObject == bufObj) { - bind_buffer_base_atomic_buffer(ctx, j, - ctx->Shared->NullBufferObj); + bind_buffer_base_atomic_buffer(ctx, j, NULL); } } @@ -3713,7 +3715,7 @@ bind_buffer_range_uniform_buffer(struct gl_context *ctx, GLuint index, struct gl_buffer_object *bufObj, GLintptr offset, GLsizeiptr size) { - if (bufObj == ctx->Shared->NullBufferObj) { + if (!bufObj) { offset = -1; size = -1; } @@ -3756,7 +3758,7 @@ bind_buffer_range_shader_storage_buffer(struct gl_context *ctx, GLintptr offset, GLsizeiptr size) { - if (bufObj == ctx->Shared->NullBufferObj) { + if (!bufObj) { offset = -1; size = -1; } @@ -3798,7 +3800,7 @@ bind_buffer_range_atomic_buffer(struct gl_context *ctx, GLuint index, struct gl_buffer_object *bufObj, GLintptr offset, GLsizeiptr size) { - if (bufObj == ctx->Shared->NullBufferObj) { + if (!bufObj) { offset = -1; size = -1; } @@ -3933,11 +3935,9 @@ error_check_bind_shader_storage_buffers(struct gl_context *ctx, static void unbind_uniform_buffers(struct gl_context *ctx, GLuint first, GLsizei count) { - struct gl_buffer_object *bufObj = ctx->Shared->NullBufferObj; - for (int i = 0; i < count; i++) set_buffer_binding(ctx, &ctx->UniformBufferBindings[first + i], - bufObj, -1, -1, GL_TRUE, 0); + NULL, -1, -1, GL_TRUE, 0); } /** @@ -3948,11 +3948,9 @@ static void unbind_shader_storage_buffers(struct gl_context *ctx, GLuint first, GLsizei count) { - struct gl_buffer_object *bufObj = ctx->Shared->NullBufferObj; - for (int i = 0; i < count; i++) set_buffer_binding(ctx, &ctx->ShaderStorageBufferBindings[first + i], - bufObj, -1, -1, GL_TRUE, 0); + NULL, -1, -1, GL_TRUE, 0); } static void @@ -4212,11 +4210,9 @@ unbind_xfb_buffers(struct gl_context *ctx, struct gl_transform_feedback_object *tfObj, GLuint first, GLsizei count) { - struct gl_buffer_object * const bufObj = ctx->Shared->NullBufferObj; - for (int i = 0; i < count; i++) _mesa_set_transform_feedback_binding(ctx, tfObj, first + i, - bufObj, 0, 0); + NULL, 0, 0); } static void @@ -4325,12 +4321,16 @@ bind_xfb_buffers(struct gl_context *ctx, if (boundBufObj && boundBufObj->Name == buffers[i]) bufObj = boundBufObj; - else - bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, caller); + else { + bool error; + bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, caller, + &error); + if (error) + continue; + } - if (bufObj) - _mesa_set_transform_feedback_binding(ctx, tfObj, index, bufObj, - offset, size); + _mesa_set_transform_feedback_binding(ctx, tfObj, index, bufObj, + offset, size); } _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); @@ -4371,11 +4371,9 @@ error_check_bind_atomic_buffers(struct gl_context *ctx, static void unbind_atomic_buffers(struct gl_context *ctx, GLuint first, GLsizei count) { - struct gl_buffer_object * const bufObj = ctx->Shared->NullBufferObj; - for (int i = 0; i < count; i++) set_buffer_binding(ctx, &ctx->AtomicBufferBindings[first + i], - bufObj, -1, -1, GL_TRUE, 0); + NULL, -1, -1, GL_TRUE, 0); } static void @@ -4492,7 +4490,7 @@ bind_buffer_range(GLenum target, GLuint index, GLuint buffer, GLintptr offset, } if (buffer == 0) { - bufObj = ctx->Shared->NullBufferObj; + bufObj = NULL; } else { bufObj = _mesa_lookup_bufferobj(ctx, buffer); if (!_mesa_handle_bind_buffer_gen(ctx, buffer, @@ -4590,7 +4588,7 @@ _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer) } if (buffer == 0) { - bufObj = ctx->Shared->NullBufferObj; + bufObj = NULL; } else { bufObj = _mesa_lookup_bufferobj(ctx, buffer); if (!_mesa_handle_bind_buffer_gen(ctx, buffer, diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h index 588a6a7449b..44ac95f40e9 100644 --- a/src/mesa/main/bufferobj.h +++ b/src/mesa/main/bufferobj.h @@ -101,7 +101,8 @@ _mesa_lookup_bufferobj_err(struct gl_context *ctx, GLuint buffer, extern struct gl_buffer_object * _mesa_multi_bind_lookup_bufferobj(struct gl_context *ctx, const GLuint *buffers, - GLuint index, const char *caller); + GLuint index, const char *caller, + bool *error); extern void _mesa_initialize_buffer_object(struct gl_context *ctx, diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index f71fd55c814..08315517167 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -2531,7 +2531,8 @@ find_value_indexed(const char *func, GLenum pname, GLuint index, union value *v) goto invalid_value; if (!ctx->Extensions.ARB_uniform_buffer_object) goto invalid_enum; - v->value_int = ctx->UniformBufferBindings[index].BufferObject->Name; + buf = ctx->UniformBufferBindings[index].BufferObject; + v->value_int = buf ? buf->Name : 0; return TYPE_INT; case GL_UNIFORM_BUFFER_START: @@ -2558,7 +2559,8 @@ find_value_indexed(const char *func, GLenum pname, GLuint index, union value *v) goto invalid_enum; if (index >= ctx->Const.MaxShaderStorageBufferBindings) goto invalid_value; - v->value_int = ctx->ShaderStorageBufferBindings[index].BufferObject->Name; + buf = ctx->ShaderStorageBufferBindings[index].BufferObject; + v->value_int = buf ? buf->Name : 0; return TYPE_INT; case GL_SHADER_STORAGE_BUFFER_START: @@ -2593,7 +2595,8 @@ find_value_indexed(const char *func, GLenum pname, GLuint index, union value *v) goto invalid_enum; if (index >= ctx->Const.MaxAtomicBufferBindings) goto invalid_value; - v->value_int = ctx->AtomicBufferBindings[index].BufferObject->Name; + buf = ctx->AtomicBufferBindings[index].BufferObject; + v->value_int = buf ? buf->Name : 0; return TYPE_INT; case GL_ATOMIC_COUNTER_BUFFER_START: diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c index b9cbe5a309b..315233b2c6b 100644 --- a/src/mesa/main/transformfeedback.c +++ b/src/mesa/main/transformfeedback.c @@ -633,7 +633,7 @@ _mesa_validate_buffer_range_xfb(struct gl_context *ctx, return false; } - if (size <= 0 && (dsa || bufObj != ctx->Shared->NullBufferObj)) { + if (size <= 0 && (dsa || bufObj)) { /* OpenGL 4.5 core profile, 6.1, pdf page 82: "An INVALID_VALUE error is * generated by BindBufferRange if buffer is non-zero and size is less * than or equal to zero." @@ -709,20 +709,22 @@ lookup_transform_feedback_object_err(struct gl_context *ctx, */ static struct gl_buffer_object * lookup_transform_feedback_bufferobj_err(struct gl_context *ctx, - GLuint buffer, const char* func) + GLuint buffer, const char* func, + bool *error) { - struct gl_buffer_object *bufObj; + struct gl_buffer_object *bufObj = NULL; + + *error = false; /* OpenGL 4.5 core profile, 13.2, pdf page 444: buffer must be zero or the * name of an existing buffer object. */ - if (buffer == 0) { - bufObj = ctx->Shared->NullBufferObj; - } else { + if (buffer) { bufObj = _mesa_lookup_bufferobj(ctx, buffer); if (!bufObj) { _mesa_error(ctx, GL_INVALID_VALUE, "%s(invalid buffer=%u)", func, buffer); + *error = true; } } @@ -742,9 +744,11 @@ _mesa_TransformFeedbackBufferBase(GLuint xfb, GLuint index, GLuint buffer) return; } + bool error; bufObj = lookup_transform_feedback_bufferobj_err(ctx, buffer, - "glTransformFeedbackBufferBase"); - if (!bufObj) { + "glTransformFeedbackBufferBase", + &error); + if (error) { return; } @@ -765,9 +769,11 @@ _mesa_TransformFeedbackBufferRange(GLuint xfb, GLuint index, GLuint buffer, return; } + bool error; bufObj = lookup_transform_feedback_bufferobj_err(ctx, buffer, - "glTransformFeedbackBufferRange"); - if (!bufObj) { + "glTransformFeedbackBufferRange", + &error); + if (error) { return; } @@ -793,7 +799,7 @@ bind_buffer_offset(struct gl_context *ctx, struct gl_buffer_object *bufObj; if (buffer == 0) { - bufObj = ctx->Shared->NullBufferObj; + bufObj = NULL; } else { bufObj = _mesa_lookup_bufferobj(ctx, buffer); if (!no_error && !bufObj) { diff --git a/src/mesa/main/transformfeedback.h b/src/mesa/main/transformfeedback.h index 729b790ed6f..c699fcb94f3 100644 --- a/src/mesa/main/transformfeedback.h +++ b/src/mesa/main/transformfeedback.h @@ -165,11 +165,11 @@ _mesa_set_transform_feedback_binding(struct gl_context *ctx, { _mesa_reference_buffer_object(ctx, &tfObj->Buffers[index], bufObj); - tfObj->BufferNames[index] = bufObj->Name; + tfObj->BufferNames[index] = bufObj ? bufObj->Name : 0; tfObj->Offset[index] = offset; tfObj->RequestedSize[index] = size; - if (bufObj != ctx->Shared->NullBufferObj) + if (bufObj) bufObj->UsageHistory |= USAGE_TRANSFORM_FEEDBACK_BUFFER; } diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c index f516d0008d6..7fa35879e58 100644 --- a/src/mesa/main/varray.c +++ b/src/mesa/main/varray.c @@ -3167,13 +3167,11 @@ vertex_array_vertex_buffers(struct gl_context *ctx, else if (binding->BufferObj && binding->BufferObj->Name == buffers[i]) vbo = binding->BufferObj; else { - vbo = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, func); - if (!vbo) + bool error; + vbo = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, func, + &error); + if (error) continue; - - /* TODO: remove this hack */ - if (vbo == ctx->Shared->NullBufferObj) - vbo = NULL; } } else { vbo = NULL; diff --git a/src/mesa/state_tracker/st_atom_constbuf.c b/src/mesa/state_tracker/st_atom_constbuf.c index 530e2f2ce2e..c1682544f49 100644 --- a/src/mesa/state_tracker/st_atom_constbuf.c +++ b/src/mesa/state_tracker/st_atom_constbuf.c @@ -212,7 +212,7 @@ st_bind_ubos(struct st_context *st, struct gl_program *prog, &st->ctx->UniformBufferBindings[prog->sh.UniformBlocks[i]->Binding]; st_obj = st_buffer_object(binding->BufferObject); - cb.buffer = st_obj->buffer; + cb.buffer = st_obj ? st_obj->buffer : NULL; if (cb.buffer) { cb.buffer_offset = binding->Offset; diff --git a/src/mesa/state_tracker/st_atom_storagebuf.c b/src/mesa/state_tracker/st_atom_storagebuf.c index b12c9164464..2c5d4885841 100644 --- a/src/mesa/state_tracker/st_atom_storagebuf.c +++ b/src/mesa/state_tracker/st_atom_storagebuf.c @@ -58,7 +58,7 @@ st_bind_ssbos(struct st_context *st, struct gl_program *prog, prog->sh.ShaderStorageBlocks[i]->Binding]; st_obj = st_buffer_object(binding->BufferObject); - sb->buffer = st_obj->buffer; + sb->buffer = st_obj ? st_obj->buffer : NULL; if (sb->buffer) { sb->buffer_offset = binding->Offset; -- 2.30.2