godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix GL buffer upload size bugs 

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.

(cherry picked from commit 614dc363ab)
This commit is contained in:
lawnjelly 2022-02-14 10:56:06 +00:00 committed by Rémi Verschelde
parent 285c2d3a84
commit f911ec589d
No known key found for this signature in database
GPG Key ID: C3336907360768E1
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; s->blend_shape_buffer_size = buffer_size;
glBufferData(GL_ARRAY_BUFFER, buffer_size * sizeof(float), transform_buffer.read().ptr(), GL_DYNAMIC_DRAW); glBufferData(GL_ARRAY_BUFFER, buffer_size * sizeof(float), transform_buffer.read().ptr(), GL_DYNAMIC_DRAW);
} else { } 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); 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); glBufferData(GL_ARRAY_BUFFER, buffer_size, p_data.read().ptr(), GL_DYNAMIC_DRAW);
} else { } else {
// this may not be best, it could be better to use glBufferData in both cases. // 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); glBindBuffer(GL_ARRAY_BUFFER, 0);

18
drivers/gles2/rasterizer_storage_gles2.h
View File

@ -1353,7 +1353,8 @@ public:
virtual String get_video_adapter_name() const; virtual String get_video_adapter_name() const;
virtual String get_video_adapter_vendor() 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; 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(); RasterizerStorageGLES2();
@ -1373,17 +1374,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 // standardize the orphan / upload in one place so it can be changed per platform as necessary, and avoid future
// bugs causing pipeline stalls // 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 // 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) // Was previously #ifndef GLES_OVER_GL however this causes stalls on desktop mac also (and possibly other)
if (!p_optional_orphan || (config.should_orphan)) { 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 #ifdef RASTERIZER_EXTRA_CHECKS
// fill with garbage off the end of the array // fill with garbage off the end of the array
if (p_buffer_size) { if (p_buffer_size_bytes) {
unsigned int start = p_offset + p_data_size; unsigned int start = p_offset_bytes + p_data_size_bytes;
unsigned int end = start + 1024; unsigned int end = start + 1024;
if (end < p_buffer_size) { if (end < p_buffer_size_bytes) {
uint8_t *garbage = (uint8_t *)alloca(1024); uint8_t *garbage = (uint8_t *)alloca(1024);
for (int n = 0; n < 1024; n++) { for (int n = 0; n < 1024; n++) {
garbage[n] = Math::random(0, 255); garbage[n] = Math::random(0, 255);
@ -1393,8 +1395,8 @@ inline void RasterizerStorageGLES2::buffer_orphan_and_upload(unsigned int p_buff
} }
#endif #endif
} }
DEV_ASSERT((p_offset + p_data_size) <= p_buffer_size); ERR_FAIL_COND((p_offset_bytes + p_data_size_bytes) > p_buffer_size_bytes);
glBufferSubData(p_target, p_offset, p_data_size, p_data); glBufferSubData(p_target, p_offset_bytes, p_data_size_bytes, p_data);
} }
#endif // RASTERIZERSTORAGEGLES2_H #endif // RASTERIZERSTORAGEGLES2_H

26
drivers/gles3/rasterizer_storage_gles3.h
View File

@ -1491,35 +1491,37 @@ public:
virtual String get_video_adapter_name() const; virtual String get_video_adapter_name() const;
virtual String get_video_adapter_vendor() 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.
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; 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 { 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 = p_offset + p_data_size; r_offset_after_bytes = p_offset_bytes + p_data_size_bytes;
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
// we are trying to write across the edge of the buffer // 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; return false;
} }
#endif #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; return true;
} }
// standardize the orphan / upload in one place so it can be changed per platform as necessary, and avoid future // standardize the orphan / upload in one place so it can be changed per platform as necessary, and avoid future
// bugs causing pipeline stalls // 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 // 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) // Was previously #ifndef GLES_OVER_GL however this causes stalls on desktop mac also (and possibly other)
if (!p_optional_orphan || (config.should_orphan)) { 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 #ifdef RASTERIZER_EXTRA_CHECKS
// fill with garbage off the end of the array // fill with garbage off the end of the array
if (p_buffer_size) { if (p_buffer_size_bytes) {
unsigned int start = p_offset + p_data_size; unsigned int start = p_offset_bytes + p_data_size_bytes;
unsigned int end = start + 1024; unsigned int end = start + 1024;
if (end < p_buffer_size) { if (end < p_buffer_size) {
uint8_t *garbage = (uint8_t *)alloca(1024); uint8_t *garbage = (uint8_t *)alloca(1024);
@ -1531,8 +1533,8 @@ inline void RasterizerStorageGLES3::buffer_orphan_and_upload(unsigned int p_buff
} }
#endif #endif
} }
DEV_ASSERT((p_offset + p_data_size) <= p_buffer_size); ERR_FAIL_COND((p_offset_bytes + p_data_size_bytes) > p_buffer_size_bytes);
glBufferSubData(p_target, p_offset, p_data_size, p_data); glBufferSubData(p_target, p_offset_bytes, p_data_size_bytes, p_data);
} }
#endif // RASTERIZERSTORAGEGLES3_H #endif // RASTERIZERSTORAGEGLES3_H