Octahedral Normal/Tangent Compression

[The-O-King]

Bugsquad edit: master version of #46800.

Implement an import option to enable Octahedral Normals in Godot 4.0

[The-O-King]

Wanted to bump this to see if this was something we were looking to get into the beta? I saw a message about the feature freeze and I know this change might be a little wide reaching so just let me know if we need to hold off on it for another release?

Thanks everyone :)

[Calinou]

Wanted to bump this to see if this was something we were looking to get into the beta? I saw a message about the feature freeze and I know this change might be a little wide reaching so just let me know if we need to hold off on it for another release?

I think the goal is still to have octahedral compression in 4.0, as it's a feature that is already present in 3.x. We should aim to avoid functional regressions when possible.

[clayjohn]

Looks good to me!

Before merging we will also need to add the corresponding changes to the GLES3 renderer as well.

[reduz]

We really should get this merged before Beta.

[The-O-King]

Thanks for the feedback! I'm at SIGGRAPH this week actually - but I should be able to also implement this in GLES3 pretty quickly by the end of the upcoming weekend if that works

[The-O-King]

Hi all! Just got a chance to get this into GLES3 - it was a little tricky to validate because I think there's something wrong with the normal map implementation on GLES3 right now? I checked in RenderDoc and the values I expect to come out of the scene shader are correct (and match what I saw in my vulkan implementation which renders correctly) so wanted to see if this was a known issue (or perhaps it has yet to be implemented?) It seems this behavior also occur on a clean master branch as well

Without normal mapping everything renders correctly though on GLES3!

[clayjohn]

Hi all! Just got a chance to get this into GLES3 - it was a little tricky to validate because I think there's something wrong with the normal map implementation on GLES3 right now? I checked in RenderDoc and the values I expect to come out of the scene shader are correct (and match what I saw in my vulkan implementation which renders correctly) so wanted to see if this was a known issue (or perhaps it has yet to be implemented?) It seems this behavior also occur on a clean master branch as well

Without normal mapping everything renders correctly though on GLES3!

I haven't tested normal mapping extensively in GLES3 yet, so my guess is there is an existing issue in GLES3 rather than the problem being with your code.

[clayjohn]

Looks great to me! I'd like @reduz to approve the changes to vector3 before merging.

[reduz]

I think there is one thing missing, which is the softbody code (soft_dynamic_body.cpp:85), which does encoding on CPU.

[clayjohn]

I think there is one thing missing, which is the softbody code (soft_dynamic_body.cpp:85), which does encoding on CPU.

For reference, change will be here:

godot/scene/3d/soft_dynamic_body_3d.cpp

Lines 87 to 95 in b35ff86

	Vector3 n;
	memcpy(&n, p_vector3, sizeof(Vector3));
	n *= Vector3(0.5, 0.5, 0.5);
	n += Vector3(0.5, 0.5, 0.5);
	uint32_t value = 0;
	value |= CLAMP(int(n.x * 1023.0), 0, 1023);
	value |= CLAMP(int(n.y * 1023.0), 0, 1023) << 10;
	value |= CLAMP(int(n.z * 1023.0), 0, 1023) << 20;
	memcpy(&write_buffer[p_vertex_id * stride + offset_normal], &value, sizeof(uint32_t));

And also in Sprite 3D:

godot/scene/3d/sprite_3d.cpp

Lines 561 to 583 in b35ff86

	uint32_t v_normal;
	{
	Vector3 n = normal * Vector3(0.5, 0.5, 0.5) + Vector3(0.5, 0.5, 0.5);
	uint32_t value = 0;
	value |= CLAMP(int(n.x * 1023.0), 0, 1023);
	value |= CLAMP(int(n.y * 1023.0), 0, 1023) << 10;
	value |= CLAMP(int(n.z * 1023.0), 0, 1023) << 20;
	v_normal = value;
	}
	uint32_t v_tangent;
	{
	Plane t = tangent;
	uint32_t value = 0;
	value |= CLAMP(int((t.normal.x * 0.5 + 0.5) * 1023.0), 0, 1023);
	value |= CLAMP(int((t.normal.y * 0.5 + 0.5) * 1023.0), 0, 1023) << 10;
	value |= CLAMP(int((t.normal.z * 0.5 + 0.5) * 1023.0), 0, 1023) << 20;
	if (t.d > 0) {
	value |= 3UL << 30;
	}
	v_tangent = value;
	}

godot/scene/3d/sprite_3d.cpp

Lines 928 to 950 in b35ff86

	uint32_t v_normal;
	{
	Vector3 n = normal * Vector3(0.5, 0.5, 0.5) + Vector3(0.5, 0.5, 0.5);
	uint32_t value = 0;
	value |= CLAMP(int(n.x * 1023.0), 0, 1023);
	value |= CLAMP(int(n.y * 1023.0), 0, 1023) << 10;
	value |= CLAMP(int(n.z * 1023.0), 0, 1023) << 20;
	v_normal = value;
	}
	uint32_t v_tangent;
	{
	Plane t = tangent;
	uint32_t value = 0;
	value |= CLAMP(int((t.normal.x * 0.5 + 0.5) * 1023.0), 0, 1023);
	value |= CLAMP(int((t.normal.y * 0.5 + 0.5) * 1023.0), 0, 1023) << 10;
	value |= CLAMP(int((t.normal.z * 0.5 + 0.5) * 1023.0), 0, 1023) << 20;
	if (t.d > 0) {
	value |= 3UL << 30;
	}
	v_tangent = value;
	}

[The-O-King]

Finished up getting those supported! Do we also have blendshapes to support too? I took a quick look but doesn't seem like they've been implemented yet but wanted to make sure I wasn't missing anything

[clayjohn]

Finished up getting those supported! Do we also have blendshapes to support too? I took a quick look but doesn't seem like they've been implemented yet but wanted to make sure I wasn't missing anything

Good thinking. It looks like we will need to add it here as well:

godot/servers/rendering/renderer_rd/shaders/skeleton.glsl

Line 134 in 0c5f254

normal = decode_abgr_2_10_10_10(src_vertices.data[src_offset]).rgb;

[The-O-King]

Finished up getting those supported! Do we also have blendshapes to support too? I took a quick look but doesn't seem like they've been implemented yet but wanted to make sure I wasn't missing anything

Good thinking. It looks like we will need to add it here as well:

godot/servers/rendering/renderer_rd/shaders/skeleton.glsl

Line 134 in 0c5f254

normal = decode_abgr_2_10_10_10(src_vertices.data[src_offset]).rgb;

Ah perfect it was in the glsl files and I just missed it! I went and added the necessary encode/decode functions! It's a little bigger than what was there previously due to the difference with the tangent's encoding of the binormal sign - if it helps with readability I can reduce the number of functions that are there by inlining the additional work for the tangents for example? - let me know what you think of the implementation :)

[akien-mga]

Thanks a ton!

And congrats for finally getting rid of your "First-time contributor" badge... seems like GitHub didn't count all your great 3.x contributions as valid :(

[The-O-King]

Oh wow it's not often that you get a chance to make two first contributions hehe :P I'm glad I could do my small part in both branches 😄

Thanks all for the help getting this landed! I'm excited to see how we can iterate on this and what other cool new features we can do next :)

[Xyotic]

Does this affect how we need to setup normals/tangets for procedural ArrayMeshes?

[clayjohn]

@Xyotic No. But it will affect how you update normals/tangents if you call RenderingServer.mesh_surface_update_vertex_region

[Xyotic]

Alright thanks.
This should probably be documented tho.
There isnt really a way to tell how to use this method.

[nikitalita]

You changed the offset value here; we're getting 2 bytes from src here (src is int8_t, and we're getting src[0] and src[1]), so the source offset needs to be incremented by 2 bytes. You're incrementing by 4 bytes here.

[nikitalita]

you changed it here, too. sizeof(float) = 4, src is float, we're getting src[0], src[1], and src[2]. 12 bytes total. You're only incrementing by 4 bytes.

[nikitalita]

same issue as above, except that it's 16 bytes total, and you changed it to 4.

[nikitalita]

same issue as above