Skip to content

Commit

Permalink
Merge pull request #58085 from lawnjelly/buffer_upload_bytes_bug
Browse files Browse the repository at this point in the history 
Fix GL buffer upload size bugs
  • Loading branch information
@akien-mga
akien-mga authored Feb 15, 2022
2 parents cd53185 + 614dc36 commit 869939c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 22 deletions.
4 changes: 2 additions & 2 deletions drivers/gles2/rasterizer_storage_gles2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
18 changes: 10 additions & 8 deletions drivers/gles2/rasterizer_storage_gles2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1356,7 +1356,8 @@ class RasterizerStorageGLES2 : public RasterizerStorage {
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();
Expand All @@ -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);
Expand All @@ -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 changes: 14 additions & 12 deletions drivers/gles3/rasterizer_storage_gles3.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1506,36 +1506,38 @@ class RasterizerStorageGLES3 : public RasterizerStorage {
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);
Expand All @@ -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

0 comments on commit 869939c

Please sign in to comment.