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.

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

Add padding to normal attribute in Compatibility renderer to match the RD renderers #83906

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

clayjohn
Copy link
Member

Fixes: #83528

This is copy-pasted from the RD renderer:

// If we have an uncompressed surface that contains normals, but not tangents, we need to differentiate the array
// from a compressed array in the shader. To do so, we allow the the normal to read 4 components out of the buffer
// But only give it 2 components per normal. So essentially, each vertex reads the next normal in normal.zw.
// This allows us to avoid adding a shader permutation, and avoid passing dummy tangents. Since the stride is kept small
// this should still be a net win for bandwidth.
// If we do this, then the last normal will read past the end of the array. So we need to pad the array with dummy data.
if (!(new_surface.format & RS::ARRAY_FLAG_COMPRESS_ATTRIBUTES) && (new_surface.format & RS::ARRAY_FORMAT_NORMAL) && !(new_surface.format & RS::ARRAY_FORMAT_TANGENT)) {
// Unfortunately, we need to copy the buffer, which is fine as doing a resize triggers a CoW anyway.
Vector<uint8_t> new_vertex_data;
new_vertex_data.resize_zeroed(new_surface.vertex_data.size() + sizeof(uint16_t) * 2);
memcpy(new_vertex_data.ptrw(), new_surface.vertex_data.ptr(), new_surface.vertex_data.size());
s->vertex_buffer = RD::get_singleton()->vertex_buffer_create(new_vertex_data.size(), new_vertex_data, use_as_storage);
s->vertex_buffer_size = new_vertex_data.size();
} else {
s->vertex_buffer = RD::get_singleton()->vertex_buffer_create(new_surface.vertex_data.size(), new_surface.vertex_data, use_as_storage);
s->vertex_buffer_size = new_surface.vertex_data.size();
}

I missed adding this to the Compatibility renderer in the original PR.

Before:
Screenshot from 2023-10-24 22-37-06

After:
Screenshot from 2023-10-24 22-36-40

Screenshot from 2023-10-24 22-39-13

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a copy paste from the RD renderer, I'm confident approving this.

@akien-mga akien-mga merged commit 2dafd06 into godotengine:master Oct 26, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@clayjohn clayjohn deleted the GL-vertex-padding branch October 27, 2023 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange Lighting Issues in 4.2 Beta using Compatibility Mode
2 participants