Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GL buffer upload size bugs #58085

Merged
merged 1 commit into from
Feb 15, 2022
Merged

Fix GL buffer upload size bugs #58085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions drivers/gles2/rasterizer_storage_gles2.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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