From e98cc412efe5db998155d752c7ccbf0ac4aa42fe Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 2 Nov 2023 22:58:26 +0200 Subject: [PATCH] Fix #114346: PLY export crash with multiple objects that have different custom attributes Custom attribute export (#114320) code did not realize that PLY exports everything as one single "mesh" with the same vertex data for everything. So if exporting several input meshes and they have different vertex attributes, the result needs to have a union of all present attributes, with fake/zero data for the ones that were not actually present. Fixes #114346. --- .../ply/exporter/ply_export_load_plydata.cc | 201 ++++++++++-------- source/blender/io/ply/intern/ply_data.hh | 5 +- .../io/ply/tests/io_ply_exporter_test.cc | 12 ++ 3 files changed, 130 insertions(+), 88 deletions(-) diff --git a/source/blender/io/ply/exporter/ply_export_load_plydata.cc b/source/blender/io/ply/exporter/ply_export_load_plydata.cc index a370673b06a..25938bd1003 100644 --- a/source/blender/io/ply/exporter/ply_export_load_plydata.cc +++ b/source/blender/io/ply/exporter/ply_export_load_plydata.cc @@ -148,11 +148,33 @@ static void generate_vertex_map(const Mesh *mesh, } } -static void load_custom_attributes(const Mesh *mesh, Vector &r_attributes) +static float *find_or_add_attribute(const StringRef name, + int64_t size, + uint32_t vertex_offset, + Vector &r_attributes) +{ + /* Do we have this attribute from some other object already? */ + for (PlyCustomAttribute &attr : r_attributes) { + if (attr.name == name) { + BLI_assert(attr.data.size() == vertex_offset); + attr.data.resize(attr.data.size() + size, 0.0f); + return attr.data.data() + vertex_offset; + } + } + /* We don't have it yet, create and fill with zero data for previous objects. */ + r_attributes.append(PlyCustomAttribute(name, vertex_offset + size)); + return r_attributes.last().data.data() + vertex_offset; +} + +static void load_custom_attributes(const Mesh *mesh, + const Vector &ply_to_vertex, + uint32_t vertex_offset, + Vector &r_attributes) { const bke::AttributeAccessor attributes = mesh->attributes(); const StringRef color_name = mesh->active_color_attribute; const StringRef uv_name = CustomData_get_active_layer_name(&mesh->loop_data, CD_PROP_FLOAT2); + const int64_t size = ply_to_vertex.size(); attributes.for_all([&](const bke::AttributeIDRef &attribute_id, const bke::AttributeMetaData &meta_data) { @@ -165,140 +187,143 @@ static void load_custom_attributes(const Mesh *mesh, Vector const GVArraySpan attribute = *mesh->attributes().lookup( attribute_id, meta_data.domain, meta_data.data_type); - const int64_t size = attribute.size(); - if (size == 0) { + if (attribute.is_empty()) { return true; } switch (meta_data.data_type) { case CD_PROP_FLOAT: { - PlyCustomAttribute attr(attribute_id.name(), size); + float *attr = find_or_add_attribute( + attribute_id.name(), size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr.data[i] = typed[i]; + for (const int64_t i : ply_to_vertex.index_range()) { + attr[i] = typed[ply_to_vertex[i]]; } - r_attributes.append(attr); break; } case CD_PROP_INT8: { - PlyCustomAttribute attr(attribute_id.name(), size); + float *attr = find_or_add_attribute( + attribute_id.name(), size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr.data[i] = typed[i]; + for (const int64_t i : ply_to_vertex.index_range()) { + attr[i] = typed[ply_to_vertex[i]]; } - r_attributes.append(attr); break; } case CD_PROP_INT32: { - PlyCustomAttribute attr(attribute_id.name(), size); + float *attr = find_or_add_attribute( + attribute_id.name(), size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr.data[i] = typed[i]; + for (const int64_t i : ply_to_vertex.index_range()) { + attr[i] = typed[ply_to_vertex[i]]; } - r_attributes.append(attr); break; } case CD_PROP_INT32_2D: { - PlyCustomAttribute attr_x(attribute_id.name() + "_x", size); - PlyCustomAttribute attr_y(attribute_id.name() + "_y", size); + float *attr_x = find_or_add_attribute( + attribute_id.name() + "_x", size, vertex_offset, r_attributes); + float *attr_y = find_or_add_attribute( + attribute_id.name() + "_y", size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr_x.data[i] = typed[i].x; - attr_y.data[i] = typed[i].y; + for (const int64_t i : ply_to_vertex.index_range()) { + int j = ply_to_vertex[i]; + attr_x[i] = typed[j].x; + attr_y[i] = typed[j].y; } - r_attributes.append(attr_x); - r_attributes.append(attr_y); break; } case CD_PROP_FLOAT2: { - PlyCustomAttribute attr_x(attribute_id.name() + "_x", size); - PlyCustomAttribute attr_y(attribute_id.name() + "_y", size); + float *attr_x = find_or_add_attribute( + attribute_id.name() + "_x", size, vertex_offset, r_attributes); + float *attr_y = find_or_add_attribute( + attribute_id.name() + "_y", size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr_x.data[i] = typed[i].x; - attr_y.data[i] = typed[i].y; + for (const int64_t i : ply_to_vertex.index_range()) { + int j = ply_to_vertex[i]; + attr_x[i] = typed[j].x; + attr_y[i] = typed[j].y; } - r_attributes.append(attr_x); - r_attributes.append(attr_y); break; } case CD_PROP_FLOAT3: { - PlyCustomAttribute attr_x(attribute_id.name() + "_x", size); - PlyCustomAttribute attr_y(attribute_id.name() + "_y", size); - PlyCustomAttribute attr_z(attribute_id.name() + "_z", size); + float *attr_x = find_or_add_attribute( + attribute_id.name() + "_x", size, vertex_offset, r_attributes); + float *attr_y = find_or_add_attribute( + attribute_id.name() + "_y", size, vertex_offset, r_attributes); + float *attr_z = find_or_add_attribute( + attribute_id.name() + "_z", size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr_x.data[i] = typed[i].x; - attr_y.data[i] = typed[i].y; - attr_z.data[i] = typed[i].z; + for (const int64_t i : ply_to_vertex.index_range()) { + int j = ply_to_vertex[i]; + attr_x[i] = typed[j].x; + attr_y[i] = typed[j].y; + attr_z[i] = typed[j].z; } - r_attributes.append(attr_x); - r_attributes.append(attr_y); - r_attributes.append(attr_z); break; } case CD_PROP_BYTE_COLOR: { - PlyCustomAttribute attr_r(attribute_id.name() + "_r", size); - PlyCustomAttribute attr_g(attribute_id.name() + "_g", size); - PlyCustomAttribute attr_b(attribute_id.name() + "_b", size); - PlyCustomAttribute attr_a(attribute_id.name() + "_a", size); + float *attr_r = find_or_add_attribute( + attribute_id.name() + "_r", size, vertex_offset, r_attributes); + float *attr_g = find_or_add_attribute( + attribute_id.name() + "_g", size, vertex_offset, r_attributes); + float *attr_b = find_or_add_attribute( + attribute_id.name() + "_b", size, vertex_offset, r_attributes); + float *attr_a = find_or_add_attribute( + attribute_id.name() + "_a", size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - ColorGeometry4f col = typed[i].decode(); - attr_r.data[i] = col.r; - attr_g.data[i] = col.g; - attr_b.data[i] = col.b; - attr_a.data[i] = col.a; + for (const int64_t i : ply_to_vertex.index_range()) { + ColorGeometry4f col = typed[ply_to_vertex[i]].decode(); + attr_r[i] = col.r; + attr_g[i] = col.g; + attr_b[i] = col.b; + attr_a[i] = col.a; } - r_attributes.append(attr_r); - r_attributes.append(attr_g); - r_attributes.append(attr_b); - r_attributes.append(attr_a); break; } case CD_PROP_COLOR: { - PlyCustomAttribute attr_r(attribute_id.name() + "_r", size); - PlyCustomAttribute attr_g(attribute_id.name() + "_g", size); - PlyCustomAttribute attr_b(attribute_id.name() + "_b", size); - PlyCustomAttribute attr_a(attribute_id.name() + "_a", size); + float *attr_r = find_or_add_attribute( + attribute_id.name() + "_r", size, vertex_offset, r_attributes); + float *attr_g = find_or_add_attribute( + attribute_id.name() + "_g", size, vertex_offset, r_attributes); + float *attr_b = find_or_add_attribute( + attribute_id.name() + "_b", size, vertex_offset, r_attributes); + float *attr_a = find_or_add_attribute( + attribute_id.name() + "_a", size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - ColorGeometry4f col = typed[i]; - attr_r.data[i] = col.r; - attr_g.data[i] = col.g; - attr_b.data[i] = col.b; - attr_a.data[i] = col.a; + for (const int64_t i : ply_to_vertex.index_range()) { + ColorGeometry4f col = typed[ply_to_vertex[i]]; + attr_r[i] = col.r; + attr_g[i] = col.g; + attr_b[i] = col.b; + attr_a[i] = col.a; } - r_attributes.append(attr_r); - r_attributes.append(attr_g); - r_attributes.append(attr_b); - r_attributes.append(attr_a); break; } case CD_PROP_BOOL: { - PlyCustomAttribute attr(attribute_id.name(), size); + float *attr = find_or_add_attribute( + attribute_id.name(), size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr.data[i] = typed[i] ? 1.0f : 0.0f; + for (const int64_t i : ply_to_vertex.index_range()) { + attr[i] = typed[ply_to_vertex[i]] ? 1.0f : 0.0f; } - r_attributes.append(attr); break; } case CD_PROP_QUATERNION: { - PlyCustomAttribute attr_x(attribute_id.name() + "_x", size); - PlyCustomAttribute attr_y(attribute_id.name() + "_y", size); - PlyCustomAttribute attr_z(attribute_id.name() + "_z", size); - PlyCustomAttribute attr_w(attribute_id.name() + "_w", size); + float *attr_x = find_or_add_attribute( + attribute_id.name() + "_x", size, vertex_offset, r_attributes); + float *attr_y = find_or_add_attribute( + attribute_id.name() + "_y", size, vertex_offset, r_attributes); + float *attr_z = find_or_add_attribute( + attribute_id.name() + "_z", size, vertex_offset, r_attributes); + float *attr_w = find_or_add_attribute( + attribute_id.name() + "_w", size, vertex_offset, r_attributes); auto typed = attribute.typed(); - for (const int64_t i : typed.index_range()) { - attr_x.data[i] = typed[i].x; - attr_y.data[i] = typed[i].y; - attr_z.data[i] = typed[i].z; - attr_w.data[i] = typed[i].w; + for (const int64_t i : ply_to_vertex.index_range()) { + int j = ply_to_vertex[i]; + attr_x[i] = typed[j].x; + attr_y[i] = typed[j].y; + attr_z[i] = typed[j].z; + attr_w[i] = typed[j].w; } - r_attributes.append(attr_x); - r_attributes.append(attr_y); - r_attributes.append(attr_z); - r_attributes.append(attr_w); break; } default: @@ -430,7 +455,7 @@ void load_plydata(PlyData &plyData, Depsgraph *depsgraph, const PLYExportParams /* Custom attributes */ if (export_params.export_attributes) { - load_custom_attributes(mesh, plyData.vertex_custom_attr); + load_custom_attributes(mesh, ply_to_vertex, vertex_offset, plyData.vertex_custom_attr); } /* Loose edges */ @@ -451,6 +476,12 @@ void load_plydata(PlyData &plyData, Depsgraph *depsgraph, const PLYExportParams } DEG_OBJECT_ITER_END; + + /* Make sure all custom attribute data arrays are encompassing all input objects */ + for (PlyCustomAttribute &attr : plyData.vertex_custom_attr) { + BLI_assert(attr.data.size() <= vertex_offset); + attr.data.resize(vertex_offset, 0.0f); + } } } // namespace blender::io::ply diff --git a/source/blender/io/ply/intern/ply_data.hh b/source/blender/io/ply/intern/ply_data.hh index e9631f86151..7555f9bc0d6 100644 --- a/source/blender/io/ply/intern/ply_data.hh +++ b/source/blender/io/ply/intern/ply_data.hh @@ -8,7 +8,6 @@ #pragma once -#include "BLI_array.hh" #include "BLI_math_vector_types.hh" #include "BLI_string_ref.hh" #include "BLI_vector.hh" @@ -18,9 +17,9 @@ namespace blender::io::ply { enum PlyDataTypes { NONE, CHAR, UCHAR, SHORT, USHORT, INT, UINT, FLOAT, DOUBLE, PLY_TYPE_COUNT }; struct PlyCustomAttribute { - PlyCustomAttribute(const StringRef name_, int64_t size) : name(name_), data(size) {} + PlyCustomAttribute(const StringRef name_, int64_t size) : name(name_), data(size, 0.0f) {} std::string name; - Array data; /* Any custom PLY attributes are converted to floats. */ + Vector data; /* Any custom PLY attributes are converted to floats. */ }; struct PlyData { diff --git a/source/blender/io/ply/tests/io_ply_exporter_test.cc b/source/blender/io/ply/tests/io_ply_exporter_test.cc index 263cffc659b..e9603a9c3d2 100644 --- a/source/blender/io/ply/tests/io_ply_exporter_test.cc +++ b/source/blender/io/ply/tests/io_ply_exporter_test.cc @@ -525,6 +525,18 @@ TEST_F(ply_exporter_ply_data_test, CubeLooseEdgesLoadPLYDataUV) EXPECT_EQ_ARRAY(exp_faces, plyData.face_vertices.data(), ARRAY_SIZE(exp_faces)); } +TEST_F(ply_exporter_ply_data_test, CubesVertexAttrs) +{ + PLYExportParams params = {}; + params.export_uv = true; + params.export_attributes = true; + PlyData plyData = load_ply_data_from_blendfile( + "io_tests/blend_geometry/cubes_vertex_attrs.blend", params); + EXPECT_EQ(plyData.vertices.size(), 28); + EXPECT_EQ(plyData.vertex_custom_attr.size(), 11); /* Float 1 + Color 4 + ByteColor 4 + Int2D 2*/ + EXPECT_EQ(plyData.vertex_custom_attr[0].data.size(), 28); +} + TEST_F(ply_exporter_ply_data_test, SuzanneLoadPLYDataUV) { PLYExportParams params = {};