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

GLES2 Compression on Blend Shapes Fix #58838

Merged
merged 1 commit into from
Mar 6, 2022

Conversation

The-O-King
Copy link
Contributor

@The-O-King The-O-King commented Mar 6, 2022

When compressed vertex positions are used in a blend shapes mesh, we
need to make sure we set the w-component of the position vector to 1.0

When octahedral compression is used on normals/tangents, they need to be
converted to cartesian floats to be used for blend shapes

This conversion also changes the number of components of that vertex
attribute, which caused issues because previously there was an
assumption that you had the same number of components in the blend shape
buffer as you did in the original mesh's buffer (which is not true for
oct norm/tang)

Bugsquad edit: Follow-up to #58808.

When compressed vertex positions are used in a blend shapes mesh, we
need to make sure we set the w-component of the position vector to 1.0

When octahedral compression is used on normals/tangents, they need to be
converted to cartesian floats to be used for blend shapes

This conversion also changes the number of components of that vertex
attribute, which caused issues because previously there was an
assumption that you had the same number of components in the blend shape
buffer as you did in the original mesh's buffer (which is not true for
oct norm/tang)
@The-O-King The-O-King requested a review from a team as a code owner March 6, 2022 16:02
glVertexAttribPointer(s->attribs[i].index, s->attribs[i].size, GL_FLOAT, GL_FALSE, 8 * 4 * sizeof(float), CAST_INT_TO_UCHAR_PTR(i * 4 * sizeof(float)));
// When using octahedral compression (2 component normal/tangent)
// decompression changes the component count to 3/4
int size;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also have a solution which creates a separate Vertex Attributes array for the blend shape vertex buffer so that we don't have to have this check here; but this was the easiest fix, not sure which would be preferable (performance wise adding these checks didn't seem to cause any issues)

@akien-mga akien-mga merged commit d93a4a8 into godotengine:3.x Mar 6, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.4.4.

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.

3 participants