asgardius/virtualx-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix GL buffer upload size bugs

Browse source 
Wrapper functions for uploading buffers to OpenGL take all sizes and offsets in bytes. Some buffer sizes are specified as units (e.g. float) so require conversion to bytes when calling the buffer upload functions.

Two such bugs have been fixed in blendshapes, and parameter names and comments have been changed to emphasize that sizes should be in bytes.

In addition DEV_ASSERTS in the upload wrappers have been changed to ERR_FAIL.
This commit is contained in:
lawnjelly 2022-02-14 10:56:06 +00:00
parent f68a6daf7d
commit 614dc363ab
3 changed files with 26 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
drivers/gles2/rasterizer_storage_gles2.cpp
View file

@ -3970,7 +3970,7 @@ void RasterizerStorageGLES2::update_dirty_blend_shapes() {
s->blend_shape_buffer_size = buffer_size;
glBufferData(GL_ARRAY_BUFFER, buffer_size * sizeof(float), transform_buffer.read().ptr(), GL_DYNAMIC_DRAW);
} else {
buffer_orphan_and_upload(s->blend_shape_buffer_size, 0, buffer_size * sizeof(float), transform_buffer.read().ptr(), GL_ARRAY_BUFFER, true);
buffer_orphan_and_upload(s->blend_shape_buffer_size * sizeof(float), 0, buffer_size * sizeof(float), transform_buffer.read().ptr(), GL_ARRAY_BUFFER, true);
}
glBindBuffer(GL_ARRAY_BUFFER, 0);
}
@ -3992,7 +3992,7 @@ void RasterizerStorageGLES2::_update_skeleton_transform_buffer(const PoolVector<
glBufferData(GL_ARRAY_BUFFER, buffer_size, p_data.read().ptr(), GL_DYNAMIC_DRAW);
} else {
// this may not be best, it could be better to use glBufferData in both cases.
buffer_orphan_and_upload(resources.skeleton_transform_buffer_size, 0, buffer_size, p_data.read().ptr(), GL_ARRAY_BUFFER, true);
buffer_orphan_and_upload(resources.skeleton_transform_buffer_size * sizeof(float), 0, buffer_size, p_data.read().ptr(), GL_ARRAY_BUFFER, true);
}
glBindBuffer(GL_ARRAY_BUFFER, 0);

18
drivers/gles2/rasterizer_storage_gles2.h
View file

@ -1356,7 +1356,8 @@ public:
virtual String get_video_adapter_name() const;
virtual String get_video_adapter_vendor() const;
void buffer_orphan_and_upload(unsigned int p_buffer_size, unsigned int p_offset, unsigned int p_data_size, const void *p_data, GLenum p_target = GL_ARRAY_BUFFER, GLenum p_usage = GL_DYNAMIC_DRAW, bool p_optional_orphan = false) const;
// NOTE : THESE SIZES ARE IN BYTES. BUFFER SIZES MAY NOT BE SPECIFIED IN BYTES SO REMEMBER TO CONVERT THEM WHEN CALLING.
void buffer_orphan_and_upload(unsigned int p_buffer_size_bytes, unsigned int p_offset_bytes, unsigned int p_data_size_bytes, const void *p_data, GLenum p_target = GL_ARRAY_BUFFER, GLenum p_usage = GL_DYNAMIC_DRAW, bool p_optional_orphan = false) const;
bool safe_buffer_sub_data(unsigned int p_total_buffer_size, GLenum p_target, unsigned int p_offset, unsigned int p_data_size, const void *p_data, unsigned int &r_offset_after) const;
RasterizerStorageGLES2();
@ -1376,17 +1377,18 @@ inline bool RasterizerStorageGLES2::safe_buffer_sub_data(unsigned int p_total_bu
// standardize the orphan / upload in one place so it can be changed per platform as necessary, and avoid future
// bugs causing pipeline stalls
inline void RasterizerStorageGLES2::buffer_orphan_and_upload(unsigned int p_buffer_size, unsigned int p_offset, unsigned int p_data_size, const void *p_data, GLenum p_target, GLenum p_usage, bool p_optional_orphan) const {
// NOTE : THESE SIZES ARE IN BYTES. BUFFER SIZES MAY NOT BE SPECIFIED IN BYTES SO REMEMBER TO CONVERT THEM WHEN CALLING.
inline void RasterizerStorageGLES2::buffer_orphan_and_upload(unsigned int p_buffer_size_bytes, unsigned int p_offset_bytes, unsigned int p_data_size_bytes, const void *p_data, GLenum p_target, GLenum p_usage, bool p_optional_orphan) const {
// Orphan the buffer to avoid CPU/GPU sync points caused by glBufferSubData
// Was previously #ifndef GLES_OVER_GL however this causes stalls on desktop mac also (and possibly other)
if (!p_optional_orphan || (config.should_orphan)) {
glBufferData(p_target, p_buffer_size, nullptr, p_usage);
glBufferData(p_target, p_buffer_size_bytes, nullptr, p_usage);
#ifdef RASTERIZER_EXTRA_CHECKS
// fill with garbage off the end of the array
if (p_buffer_size) {
unsigned int start = p_offset + p_data_size;
if (p_buffer_size_bytes) {
unsigned int start = p_offset_bytes + p_data_size_bytes;
unsigned int end = start + 1024;
if (end < p_buffer_size) {
if (end < p_buffer_size_bytes) {
uint8_t *garbage = (uint8_t *)alloca(1024);
for (int n = 0; n < 1024; n++) {
garbage[n] = Math::random(0, 255);
@ -1396,8 +1398,8 @@ inline void RasterizerStorageGLES2::buffer_orphan_and_upload(unsigned int p_buff
}
#endif
}
DEV_ASSERT((p_offset + p_data_size) <= p_buffer_size);
glBufferSubData(p_target, p_offset, p_data_size, p_data);
ERR_FAIL_COND((p_offset_bytes + p_data_size_bytes) > p_buffer_size_bytes);
glBufferSubData(p_target, p_offset_bytes, p_data_size_bytes, p_data);
}
#endif // RASTERIZERSTORAGEGLES2_H

26
drivers/gles3/rasterizer_storage_gles3.h
View file

@ -1506,36 +1506,38 @@ public:
virtual String get_video_adapter_name() const;
virtual String get_video_adapter_vendor() const;
void buffer_orphan_and_upload(unsigned int p_buffer_size, unsigned int p_offset, unsigned int p_data_size, const void *p_data, GLenum p_target = GL_ARRAY_BUFFER, GLenum p_usage = GL_DYNAMIC_DRAW, bool p_optional_orphan = false) const;
bool safe_buffer_sub_data(unsigned int p_total_buffer_size, GLenum p_target, unsigned int p_offset, unsigned int p_data_size, const void *p_data, unsigned int &r_offset_after) const;
// NOTE : THESE SIZES ARE IN BYTES. BUFFER SIZES MAY NOT BE SPECIFIED IN BYTES SO REMEMBER TO CONVERT THEM WHEN CALLING.
void buffer_orphan_and_upload(unsigned int p_buffer_size_bytes, unsigned int p_offset_bytes, unsigned int p_data_size_bytes, const void *p_data, GLenum p_target = GL_ARRAY_BUFFER, GLenum p_usage = GL_DYNAMIC_DRAW, bool p_optional_orphan = false) const;
bool safe_buffer_sub_data(unsigned int p_total_buffer_size_bytes, GLenum p_target, unsigned int p_offset_bytes, unsigned int p_data_size_bytes, const void *p_data, unsigned int &r_offset_after_bytes) const;
RasterizerStorageGLES3();
~RasterizerStorageGLES3();
};
inline bool RasterizerStorageGLES3::safe_buffer_sub_data(unsigned int p_total_buffer_size, GLenum p_target, unsigned int p_offset, unsigned int p_data_size, const void *p_data, unsigned int &r_offset_after) const {
r_offset_after = p_offset + p_data_size;
inline bool RasterizerStorageGLES3::safe_buffer_sub_data(unsigned int p_total_buffer_size_bytes, GLenum p_target, unsigned int p_offset_bytes, unsigned int p_data_size_bytes, const void *p_data, unsigned int &r_offset_after_bytes) const {
r_offset_after_bytes = p_offset_bytes + p_data_size_bytes;
#ifdef DEBUG_ENABLED
// we are trying to write across the edge of the buffer
if (r_offset_after > p_total_buffer_size) {
if (r_offset_after_bytes > p_total_buffer_size_bytes) {
return false;
}
#endif
glBufferSubData(p_target, p_offset, p_data_size, p_data);
glBufferSubData(p_target, p_offset_bytes, p_data_size_bytes, p_data);
return true;
}
// standardize the orphan / upload in one place so it can be changed per platform as necessary, and avoid future
// bugs causing pipeline stalls
inline void RasterizerStorageGLES3::buffer_orphan_and_upload(unsigned int p_buffer_size, unsigned int p_offset, unsigned int p_data_size, const void *p_data, GLenum p_target, GLenum p_usage, bool p_optional_orphan) const {
// NOTE : THESE SIZES ARE IN BYTES. BUFFER SIZES MAY NOT BE SPECIFIED IN BYTES SO REMEMBER TO CONVERT THEM WHEN CALLING.
inline void RasterizerStorageGLES3::buffer_orphan_and_upload(unsigned int p_buffer_size_bytes, unsigned int p_offset_bytes, unsigned int p_data_size_bytes, const void *p_data, GLenum p_target, GLenum p_usage, bool p_optional_orphan) const {
// Orphan the buffer to avoid CPU/GPU sync points caused by glBufferSubData
// Was previously #ifndef GLES_OVER_GL however this causes stalls on desktop mac also (and possibly other)
if (!p_optional_orphan || (config.should_orphan)) {
glBufferData(p_target, p_buffer_size, nullptr, p_usage);
glBufferData(p_target, p_buffer_size_bytes, nullptr, p_usage);
#ifdef RASTERIZER_EXTRA_CHECKS
// fill with garbage off the end of the array
if (p_buffer_size) {
unsigned int start = p_offset + p_data_size;
if (p_buffer_size_bytes) {
unsigned int start = p_offset_bytes + p_data_size_bytes;
unsigned int end = start + 1024;
if (end < p_buffer_size) {
uint8_t *garbage = (uint8_t *)alloca(1024);
@ -1547,8 +1549,8 @@ inline void RasterizerStorageGLES3::buffer_orphan_and_upload(unsigned int p_buff
}
#endif
}
DEV_ASSERT((p_offset + p_data_size) <= p_buffer_size);
glBufferSubData(p_target, p_offset, p_data_size, p_data);
ERR_FAIL_COND((p_offset_bytes + p_data_size_bytes) > p_buffer_size_bytes);
glBufferSubData(p_target, p_offset_bytes, p_data_size_bytes, p_data);
}
#endif // RASTERIZERSTORAGEGLES3_H