Skip to content

Commit d642e1f

Browse files
committed
Use Vector4 storage for tangents in SurfaceTool for consistency
The interface SurfaceTool presents for tangents and all its usages work with tangent frames represented as 4 values: tangent direction and scalar orientation (+1 or -1). However, internally it was represented as tangent+bitangent (mistakenly called binormal), which introduces additional conversions back & forth and is problematic for replacing the tangent space generation algorithm. This change converts the internal storage and adjusts the calling code accordingly. Note that since SurfaceTool::set_tangent is exposed to scripts, I've kept the Plane interface there even though it's misleading to use this type here.
1 parent 3e4f548 commit d642e1f

4 files changed

Lines changed: 15 additions & 32 deletions

File tree

scene/resources/3d/importer_mesh.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,10 +1482,7 @@ Error ImporterMesh::lightmap_unwrap_cached(const Transform3D &p_base_transform,
14821482
surfaces_tools[surface]->set_normal(v.normal);
14831483
}
14841484
if (lightmap_surfaces[surface].format & Mesh::ARRAY_FORMAT_TANGENT) {
1485-
Plane t;
1486-
t.normal = v.tangent;
1487-
t.d = v.binormal.dot(v.normal.cross(v.tangent)) < 0 ? -1 : 1;
1488-
surfaces_tools[surface]->set_tangent(t);
1485+
surfaces_tools[surface]->set_tangent(Plane(v.tangent.x, v.tangent.y, v.tangent.z, v.tangent.w));
14891486
}
14901487
if (lightmap_surfaces[surface].format & Mesh::ARRAY_FORMAT_BONES) {
14911488
surfaces_tools[surface]->set_bones(v.bones);

scene/resources/mesh.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,10 +2235,7 @@ Error ArrayMesh::lightmap_unwrap_cached(const Transform3D &p_base_transform, flo
22352235
surfaces_tools[surface]->set_normal(v.normal);
22362236
}
22372237
if (lightmap_surfaces[surface].format & ARRAY_FORMAT_TANGENT) {
2238-
Plane t;
2239-
t.normal = v.tangent;
2240-
t.d = v.binormal.dot(v.normal.cross(v.tangent)) < 0 ? -1 : 1;
2241-
surfaces_tools[surface]->set_tangent(t);
2238+
surfaces_tools[surface]->set_tangent(Plane(v.tangent.x, v.tangent.y, v.tangent.z, v.tangent.w));
22422239
}
22432240
if (lightmap_surfaces[surface].format & ARRAY_FORMAT_BONES) {
22442241
surfaces_tools[surface]->set_bones(v.bones);

scene/resources/surface_tool.cpp

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,6 @@ bool SurfaceTool::Vertex::operator==(const Vertex &p_vertex) const {
9696
return false;
9797
}
9898

99-
if (binormal != p_vertex.binormal) {
100-
return false;
101-
}
102-
10399
if (tangent != p_vertex.tangent) {
104100
return false;
105101
}
@@ -235,8 +231,7 @@ void SurfaceTool::add_vertex(const Vector3 &p_vertex) {
235231
vtx.uv2 = last_uv2;
236232
vtx.weights = last_weights;
237233
vtx.bones = last_bones;
238-
vtx.tangent = last_tangent.normal;
239-
vtx.binormal = last_normal.cross(last_tangent.normal).normalized() * last_tangent.d;
234+
vtx.tangent = last_tangent;
240235
vtx.smooth_group = last_smooth_group;
241236

242237
for (int i = 0; i < RSE::ARRAY_CUSTOM_COUNT; i++) {
@@ -317,7 +312,7 @@ void SurfaceTool::set_tangent(const Plane &p_tangent) {
317312
ERR_FAIL_COND(!first && !(format & Mesh::ARRAY_FORMAT_TANGENT));
318313

319314
format |= Mesh::ARRAY_FORMAT_TANGENT;
320-
last_tangent = p_tangent;
315+
last_tangent = Vector4(p_tangent.normal.x, p_tangent.normal.y, p_tangent.normal.z, p_tangent.d);
321316
}
322317

323318
void SurfaceTool::set_uv(const Vector2 &p_uv) {
@@ -483,10 +478,7 @@ Array SurfaceTool::commit_to_arrays() {
483478
w[idx * 4 + 0] = v.tangent.x;
484479
w[idx * 4 + 1] = v.tangent.y;
485480
w[idx * 4 + 2] = v.tangent.z;
486-
487-
//float d = v.tangent.dot(v.binormal,v.normal);
488-
float d = v.binormal.dot(v.normal.cross(v.tangent));
489-
w[idx * 4 + 3] = d < 0 ? -1 : 1;
481+
w[idx * 4 + 3] = v.tangent.w;
490482
}
491483

492484
a[i] = array;
@@ -882,9 +874,7 @@ void SurfaceTool::create_vertex_array_from_arrays(const Array &p_arrays, LocalVe
882874
v.normal = narr[i];
883875
}
884876
if (lformat & RSE::ARRAY_FORMAT_TANGENT) {
885-
v.tangent = Vector3(tarr[i * 4 + 0], tarr[i * 4 + 1], tarr[i * 4 + 2]);
886-
float d = tarr[i * 4 + 3];
887-
v.binormal = v.normal.cross(v.tangent).normalized() * d;
877+
v.tangent = Vector4(tarr[i * 4 + 0], tarr[i * 4 + 1], tarr[i * 4 + 2], tarr[i * 4 + 3]);
888878
}
889879
if (lformat & RSE::ARRAY_FORMAT_COLOR) {
890880
v.color = carr[i];
@@ -1039,15 +1029,16 @@ void SurfaceTool::append_from(const Ref<Mesh> &p_existing, int p_surface, const
10391029
}
10401030
}
10411031
int vfrom = vertex_array.size();
1032+
const float tangent_orientation = p_xform.basis.determinant() < 0 ? -1.0f : 1.0f;
10421033

10431034
for (Vertex &v : nvertices) {
10441035
v.vertex = p_xform.xform(v.vertex);
10451036
if (nformat & RSE::ARRAY_FORMAT_NORMAL) {
10461037
v.normal = p_xform.basis.xform(v.normal);
10471038
}
10481039
if (nformat & RSE::ARRAY_FORMAT_TANGENT) {
1049-
v.tangent = p_xform.basis.xform(v.tangent);
1050-
v.binormal = p_xform.basis.xform(v.binormal);
1040+
Vector3 tangent = p_xform.basis.xform(Vector3(v.tangent.x, v.tangent.y, v.tangent.z));
1041+
v.tangent = Vector4(tangent.x, tangent.y, tangent.z, v.tangent.w * tangent_orientation);
10511042
}
10521043

10531044
vertex_array.push_back(v);
@@ -1148,8 +1139,7 @@ void SurfaceTool::mikktSetTSpaceDefault(const SMikkTSpaceContext *pContext, cons
11481139
}
11491140

11501141
if (vtx != nullptr) {
1151-
vtx->tangent = Vector3(fvTangent[0], fvTangent[1], fvTangent[2]);
1152-
vtx->binormal = Vector3(-fvBiTangent[0], -fvBiTangent[1], -fvBiTangent[2]); // for some reason these are reversed, something with the coordinate system in Godot
1142+
vtx->tangent = Vector4(fvTangent[0], fvTangent[1], fvTangent[2], bIsOrientationPreserving ? 1.0f : -1.0f);
11531143
}
11541144
}
11551145

@@ -1172,8 +1162,7 @@ void SurfaceTool::generate_tangents() {
11721162
TangentGenerationContextUserData triangle_data;
11731163
triangle_data.vertices = &vertex_array;
11741164
for (Vertex &vertex : vertex_array) {
1175-
vertex.binormal = Vector3();
1176-
vertex.tangent = Vector3();
1165+
vertex.tangent = Vector4();
11771166
}
11781167
triangle_data.indices = &index_array;
11791168
msc.m_pUserData = &triangle_data;

scene/resources/surface_tool.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#pragma once
3232

33+
#include "core/math/vector4.h"
3334
#include "core/templates/local_vector.h"
3435
#include "scene/resources/mesh.h"
3536
#include "servers/rendering/rendering_server_enums.h"
@@ -49,9 +50,8 @@ class SurfaceTool : public RefCounted {
4950
uint32_t smooth_group = 0; // Must be first.
5051

5152
Color color;
52-
Vector3 normal; // normal, binormal, tangent.
53-
Vector3 binormal;
54-
Vector3 tangent;
53+
Vector3 normal;
54+
Vector4 tangent; // xyz tangent, w orientation.
5555
Vector2 uv;
5656
Vector2 uv2;
5757
Color custom[RSE::ARRAY_CUSTOM_COUNT];
@@ -164,7 +164,7 @@ class SurfaceTool : public RefCounted {
164164
Vector2 last_uv2;
165165
Vector<int> last_bones;
166166
Vector<float> last_weights;
167-
Plane last_tangent;
167+
Vector4 last_tangent;
168168
uint32_t last_smooth_group = 0;
169169

170170
SkinWeightCount skin_weights = SKIN_4_WEIGHTS;

0 commit comments

Comments
 (0)