From 6ec541ca4e586dc3b05884c357cccf50cc27e5a2 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Thu, 12 Dec 2024 21:20:24 +0100 Subject: [PATCH] Refactor: Cycles: Remove face normal attribute It's already computed on demand in the kernel, no need to have it host side. Pull Request: https://projects.blender.org/blender/blender/pulls/135681 --- intern/cycles/hydra/mesh.cpp | 22 ++--------- intern/cycles/kernel/types.h | 1 - intern/cycles/scene/attribute.cpp | 11 +----- intern/cycles/scene/geometry.cpp | 2 - intern/cycles/scene/mesh.cpp | 38 ++---------------- intern/cycles/scene/mesh.h | 1 - intern/cycles/scene/mesh_displace.cpp | 57 +++++++++++---------------- intern/cycles/scene/volume.cpp | 31 +++------------ 8 files changed, 35 insertions(+), 128 deletions(-) diff --git a/intern/cycles/hydra/mesh.cpp b/intern/cycles/hydra/mesh.cpp index d3749e1ac8f..836d52bfeb8 100644 --- a/intern/cycles/hydra/mesh.cpp +++ b/intern/cycles/hydra/mesh.cpp @@ -199,7 +199,6 @@ void HdCyclesMesh::PopulatePoints(HdSceneDelegate *sceneDelegate) void HdCyclesMesh::PopulateNormals(HdSceneDelegate *sceneDelegate) { - _geom->attributes.remove(ATTR_STD_FACE_NORMAL); _geom->attributes.remove(ATTR_STD_VERTEX_NORMAL); // Authored normals should only exist on triangle meshes @@ -255,13 +254,7 @@ void HdCyclesMesh::PopulateNormals(HdSceneDelegate *sceneDelegate) } else if (interpolation == HdInterpolationUniform) { TF_VERIFY(normals.size() == static_cast(_topology.GetNumFaces())); - - float3 *const N = _geom->attributes.add(ATTR_STD_FACE_NORMAL)->data_float3(); - for (size_t i = 0; i < _geom->num_triangles(); ++i) { - const int faceIndex = HdMeshUtil::DecodeFaceIndexFromCoarseFaceParam(_primitiveParams[i]); - - N[i] = make_float3(normals[faceIndex][0], normals[faceIndex][1], normals[faceIndex][2]); - } + /* Nothing to do, face normals are computed on demand in the kernel. */ } else if (interpolation == HdInterpolationVertex || interpolation == HdInterpolationVarying) { TF_VERIFY(normals.size() == static_cast(_topology.GetNumPoints()) && @@ -275,6 +268,8 @@ void HdCyclesMesh::PopulateNormals(HdSceneDelegate *sceneDelegate) else if (interpolation == HdInterpolationFaceVarying) { TF_VERIFY(normals.size() == static_cast(_topology.GetNumFaceVaryings())); + // TODO: Cycles has no per-corner normals, so ignore until supported. +#if 0 if (!_util.ComputeTriangulatedFaceVaryingPrimvar( normals.data(), normals.size(), HdTypeFloatVec3, &value)) { @@ -282,16 +277,7 @@ void HdCyclesMesh::PopulateNormals(HdSceneDelegate *sceneDelegate) } const auto &normalsTriangulated = value.UncheckedGet(); - - // Cycles has no standard attribute for face-varying normals, so this is a lossy transformation - float3 *const N = _geom->attributes.add(ATTR_STD_FACE_NORMAL)->data_float3(); - for (size_t i = 0; i < _geom->num_triangles(); ++i) { - GfVec3f averageNormal = normalsTriangulated[i * 3] + normalsTriangulated[i * 3 + 1] + - normalsTriangulated[i * 3 + 2]; - GfNormalize(&averageNormal); - - N[i] = make_float3(averageNormal[0], averageNormal[1], averageNormal[2]); - } +#endif } } diff --git a/intern/cycles/kernel/types.h b/intern/cycles/kernel/types.h index 58b803d56ff..e0c13e17cca 100644 --- a/intern/cycles/kernel/types.h +++ b/intern/cycles/kernel/types.h @@ -881,7 +881,6 @@ enum AttributeElement { enum AttributeStandard { ATTR_STD_NONE = 0, ATTR_STD_VERTEX_NORMAL, - ATTR_STD_FACE_NORMAL, ATTR_STD_UV, ATTR_STD_UV_TANGENT, ATTR_STD_UV_TANGENT_SIGN, diff --git a/intern/cycles/scene/attribute.cpp b/intern/cycles/scene/attribute.cpp index 2461d90d4ee..fca3565bfd8 100644 --- a/intern/cycles/scene/attribute.cpp +++ b/intern/cycles/scene/attribute.cpp @@ -332,8 +332,6 @@ const char *Attribute::standard_name(AttributeStandard std) switch (std) { case ATTR_STD_VERTEX_NORMAL: return "N"; - case ATTR_STD_FACE_NORMAL: - return "Ng"; case ATTR_STD_UV: return "uv"; case ATTR_STD_GENERATED: @@ -537,9 +535,6 @@ Attribute *AttributeSet::add(AttributeStandard std, ustring name) case ATTR_STD_VERTEX_NORMAL: attr = add(name, TypeNormal, ATTR_ELEMENT_VERTEX); break; - case ATTR_STD_FACE_NORMAL: - attr = add(name, TypeNormal, ATTR_ELEMENT_FACE); - break; case ATTR_STD_UV: attr = add(name, TypeFloat2, ATTR_ELEMENT_CORNER); break; @@ -610,9 +605,6 @@ Attribute *AttributeSet::add(AttributeStandard std, ustring name) case ATTR_STD_VERTEX_NORMAL: attr = add(name, TypeNormal, ATTR_ELEMENT_VERTEX); break; - case ATTR_STD_FACE_NORMAL: - attr = add(name, TypeNormal, ATTR_ELEMENT_FACE); - break; case ATTR_STD_VOLUME_DENSITY: case ATTR_STD_VOLUME_FLAME: case ATTR_STD_VOLUME_HEAT: @@ -814,8 +806,7 @@ void AttributeSet::tag_modified(const Attribute &attr) /* Some attributes are not stored in the various kernel attribute arrays * (DeviceScene::attribute_*), so the modified flags are only set if the associated standard * corresponds to an attribute which will be stored in the kernel's attribute arrays. */ - const bool modifies_device_array = (attr.std != ATTR_STD_FACE_NORMAL && - attr.std != ATTR_STD_VERTEX_NORMAL); + const bool modifies_device_array = (attr.std != ATTR_STD_VERTEX_NORMAL); if (modifies_device_array) { const AttrKernelDataType kernel_type = Attribute::kernel_type(attr); diff --git a/intern/cycles/scene/geometry.cpp b/intern/cycles/scene/geometry.cpp index 7443dbbd8f2..fb3a5ca27cf 100644 --- a/intern/cycles/scene/geometry.cpp +++ b/intern/cycles/scene/geometry.cpp @@ -740,12 +740,10 @@ void GeometryManager::device_update(Device *device, if (mesh->get_subdivision_type() != Mesh::SUBDIVISION_CATMULL_CLARK) #endif { - mesh->add_face_normals(); mesh->add_vertex_normals(); } } else { - mesh->add_face_normals(); mesh->add_vertex_normals(); } diff --git a/intern/cycles/scene/mesh.cpp b/intern/cycles/scene/mesh.cpp index ce297e38306..ab28e52bb23 100644 --- a/intern/cycles/scene/mesh.cpp +++ b/intern/cycles/scene/mesh.cpp @@ -532,38 +532,6 @@ void Mesh::apply_transform(const Transform &tfm, const bool apply_to_motion) } } -void Mesh::add_face_normals() -{ - /* don't compute if already there */ - if (attributes.find(ATTR_STD_FACE_NORMAL)) { - return; - } - - /* get attributes */ - Attribute *attr_fN = attributes.add(ATTR_STD_FACE_NORMAL); - float3 *fN = attr_fN->data_float3(); - - /* compute face normals */ - const size_t triangles_size = num_triangles(); - - if (triangles_size) { - float3 *verts_ptr = verts.data(); - - for (size_t i = 0; i < triangles_size; i++) { - fN[i] = get_triangle(i).compute_normal(verts_ptr); - } - } - - /* expected to be in local space */ - if (transform_applied) { - const Transform ntfm = transform_inverse(transform_normal); - - for (size_t i = 0; i < triangles_size; i++) { - fN[i] = normalize(transform_direction(&ntfm, fN[i])); - } - } -} - void Mesh::add_vertex_normals() { const bool flip = transform_negative_scaled; @@ -573,18 +541,18 @@ void Mesh::add_vertex_normals() /* static vertex normals */ if (!attributes.find(ATTR_STD_VERTEX_NORMAL) && triangles_size) { /* get attributes */ - Attribute *attr_fN = attributes.find(ATTR_STD_FACE_NORMAL); Attribute *attr_vN = attributes.add(ATTR_STD_VERTEX_NORMAL); - float3 *fN = attr_fN->data_float3(); + float3 *verts_ptr = verts.data(); float3 *vN = attr_vN->data_float3(); /* compute vertex normals */ std::fill_n(vN, verts.size(), zero_float3()); for (size_t i = 0; i < triangles_size; i++) { + const float3 fN = get_triangle(i).compute_normal(verts_ptr); for (size_t j = 0; j < 3; j++) { - vN[get_triangle(i).v[j]] += fN[i]; + vN[get_triangle(i).v[j]] += fN; } } diff --git a/intern/cycles/scene/mesh.h b/intern/cycles/scene/mesh.h index 2bf10f65d9f..da1bd45c392 100644 --- a/intern/cycles/scene/mesh.h +++ b/intern/cycles/scene/mesh.h @@ -207,7 +207,6 @@ class Mesh : public Geometry { void compute_bounds() override; void apply_transform(const Transform &tfm, const bool apply_to_motion) override; - void add_face_normals(); void add_vertex_normals(); void add_undisplaced(); diff --git a/intern/cycles/scene/mesh_displace.cpp b/intern/cycles/scene/mesh_displace.cpp index 1915ebfed84..fe3835f6d9b 100644 --- a/intern/cycles/scene/mesh_displace.cpp +++ b/intern/cycles/scene/mesh_displace.cpp @@ -17,22 +17,6 @@ CCL_NAMESPACE_BEGIN -static float3 compute_face_normal(const Mesh::Triangle &t, float3 *verts) -{ - const float3 v0 = verts[t.v[0]]; - const float3 v1 = verts[t.v[1]]; - const float3 v2 = verts[t.v[2]]; - - const float3 norm = cross(v1 - v0, v2 - v0); - const float normlen = len(norm); - - if (normlen == 0.0f) { - return make_float3(1.0f, 0.0f, 0.0f); - } - - return norm / normlen; -} - /* Fill in coordinates for mesh displacement shader evaluation on device. */ static int fill_shader_input(const Scene *scene, const Mesh *mesh, @@ -228,13 +212,9 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres } } - /* for displacement method both, we only need to recompute the face - * normals, as bump mapping in the shader will already alter the - * vertex normal, so we start from the non-displaced vertex normals - * to avoid applying the perturbation twice. */ - mesh->attributes.remove(ATTR_STD_FACE_NORMAL); - mesh->add_face_normals(); - + /* For displacement method both, we don't need to recompute the vertex normals + * as bump mapping in the shader will already alter the vertex normal, so we start + * from the non-displaced vertex normals to avoid applying the perturbation twice. */ bool need_recompute_vertex_normals = false; for (Node *node : mesh->get_used_shaders()) { @@ -262,10 +242,7 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres /* static vertex normals */ /* get attributes */ - Attribute *attr_fN = mesh->attributes.find(ATTR_STD_FACE_NORMAL); Attribute *attr_vN = mesh->attributes.find(ATTR_STD_VERTEX_NORMAL); - - float3 *fN = attr_fN->data_float3(); float3 *vN = attr_vN->data_float3(); /* compute vertex normals */ @@ -273,18 +250,23 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres /* zero vertex normals on triangles with true displacement */ for (size_t i = 0; i < num_triangles; i++) { if (tri_has_true_disp[i]) { + const Mesh::Triangle triangle = mesh->get_triangle(i); for (size_t j = 0; j < 3; j++) { - vN[mesh->get_triangle(i).v[j]] = zero_float3(); + vN[triangle.v[j]] = zero_float3(); } } } /* add face normals to vertex normals */ + const float3 *verts_data = mesh->get_verts().data(); for (size_t i = 0; i < num_triangles; i++) { if (tri_has_true_disp[i]) { + const Mesh::Triangle triangle = mesh->get_triangle(i); + const float3 fN = triangle.compute_normal(verts_data); + for (size_t j = 0; j < 3; j++) { - const int vert = mesh->get_triangle(i).v[j]; - vN[vert] += fN[i]; + const int vert = triangle.v[j]; + vN[vert] += fN; /* add face normals to stitched vertices */ if (!stitch_keys.empty()) { @@ -299,7 +281,7 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres continue; } - vN[v->second] += fN[i]; + vN[v->second] += fN; } } } @@ -312,8 +294,9 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres for (size_t i = 0; i < num_triangles; i++) { if (tri_has_true_disp[i]) { + const Mesh::Triangle triangle = mesh->get_triangle(i); for (size_t j = 0; j < 3; j++) { - const int vert = mesh->get_triangle(i).v[j]; + const int vert = triangle.v[j]; if (done[vert]) { continue; @@ -343,8 +326,9 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres /* zero vertex normals on triangles with true displacement */ for (size_t i = 0; i < num_triangles; i++) { if (tri_has_true_disp[i]) { + const Mesh::Triangle triangle = mesh->get_triangle(i); for (size_t j = 0; j < 3; j++) { - mN[mesh->get_triangle(i).v[j]] = zero_float3(); + mN[triangle.v[j]] = zero_float3(); } } } @@ -352,9 +336,11 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres /* add face normals to vertex normals */ for (size_t i = 0; i < num_triangles; i++) { if (tri_has_true_disp[i]) { + const Mesh::Triangle triangle = mesh->get_triangle(i); + const float3 fN = triangle.compute_normal(mP); + for (size_t j = 0; j < 3; j++) { - const int vert = mesh->get_triangle(i).v[j]; - const float3 fN = compute_face_normal(mesh->get_triangle(i), mP); + const int vert = triangle.v[j]; mN[vert] += fN; /* add face normals to stitched vertices */ @@ -383,8 +369,9 @@ bool GeometryManager::displace(Device *device, Scene *scene, Mesh *mesh, Progres for (size_t i = 0; i < num_triangles; i++) { if (tri_has_true_disp[i]) { + const Mesh::Triangle triangle = mesh->get_triangle(i); for (size_t j = 0; j < 3; j++) { - const int vert = mesh->get_triangle(i).v[j]; + const int vert = triangle.v[j]; if (done[vert]) { continue; diff --git a/intern/cycles/scene/volume.cpp b/intern/cycles/scene/volume.cpp index 67eb5b482c5..71d8f0504b3 100644 --- a/intern/cycles/scene/volume.cpp +++ b/intern/cycles/scene/volume.cpp @@ -158,7 +158,6 @@ class VolumeMeshBuilder { void create_mesh(vector &vertices, vector &indices, - vector &face_normals, const float face_overlap_avoidance); void generate_vertices_and_quads(vector &vertices_is, vector &quads); @@ -167,9 +166,7 @@ class VolumeMeshBuilder { vector &out_vertices, const float face_overlap_avoidance); - void convert_quads_to_tris(const vector &quads, - vector &tris, - vector &face_normals); + void convert_quads_to_tris(const vector &quads, vector &tris); bool empty_grid() const; @@ -270,7 +267,6 @@ void VolumeMeshBuilder::add_padding(const int pad_size) void VolumeMeshBuilder::create_mesh(vector &vertices, vector &indices, - vector &face_normals, const float face_overlap_avoidance) { #ifdef WITH_OPENVDB @@ -287,11 +283,10 @@ void VolumeMeshBuilder::create_mesh(vector &vertices, convert_object_space(vertices_is, vertices, face_overlap_avoidance); - convert_quads_to_tris(quads, indices, face_normals); + convert_quads_to_tris(quads, indices); #else (void)vertices; (void)indices; - (void)face_normals; (void)face_overlap_avoidance; #endif } @@ -404,26 +399,19 @@ void VolumeMeshBuilder::convert_object_space(const vector &vertices, #endif } -void VolumeMeshBuilder::convert_quads_to_tris(const vector &quads, - vector &tris, - vector &face_normals) +void VolumeMeshBuilder::convert_quads_to_tris(const vector &quads, vector &tris) { int index_offset = 0; tris.resize(quads.size() * 6); - face_normals.reserve(quads.size() * 2); for (size_t i = 0; i < quads.size(); ++i) { tris[index_offset++] = quads[i].v0; tris[index_offset++] = quads[i].v2; tris[index_offset++] = quads[i].v1; - face_normals.push_back(quads[i].normal); - tris[index_offset++] = quads[i].v0; tris[index_offset++] = quads[i].v3; tris[index_offset++] = quads[i].v2; - - face_normals.push_back(quads[i].normal); } } @@ -748,8 +736,7 @@ void GeometryManager::create_volume_mesh(const Scene *scene, Volume *volume, Pro /* Create mesh. */ vector vertices; vector indices; - vector face_normals; - builder.create_mesh(vertices, indices, face_normals, face_overlap_avoidance); + builder.create_mesh(vertices, indices, face_overlap_avoidance); volume->reserve_mesh(vertices.size(), indices.size() / 3); volume->used_shaders.clear(); @@ -763,17 +750,9 @@ void GeometryManager::create_volume_mesh(const Scene *scene, Volume *volume, Pro volume->add_triangle(indices[i], indices[i + 1], indices[i + 2], 0, false); } - Attribute *attr_fN = volume->attributes.add(ATTR_STD_FACE_NORMAL); - float3 *fN = attr_fN->data_float3(); - - for (size_t i = 0; i < face_normals.size(); ++i) { - fN[i] = face_normals[i]; - } - /* Print stats. */ VLOG_WORK << "Memory usage volume mesh: " - << ((vertices.size() + face_normals.size()) * sizeof(float3) + - indices.size() * sizeof(int)) / + << (vertices.size() * sizeof(float3) + indices.size() * sizeof(int)) / (1024.0 * 1024.0) << "Mb."; }