From 5d5e326e91575d450859fe33fae6a9c6ed9b082a Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Tue, 14 Mar 2023 11:01:52 +0200 Subject: [PATCH 01/10] PLY: simplify import tests Instead of loading dummy .blend file and importing a ply file into that, test only the "read ply file and import into a PlyData object" bit. Also rename the PLY import suite/fixture name to start with "ply_" so that it's the same prefix as for export tests. --- source/blender/io/ply/CMakeLists.txt | 8 +- source/blender/io/ply/importer/ply_import.cc | 135 ++++----- source/blender/io/ply/importer/ply_import.hh | 6 + .../io/ply/tests/io_ply_importer_test.cc | 262 ++++++------------ 4 files changed, 159 insertions(+), 252 deletions(-) diff --git a/source/blender/io/ply/CMakeLists.txt b/source/blender/io/ply/CMakeLists.txt index 980a2c0b4cb..5414f9a6971 100644 --- a/source/blender/io/ply/CMakeLists.txt +++ b/source/blender/io/ply/CMakeLists.txt @@ -36,8 +36,6 @@ set(SRC importer/ply_import_mesh.cc IO_ply.cc - - exporter/ply_export.hh exporter/ply_export_data.hh exporter/ply_export_header.hh @@ -65,10 +63,8 @@ blender_add_lib(bf_ply "${SRC}" "${INC}" "${INC_SYS}" "${LIB}") if (WITH_GTESTS) set(TEST_SRC - tests/io_ply_importer_test.cc - - tests/io_ply_exporter_test.cc - + tests/io_ply_importer_test.cc + tests/io_ply_exporter_test.cc ) set(TEST_INC ../../blenloader diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index d06b7025a4d..25b02b96cea 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -71,6 +71,64 @@ enum PlyDataTypes from_string(const StringRef &input) return PlyDataTypes::FLOAT; } +const char *read_header(fstream &file, PlyHeader &r_header) +{ + std::string line; + while (true) { /* We break when end_header is encountered. */ + safe_getline(file, line); + if (r_header.header_size == 0 && line != "ply") { + return "Invalid PLY header."; + } + r_header.header_size++; + Vector words{}; + splitstr(line, words, " "); + + if (strcmp(words[0].c_str(), "format") == 0) { + if (strcmp(words[1].c_str(), "ascii") == 0) { + r_header.type = PlyFormatType::ASCII; + } + else if (strcmp(words[1].c_str(), "binary_big_endian") == 0) { + r_header.type = PlyFormatType::BINARY_BE; + } + else if (strcmp(words[1].c_str(), "binary_little_endian") == 0) { + r_header.type = PlyFormatType::BINARY_LE; + } + } + else if (strcmp(words[0].c_str(), "element") == 0) { + r_header.elements.append(std::make_pair(words[1], std::stoi(words[2]))); + if (strcmp(words[1].c_str(), "vertex") == 0) { + r_header.vertex_count = std::stoi(words[2]); + } + else if (strcmp(words[1].c_str(), "face") == 0) { + r_header.face_count = std::stoi(words[2]); + } + else if (strcmp(words[1].c_str(), "edge") == 0) { + r_header.edge_count = std::stoi(words[2]); + } + } + else if (strcmp(words[0].c_str(), "property") == 0) { + std::pair property; + property.first = words[2]; + property.second = from_string(words[1]); + + while (r_header.properties.size() < r_header.elements.size()) { + Vector> temp; + r_header.properties.append(temp); + } + r_header.properties[r_header.elements.size() - 1].append(property); + } + else if (words[0] == "end_header") { + break; + } + else if ((words[0][0] >= '0' && words[0][0] <= '9') || words[0][0] == '-' || line.empty() || + file.eof()) { + /* A value was found before we broke out of the loop. No end_header. */ + return "No end_header."; + } + } + return nullptr; +} + void importer_main(bContext *C, const PLYImportParams &import_params, wmOperator *op) { Main *bmain = CTX_data_main(C); @@ -85,73 +143,22 @@ void importer_main(Main *bmain, const PLYImportParams &import_params, wmOperator *op) { - - std::string line; - fstream infile(import_params.filepath, std::ios::in | std::ios::binary); - - PlyHeader header; - - while (true) { /* We break when end_header is encountered. */ - safe_getline(infile, line); - if (header.header_size == 0 && line != "ply") { - fprintf(stderr, "PLY Importer: failed to read file. Invalid PLY header.\n"); - BKE_report(op->reports, RPT_ERROR, "PLY Importer: Invalid PLY header."); - return; - } - header.header_size++; - Vector words{}; - splitstr(line, words, " "); - - if (STREQ(words[0].c_str(), "format")) { - if (STREQ(words[1].c_str(), "ascii")) { - header.type = PlyFormatType::ASCII; - } - else if (STREQ(words[1].c_str(), "binary_big_endian")) { - header.type = PlyFormatType::BINARY_BE; - } - else if (STREQ(words[1].c_str(), "binary_little_endian")) { - header.type = PlyFormatType::BINARY_LE; - } - } - else if (STREQ(words[0].c_str(), "element")) { - header.elements.append(std::make_pair(words[1], std::stoi(words[2]))); - if (STREQ(words[1].c_str(), "vertex")) { - header.vertex_count = std::stoi(words[2]); - } - else if (STREQ(words[1].c_str(), "face")) { - header.face_count = std::stoi(words[2]); - } - else if (STREQ(words[1].c_str(), "edge")) { - header.edge_count = std::stoi(words[2]); - } - } - else if (STREQ(words[0].c_str(), "property")) { - std::pair property; - property.first = words[2]; - property.second = from_string(words[1]); - - while (header.properties.size() < header.elements.size()) { - Vector> temp; - header.properties.append(temp); - } - header.properties[header.elements.size() - 1].append(property); - } - else if (words[0] == "end_header") { - break; - } - else if ((words[0][0] >= '0' && words[0][0] <= '9') || words[0][0] == '-' || line.empty() || - infile.eof()) { - /* A value was found before we broke out of the loop. No end_header. */ - BKE_report(op->reports, RPT_ERROR, "PLY Importer: No end_header"); - return; - } - } - - /* Name used for both mesh and object. */ + /* File base name used for both mesh and object. */ char ob_name[FILE_MAX]; BLI_strncpy(ob_name, BLI_path_basename(import_params.filepath), FILE_MAX); BLI_path_extension_replace(ob_name, FILE_MAX, ""); + /* Parse header. */ + fstream infile(import_params.filepath, std::ios::in | std::ios::binary); + PlyHeader header; + const char *err = read_header(infile, header); + if (err != nullptr) { + fprintf(stderr, "PLY Importer: %s: %s\n", ob_name, err); + BKE_reportf(op->reports, RPT_ERROR, "PLY Importer: %s: %s", ob_name, err); + return; + } + + /* Create mesh and do all prep work. */ Mesh *mesh = BKE_mesh_add(bmain, ob_name); BKE_view_layer_base_deselect_all(scene, view_layer); @@ -163,6 +170,7 @@ void importer_main(Main *bmain, Base *base = BKE_view_layer_base_find(view_layer, obj); BKE_view_layer_base_select_and_set_active(view_layer, base); + /* Parse actual file data. */ try { std::unique_ptr data; if (header.type == PlyFormatType::ASCII) { @@ -183,6 +191,7 @@ void importer_main(Main *bmain, return; } + /* Object matrix and finishing up. */ float global_scale = import_params.global_scale; if ((scene->unit.system != USER_UNIT_NONE) && import_params.use_scene_unit) { global_scale *= scene->unit.scale_length; diff --git a/source/blender/io/ply/importer/ply_import.hh b/source/blender/io/ply/importer/ply_import.hh index a19f32cc5ad..de70c58e3bd 100644 --- a/source/blender/io/ply/importer/ply_import.hh +++ b/source/blender/io/ply/importer/ply_import.hh @@ -9,6 +9,10 @@ #include "IO_ply.h" #include "ply_data.hh" +namespace blender { +class fstream; +} + namespace blender::io::ply { enum PlyDataTypes from_string(const StringRef &input); @@ -25,4 +29,6 @@ void importer_main(Main *bmain, const PLYImportParams &import_params, wmOperator *op); +const char *read_header(fstream &file, PlyHeader &r_header); + } // namespace blender::io::ply diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index c3e01727fae..124401101a2 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -1,24 +1,15 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ -#include "tests/blendfile_loading_base_test.h" +#include "testing/testing.h" -#include "BKE_attribute.hh" -#include "BKE_mesh.hh" -#include "BKE_object.h" - -#include "BLO_readfile.h" - -#include "DEG_depsgraph_query.h" - -#include "IO_ply.h" -#include "ply_data.hh" +#include "BLI_fileops.hh" #include "ply_import.hh" +#include "ply_import_ascii.hh" #include "ply_import_binary.hh" namespace blender::io::ply { struct Expectation { - std::string name; PlyFormatType type; int totvert, totpoly, totedge; float3 vert_first, vert_last; @@ -27,202 +18,107 @@ struct Expectation { float4 color_first = {-1, -1, -1, -1}; }; -class PlyImportTest : public BlendfileLoadingBaseTest { +class ply_import_test : public testing::Test { public: - void import_and_check(const char *path, const Expectation *expect, size_t expect_count) + void import_and_check(const char *path, const Expectation &exp) { - if (!blendfile_load("io_tests/blend_geometry/all_quads.blend")) { + std::string ply_path = blender::tests::flags_test_asset_dir() + "/io_tests/ply/" + path; + + fstream infile(ply_path, std::ios::in | std::ios::binary); + PlyHeader header; + const char *header_err = read_header(infile, header); + if (header_err != nullptr) { ADD_FAILURE(); return; } - - PLYImportParams params; - params.global_scale = 1.0f; - params.forward_axis = IO_AXIS_NEGATIVE_Z; - params.up_axis = IO_AXIS_Y; - params.merge_verts = false; - params.vertex_colors = PLY_VERTEX_COLOR_NONE; - - /* Import the test file. */ - std::string ply_path = blender::tests::flags_test_asset_dir() + "/io_tests/ply/" + path; - strncpy(params.filepath, ply_path.c_str(), FILE_MAX - 1); - importer_main(bfile->main, bfile->curscene, bfile->cur_view_layer, params, nullptr); - - depsgraph_create(DAG_EVAL_VIEWPORT); - - DEGObjectIterSettings deg_iter_settings{}; - deg_iter_settings.depsgraph = depsgraph; - deg_iter_settings.flags = DEG_ITER_OBJECT_FLAG_LINKED_DIRECTLY | - DEG_ITER_OBJECT_FLAG_LINKED_VIA_SET | DEG_ITER_OBJECT_FLAG_VISIBLE | - DEG_ITER_OBJECT_FLAG_DUPLI; - size_t object_index = 0; - - /* Iterate over the objects in the viewport */ - DEG_OBJECT_ITER_BEGIN (°_iter_settings, object) { - if (object_index >= expect_count) { - ADD_FAILURE(); - break; - } - - const Expectation &exp = expect[object_index]; - - ASSERT_STREQ(object->id.name, exp.name.c_str()); - EXPECT_V3_NEAR(object->loc, float3(0, 0, 0), 0.0001f); - - EXPECT_V3_NEAR(object->scale, float3(1, 1, 1), 0.0001f); - if (object->type == OB_MESH) { - Mesh *mesh = BKE_object_get_evaluated_mesh(object); - - /* Test if mesh has expected amount of vertices, edges, and faces. */ - ASSERT_EQ(mesh->totvert, exp.totvert); - ASSERT_EQ(mesh->totedge, exp.totedge); - ASSERT_EQ(mesh->totpoly, exp.totpoly); - - /* Test if first and last vertices match. */ - const Span verts = mesh->vert_positions(); - EXPECT_V3_NEAR(verts.first(), exp.vert_first, 0.0001f); - EXPECT_V3_NEAR(verts.last(), exp.vert_last, 0.0001f); - - /* Fetch normal data from mesh and test if it matches expectation. */ - if (BKE_mesh_has_custom_loop_normals(mesh)) { - const Span vertex_normals = mesh->vert_normals(); - ASSERT_FALSE(vertex_normals.is_empty()); - EXPECT_V3_NEAR(vertex_normals[0], exp.normal_first, 0.0001f); - } - - /* Fetch UV data from mesh and test if it matches expectation. */ - blender::bke::AttributeAccessor attributes = mesh->attributes(); - VArray uvs = attributes.lookup("UVMap"); - float2 uv_first = !uvs.is_empty() ? uvs[0] : float2(0, 0); - EXPECT_V2_NEAR(uv_first, exp.uv_first, 0.0001f); - - /* Check if expected mesh has vertex colors, and tests if it matches. */ - if (CustomData_has_layer(&mesh->vdata, CD_PROP_COLOR)) { - const float4 *colors = (const float4 *)CustomData_get_layer(&mesh->vdata, CD_PROP_COLOR); - ASSERT_TRUE(colors != nullptr); - EXPECT_V4_NEAR(colors[0], exp.color_first, 0.0001f); - } - } - ++object_index; + std::unique_ptr data; + if (header.type == PlyFormatType::ASCII) { + data = import_ply_ascii(infile, &header); + } + else { + data = import_ply_binary(infile, &header); } - DEG_OBJECT_ITER_END; - EXPECT_EQ(object_index, expect_count); + /* Test expected amount of vertices, edges, and faces. */ + ASSERT_EQ(header.vertex_count, exp.totvert); + ASSERT_EQ(data->vertices.size(), exp.totvert); + ASSERT_EQ(header.edge_count, exp.totedge); + ASSERT_EQ(data->edges.size(), exp.totedge); + ASSERT_EQ(header.face_count, exp.totpoly); + ASSERT_EQ(data->faces.size(), exp.totpoly); + + /* Test if first and last vertices match. */ + EXPECT_V3_NEAR(data->vertices.first(), exp.vert_first, 0.0001f); + EXPECT_V3_NEAR(data->vertices.last(), exp.vert_last, 0.0001f); + + /* Check if first normal matches. */ + float3 got_normal = data->vertex_normals.is_empty() ? float3(0, 0, 0) : + data->vertex_normals.first(); + EXPECT_V3_NEAR(got_normal, exp.normal_first, 0.0001f); + + /* Check if first UV matches. */ + float2 got_uv = data->uv_coordinates.is_empty() ? float2(0, 0) : data->uv_coordinates.first(); + EXPECT_V2_NEAR(got_uv, exp.uv_first, 0.0001f); + + /* Check if first color matches. */ + float4 got_color = data->vertex_colors.is_empty() ? float4(-1, -1, -1, -1) : + data->vertex_colors.first(); + EXPECT_V4_NEAR(got_color, exp.color_first, 0.0001f); } }; -TEST_F(PlyImportTest, PLYImportCube) +TEST_F(ply_import_test, PLYImportCube) { - Expectation expect[] = {{"OBCube", - ASCII, - 8, - 6, - 12, - float3(1, 1, -1), - float3(-1, 1, 1), - float3(0.5773, 0.5773, -0.5773), - float2(0, 0)}, - {"OBcube_ascii", - ASCII, - 24, - 6, - 24, - float3(1, 1, -1), - float3(-1, 1, 1), - float3(0, 0, -1), - float2(0.979336, 0.844958), - float4(1, 0.8470, 0, 1)}}; - import_and_check("cube_ascii.ply", expect, 2); + Expectation expect = {ASCII, + 24, + 6, + 0, + float3(1, 1, -1), + float3(-1, 1, 1), + float3(0, 0, -1), + float2(0.979336, 0.844958), + float4(1, 0.8470, 0, 1)}; + import_and_check("cube_ascii.ply", expect); } -TEST_F(PlyImportTest, PLYImportASCIIEdgeTest) +TEST_F(ply_import_test, PLYImportASCIIEdgeTest) { - Expectation expect[] = {{"OBCube", - ASCII, - 8, - 6, - 12, - float3(1, 1, -1), - float3(-1, 1, 1), - float3(0.5773, 0.5773, -0.5773)}, - {"OBASCII_wireframe_cube", - ASCII, - 8, - 0, - 12, - float3(-1, -1, -1), - float3(1, 1, 1), - float3(-2, 0, -1)}}; - - import_and_check("ASCII_wireframe_cube.ply", expect, 2); + Expectation expect = {ASCII, 8, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; + import_and_check("ASCII_wireframe_cube.ply", expect); } -TEST_F(PlyImportTest, PLYImportBunny) +TEST_F(ply_import_test, PLYImportBunny) { - Expectation expect[] = {{"OBCube", - ASCII, - 8, - 6, - 12, - float3(1, 1, -1), - float3(-1, 1, 1), - float3(0.5773, 0.5773, -0.5773)}, - {"OBbunny2", - BINARY_LE, - 1623, - 1000, - 1513, - float3(0.0380425, 0.109755, 0.0161689), - float3(-0.0722821, 0.143895, -0.0129091), - float3(-2, -2, -2)}}; - import_and_check("bunny2.ply", expect, 2); + Expectation expect = {BINARY_LE, + 1623, + 1000, + 0, + float3(0.0380425, 0.109755, 0.0161689), + float3(-0.0722821, 0.143895, -0.0129091)}; + import_and_check("bunny2.ply", expect); } -TEST_F(PlyImportTest, PlyImportManySmallHoles) +TEST_F(ply_import_test, PlyImportManySmallHoles) { - Expectation expect[] = {{"OBCube", - ASCII, - 8, - 6, - 12, - float3(1, 1, -1), - float3(-1, 1, 1), - float3(0.5773, 0.5773, -0.5773)}, - {"OBmany_small_holes", - BINARY_LE, - 2004, - 3524, - 5564, - float3(-0.0131592, -0.0598382, 1.58958), - float3(-0.0177622, 0.0105153, 1.61977), - float3(-2, -2, -2), - float2(0, 0), - float4(0.7215, 0.6784, 0.6627, 1)}}; - import_and_check("many_small_holes.ply", expect, 2); + Expectation expect = {BINARY_LE, + 2004, + 3524, + 0, + float3(-0.0131592, -0.0598382, 1.58958), + float3(-0.0177622, 0.0105153, 1.61977), + float3(0, 0, 0), + float2(0, 0), + float4(0.7215, 0.6784, 0.6627, 1)}; + import_and_check("many_small_holes.ply", expect); } -TEST_F(PlyImportTest, PlyImportWireframeCube) +TEST_F(ply_import_test, PlyImportWireframeCube) { - Expectation expect[] = {{"OBCube", - ASCII, - 8, - 6, - 12, - float3(1, 1, -1), - float3(-1, 1, 1), - float3(0.5773, 0.5773, -0.5773)}, - {"OBwireframe_cube", - BINARY_LE, - 8, - 0, - 12, - float3(-1, -1, -1), - float3(1, 1, 1), - float3(-2, -2, -2)}}; - import_and_check("wireframe_cube.ply", expect, 2); + Expectation expect = {BINARY_LE, 8, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; + import_and_check("wireframe_cube.ply", expect); } -TEST(PlyImportFunctionsTest, PlySwapBytes) +TEST(ply_import_functions_test, PlySwapBytes) { /* Individual bits shouldn't swap with each other. */ uint8_t val8 = 0xA8; -- 2.30.2 From 1522d4b82a601c8d42fc9aa6c842aee1326f2d92 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Tue, 14 Mar 2023 16:24:53 +0200 Subject: [PATCH 02/10] PLY: improve ascii parser robustness and performance - Was not handling type alias names (float32, uint32, int8 etc.), - Was assuming that elements always follow in this order: vertex, face, edge. This is common but not always the case. - Was not handling whitespace that is not literal ASCII space character (but e.g. a tab instead). - Was not checking for partially present properties (e.g. if just "nx" is present but not "ny" and "nz", it was still assuming whole normal is there). - Was assuming that positions/normals/UVs are always floats, and colors are always uchars. But e.g. positions are sometimes doubles, or colors are floats. Now that is handled. Added a bunch of tests that cover the above, that were returning wrong data or crashing previously. It's now faster too, due to no longer splitting each input line into a vector of std::string objects. --- source/blender/io/ply/CMakeLists.txt | 1 + source/blender/io/ply/importer/ply_import.cc | 30 +- source/blender/io/ply/importer/ply_import.hh | 2 - .../io/ply/importer/ply_import_ascii.cc | 356 ++++++++++-------- .../io/ply/importer/ply_import_ascii.hh | 20 +- .../io/ply/importer/ply_import_binary.cc | 50 +-- .../io/ply/importer/ply_import_binary.hh | 5 +- source/blender/io/ply/intern/ply_data.hh | 15 +- .../io/ply/tests/io_ply_importer_test.cc | 143 ++++++- 9 files changed, 390 insertions(+), 232 deletions(-) diff --git a/source/blender/io/ply/CMakeLists.txt b/source/blender/io/ply/CMakeLists.txt index 5414f9a6971..ddaa2d73150 100644 --- a/source/blender/io/ply/CMakeLists.txt +++ b/source/blender/io/ply/CMakeLists.txt @@ -14,6 +14,7 @@ set(INC ../../makesdna ../../makesrna ../../windowmanager + ../../../../extern/fast_float ../../../../extern/fmtlib/include ../../../../intern/guardedalloc ) diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index 25b02b96cea..c78162f8a98 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -44,28 +44,28 @@ void splitstr(std::string str, Vector &words, const StringRef &deli enum PlyDataTypes from_string(const StringRef &input) { - if (input == "uchar") { + if (input == "uchar" || input == "uint8") { return PlyDataTypes::UCHAR; } - if (input == "char") { + if (input == "char" || input == "int8") { return PlyDataTypes::CHAR; } - if (input == "ushort") { + if (input == "ushort" || input == "uint16") { return PlyDataTypes::USHORT; } - if (input == "short") { + if (input == "short" || input == "int16") { return PlyDataTypes::SHORT; } - if (input == "uint") { + if (input == "uint" || input == "uint32") { return PlyDataTypes::UINT; } - if (input == "int") { + if (input == "int" || input == "int32") { return PlyDataTypes::INT; } - if (input == "float") { + if (input == "float" || input == "float32") { return PlyDataTypes::FLOAT; } - if (input == "double") { + if (input == "double" || input == "float64") { return PlyDataTypes::DOUBLE; } return PlyDataTypes::FLOAT; @@ -95,7 +95,10 @@ const char *read_header(fstream &file, PlyHeader &r_header) } } else if (strcmp(words[0].c_str(), "element") == 0) { - r_header.elements.append(std::make_pair(words[1], std::stoi(words[2]))); + PlyElement element; + element.name = words[1]; + element.count = std::stoi(words[2]); + r_header.elements.append(element); if (strcmp(words[1].c_str(), "vertex") == 0) { r_header.vertex_count = std::stoi(words[2]); } @@ -110,12 +113,7 @@ const char *read_header(fstream &file, PlyHeader &r_header) std::pair property; property.first = words[2]; property.second = from_string(words[1]); - - while (r_header.properties.size() < r_header.elements.size()) { - Vector> temp; - r_header.properties.append(temp); - } - r_header.properties[r_header.elements.size() - 1].append(property); + r_header.elements.last().properties.append(property); } else if (words[0] == "end_header") { break; @@ -174,7 +172,7 @@ void importer_main(Main *bmain, try { std::unique_ptr data; if (header.type == PlyFormatType::ASCII) { - data = import_ply_ascii(infile, &header); + data = import_ply_ascii(infile, header); } else { data = import_ply_binary(infile, &header); diff --git a/source/blender/io/ply/importer/ply_import.hh b/source/blender/io/ply/importer/ply_import.hh index de70c58e3bd..0141ebd98a4 100644 --- a/source/blender/io/ply/importer/ply_import.hh +++ b/source/blender/io/ply/importer/ply_import.hh @@ -15,8 +15,6 @@ class fstream; namespace blender::io::ply { -enum PlyDataTypes from_string(const StringRef &input); - void splitstr(std::string str, Vector &words, const StringRef &deli); /* Main import function used from within Blender. */ diff --git a/source/blender/io/ply/importer/ply_import_ascii.cc b/source/blender/io/ply/importer/ply_import_ascii.cc index 4a1e5c43750..494a29a46ec 100644 --- a/source/blender/io/ply/importer/ply_import_ascii.cc +++ b/source/blender/io/ply/importer/ply_import_ascii.cc @@ -10,213 +10,265 @@ #include #include -namespace blender::io::ply { +#include "fast_float.h" +#include -std::unique_ptr import_ply_ascii(fstream &file, PlyHeader *header) +static bool is_whitespace(char c) { - std::unique_ptr data = std::make_unique(load_ply_ascii(file, header)); - return data; + return c <= ' '; } -PlyData load_ply_ascii(fstream &file, const PlyHeader *header) +static const char *drop_whitespace(const char *p, const char *end) { - PlyData data; - /* Check if header contains alpha. */ - std::pair alpha = {"alpha", PlyDataTypes::UCHAR}; - bool has_alpha = std::find(header->properties[0].begin(), header->properties[0].end(), alpha) != - header->properties[0].end(); + while (p < end && is_whitespace(*p)) { + ++p; + } + return p; +} - /* Check if header contains colors. */ - std::pair red = {"red", PlyDataTypes::UCHAR}; - bool has_color = std::find(header->properties[0].begin(), header->properties[0].end(), red) != - header->properties[0].end(); +static const char *drop_non_whitespace(const char *p, const char *end) +{ + while (p < end && !is_whitespace(*p)) { + ++p; + } + return p; +} - /* Check if header contains normals. */ - std::pair normalx = {"nx", PlyDataTypes::FLOAT}; - bool has_normals = std::find(header->properties[0].begin(), - header->properties[0].end(), - normalx) != header->properties[0].end(); +static const char *drop_plus(const char *p, const char *end) +{ + if (p < end && *p == '+') { + ++p; + } + return p; +} - /* Check if header contains uv data. */ - std::pair uv = {"s", PlyDataTypes::FLOAT}; - const bool has_uv = std::find(header->properties[0].begin(), header->properties[0].end(), uv) != - header->properties[0].end(); +static const char *parse_float(const char *p, + const char *end, + float fallback, + float &dst, + bool skip_space = true, + bool require_trailing_space = false) +{ + if (skip_space) { + p = drop_whitespace(p, end); + } + p = drop_plus(p, end); + fast_float::from_chars_result res = fast_float::from_chars(p, end, dst); + if (ELEM(res.ec, std::errc::invalid_argument, std::errc::result_out_of_range)) { + dst = fallback; + } + else if (require_trailing_space && res.ptr < end && !is_whitespace(*res.ptr)) { + /* If there are trailing non-space characters, do not eat up the number. */ + dst = fallback; + return p; + } + return res.ptr; +} - int3 vertex_index = get_vertex_index(header); - int alpha_index; - int3 color_index; - int3 normal_index; - int2 uv_index; +static const char *parse_int( + const char *p, const char *end, int fallback, int &dst, bool skip_space = true) +{ + if (skip_space) { + p = drop_whitespace(p, end); + } + p = drop_plus(p, end); + std::from_chars_result res = std::from_chars(p, end, dst); + if (ELEM(res.ec, std::errc::invalid_argument, std::errc::result_out_of_range)) { + dst = fallback; + } + return res.ptr; +} - if (has_alpha) { - alpha_index = get_index(header, "alpha", PlyDataTypes::UCHAR); +namespace blender::io::ply { + +static const float data_type_normalizer[] = { + 127.0f, 255.0f, 32767.0f, 65535.0f, INT_MAX, UINT_MAX, 1.0f, 1.0f}; +static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, + "PLY data type normalization factor table mismatch"); + +static int get_index(const PlyElement &element, StringRef property) +{ + for (int i = 0, n = int(element.properties.size()); i != n; i++) { + const auto &prop = element.properties[i]; + if (prop.first == property) { + return i; + } + } + return -1; +} + +static int3 get_vertex_index(const PlyElement &element) +{ + return {get_index(element, "x"), get_index(element, "y"), get_index(element, "z")}; +} + +static int3 get_color_index(const PlyElement &element) +{ + return {get_index(element, "red"), get_index(element, "green"), get_index(element, "blue")}; +} + +static int3 get_normal_index(const PlyElement &element) +{ + return {get_index(element, "nx"), get_index(element, "ny"), get_index(element, "nz")}; +} + +static int2 get_uv_index(const PlyElement &element) +{ + return {get_index(element, "s"), get_index(element, "t")}; +} + +static void load_vertex_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + /* Figure out vertex component indices. */ + int3 vertex_index = get_vertex_index(element); + int3 color_index = get_color_index(element); + int3 normal_index = get_normal_index(element); + int2 uv_index = get_uv_index(element); + int alpha_index = get_index(element, "alpha"); + + bool has_vertex = vertex_index.x >= 0 && vertex_index.y >= 0 && vertex_index.z >= 0; + bool has_color = color_index.x >= 0 && color_index.y >= 0 && color_index.z >= 0; + bool has_normal = normal_index.x >= 0 && normal_index.y >= 0 && normal_index.z >= 0; + bool has_uv = uv_index.x >= 0 && uv_index.y >= 0; + bool has_alpha = alpha_index >= 0; + + if (!has_vertex) { + throw std::runtime_error("Vertex positions are not present in the file"); } + float4 color_norm = {1, 1, 1, 1}; if (has_color) { - /* x=red, y=green, z=blue */ - color_index = get_color_index(header); + color_norm.x = data_type_normalizer[element.properties[color_index.x].second]; + color_norm.y = data_type_normalizer[element.properties[color_index.y].second]; + color_norm.z = data_type_normalizer[element.properties[color_index.z].second]; + } + if (has_alpha) { + color_norm.w = data_type_normalizer[element.properties[alpha_index].second]; } - if (has_normals) { - normal_index = get_normal_index(header); - } + Vector value_vec(element.properties.size()); - if (has_uv) { - uv_index = get_uv_index(header); - } - - for (int i = 0; i < header->vertex_count; i++) { + for (int i = 0; i < header.vertex_count; i++) { std::string line; safe_getline(file, line); - Vector value_vec = explode(line, ' '); - /* Vertex coords */ + /* Parse whole line as floats. */ + const char *p = line.data(); + const char *end = p + line.size(); + int value_idx = 0; + while (p < end && value_idx < value_vec.size()) { + float val; + p = parse_float(p, end, 0.0f, val); + value_vec[value_idx++] = val; + } + + /* Vertex coord */ float3 vertex3; - vertex3.x = std::stof(value_vec[vertex_index.x]); - vertex3.y = std::stof(value_vec[vertex_index.y]); - vertex3.z = std::stof(value_vec[vertex_index.z]); + vertex3.x = value_vec[vertex_index.x]; + vertex3.y = value_vec[vertex_index.y]; + vertex3.z = value_vec[vertex_index.z]; + data->vertices.append(vertex3); - data.vertices.append(vertex3); - - /* Vertex colors */ + /* Vertex color */ if (has_color) { float4 colors4; - colors4.x = std::stof(value_vec[color_index.x]) / 255.0f; - colors4.y = std::stof(value_vec[color_index.y]) / 255.0f; - colors4.z = std::stof(value_vec[color_index.z]) / 255.0f; + colors4.x = value_vec[color_index.x] / color_norm.x; + colors4.y = value_vec[color_index.y] / color_norm.y; + colors4.z = value_vec[color_index.z] / color_norm.z; if (has_alpha) { - colors4.w = std::stof(value_vec[alpha_index]) / 255.0f; + colors4.w = value_vec[alpha_index] / color_norm.w; } else { colors4.w = 1.0f; } - - data.vertex_colors.append(colors4); + data->vertex_colors.append(colors4); } /* If normals */ - if (has_normals) { + if (has_normal) { float3 normals3; - normals3.x = std::stof(value_vec[normal_index.x]); - normals3.y = std::stof(value_vec[normal_index.y]); - normals3.z = std::stof(value_vec[normal_index.z]); - - data.vertex_normals.append(normals3); + normals3.x = value_vec[normal_index.x]; + normals3.y = value_vec[normal_index.y]; + normals3.z = value_vec[normal_index.z]; + data->vertex_normals.append(normals3); } /* If uv */ if (has_uv) { float2 uvmap; - uvmap.x = std::stof(value_vec[uv_index.x]); - uvmap.y = std::stof(value_vec[uv_index.y]); - - data.uv_coordinates.append(uvmap); + uvmap.x = value_vec[uv_index.x]; + uvmap.y = value_vec[uv_index.y]; + data->uv_coordinates.append(uvmap); } } - for (int i = 0; i < header->face_count; i++) { +} + +static void load_face_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + for (int i = 0; i < header.face_count; i++) { std::string line; getline(file, line); - Vector value_vec = explode(line, ' '); - int count = std::stoi(value_vec[0]); - Array vertex_indices(count); - for (int j = 1; j <= count; j++) { - int index = std::stoi(value_vec[j]); + const char *p = line.data(); + const char *end = p + line.size(); + int count = 0; + p = parse_int(p, end, 0, count); + + Array vertex_indices(count); + for (int j = 0; j < count; j++) { + int index; + p = parse_int(p, end, 0, index); /* If the face has a vertex index that is outside the range. */ - if (index >= data.vertices.size()) { + if (index >= header.vertex_count) { throw std::runtime_error("Vertex index out of bounds"); } - vertex_indices[j - 1] = index; + vertex_indices[j] = index; } - data.faces.append(vertex_indices); + data->faces.append(vertex_indices); } +} - for (int i = 0; i < header->edge_count; i++) { +static void load_edge_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + for (int i = 0; i < header.edge_count; i++) { std::string line; getline(file, line); - Vector value_vec = explode(line, ' '); + const char *p = line.data(); + const char *end = p + line.size(); + int index0, index1; + p = parse_int(p, end, 0, index0); + p = parse_int(p, end, 0, index1); + data->edges.append(std::make_pair(index0, index1)); + } +} - std::pair edge = std::make_pair(stoi(value_vec[0]), stoi(value_vec[1])); - data.edges.append(edge); +std::unique_ptr import_ply_ascii(fstream &file, const PlyHeader &header) +{ + std::unique_ptr data = std::make_unique(); + + for (const PlyElement &element : header.elements) { + if (element.name == "vertex") { + load_vertex_element(file, header, element, data.get()); + } + else if (element.name == "face") { + load_face_element(file, header, element, data.get()); + } + else if (element.name == "edge") { + load_edge_element(file, header, element, data.get()); + } } return data; } -int3 get_vertex_index(const PlyHeader *header) -{ - int3 vertex_index; - vertex_index.x = get_index(header, "x", PlyDataTypes::FLOAT); - vertex_index.y = get_index(header, "y", PlyDataTypes::FLOAT); - vertex_index.z = get_index(header, "z", PlyDataTypes::FLOAT); - - return vertex_index; -} - -int3 get_color_index(const PlyHeader *header) -{ - int3 color_index; - color_index.x = get_index(header, "red", PlyDataTypes::UCHAR); - color_index.y = get_index(header, "green", PlyDataTypes::UCHAR); - color_index.z = get_index(header, "blue", PlyDataTypes::UCHAR); - - return color_index; -} - -int3 get_normal_index(const PlyHeader *header) -{ - int3 normal_index; - normal_index.x = get_index(header, "nx", PlyDataTypes::FLOAT); - normal_index.y = get_index(header, "ny", PlyDataTypes::FLOAT); - normal_index.z = get_index(header, "nz", PlyDataTypes::FLOAT); - - return normal_index; -} - -int2 get_uv_index(const PlyHeader *header) -{ - int2 uv_index; - uv_index.x = get_index(header, "s", PlyDataTypes::FLOAT); - uv_index.y = get_index(header, "t", PlyDataTypes::FLOAT); - - return uv_index; -} - -int get_index(const PlyHeader *header, std::string property, PlyDataTypes datatype) -{ - std::pair pair = {property, datatype}; - const std::pair *it = std::find( - header->properties[0].begin(), header->properties[0].end(), pair); - return int(it - header->properties[0].begin()); -} - -Vector explode(const StringRef str, const char &ch) -{ - std::string next; - Vector result; - - /* For each character in the string. */ - for (char c : str) { - /* If we've hit the terminal character. */ - if (c == ch) { - /* If we have some characters accumulated. */ - if (!next.empty()) { - /* Add them to the result vector. */ - result.append(next); - next.clear(); - } - } - else { - /* Accumulate the next character into the sequence. */ - next += c; - } - } - - if (!next.empty()) { - result.append(next); - } - - return result; -} } // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_ascii.hh b/source/blender/io/ply/importer/ply_import_ascii.hh index 3506dd3ed54..963e92dd6fc 100644 --- a/source/blender/io/ply/importer/ply_import_ascii.hh +++ b/source/blender/io/ply/importer/ply_import_ascii.hh @@ -8,32 +8,16 @@ #include "BLI_fileops.hh" -#include "DNA_mesh_types.h" - -#include "IO_ply.h" #include "ply_data.hh" namespace blender::io::ply { /** - * The function that gets called from the importer. - * \param file: The PLY file that was opened. - * \param header: The information in the PLY header. - */ -std::unique_ptr import_ply_ascii(fstream &file, PlyHeader *header); - -/** - * Loads the information from the PLY file in ASCII format to the #PlyData data-structure. + * Loads the information from the PLY file in ASCII format to a #PlyData data-structure. * \param file: The PLY file that was opened. * \param header: The information in the PLY header. * \return The #PlyData data-structure that can be used for conversion to a Mesh. */ -PlyData load_ply_ascii(fstream &file, const PlyHeader *header); +std::unique_ptr import_ply_ascii(fstream &file, const PlyHeader &header); -int3 get_vertex_index(const PlyHeader *header); -int3 get_color_index(const PlyHeader *header); -int3 get_normal_index(const PlyHeader *header); -int2 get_uv_index(const PlyHeader *header); -int get_index(const PlyHeader *header, std::string property, PlyDataTypes datatype); -Vector explode(const StringRef str, const char &ch); } // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_binary.cc b/source/blender/io/ply/importer/ply_import_binary.cc index 163d3fdbeb7..0017e286a31 100644 --- a/source/blender/io/ply/importer/ply_import_binary.cc +++ b/source/blender/io/ply/importer/ply_import_binary.cc @@ -84,36 +84,35 @@ PlyData load_ply_binary(fstream &file, const PlyHeader *header) PlyData data; bool is_big_endian = header->type == PlyFormatType::BINARY_BE; - for (int i = 0; i < header->elements.size(); i++) { - if (header->elements[i].first == "vertex") { - /* Import vertices. */ - load_vertex_data(file, header, &data, i); + for (const PlyElement &element : header->elements) { + if (element.name == "vertex") { + /* Read vertices */ + load_vertex_data(file, header, element, &data); } - else if (header->elements[i].first == "edge") { - /* Import edges. */ - for (int j = 0; j < header->elements[i].second; j++) { - std::pair vertex_indices; - for (auto [name, type] : header->properties[i]) { + else if (element.name == "edge") { + /* Read edges */ + for (int j = 0; j < element.count; j++) { + std::pair edge; + for (auto [name, type] : element.properties) { if (name == "vertex1") { - vertex_indices.first = int(read(file, is_big_endian)); + edge.first = int(read(file, is_big_endian)); } else if (name == "vertex2") { - vertex_indices.second = int(read(file, is_big_endian)); + edge.second = int(read(file, is_big_endian)); } else { discard_value(file, type); } } - data.edges.append(vertex_indices); + data.edges.append(edge); } } - else if (header->elements[i].first == "face") { - - /* Import faces. */ - for (int j = 0; j < header->elements[i].second; j++) { + else if (element.name == "face") { + /* Read faces */ + for (int j = 0; j < element.count; j++) { /* Assume vertex_index_count_type is uchar. */ uint8_t count = read(file, is_big_endian); - Array vertex_indices(count); + Array face(count); /* Loop over the amount of vertex indices in this face. */ for (uint8_t k = 0; k < count; k++) { @@ -122,15 +121,15 @@ PlyData load_ply_binary(fstream &file, const PlyHeader *header) if (index >= data.vertices.size()) { throw std::runtime_error("Vertex index out of bounds"); } - vertex_indices[k] = index; + face[k] = index; } - data.faces.append(vertex_indices); + data.faces.append(face); } } else { - /* Nothing else is supported. */ - for (int j = 0; j < header->elements[i].second; j++) { - for (auto [name, type] : header->properties[i]) { + /* Skip everything else. */ + for (int j = 0; j < element.count; j++) { + for (auto [name, type] : element.properties) { discard_value(file, type); } } @@ -140,7 +139,10 @@ PlyData load_ply_binary(fstream &file, const PlyHeader *header) return data; } -void load_vertex_data(fstream &file, const PlyHeader *header, PlyData *r_data, int index) +void load_vertex_data(fstream &file, + const PlyHeader *header, + const PlyElement &element, + PlyData *r_data) { bool hasNormal = false; bool hasColor = false; @@ -153,7 +155,7 @@ void load_vertex_data(fstream &file, const PlyHeader *header, PlyData *r_data, i float4 color{1}; float2 uv{0}; - for (auto [name, type] : header->properties[index]) { + for (auto [name, type] : element.properties) { if (name == "x") { coord.x = read(file, is_big_endian); } diff --git a/source/blender/io/ply/importer/ply_import_binary.hh b/source/blender/io/ply/importer/ply_import_binary.hh index 91f471e5eb2..da83f57dfd4 100644 --- a/source/blender/io/ply/importer/ply_import_binary.hh +++ b/source/blender/io/ply/importer/ply_import_binary.hh @@ -29,7 +29,10 @@ std::unique_ptr import_ply_binary(fstream &file, const PlyHeader *heade */ PlyData load_ply_binary(fstream &file, const PlyHeader *header); -void load_vertex_data(fstream &file, const PlyHeader *header, PlyData *r_data, int index); +void load_vertex_data(fstream &file, + const PlyHeader *header, + const PlyElement &element, + PlyData *r_data); void check_file_errors(const fstream &file); diff --git a/source/blender/io/ply/intern/ply_data.hh b/source/blender/io/ply/intern/ply_data.hh index 087d6e61269..d8369d25240 100644 --- a/source/blender/io/ply/intern/ply_data.hh +++ b/source/blender/io/ply/intern/ply_data.hh @@ -12,7 +12,7 @@ namespace blender::io::ply { -enum PlyDataTypes { CHAR, UCHAR, SHORT, USHORT, INT, UINT, FLOAT, DOUBLE }; +enum PlyDataTypes { CHAR, UCHAR, SHORT, USHORT, INT, UINT, FLOAT, DOUBLE, PLY_TYPE_COUNT }; struct PlyData { Vector vertices; @@ -27,15 +27,20 @@ struct PlyData { enum PlyFormatType { ASCII, BINARY_LE, BINARY_BE }; +struct PlyElement { + std::string name; + int count = 0; + Vector> properties; +}; + struct PlyHeader { int vertex_count = 0; int edge_count = 0; int face_count = 0; int header_size = 0; - /* List of elements in ply file with their count. */ - Vector> elements; - /* List of properties (Name, type) per element. */ - Vector>> properties; + + Vector elements; + PlyFormatType type; }; diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index 124401101a2..6f6ba97eb11 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -10,8 +10,7 @@ namespace blender::io::ply { struct Expectation { - PlyFormatType type; - int totvert, totpoly, totedge; + int totvert, totpoly, totindex, totedge; float3 vert_first, vert_last; float3 normal_first = {0, 0, 0}; float2 uv_first; @@ -32,11 +31,18 @@ class ply_import_test : public testing::Test { return; } std::unique_ptr data; - if (header.type == PlyFormatType::ASCII) { - data = import_ply_ascii(infile, &header); + try { + if (header.type == PlyFormatType::ASCII) { + data = import_ply_ascii(infile, header); + } + else { + data = import_ply_binary(infile, &header); + } } - else { - data = import_ply_binary(infile, &header); + catch (std::exception &e) { + ASSERT_EQ(0, exp.totvert); + ASSERT_EQ(0, exp.totpoly); + return; } /* Test expected amount of vertices, edges, and faces. */ @@ -47,6 +53,12 @@ class ply_import_test : public testing::Test { ASSERT_EQ(header.face_count, exp.totpoly); ASSERT_EQ(data->faces.size(), exp.totpoly); + int indexCount = 0; + for (const auto &f : data->faces) { + indexCount += f.size(); + } + ASSERT_EQ(indexCount, exp.totindex); + /* Test if first and last vertices match. */ EXPECT_V3_NEAR(data->vertices.first(), exp.vert_first, 0.0001f); EXPECT_V3_NEAR(data->vertices.last(), exp.vert_last, 0.0001f); @@ -69,9 +81,9 @@ class ply_import_test : public testing::Test { TEST_F(ply_import_test, PLYImportCube) { - Expectation expect = {ASCII, - 24, + Expectation expect = {24, 6, + 24, 0, float3(1, 1, -1), float3(-1, 1, 1), @@ -83,15 +95,15 @@ TEST_F(ply_import_test, PLYImportCube) TEST_F(ply_import_test, PLYImportASCIIEdgeTest) { - Expectation expect = {ASCII, 8, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; + Expectation expect = {8, 0, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; import_and_check("ASCII_wireframe_cube.ply", expect); } TEST_F(ply_import_test, PLYImportBunny) { - Expectation expect = {BINARY_LE, - 1623, + Expectation expect = {1623, 1000, + 3000, 0, float3(0.0380425, 0.109755, 0.0161689), float3(-0.0722821, 0.143895, -0.0129091)}; @@ -100,9 +112,9 @@ TEST_F(ply_import_test, PLYImportBunny) TEST_F(ply_import_test, PlyImportManySmallHoles) { - Expectation expect = {BINARY_LE, - 2004, + Expectation expect = {2004, 3524, + 10572, 0, float3(-0.0131592, -0.0598382, 1.58958), float3(-0.0177622, 0.0105153, 1.61977), @@ -114,10 +126,113 @@ TEST_F(ply_import_test, PlyImportManySmallHoles) TEST_F(ply_import_test, PlyImportWireframeCube) { - Expectation expect = {BINARY_LE, 8, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; + Expectation expect = {8, 0, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; import_and_check("wireframe_cube.ply", expect); } +TEST_F(ply_import_test, PlyImportColorNotFull) +{ + Expectation expect = {4, 1, 4, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + import_and_check("color_not_full_a.ply", expect); + // import_and_check("color_not_full_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportDoubleXYZ) +{ + Expectation expect = {4, + 1, + 4, + 0, + float3(1, 0, 1), + float3(-1, 0, 1), + float3(0, 0, 0), + float2(0, 0), + float4(1, 0, 0, 1)}; + import_and_check("double_xyz_a.ply", expect); + // import_and_check("double_xyz_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportFaceUVsColors) +{ + Expectation expect = {4, 1, 4, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + import_and_check("face_uvs_colors_a.ply", expect); + // import_and_check("face_uvs_colors_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportFacesFirst) +{ + Expectation expect = {4, + 1, + 4, + 0, + float3(1, 0, 1), + float3(-1, 0, 1), + float3(0, 0, 0), + float2(0, 0), + float4(1, 0, 0, 1)}; + import_and_check("faces_first_a.ply", expect); + // import_and_check("faces_first_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportFloatFormats) +{ + Expectation expect = {4, + 1, + 4, + 0, + float3(1, 0, 1), + float3(-1, 0, 1), + float3(0, 0, 0), + float2(0, 0), + float4(0.5f, 0, 0.25f, 1)}; + import_and_check("float_formats_a.ply", expect); + // import_and_check("float_formats_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportPositionNotFull) +{ + Expectation expect = {0, 0, 0, 0}; + import_and_check("position_not_full_a.ply", expect); + // import_and_check("position_not_full_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportTristrips) +{ + Expectation expect = {6, 0, 0, 0, float3(1, 0, 1), float3(-3, 0, 1)}; //@TODO: incorrect + import_and_check("tristrips_a.ply", expect); + // import_and_check("tristrips_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportTypeAliases) +{ + Expectation expect = {4, + 1, + 4, + 0, + float3(1, 0, 1), + float3(-1, 0, 1), + float3(0, 0, 0), + float2(0, 0), + float4(220 / 255.0f, 20 / 255.0f, 20 / 255.0f, 1)}; + import_and_check("type_aliases_a.ply", expect); + // import_and_check("type_aliases_b.ply", expect); +} + +TEST_F(ply_import_test, PlyImportVertexCompOrder) +{ + Expectation expect = {4, + 1, + 4, + 0, + float3(1, 0, 1), + float3(-1, 0, 1), + float3(0, 0, 0), + float2(0, 0), + float4(0.8f, 0.2f, 0, 1)}; + import_and_check("vertex_comp_order_a.ply", expect); + // import_and_check("vertex_comp_order_b.ply", expect); +} + TEST(ply_import_functions_test, PlySwapBytes) { /* Individual bits shouldn't swap with each other. */ -- 2.30.2 From 20e8222c4456c7cc8a2d01e973faf8a4d7273258 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Wed, 15 Mar 2023 12:13:25 +0200 Subject: [PATCH 03/10] PLY: improve binary parser robustness - Was assuming that positions/normals/UVs are floats and colors are bytes. - Was assuming that faces are always byte list size and int32 indices. - Was assuming that edge vertex indices are always int32. Added a bunch of tests that cover the above, that were returning wrong data or crashing previously. It should now be faster too, due to reading not one property at a time, but whole row or list at a time. Also the whole code is merged together between ascii and binary into ply_import_data.cc/hh, since it shares most of the "figure out property indices" logic. There's also a test case with big endian binary file; coverage of that was completely non-existent before. --- source/blender/io/ply/CMakeLists.txt | 6 +- source/blender/io/ply/importer/ply_import.cc | 35 +- .../io/ply/importer/ply_import_ascii.cc | 274 ----------- .../io/ply/importer/ply_import_binary.cc | 217 --------- .../io/ply/importer/ply_import_binary.hh | 74 --- .../io/ply/importer/ply_import_data.cc | 443 ++++++++++++++++++ ...ply_import_ascii.hh => ply_import_data.hh} | 4 +- source/blender/io/ply/intern/ply_data.hh | 13 +- .../io/ply/tests/io_ply_importer_test.cc | 121 +++-- 9 files changed, 548 insertions(+), 639 deletions(-) delete mode 100644 source/blender/io/ply/importer/ply_import_ascii.cc delete mode 100644 source/blender/io/ply/importer/ply_import_binary.cc delete mode 100644 source/blender/io/ply/importer/ply_import_binary.hh create mode 100644 source/blender/io/ply/importer/ply_import_data.cc rename source/blender/io/ply/importer/{ply_import_ascii.hh => ply_import_data.hh} (69%) diff --git a/source/blender/io/ply/CMakeLists.txt b/source/blender/io/ply/CMakeLists.txt index ddaa2d73150..266455cea70 100644 --- a/source/blender/io/ply/CMakeLists.txt +++ b/source/blender/io/ply/CMakeLists.txt @@ -32,8 +32,7 @@ set(SRC exporter/ply_file_buffer_ascii.cc exporter/ply_file_buffer_binary.cc importer/ply_import.cc - importer/ply_import_ascii.cc - importer/ply_import_binary.cc + importer/ply_import_data.cc importer/ply_import_mesh.cc IO_ply.cc @@ -45,8 +44,7 @@ set(SRC exporter/ply_file_buffer_ascii.hh exporter/ply_file_buffer_binary.hh importer/ply_import.hh - importer/ply_import_ascii.hh - importer/ply_import_binary.hh + importer/ply_import_data.hh importer/ply_import_mesh.hh IO_ply.h diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index c78162f8a98..d44aaecdca3 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -24,8 +24,7 @@ #include "ply_data.hh" #include "ply_functions.hh" #include "ply_import.hh" -#include "ply_import_ascii.hh" -#include "ply_import_binary.hh" +#include "ply_import_data.hh" #include "ply_import_mesh.hh" namespace blender::io::ply { @@ -42,7 +41,7 @@ void splitstr(std::string str, Vector &words, const StringRef &deli words.append(str.substr()); } -enum PlyDataTypes from_string(const StringRef &input) +static PlyDataTypes type_from_string(const StringRef &input) { if (input == "uchar" || input == "uint8") { return PlyDataTypes::UCHAR; @@ -68,7 +67,7 @@ enum PlyDataTypes from_string(const StringRef &input) if (input == "double" || input == "float64") { return PlyDataTypes::DOUBLE; } - return PlyDataTypes::FLOAT; + return PlyDataTypes::NONE; } const char *read_header(fstream &file, PlyHeader &r_header) @@ -110,9 +109,19 @@ const char *read_header(fstream &file, PlyHeader &r_header) } } else if (strcmp(words[0].c_str(), "property") == 0) { - std::pair property; - property.first = words[2]; - property.second = from_string(words[1]); + PlyProperty property; + if (words.size() >= 3 && words[1] != "list") { + property.type = type_from_string(words[1]); + property.name = words[2]; + } + else if (words.size() >= 5 && words[1] == "list") { + property.count_type = type_from_string(words[2]); + property.type = type_from_string(words[3]); + property.name = words[4]; + } + else { + return "Malformed property"; + } r_header.elements.last().properties.append(property); } else if (words[0] == "end_header") { @@ -124,6 +133,10 @@ const char *read_header(fstream &file, PlyHeader &r_header) return "No end_header."; } } + + for (PlyElement &el : r_header.elements) { + el.calc_stride(); + } return nullptr; } @@ -170,13 +183,7 @@ void importer_main(Main *bmain, /* Parse actual file data. */ try { - std::unique_ptr data; - if (header.type == PlyFormatType::ASCII) { - data = import_ply_ascii(infile, header); - } - else { - data = import_ply_binary(infile, &header); - } + std::unique_ptr data = import_ply_data(infile, header); Mesh *temp_val = convert_ply_to_mesh(*data, mesh, import_params); if (import_params.merge_verts && temp_val != mesh) { diff --git a/source/blender/io/ply/importer/ply_import_ascii.cc b/source/blender/io/ply/importer/ply_import_ascii.cc deleted file mode 100644 index 494a29a46ec..00000000000 --- a/source/blender/io/ply/importer/ply_import_ascii.cc +++ /dev/null @@ -1,274 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -/** \file - * \ingroup ply - */ - -#include "ply_import_ascii.hh" -#include "ply_functions.hh" - -#include -#include - -#include "fast_float.h" -#include - -static bool is_whitespace(char c) -{ - return c <= ' '; -} - -static const char *drop_whitespace(const char *p, const char *end) -{ - while (p < end && is_whitespace(*p)) { - ++p; - } - return p; -} - -static const char *drop_non_whitespace(const char *p, const char *end) -{ - while (p < end && !is_whitespace(*p)) { - ++p; - } - return p; -} - -static const char *drop_plus(const char *p, const char *end) -{ - if (p < end && *p == '+') { - ++p; - } - return p; -} - -static const char *parse_float(const char *p, - const char *end, - float fallback, - float &dst, - bool skip_space = true, - bool require_trailing_space = false) -{ - if (skip_space) { - p = drop_whitespace(p, end); - } - p = drop_plus(p, end); - fast_float::from_chars_result res = fast_float::from_chars(p, end, dst); - if (ELEM(res.ec, std::errc::invalid_argument, std::errc::result_out_of_range)) { - dst = fallback; - } - else if (require_trailing_space && res.ptr < end && !is_whitespace(*res.ptr)) { - /* If there are trailing non-space characters, do not eat up the number. */ - dst = fallback; - return p; - } - return res.ptr; -} - -static const char *parse_int( - const char *p, const char *end, int fallback, int &dst, bool skip_space = true) -{ - if (skip_space) { - p = drop_whitespace(p, end); - } - p = drop_plus(p, end); - std::from_chars_result res = std::from_chars(p, end, dst); - if (ELEM(res.ec, std::errc::invalid_argument, std::errc::result_out_of_range)) { - dst = fallback; - } - return res.ptr; -} - -namespace blender::io::ply { - -static const float data_type_normalizer[] = { - 127.0f, 255.0f, 32767.0f, 65535.0f, INT_MAX, UINT_MAX, 1.0f, 1.0f}; -static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, - "PLY data type normalization factor table mismatch"); - -static int get_index(const PlyElement &element, StringRef property) -{ - for (int i = 0, n = int(element.properties.size()); i != n; i++) { - const auto &prop = element.properties[i]; - if (prop.first == property) { - return i; - } - } - return -1; -} - -static int3 get_vertex_index(const PlyElement &element) -{ - return {get_index(element, "x"), get_index(element, "y"), get_index(element, "z")}; -} - -static int3 get_color_index(const PlyElement &element) -{ - return {get_index(element, "red"), get_index(element, "green"), get_index(element, "blue")}; -} - -static int3 get_normal_index(const PlyElement &element) -{ - return {get_index(element, "nx"), get_index(element, "ny"), get_index(element, "nz")}; -} - -static int2 get_uv_index(const PlyElement &element) -{ - return {get_index(element, "s"), get_index(element, "t")}; -} - -static void load_vertex_element(fstream &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) -{ - /* Figure out vertex component indices. */ - int3 vertex_index = get_vertex_index(element); - int3 color_index = get_color_index(element); - int3 normal_index = get_normal_index(element); - int2 uv_index = get_uv_index(element); - int alpha_index = get_index(element, "alpha"); - - bool has_vertex = vertex_index.x >= 0 && vertex_index.y >= 0 && vertex_index.z >= 0; - bool has_color = color_index.x >= 0 && color_index.y >= 0 && color_index.z >= 0; - bool has_normal = normal_index.x >= 0 && normal_index.y >= 0 && normal_index.z >= 0; - bool has_uv = uv_index.x >= 0 && uv_index.y >= 0; - bool has_alpha = alpha_index >= 0; - - if (!has_vertex) { - throw std::runtime_error("Vertex positions are not present in the file"); - } - - float4 color_norm = {1, 1, 1, 1}; - if (has_color) { - color_norm.x = data_type_normalizer[element.properties[color_index.x].second]; - color_norm.y = data_type_normalizer[element.properties[color_index.y].second]; - color_norm.z = data_type_normalizer[element.properties[color_index.z].second]; - } - if (has_alpha) { - color_norm.w = data_type_normalizer[element.properties[alpha_index].second]; - } - - Vector value_vec(element.properties.size()); - - for (int i = 0; i < header.vertex_count; i++) { - std::string line; - safe_getline(file, line); - - /* Parse whole line as floats. */ - const char *p = line.data(); - const char *end = p + line.size(); - int value_idx = 0; - while (p < end && value_idx < value_vec.size()) { - float val; - p = parse_float(p, end, 0.0f, val); - value_vec[value_idx++] = val; - } - - /* Vertex coord */ - float3 vertex3; - vertex3.x = value_vec[vertex_index.x]; - vertex3.y = value_vec[vertex_index.y]; - vertex3.z = value_vec[vertex_index.z]; - data->vertices.append(vertex3); - - /* Vertex color */ - if (has_color) { - float4 colors4; - colors4.x = value_vec[color_index.x] / color_norm.x; - colors4.y = value_vec[color_index.y] / color_norm.y; - colors4.z = value_vec[color_index.z] / color_norm.z; - if (has_alpha) { - colors4.w = value_vec[alpha_index] / color_norm.w; - } - else { - colors4.w = 1.0f; - } - data->vertex_colors.append(colors4); - } - - /* If normals */ - if (has_normal) { - float3 normals3; - normals3.x = value_vec[normal_index.x]; - normals3.y = value_vec[normal_index.y]; - normals3.z = value_vec[normal_index.z]; - data->vertex_normals.append(normals3); - } - - /* If uv */ - if (has_uv) { - float2 uvmap; - uvmap.x = value_vec[uv_index.x]; - uvmap.y = value_vec[uv_index.y]; - data->uv_coordinates.append(uvmap); - } - } -} - -static void load_face_element(fstream &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) -{ - for (int i = 0; i < header.face_count; i++) { - std::string line; - getline(file, line); - - const char *p = line.data(); - const char *end = p + line.size(); - int count = 0; - p = parse_int(p, end, 0, count); - - Array vertex_indices(count); - for (int j = 0; j < count; j++) { - int index; - p = parse_int(p, end, 0, index); - /* If the face has a vertex index that is outside the range. */ - if (index >= header.vertex_count) { - throw std::runtime_error("Vertex index out of bounds"); - } - vertex_indices[j] = index; - } - data->faces.append(vertex_indices); - } -} - -static void load_edge_element(fstream &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) -{ - for (int i = 0; i < header.edge_count; i++) { - std::string line; - getline(file, line); - const char *p = line.data(); - const char *end = p + line.size(); - int index0, index1; - p = parse_int(p, end, 0, index0); - p = parse_int(p, end, 0, index1); - data->edges.append(std::make_pair(index0, index1)); - } -} - -std::unique_ptr import_ply_ascii(fstream &file, const PlyHeader &header) -{ - std::unique_ptr data = std::make_unique(); - - for (const PlyElement &element : header.elements) { - if (element.name == "vertex") { - load_vertex_element(file, header, element, data.get()); - } - else if (element.name == "face") { - load_face_element(file, header, element, data.get()); - } - else if (element.name == "edge") { - load_edge_element(file, header, element, data.get()); - } - } - - return data; -} - - -} // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_binary.cc b/source/blender/io/ply/importer/ply_import_binary.cc deleted file mode 100644 index 0017e286a31..00000000000 --- a/source/blender/io/ply/importer/ply_import_binary.cc +++ /dev/null @@ -1,217 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -/** \file - * \ingroup ply - */ -#include "BLI_array.hh" - -#include "ply_import_binary.hh" - -#include - -namespace blender::io::ply { -std::unique_ptr import_ply_binary(fstream &file, const PlyHeader *header) -{ - std::unique_ptr data = std::make_unique(load_ply_binary(file, header)); - return data; -} - -template T read(fstream &file, bool is_big_endian) -{ - T returnVal; - file.read((char *)&returnVal, sizeof(returnVal)); - check_file_errors(file); - if (is_big_endian) { - returnVal = swap_bytes(returnVal); - } - return returnVal; -} - -template uint8_t read(fstream &file, bool is_big_endian); -template int8_t read(fstream &file, bool is_big_endian); -template uint16_t read(fstream &file, bool is_big_endian); -template int16_t read(fstream &file, bool is_big_endian); -template uint32_t read(fstream &file, bool is_big_endian); -template int32_t read(fstream &file, bool is_big_endian); -template float read(fstream &file, bool is_big_endian); -template double read(fstream &file, bool is_big_endian); - -void check_file_errors(const fstream &file) -{ - if (file.bad()) { - throw std::ios_base::failure("Read/Write error on io operation"); - } - if (file.fail()) { - throw std::ios_base::failure("Logical error on io operation"); - } - if (file.eof()) { - throw std::ios_base::failure("Reached end of the file"); - } -} - -void discard_value(fstream &file, const PlyDataTypes type) -{ - switch (type) { - case CHAR: - read(file, false); - break; - case UCHAR: - read(file, false); - break; - case SHORT: - read(file, false); - break; - case USHORT: - read(file, false); - break; - case INT: - read(file, false); - break; - case UINT: - read(file, false); - break; - case FLOAT: - read(file, false); - break; - case DOUBLE: - read(file, false); - break; - } -} - -PlyData load_ply_binary(fstream &file, const PlyHeader *header) -{ - PlyData data; - bool is_big_endian = header->type == PlyFormatType::BINARY_BE; - - for (const PlyElement &element : header->elements) { - if (element.name == "vertex") { - /* Read vertices */ - load_vertex_data(file, header, element, &data); - } - else if (element.name == "edge") { - /* Read edges */ - for (int j = 0; j < element.count; j++) { - std::pair edge; - for (auto [name, type] : element.properties) { - if (name == "vertex1") { - edge.first = int(read(file, is_big_endian)); - } - else if (name == "vertex2") { - edge.second = int(read(file, is_big_endian)); - } - else { - discard_value(file, type); - } - } - data.edges.append(edge); - } - } - else if (element.name == "face") { - /* Read faces */ - for (int j = 0; j < element.count; j++) { - /* Assume vertex_index_count_type is uchar. */ - uint8_t count = read(file, is_big_endian); - Array face(count); - - /* Loop over the amount of vertex indices in this face. */ - for (uint8_t k = 0; k < count; k++) { - uint32_t index = read(file, is_big_endian); - /* If the face has a vertex index that is outside the range. */ - if (index >= data.vertices.size()) { - throw std::runtime_error("Vertex index out of bounds"); - } - face[k] = index; - } - data.faces.append(face); - } - } - else { - /* Skip everything else. */ - for (int j = 0; j < element.count; j++) { - for (auto [name, type] : element.properties) { - discard_value(file, type); - } - } - } - } - - return data; -} - -void load_vertex_data(fstream &file, - const PlyHeader *header, - const PlyElement &element, - PlyData *r_data) -{ - bool hasNormal = false; - bool hasColor = false; - bool hasUv = false; - bool is_big_endian = header->type == PlyFormatType::BINARY_BE; - - for (int i = 0; i < header->vertex_count; i++) { - float3 coord{0}; - float3 normal{0}; - float4 color{1}; - float2 uv{0}; - - for (auto [name, type] : element.properties) { - if (name == "x") { - coord.x = read(file, is_big_endian); - } - else if (name == "y") { - coord.y = read(file, is_big_endian); - } - else if (name == "z") { - coord.z = read(file, is_big_endian); - } - else if (name == "nx") { - normal.x = read(file, is_big_endian); - hasNormal = true; - } - else if (name == "ny") { - normal.y = read(file, is_big_endian); - } - else if (name == "nz") { - normal.z = read(file, is_big_endian); - } - else if (name == "red") { - color.x = read(file, is_big_endian) / 255.0f; - hasColor = true; - } - else if (name == "green") { - color.y = read(file, is_big_endian) / 255.0f; - } - else if (name == "blue") { - color.z = read(file, is_big_endian) / 255.0f; - } - else if (name == "alpha") { - color.w = read(file, is_big_endian) / 255.0f; - } - else if (name == "s") { - uv.x = read(file, is_big_endian); - hasUv = true; - } - else if (name == "t") { - uv.y = read(file, is_big_endian); - } - else { - /* No other properties are supported yet. */ - discard_value(file, type); - } - } - - r_data->vertices.append(coord); - if (hasNormal) { - r_data->vertex_normals.append(normal); - } - if (hasColor) { - r_data->vertex_colors.append(color); - } - if (hasUv) { - r_data->uv_coordinates.append(uv); - } - } -} - -} // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_binary.hh b/source/blender/io/ply/importer/ply_import_binary.hh deleted file mode 100644 index da83f57dfd4..00000000000 --- a/source/blender/io/ply/importer/ply_import_binary.hh +++ /dev/null @@ -1,74 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -/** \file - * \ingroup ply - */ - -#pragma once - -#include "BLI_endian_switch.h" -#include "BLI_fileops.hh" - -#include "ply_data.hh" - -namespace blender::io::ply { - -/** - * The function that gets called from the importer. - * \param file: The PLY file that was opened. - * \param header: The information in the PLY header. - * \return The #PlyData data-structure that can be used for conversion to a #Mesh. - */ -std::unique_ptr import_ply_binary(fstream &file, const PlyHeader *header); - -/** - * Loads the information from the PLY file in binary format to the #PlyData data-structure. - * \param file: The PLY file that was opened. - * \param header: The information in the PLY header. - * \return The #PlyData data-structure that can be used for conversion to a Mesh. - */ -PlyData load_ply_binary(fstream &file, const PlyHeader *header); - -void load_vertex_data(fstream &file, - const PlyHeader *header, - const PlyElement &element, - PlyData *r_data); - -void check_file_errors(const fstream &file); - -void discard_value(fstream &file, const PlyDataTypes type); - -template T swap_bytes(T input) -{ - /* In big endian, the most-significant byte is first. - * So, we need to swap the byte order. */ - - /* 0xAC in LE should become 0xCA in BE. */ - if (sizeof(T) == 1) { - return input; - } - - if constexpr (sizeof(T) == 2) { - uint16_t value = reinterpret_cast(input); - BLI_endian_switch_uint16(&value); - return reinterpret_cast(value); - } - - if constexpr (sizeof(T) == 4) { - /* Reinterpret this data as uint32 for easy rearranging of bytes. */ - uint32_t value = reinterpret_cast(input); - BLI_endian_switch_uint32(&value); - return reinterpret_cast(value); - } - - if constexpr (sizeof(T) == 8) { - /* Reinterpret this data as uint64 for easy rearranging of bytes. */ - uint64_t value = reinterpret_cast(input); - BLI_endian_switch_uint64(&value); - return reinterpret_cast(value); - } -} - -template T read(fstream &file, bool isBigEndian); - -} // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc new file mode 100644 index 00000000000..745800b0609 --- /dev/null +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -0,0 +1,443 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup ply + */ + +#include "ply_import_data.hh" +#include "ply_functions.hh" + +#include "BLI_endian_switch.h" + +#include "fast_float.h" + +#include +#include +#include + +static bool is_whitespace(char c) +{ + return c <= ' '; +} + +static const char *drop_whitespace(const char *p, const char *end) +{ + while (p < end && is_whitespace(*p)) { + ++p; + } + return p; +} + +static const char *drop_plus(const char *p, const char *end) +{ + if (p < end && *p == '+') { + ++p; + } + return p; +} + +static const char *parse_float(const char *p, + const char *end, + float fallback, + float &dst) +{ + p = drop_whitespace(p, end); + p = drop_plus(p, end); + fast_float::from_chars_result res = fast_float::from_chars(p, end, dst); + if (ELEM(res.ec, std::errc::invalid_argument, std::errc::result_out_of_range)) { + dst = fallback; + } + return res.ptr; +} + +static const char *parse_int( + const char *p, const char *end, int fallback, int &dst) +{ + p = drop_whitespace(p, end); + p = drop_plus(p, end); + std::from_chars_result res = std::from_chars(p, end, dst); + if (ELEM(res.ec, std::errc::invalid_argument, std::errc::result_out_of_range)) { + dst = fallback; + } + return res.ptr; +} + +static void endian_switch(uint8_t *ptr, int type_size) +{ + if (type_size == 2) { + BLI_endian_switch_uint16((uint16_t*)ptr); + } + else if (type_size == 4) { + BLI_endian_switch_uint32((uint32_t*)ptr); + } + else if (type_size == 8) { + BLI_endian_switch_uint64((uint64_t*)ptr); + } +} + +static void endian_switch_array(uint8_t *ptr, int type_size, int size) +{ + if (type_size == 2) { + BLI_endian_switch_uint16_array((uint16_t*)ptr, size); + } + else if (type_size == 4) { + BLI_endian_switch_uint32_array((uint32_t*)ptr, size); + } + else if (type_size == 8) { + BLI_endian_switch_uint64_array((uint64_t*)ptr, size); + } +} + +namespace blender::io::ply { + +static const int data_type_size[] = {0, 1, 1, 2, 2, 4, 4, 4, 8}; +static_assert(std::size(data_type_size) == PLY_TYPE_COUNT, "PLY data type size table mismatch"); + +void PlyElement::calc_stride() +{ + stride = 0; + for (PlyProperty &p : properties) { + if (p.count_type != PlyDataTypes::NONE) { + stride = 0; + return; + } + stride += data_type_size[p.type]; + } +} + + +static const float data_type_normalizer[] = { + 1.0f, 127.0f, 255.0f, 32767.0f, 65535.0f, INT_MAX, UINT_MAX, 1.0f, 1.0f}; +static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, + "PLY data type normalization factor table mismatch"); + +static int get_index(const PlyElement &element, StringRef property) +{ + for (int i = 0, n = int(element.properties.size()); i != n; i++) { + const PlyProperty &prop = element.properties[i]; + if (prop.name == property) { + return i; + } + } + return -1; +} + +static int3 get_vertex_index(const PlyElement &element) +{ + return {get_index(element, "x"), get_index(element, "y"), get_index(element, "z")}; +} + +static int3 get_color_index(const PlyElement &element) +{ + return {get_index(element, "red"), get_index(element, "green"), get_index(element, "blue")}; +} + +static int3 get_normal_index(const PlyElement &element) +{ + return {get_index(element, "nx"), get_index(element, "ny"), get_index(element, "nz")}; +} + +static int2 get_uv_index(const PlyElement &element) +{ + return {get_index(element, "s"), get_index(element, "t")}; +} + +static void parse_row_ascii(fstream &file, Vector &r_values) +{ + std::string line; + safe_getline(file, line); + + /* Parse whole line as floats. */ + const char *p = line.data(); + const char *end = p + line.size(); + int value_idx = 0; + while (p < end && value_idx < r_values.size()) { + float val; + p = parse_float(p, end, 0.0f, val); + r_values[value_idx++] = val; + } +} + +static void check_file_errors(const fstream &file) +{ + if (file.bad()) { + throw std::ios_base::failure("Read/Write error on io operation"); + } + if (file.fail()) { + throw std::ios_base::failure("Logical error on io operation"); + } + if (file.eof()) { + throw std::ios_base::failure("Reached end of the file"); + } +} + +template +static T get_binary_value(PlyDataTypes type, const uint8_t*& r_ptr) +{ + T val = 0; + switch (type) { + case NONE: break; + case CHAR: val = *(int8_t*)r_ptr; r_ptr += 1; break; + case UCHAR: val = *(uint8_t*)r_ptr; r_ptr += 1; break; + case SHORT: val = *(int16_t*)r_ptr; r_ptr += 2; break; + case USHORT: val = *(uint16_t*)r_ptr; r_ptr += 2; break; + case INT: val = *(int32_t*)r_ptr; r_ptr += 4; break; + case UINT: val = *(int32_t*)r_ptr; r_ptr += 4; break; + case FLOAT: val = *(float*)r_ptr; r_ptr += 4; break; + case DOUBLE: val = *(double*)r_ptr; r_ptr += 8; break; + default: throw std::runtime_error("Unknown property type"); + } + return val; +} + +static void parse_row_binary(fstream &file, + const PlyHeader &header, + const PlyElement &element, + Vector &r_scratch, + Vector &r_values) +{ + if (element.stride == 0) { + throw std::runtime_error("Vertex/Edge element contains list properties, this is not supported"); + } + BLI_assert(r_scratch.size() == element.stride); + BLI_assert(r_values.size() == element.properties.size()); + file.read((char*)r_scratch.data(), r_scratch.size()); + check_file_errors(file); + + const uint8_t* ptr = r_scratch.data(); + if (header.type == PlyFormatType::BINARY_LE) { + /* Little endian: just read/convert the values. */ + for (int i = 0, n = int(element.properties.size()); i != n; i++) { + const PlyProperty& prop = element.properties[i]; + float val = get_binary_value(prop.type, ptr); + r_values[i] = val; + } + } + else if (header.type == PlyFormatType::BINARY_BE) { + /* Big endian: read, switch endian, convert the values. */ + for (int i = 0, n = int(element.properties.size()); i != n; i++) { + const PlyProperty& prop = element.properties[i]; + endian_switch((uint8_t*)ptr, data_type_size[prop.type]); + float val = get_binary_value(prop.type, ptr); + r_values[i] = val; + } + } + else { + throw std::runtime_error("Unknown binary ply format for vertex element"); + } +} + +static void load_vertex_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + /* Figure out vertex component indices. */ + int3 vertex_index = get_vertex_index(element); + int3 color_index = get_color_index(element); + int3 normal_index = get_normal_index(element); + int2 uv_index = get_uv_index(element); + int alpha_index = get_index(element, "alpha"); + + bool has_vertex = vertex_index.x >= 0 && vertex_index.y >= 0 && vertex_index.z >= 0; + bool has_color = color_index.x >= 0 && color_index.y >= 0 && color_index.z >= 0; + bool has_normal = normal_index.x >= 0 && normal_index.y >= 0 && normal_index.z >= 0; + bool has_uv = uv_index.x >= 0 && uv_index.y >= 0; + bool has_alpha = alpha_index >= 0; + + if (!has_vertex) { + throw std::runtime_error("Vertex positions are not present in the file"); + } + + float4 color_norm = {1, 1, 1, 1}; + if (has_color) { + color_norm.x = data_type_normalizer[element.properties[color_index.x].type]; + color_norm.y = data_type_normalizer[element.properties[color_index.y].type]; + color_norm.z = data_type_normalizer[element.properties[color_index.z].type]; + } + if (has_alpha) { + color_norm.w = data_type_normalizer[element.properties[alpha_index].type]; + } + + Vector value_vec(element.properties.size()); + Vector scratch; + if (header.type != PlyFormatType::ASCII) { + scratch.resize(element.stride); + } + + for (int i = 0; i < header.vertex_count; i++) { + + if (header.type == PlyFormatType::ASCII) { + parse_row_ascii(file, value_vec); + } + else { + parse_row_binary(file, header, element, scratch, value_vec); + } + + /* Vertex coord */ + float3 vertex3; + vertex3.x = value_vec[vertex_index.x]; + vertex3.y = value_vec[vertex_index.y]; + vertex3.z = value_vec[vertex_index.z]; + data->vertices.append(vertex3); + + /* Vertex color */ + if (has_color) { + float4 colors4; + colors4.x = value_vec[color_index.x] / color_norm.x; + colors4.y = value_vec[color_index.y] / color_norm.y; + colors4.z = value_vec[color_index.z] / color_norm.z; + if (has_alpha) { + colors4.w = value_vec[alpha_index] / color_norm.w; + } + else { + colors4.w = 1.0f; + } + data->vertex_colors.append(colors4); + } + + /* If normals */ + if (has_normal) { + float3 normals3; + normals3.x = value_vec[normal_index.x]; + normals3.y = value_vec[normal_index.y]; + normals3.z = value_vec[normal_index.z]; + data->vertex_normals.append(normals3); + } + + /* If uv */ + if (has_uv) { + float2 uvmap; + uvmap.x = value_vec[uv_index.x]; + uvmap.y = value_vec[uv_index.y]; + data->uv_coordinates.append(uvmap); + } + } +} + +static void load_face_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + int prop_index = get_index(element, "vertex_indices"); + if (prop_index < 0 && element.properties.size() == 1) { + prop_index = 0; + } + if (prop_index < 0) { + throw std::runtime_error("Face element does not contain vertex indices property"); + } + const PlyProperty &prop = element.properties[prop_index]; + if (prop.count_type == PlyDataTypes::NONE) { + throw std::runtime_error("Face element vertex indices property must be a list"); + } + + if (header.type == PlyFormatType::ASCII) { + for (int i = 0; i < header.face_count; i++) { + std::string line; + getline(file, line); + + const char *p = line.data(); + const char *end = p + line.size(); + int count = 0; + p = parse_int(p, end, 0, count); + + Array vertex_indices(count); + for (int j = 0; j < count; j++) { + int index; + p = parse_int(p, end, 0, index); + /* If the face has a vertex index that is outside the range. */ + if (index >= header.vertex_count) { + throw std::runtime_error("Vertex index out of bounds"); + } + vertex_indices[j] = index; + } + data->faces.append(vertex_indices); + } + } + else { + Vector scratch(64); + + for (int i = 0; i < header.face_count; i++) { + const uint8_t* ptr; + /* Read list size. */ + scratch.resize(8); + file.read((char*)scratch.data(), data_type_size[prop.count_type]); + ptr = scratch.data(); + if (header.type == PlyFormatType::BINARY_BE) + endian_switch((uint8_t*)ptr, data_type_size[prop.count_type]); + uint32_t count = get_binary_value(prop.count_type, ptr); + + //@TODO: check invalid face size values? + + /* Read list values. */ + Array face(count); + scratch.resize(count * data_type_size[prop.type]); + file.read((char*)scratch.data(), scratch.size()); + ptr = scratch.data(); + if (header.type == PlyFormatType::BINARY_BE) + endian_switch_array((uint8_t*)ptr, data_type_size[prop.type], count); + for (int j = 0; j < count; ++j) { + uint32_t index = get_binary_value(prop.type, ptr); + if (index >= header.vertex_count) { + throw std::runtime_error("Vertex index out of bounds"); + } + face[j] = index; + } + data->faces.append(face); + } + } +} + + +static void load_edge_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + int prop_vertex1 = get_index(element, "vertex1"); + int prop_vertex2 = get_index(element, "vertex2"); + if (prop_vertex1 < 0 || prop_vertex2 < 0) { + throw std::runtime_error("Edge element does not contain vertex1 and vertex2 properties"); + } + + Vector value_vec(element.properties.size()); + Vector scratch; + if (header.type != PlyFormatType::ASCII) { + scratch.resize(element.stride); + } + + for (int i = 0; i < header.edge_count; i++) { + if (header.type == PlyFormatType::ASCII) { + parse_row_ascii(file, value_vec); + } + else { + parse_row_binary(file, header, element, scratch, value_vec); + } + int index1 = value_vec[prop_vertex1]; + int index2 = value_vec[prop_vertex2]; + //@TODO: bounds check + data->edges.append(std::make_pair(index1, index2)); + } +} + +std::unique_ptr import_ply_data(fstream &file, const PlyHeader &header) +{ + std::unique_ptr data = std::make_unique(); + + for (const PlyElement &element : header.elements) { + if (element.name == "vertex") { + load_vertex_element(file, header, element, data.get()); + } + else if (element.name == "face") { + load_face_element(file, header, element, data.get()); + } + else if (element.name == "edge") { + load_edge_element(file, header, element, data.get()); + } + } + + return data; +} + +} // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_ascii.hh b/source/blender/io/ply/importer/ply_import_data.hh similarity index 69% rename from source/blender/io/ply/importer/ply_import_ascii.hh rename to source/blender/io/ply/importer/ply_import_data.hh index 963e92dd6fc..b58577f9375 100644 --- a/source/blender/io/ply/importer/ply_import_ascii.hh +++ b/source/blender/io/ply/importer/ply_import_data.hh @@ -13,11 +13,11 @@ namespace blender::io::ply { /** - * Loads the information from the PLY file in ASCII format to a #PlyData data-structure. + * Loads the information from a PLY file to a #PlyData data-structure. * \param file: The PLY file that was opened. * \param header: The information in the PLY header. * \return The #PlyData data-structure that can be used for conversion to a Mesh. */ -std::unique_ptr import_ply_ascii(fstream &file, const PlyHeader &header); +std::unique_ptr import_ply_data(fstream &file, const PlyHeader &header); } // 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 d8369d25240..36d1519e353 100644 --- a/source/blender/io/ply/intern/ply_data.hh +++ b/source/blender/io/ply/intern/ply_data.hh @@ -12,7 +12,7 @@ namespace blender::io::ply { -enum PlyDataTypes { CHAR, UCHAR, SHORT, USHORT, INT, UINT, FLOAT, DOUBLE, PLY_TYPE_COUNT }; +enum PlyDataTypes { NONE, CHAR, UCHAR, SHORT, USHORT, INT, UINT, FLOAT, DOUBLE, PLY_TYPE_COUNT }; struct PlyData { Vector vertices; @@ -27,10 +27,19 @@ struct PlyData { enum PlyFormatType { ASCII, BINARY_LE, BINARY_BE }; +struct PlyProperty { + std::string name; + PlyDataTypes type = PlyDataTypes::NONE; + PlyDataTypes count_type = PlyDataTypes::NONE; /* NONE means it's not a list property */ +}; + struct PlyElement { std::string name; int count = 0; - Vector> properties; + Vector properties; + int stride = 0; + + void calc_stride(); }; struct PlyHeader { diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index 6f6ba97eb11..eefcf631fb1 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -3,17 +3,19 @@ #include "testing/testing.h" #include "BLI_fileops.hh" +#include "BLI_hash_mm2a.h" + #include "ply_import.hh" -#include "ply_import_ascii.hh" -#include "ply_import_binary.hh" +#include "ply_import_data.hh" namespace blender::io::ply { struct Expectation { int totvert, totpoly, totindex, totedge; + uint16_t polyhash = 0, edgehash = 0; float3 vert_first, vert_last; float3 normal_first = {0, 0, 0}; - float2 uv_first; + float2 uv_first = {0, 0}; float4 color_first = {-1, -1, -1, -1}; }; @@ -32,12 +34,7 @@ class ply_import_test : public testing::Test { } std::unique_ptr data; try { - if (header.type == PlyFormatType::ASCII) { - data = import_ply_ascii(infile, header); - } - else { - data = import_ply_binary(infile, &header); - } + data = import_ply_data(infile, header); } catch (std::exception &e) { ASSERT_EQ(0, exp.totvert); @@ -53,11 +50,27 @@ class ply_import_test : public testing::Test { ASSERT_EQ(header.face_count, exp.totpoly); ASSERT_EQ(data->faces.size(), exp.totpoly); + /* Test hash of face and edge index data. */ + BLI_HashMurmur2A hash; + BLI_hash_mm2a_init(&hash, 0); int indexCount = 0; for (const auto &f : data->faces) { indexCount += f.size(); + BLI_hash_mm2a_add(&hash, (const unsigned char *)f.data(), f.size() * sizeof(f[0])); } ASSERT_EQ(indexCount, exp.totindex); + uint16_t face_hash = BLI_hash_mm2a_end(&hash); + + if (!data->faces.is_empty()) { + ASSERT_EQ(face_hash, exp.polyhash); + } + + if (!data->edges.is_empty()) { + uint16_t edge_hash = BLI_hash_mm2((const unsigned char *)data->edges.data(), + data->edges.size() * sizeof(data->edges[0]), + 0); + ASSERT_EQ(edge_hash, exp.edgehash); + } /* Test if first and last vertices match. */ EXPECT_V3_NEAR(data->vertices.first(), exp.vert_first, 0.0001f); @@ -85,6 +98,8 @@ TEST_F(ply_import_test, PLYImportCube) 6, 24, 0, + 26429, + 0, float3(1, 1, -1), float3(-1, 1, 1), float3(0, 0, -1), @@ -93,10 +108,11 @@ TEST_F(ply_import_test, PLYImportCube) import_and_check("cube_ascii.ply", expect); } -TEST_F(ply_import_test, PLYImportASCIIEdgeTest) +TEST_F(ply_import_test, PLYImportWireframeCube) { - Expectation expect = {8, 0, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; + Expectation expect = {8, 0, 0, 12, 0, 31435, float3(-1, -1, -1), float3(1, 1, 1)}; import_and_check("ASCII_wireframe_cube.ply", expect); + import_and_check("wireframe_cube.ply", expect); } TEST_F(ply_import_test, PLYImportBunny) @@ -105,6 +121,8 @@ TEST_F(ply_import_test, PLYImportBunny) 1000, 3000, 0, + 62556, + 0, float3(0.0380425, 0.109755, 0.0161689), float3(-0.0722821, 0.143895, -0.0129091)}; import_and_check("bunny2.ply", expect); @@ -116,6 +134,8 @@ TEST_F(ply_import_test, PlyImportManySmallHoles) 3524, 10572, 0, + 15143, + 0, float3(-0.0131592, -0.0598382, 1.58958), float3(-0.0177622, 0.0105153, 1.61977), float3(0, 0, 0), @@ -124,17 +144,11 @@ TEST_F(ply_import_test, PlyImportManySmallHoles) import_and_check("many_small_holes.ply", expect); } -TEST_F(ply_import_test, PlyImportWireframeCube) -{ - Expectation expect = {8, 0, 0, 12, float3(-1, -1, -1), float3(1, 1, 1)}; - import_and_check("wireframe_cube.ply", expect); -} - TEST_F(ply_import_test, PlyImportColorNotFull) { - Expectation expect = {4, 1, 4, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + Expectation expect = {4, 1, 4, 0, 37235, 0, float3(1, 0, 1), float3(-1, 0, 1)}; import_and_check("color_not_full_a.ply", expect); - // import_and_check("color_not_full_b.ply", expect); + import_and_check("color_not_full_b.ply", expect); } TEST_F(ply_import_test, PlyImportDoubleXYZ) @@ -143,20 +157,31 @@ TEST_F(ply_import_test, PlyImportDoubleXYZ) 1, 4, 0, + 37235, + 0, float3(1, 0, 1), float3(-1, 0, 1), float3(0, 0, 0), float2(0, 0), float4(1, 0, 0, 1)}; import_and_check("double_xyz_a.ply", expect); - // import_and_check("double_xyz_b.ply", expect); + import_and_check("double_xyz_b.ply", expect); } +/* +TEST_F(ply_import_test, PlyImportFaceIndicesNotFirstProp) +{ + Expectation expect = {4, 1, 4, 0, 37235, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + import_and_check("face_indices_not_first_prop_a.ply", expect); + import_and_check("face_indices_not_first_prop_b.ply", expect); +} + */ + TEST_F(ply_import_test, PlyImportFaceUVsColors) { - Expectation expect = {4, 1, 4, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + Expectation expect = {4, 1, 4, 0, 37235, 0, float3(1, 0, 1), float3(-1, 0, 1)}; import_and_check("face_uvs_colors_a.ply", expect); - // import_and_check("face_uvs_colors_b.ply", expect); + import_and_check("face_uvs_colors_b.ply", expect); } TEST_F(ply_import_test, PlyImportFacesFirst) @@ -165,13 +190,15 @@ TEST_F(ply_import_test, PlyImportFacesFirst) 1, 4, 0, + 37235, + 0, float3(1, 0, 1), float3(-1, 0, 1), float3(0, 0, 0), float2(0, 0), float4(1, 0, 0, 1)}; import_and_check("faces_first_a.ply", expect); - // import_and_check("faces_first_b.ply", expect); + import_and_check("faces_first_b.ply", expect); } TEST_F(ply_import_test, PlyImportFloatFormats) @@ -180,27 +207,29 @@ TEST_F(ply_import_test, PlyImportFloatFormats) 1, 4, 0, + 37235, + 0, float3(1, 0, 1), float3(-1, 0, 1), float3(0, 0, 0), float2(0, 0), float4(0.5f, 0, 0.25f, 1)}; import_and_check("float_formats_a.ply", expect); - // import_and_check("float_formats_b.ply", expect); + import_and_check("float_formats_b.ply", expect); } TEST_F(ply_import_test, PlyImportPositionNotFull) { Expectation expect = {0, 0, 0, 0}; import_and_check("position_not_full_a.ply", expect); - // import_and_check("position_not_full_b.ply", expect); + import_and_check("position_not_full_b.ply", expect); } TEST_F(ply_import_test, PlyImportTristrips) { - Expectation expect = {6, 0, 0, 0, float3(1, 0, 1), float3(-3, 0, 1)}; //@TODO: incorrect + Expectation expect = {6, 0, 0, 0, 0, 0, float3(1, 0, 1), float3(-3, 0, 1)}; //@TODO: incorrect import_and_check("tristrips_a.ply", expect); - // import_and_check("tristrips_b.ply", expect); + import_and_check("tristrips_b.ply", expect); } TEST_F(ply_import_test, PlyImportTypeAliases) @@ -209,13 +238,16 @@ TEST_F(ply_import_test, PlyImportTypeAliases) 1, 4, 0, + 37235, + 0, float3(1, 0, 1), float3(-1, 0, 1), float3(0, 0, 0), float2(0, 0), float4(220 / 255.0f, 20 / 255.0f, 20 / 255.0f, 1)}; import_and_check("type_aliases_a.ply", expect); - // import_and_check("type_aliases_b.ply", expect); + import_and_check("type_aliases_b.ply", expect); + import_and_check("type_aliases_be_b.ply", expect); } TEST_F(ply_import_test, PlyImportVertexCompOrder) @@ -224,37 +256,22 @@ TEST_F(ply_import_test, PlyImportVertexCompOrder) 1, 4, 0, + 37235, + 0, float3(1, 0, 1), float3(-1, 0, 1), float3(0, 0, 0), float2(0, 0), float4(0.8f, 0.2f, 0, 1)}; import_and_check("vertex_comp_order_a.ply", expect); - // import_and_check("vertex_comp_order_b.ply", expect); + import_and_check("vertex_comp_order_b.ply", expect); } -TEST(ply_import_functions_test, PlySwapBytes) -{ - /* Individual bits shouldn't swap with each other. */ - uint8_t val8 = 0xA8; - uint8_t exp8 = 0xA8; - uint8_t actual8 = swap_bytes(val8); - ASSERT_EQ(exp8, actual8); - - uint16_t val16 = 0xFEB0; - uint16_t exp16 = 0xB0FE; - uint16_t actual16 = swap_bytes(val16); - ASSERT_EQ(exp16, actual16); - - uint32_t val32 = 0x80A37B0A; - uint32_t exp32 = 0x0A7BA380; - uint32_t actual32 = swap_bytes(val32); - ASSERT_EQ(exp32, actual32); - - uint64_t val64 = 0x0102030405060708; - uint64_t exp64 = 0x0807060504030201; - uint64_t actual64 = swap_bytes(val64); - ASSERT_EQ(exp64, actual64); -} +//@TODO: test with vertex element having list properties +//@TODO: test with face element having vertex_indices list as non-first property +//@TODO: test big endian binaries +//@TODO: test with edges starting with non-vertex index properties +//@TODO: test various malformed headers +//@TODO: line endings } // namespace blender::io::ply -- 2.30.2 From a007a81f2fb7fdbddbfd6575b061f84e0b694378 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 16 Mar 2023 09:16:55 +0200 Subject: [PATCH 04/10] PLY: handle case where face element is not only a single vertex indices property Both binary and ascii cases, added test coverage --- .../io/ply/importer/ply_import_data.cc | 191 +++++++++++++----- .../io/ply/tests/io_ply_importer_test.cc | 14 +- 2 files changed, 150 insertions(+), 55 deletions(-) diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index 745800b0609..a1ab9315698 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -28,6 +28,14 @@ static const char *drop_whitespace(const char *p, const char *end) return p; } +const char *drop_non_whitespace(const char *p, const char *end) +{ + while (p < end && !is_whitespace(*p)) { + ++p; + } + return p; +} + static const char *drop_plus(const char *p, const char *end) { if (p < end && *p == '+') { @@ -36,10 +44,7 @@ static const char *drop_plus(const char *p, const char *end) return p; } -static const char *parse_float(const char *p, - const char *end, - float fallback, - float &dst) +static const char *parse_float(const char *p, const char *end, float fallback, float &dst) { p = drop_whitespace(p, end); p = drop_plus(p, end); @@ -50,8 +55,7 @@ static const char *parse_float(const char *p, return res.ptr; } -static const char *parse_int( - const char *p, const char *end, int fallback, int &dst) +static const char *parse_int(const char *p, const char *end, int fallback, int &dst) { p = drop_whitespace(p, end); p = drop_plus(p, end); @@ -65,26 +69,26 @@ static const char *parse_int( static void endian_switch(uint8_t *ptr, int type_size) { if (type_size == 2) { - BLI_endian_switch_uint16((uint16_t*)ptr); + BLI_endian_switch_uint16((uint16_t *)ptr); } else if (type_size == 4) { - BLI_endian_switch_uint32((uint32_t*)ptr); + BLI_endian_switch_uint32((uint32_t *)ptr); } else if (type_size == 8) { - BLI_endian_switch_uint64((uint64_t*)ptr); + BLI_endian_switch_uint64((uint64_t *)ptr); } } static void endian_switch_array(uint8_t *ptr, int type_size, int size) { if (type_size == 2) { - BLI_endian_switch_uint16_array((uint16_t*)ptr, size); + BLI_endian_switch_uint16_array((uint16_t *)ptr, size); } else if (type_size == 4) { - BLI_endian_switch_uint32_array((uint32_t*)ptr, size); + BLI_endian_switch_uint32_array((uint32_t *)ptr, size); } else if (type_size == 8) { - BLI_endian_switch_uint64_array((uint64_t*)ptr, size); + BLI_endian_switch_uint64_array((uint64_t *)ptr, size); } } @@ -105,7 +109,6 @@ void PlyElement::calc_stride() } } - static const float data_type_normalizer[] = { 1.0f, 127.0f, 255.0f, 32767.0f, 65535.0f, INT_MAX, UINT_MAX, 1.0f, 1.0f}; static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, @@ -171,21 +174,46 @@ static void check_file_errors(const fstream &file) } } -template -static T get_binary_value(PlyDataTypes type, const uint8_t*& r_ptr) +template static T get_binary_value(PlyDataTypes type, const uint8_t *&r_ptr) { T val = 0; switch (type) { - case NONE: break; - case CHAR: val = *(int8_t*)r_ptr; r_ptr += 1; break; - case UCHAR: val = *(uint8_t*)r_ptr; r_ptr += 1; break; - case SHORT: val = *(int16_t*)r_ptr; r_ptr += 2; break; - case USHORT: val = *(uint16_t*)r_ptr; r_ptr += 2; break; - case INT: val = *(int32_t*)r_ptr; r_ptr += 4; break; - case UINT: val = *(int32_t*)r_ptr; r_ptr += 4; break; - case FLOAT: val = *(float*)r_ptr; r_ptr += 4; break; - case DOUBLE: val = *(double*)r_ptr; r_ptr += 8; break; - default: throw std::runtime_error("Unknown property type"); + case NONE: + break; + case CHAR: + val = *(int8_t *)r_ptr; + r_ptr += 1; + break; + case UCHAR: + val = *(uint8_t *)r_ptr; + r_ptr += 1; + break; + case SHORT: + val = *(int16_t *)r_ptr; + r_ptr += 2; + break; + case USHORT: + val = *(uint16_t *)r_ptr; + r_ptr += 2; + break; + case INT: + val = *(int32_t *)r_ptr; + r_ptr += 4; + break; + case UINT: + val = *(int32_t *)r_ptr; + r_ptr += 4; + break; + case FLOAT: + val = *(float *)r_ptr; + r_ptr += 4; + break; + case DOUBLE: + val = *(double *)r_ptr; + r_ptr += 8; + break; + default: + throw std::runtime_error("Unknown property type"); } return val; } @@ -197,18 +225,19 @@ static void parse_row_binary(fstream &file, Vector &r_values) { if (element.stride == 0) { - throw std::runtime_error("Vertex/Edge element contains list properties, this is not supported"); + throw std::runtime_error( + "Vertex/Edge element contains list properties, this is not supported"); } BLI_assert(r_scratch.size() == element.stride); BLI_assert(r_values.size() == element.properties.size()); - file.read((char*)r_scratch.data(), r_scratch.size()); + file.read((char *)r_scratch.data(), r_scratch.size()); check_file_errors(file); - - const uint8_t* ptr = r_scratch.data(); + + const uint8_t *ptr = r_scratch.data(); if (header.type == PlyFormatType::BINARY_LE) { /* Little endian: just read/convert the values. */ for (int i = 0, n = int(element.properties.size()); i != n; i++) { - const PlyProperty& prop = element.properties[i]; + const PlyProperty &prop = element.properties[i]; float val = get_binary_value(prop.type, ptr); r_values[i] = val; } @@ -216,8 +245,8 @@ static void parse_row_binary(fstream &file, else if (header.type == PlyFormatType::BINARY_BE) { /* Big endian: read, switch endian, convert the values. */ for (int i = 0, n = int(element.properties.size()); i != n; i++) { - const PlyProperty& prop = element.properties[i]; - endian_switch((uint8_t*)ptr, data_type_size[prop.type]); + const PlyProperty &prop = element.properties[i]; + endian_switch((uint8_t *)ptr, data_type_size[prop.type]); float val = get_binary_value(prop.type, ptr); r_values[i] = val; } @@ -263,10 +292,10 @@ static void load_vertex_element(fstream &file, Vector scratch; if (header.type != PlyFormatType::ASCII) { scratch.resize(element.stride); - } + } for (int i = 0; i < header.vertex_count; i++) { - + if (header.type == PlyFormatType::ASCII) { parse_row_ascii(file, value_vec); } @@ -315,12 +344,45 @@ static void load_vertex_element(fstream &file, } } +static uint32_t read_list_count(fstream &file, + const PlyProperty &prop, + Vector &scratch, + bool big_endian) +{ + scratch.resize(8); + file.read((char *)scratch.data(), data_type_size[prop.count_type]); + const uint8_t *ptr = scratch.data(); + if (big_endian) + endian_switch((uint8_t *)ptr, data_type_size[prop.count_type]); + uint32_t count = get_binary_value(prop.count_type, ptr); + return count; +} + +static void skip_property(fstream &file, + const PlyProperty &prop, + Vector &scratch, + bool big_endian) +{ + if (prop.count_type == PlyDataTypes::NONE) { + scratch.resize(8); + file.read((char *)scratch.data(), data_type_size[prop.type]); + } + else { + uint32_t count = read_list_count(file, prop, scratch, big_endian); + scratch.resize(count * data_type_size[prop.type]); + file.read((char *)scratch.data(), scratch.size()); + } +} + static void load_face_element(fstream &file, const PlyHeader &header, const PlyElement &element, PlyData *data) { int prop_index = get_index(element, "vertex_indices"); + if (prop_index < 0) { + prop_index = get_index(element, "vertex_index"); + } if (prop_index < 0 && element.properties.size() == 1) { prop_index = 0; } @@ -331,15 +393,33 @@ static void load_face_element(fstream &file, if (prop.count_type == PlyDataTypes::NONE) { throw std::runtime_error("Face element vertex indices property must be a list"); } - + if (header.type == PlyFormatType::ASCII) { for (int i = 0; i < header.face_count; i++) { + /* Read line */ std::string line; getline(file, line); const char *p = line.data(); const char *end = p + line.size(); int count = 0; + + /* Skip any properties before vertex indices. */ + for (int j = 0; j < prop_index; j++) { + p = drop_whitespace(p, end); + if (element.properties[j].count_type == PlyDataTypes::NONE) { + p = drop_non_whitespace(p, end); + } + else { + p = parse_int(p, end, 0, count); + for (int k = 0; k < count; ++k) { + p = drop_whitespace(p, end); + p = drop_non_whitespace(p, end); + } + } + } + + /* Parse vertex indices list. */ p = parse_int(p, end, 0, count); Array vertex_indices(count); @@ -357,26 +437,28 @@ static void load_face_element(fstream &file, } else { Vector scratch(64); - + for (int i = 0; i < header.face_count; i++) { - const uint8_t* ptr; - /* Read list size. */ - scratch.resize(8); - file.read((char*)scratch.data(), data_type_size[prop.count_type]); - ptr = scratch.data(); - if (header.type == PlyFormatType::BINARY_BE) - endian_switch((uint8_t*)ptr, data_type_size[prop.count_type]); - uint32_t count = get_binary_value(prop.count_type, ptr); - + const uint8_t *ptr; + + /* Skip any properties before vertex indices. */ + for (int j = 0; j < prop_index; j++) { + skip_property( + file, element.properties[j], scratch, header.type == PlyFormatType::BINARY_BE); + } + + /* Read vertex indices list. */ + uint32_t count = read_list_count( + file, prop, scratch, header.type == PlyFormatType::BINARY_BE); + //@TODO: check invalid face size values? - - /* Read list values. */ + Array face(count); scratch.resize(count * data_type_size[prop.type]); - file.read((char*)scratch.data(), scratch.size()); + file.read((char *)scratch.data(), scratch.size()); ptr = scratch.data(); if (header.type == PlyFormatType::BINARY_BE) - endian_switch_array((uint8_t*)ptr, data_type_size[prop.type], count); + endian_switch_array((uint8_t *)ptr, data_type_size[prop.type], count); for (int j = 0; j < count; ++j) { uint32_t index = get_binary_value(prop.type, ptr); if (index >= header.vertex_count) { @@ -385,11 +467,16 @@ static void load_face_element(fstream &file, face[j] = index; } data->faces.append(face); + + /* Skip any properties after vertex indices. */ + for (int j = prop_index + 1; j < element.properties.size(); j++) { + skip_property( + file, element.properties[j], scratch, header.type == PlyFormatType::BINARY_BE); + } } } } - static void load_edge_element(fstream &file, const PlyHeader &header, const PlyElement &element, @@ -400,13 +487,13 @@ static void load_edge_element(fstream &file, if (prop_vertex1 < 0 || prop_vertex2 < 0) { throw std::runtime_error("Edge element does not contain vertex1 and vertex2 properties"); } - + Vector value_vec(element.properties.size()); Vector scratch; if (header.type != PlyFormatType::ASCII) { scratch.resize(element.stride); } - + for (int i = 0; i < header.edge_count; i++) { if (header.type == PlyFormatType::ASCII) { parse_row_ascii(file, value_vec); diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index eefcf631fb1..7d03853e6b5 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -168,14 +168,19 @@ TEST_F(ply_import_test, PlyImportDoubleXYZ) import_and_check("double_xyz_b.ply", expect); } -/* TEST_F(ply_import_test, PlyImportFaceIndicesNotFirstProp) { - Expectation expect = {4, 1, 4, 0, 37235, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + Expectation expect = {4, 2, 6, 0, 4136, 0, float3(1, 0, 1), float3(-1, 0, 1)}; import_and_check("face_indices_not_first_prop_a.ply", expect); import_and_check("face_indices_not_first_prop_b.ply", expect); } - */ + +TEST_F(ply_import_test, PlyImportFaceIndicesPrecededByList) +{ + Expectation expect = {4, 2, 6, 0, 4136, 0, float3(1, 0, 1), float3(-1, 0, 1)}; + import_and_check("face_indices_preceded_by_list_a.ply", expect); + import_and_check("face_indices_preceded_by_list_b.ply", expect); +} TEST_F(ply_import_test, PlyImportFaceUVsColors) { @@ -273,5 +278,8 @@ TEST_F(ply_import_test, PlyImportVertexCompOrder) //@TODO: test with edges starting with non-vertex index properties //@TODO: test various malformed headers //@TODO: line endings +//@TODO: UVs with: s,t; u,v; texture_u,texture_v; texture_s,texture_t (from miniply) +//@TODO: colors with: r,g,b in addition to red,green,blue (from miniply) +//@TODO: vertex_index in addition to vertex_indices (from miniply) } // namespace blender::io::ply -- 2.30.2 From e504dab2afbbede269a6d783bea51067ccaedafb Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 16 Mar 2023 09:54:34 +0200 Subject: [PATCH 05/10] PLY: support "tristrips" element --- source/blender/io/ply/importer/ply_import.cc | 4 + .../io/ply/importer/ply_import_data.cc | 93 ++++++++++++++++++- .../io/ply/importer/ply_import_data.hh | 2 +- .../io/ply/tests/io_ply_importer_test.cc | 2 +- 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index d44aaecdca3..eb44c1f6da8 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -104,6 +104,10 @@ const char *read_header(fstream &file, PlyHeader &r_header) else if (strcmp(words[1].c_str(), "face") == 0) { r_header.face_count = std::stoi(words[2]); } + else if (strcmp(words[1].c_str(), "tristrips") == 0) { + /* Will get changed later after strip decoding. */ + r_header.face_count = std::stoi(words[2]); + } else if (strcmp(words[1].c_str(), "edge") == 0) { r_header.edge_count = std::stoi(words[2]); } diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index a1ab9315698..85f8248eac9 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -477,6 +477,91 @@ static void load_face_element(fstream &file, } } +static void load_tristrips_element(fstream &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) +{ + if (header.face_count != 1) { + throw std::runtime_error("Tristrips element should contain one row"); + } + if (element.properties.size() != 1) { + throw std::runtime_error("Tristrips element should contain one property"); + } + const PlyProperty &prop = element.properties[0]; + if (prop.count_type == PlyDataTypes::NONE) { + throw std::runtime_error("Tristrips element property must be a list"); + } + + Vector strip; + + if (header.type == PlyFormatType::ASCII) { + std::string line; + getline(file, line); + + const char *p = line.data(); + const char *end = p + line.size(); + int count = 0; + p = parse_int(p, end, 0, count); + + strip.resize(count); + for (int j = 0; j < count; j++) { + int index; + p = parse_int(p, end, 0, index); + if (index >= header.vertex_count) { + throw std::runtime_error("Tristrip vertex index out of bounds"); + } + strip[j] = index; + } + } + else { + Vector scratch(64); + + const uint8_t *ptr; + + uint32_t count = read_list_count(file, prop, scratch, header.type == PlyFormatType::BINARY_BE); + + strip.resize(count); + scratch.resize(count * data_type_size[prop.type]); + file.read((char *)scratch.data(), scratch.size()); + ptr = scratch.data(); + if (header.type == PlyFormatType::BINARY_BE) + endian_switch_array((uint8_t *)ptr, data_type_size[prop.type], count); + for (int j = 0; j < count; ++j) { + int index = get_binary_value(prop.type, ptr); + if (index >= header.vertex_count) { + throw std::runtime_error("Tristrip vertex index out of bounds"); + } + strip[j] = index; + } + } + + /* Decode triangle strip (with possible -1 restart indices) into faces. */ + Array triangle(3); + size_t start = 0; + + for (size_t i = 0; i < strip.size(); i++) { + if (strip[i] == -1) { + /* Restart strip. */ + start = i + 1; + } + else if (i - start >= 2) { + int a = strip[i - 2], b = strip[i - 1], c = strip[i]; + /* Flip odd triangles. */ + if ((i - start) & 1) { + SWAP(int, a, b); + } + /* Add triangle if it's not degenerate. */ + if (a != b && a != c && b != c) { + triangle[0] = a; + triangle[1] = b; + triangle[2] = c; + data->faces.append(triangle); + } + } + } +} + static void load_edge_element(fstream &file, const PlyHeader &header, const PlyElement &element, @@ -508,7 +593,7 @@ static void load_edge_element(fstream &file, } } -std::unique_ptr import_ply_data(fstream &file, const PlyHeader &header) +std::unique_ptr import_ply_data(fstream &file, PlyHeader &header) { std::unique_ptr data = std::make_unique(); @@ -519,6 +604,12 @@ std::unique_ptr import_ply_data(fstream &file, const PlyHeader &header) else if (element.name == "face") { load_face_element(file, header, element, data.get()); } + else if (element.name == "tristrips") { + int64_t prev_face_count = data->faces.size(); + load_tristrips_element(file, header, element, data.get()); + header.face_count -= 1; + header.face_count += data->faces.size() - prev_face_count; + } else if (element.name == "edge") { load_edge_element(file, header, element, data.get()); } diff --git a/source/blender/io/ply/importer/ply_import_data.hh b/source/blender/io/ply/importer/ply_import_data.hh index b58577f9375..cd023d48035 100644 --- a/source/blender/io/ply/importer/ply_import_data.hh +++ b/source/blender/io/ply/importer/ply_import_data.hh @@ -18,6 +18,6 @@ namespace blender::io::ply { * \param header: The information in the PLY header. * \return The #PlyData data-structure that can be used for conversion to a Mesh. */ -std::unique_ptr import_ply_data(fstream &file, const PlyHeader &header); +std::unique_ptr import_ply_data(fstream &file, PlyHeader &header); } // namespace blender::io::ply diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index 7d03853e6b5..1233817278f 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -232,7 +232,7 @@ TEST_F(ply_import_test, PlyImportPositionNotFull) TEST_F(ply_import_test, PlyImportTristrips) { - Expectation expect = {6, 0, 0, 0, 0, 0, float3(1, 0, 1), float3(-3, 0, 1)}; //@TODO: incorrect + Expectation expect = {6, 4, 12, 0, 3404, 0, float3(1, 0, 1), float3(-3, 0, 1)}; import_and_check("tristrips_a.ply", expect); import_and_check("tristrips_b.ply", expect); } -- 2.30.2 From 1a8d3adfeb5dc9c1e1f459b9c5dcdee9bca29318 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 16 Mar 2023 19:16:40 +0200 Subject: [PATCH 06/10] PLY: optimize importing by avoiding tiny file reads Conceptually similar to OBJ importer, this one reads input file in chunks (64kb), and then parses lines (header or ascii ply files) or reads binary data from that, refilling the buffer as necessary. Pure parsing performance is about 2x faster (tested on a Mac). --- source/blender/io/ply/CMakeLists.txt | 4 +- source/blender/io/ply/importer/ply_import.cc | 127 +++++++++++------- source/blender/io/ply/importer/ply_import.hh | 8 +- .../io/ply/importer/ply_import_buffer.cc | 114 ++++++++++++++++ .../io/ply/importer/ply_import_buffer.hh | 41 ++++++ .../io/ply/importer/ply_import_data.cc | 59 +++----- .../io/ply/importer/ply_import_data.hh | 6 +- source/blender/io/ply/intern/ply_functions.cc | 51 ------- source/blender/io/ply/intern/ply_functions.hh | 26 ---- .../io/ply/tests/io_ply_importer_test.cc | 4 +- 10 files changed, 263 insertions(+), 177 deletions(-) create mode 100644 source/blender/io/ply/importer/ply_import_buffer.cc create mode 100644 source/blender/io/ply/importer/ply_import_buffer.hh delete mode 100644 source/blender/io/ply/intern/ply_functions.cc delete mode 100644 source/blender/io/ply/intern/ply_functions.hh diff --git a/source/blender/io/ply/CMakeLists.txt b/source/blender/io/ply/CMakeLists.txt index 266455cea70..a3235dd7170 100644 --- a/source/blender/io/ply/CMakeLists.txt +++ b/source/blender/io/ply/CMakeLists.txt @@ -32,6 +32,7 @@ set(SRC exporter/ply_file_buffer_ascii.cc exporter/ply_file_buffer_binary.cc importer/ply_import.cc + importer/ply_import_buffer.cc importer/ply_import_data.cc importer/ply_import_mesh.cc IO_ply.cc @@ -44,13 +45,12 @@ set(SRC exporter/ply_file_buffer_ascii.hh exporter/ply_file_buffer_binary.hh importer/ply_import.hh + importer/ply_import_buffer.hh importer/ply_import_data.hh importer/ply_import_mesh.hh IO_ply.h intern/ply_data.hh - intern/ply_functions.cc - intern/ply_functions.hh ) set(LIB diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index eb44c1f6da8..33b86f05720 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -14,7 +14,6 @@ #include "DNA_object_types.h" #include "DNA_scene_types.h" -#include "BLI_fileops.hh" #include "BLI_math_vector.h" #include "BLI_memory_utils.hh" @@ -22,27 +21,47 @@ #include "DEG_depsgraph_build.h" #include "ply_data.hh" -#include "ply_functions.hh" #include "ply_import.hh" +#include "ply_import_buffer.hh" #include "ply_import_data.hh" #include "ply_import_mesh.hh" namespace blender::io::ply { -void splitstr(std::string str, Vector &words, const StringRef &deli) +/* If line starts with keyword, returns true and drops it from the line. */ +static bool parse_keyword(Span &str, StringRef keyword) { - int pos; - - while ((pos = int(str.find(deli))) != std::string::npos) { - words.append(str.substr(0, pos)); - str.erase(0, pos + deli.size()); + const size_t keyword_len = keyword.size(); + if (str.size() < keyword_len) { + return false; } - /* We add the final word to the vector. */ - words.append(str.substr()); + if (memcmp(str.data(), keyword.data(), keyword_len) != 0) { + return false; + } + str = str.drop_front(keyword_len); + return true; } -static PlyDataTypes type_from_string(const StringRef &input) +static Span parse_word(Span &str) { + size_t len = 0; + while (len < str.size() && str[len] > ' ') { + ++len; + } + Span word(str.begin(), len); + str = str.drop_front(len); + return word; +} + +static void skip_space(Span &str) +{ + while (!str.is_empty() && str[0] <= ' ') + str = str.drop_front(1); +} + +static PlyDataTypes type_from_string(Span word) +{ + StringRef input(word.data(), word.size()); if (input == "uchar" || input == "uint8") { return PlyDataTypes::UCHAR; } @@ -70,74 +89,79 @@ static PlyDataTypes type_from_string(const StringRef &input) return PlyDataTypes::NONE; } -const char *read_header(fstream &file, PlyHeader &r_header) +const char *read_header(PlyReadBuffer &file, PlyHeader &r_header) { - std::string line; + Span word; + while (true) { /* We break when end_header is encountered. */ - safe_getline(file, line); - if (r_header.header_size == 0 && line != "ply") { + Span line = file.read_line(); + if (r_header.header_size == 0 && StringRef(line.data(), line.size()) != "ply") { return "Invalid PLY header."; } r_header.header_size++; - Vector words{}; - splitstr(line, words, " "); - if (strcmp(words[0].c_str(), "format") == 0) { - if (strcmp(words[1].c_str(), "ascii") == 0) { + if (parse_keyword(line, "format")) { + skip_space(line); + if (parse_keyword(line, "ascii")) { r_header.type = PlyFormatType::ASCII; } - else if (strcmp(words[1].c_str(), "binary_big_endian") == 0) { + else if (parse_keyword(line, "binary_big_endian")) { r_header.type = PlyFormatType::BINARY_BE; } - else if (strcmp(words[1].c_str(), "binary_little_endian") == 0) { + else if (parse_keyword(line, "binary_little_endian")) { r_header.type = PlyFormatType::BINARY_LE; } } - else if (strcmp(words[0].c_str(), "element") == 0) { + else if (parse_keyword(line, "element")) { PlyElement element; - element.name = words[1]; - element.count = std::stoi(words[2]); + + skip_space(line); + word = parse_word(line); + element.name = std::string(word.data(), word.size()); + skip_space(line); + word = parse_word(line); + element.count = std::stoi(std::string(word.data(), word.size())); r_header.elements.append(element); - if (strcmp(words[1].c_str(), "vertex") == 0) { - r_header.vertex_count = std::stoi(words[2]); + + if (element.name == "vertex") { + r_header.vertex_count = element.count; } - else if (strcmp(words[1].c_str(), "face") == 0) { - r_header.face_count = std::stoi(words[2]); + else if (element.name == "face") { + r_header.face_count = element.count; } - else if (strcmp(words[1].c_str(), "tristrips") == 0) { + else if (element.name == "tristrips") { /* Will get changed later after strip decoding. */ - r_header.face_count = std::stoi(words[2]); + r_header.face_count = element.count; } - else if (strcmp(words[1].c_str(), "edge") == 0) { - r_header.edge_count = std::stoi(words[2]); + else if (element.name == "edge") { + r_header.edge_count = element.count; } } - else if (strcmp(words[0].c_str(), "property") == 0) { + else if (parse_keyword(line, "property")) { PlyProperty property; - if (words.size() >= 3 && words[1] != "list") { - property.type = type_from_string(words[1]); - property.name = words[2]; - } - else if (words.size() >= 5 && words[1] == "list") { - property.count_type = type_from_string(words[2]); - property.type = type_from_string(words[3]); - property.name = words[4]; - } - else { - return "Malformed property"; + skip_space(line); + if (parse_keyword(line, "list")) { + skip_space(line); + property.count_type = type_from_string(parse_word(line)); } + skip_space(line); + property.type = type_from_string(parse_word(line)); + skip_space(line); + word = parse_word(line); + property.name = std::string(word.data(), word.size()); r_header.elements.last().properties.append(property); } - else if (words[0] == "end_header") { + else if (parse_keyword(line, "end_header")) { break; } - else if ((words[0][0] >= '0' && words[0][0] <= '9') || words[0][0] == '-' || line.empty() || - file.eof()) { + else if (line.is_empty() || (line.first() >= '0' && line.first() <= '9') || + line.first() == '-') { /* A value was found before we broke out of the loop. No end_header. */ return "No end_header."; } } + file.after_header(r_header.type != PlyFormatType::ASCII); for (PlyElement &el : r_header.elements) { el.calc_stride(); } @@ -164,9 +188,10 @@ void importer_main(Main *bmain, BLI_path_extension_replace(ob_name, FILE_MAX, ""); /* Parse header. */ - fstream infile(import_params.filepath, std::ios::in | std::ios::binary); + PlyReadBuffer file(import_params.filepath, 64 * 1024); + PlyHeader header; - const char *err = read_header(infile, header); + const char *err = read_header(file, header); if (err != nullptr) { fprintf(stderr, "PLY Importer: %s: %s\n", ob_name, err); BKE_reportf(op->reports, RPT_ERROR, "PLY Importer: %s: %s", ob_name, err); @@ -187,7 +212,7 @@ void importer_main(Main *bmain, /* Parse actual file data. */ try { - std::unique_ptr data = import_ply_data(infile, header); + std::unique_ptr data = import_ply_data(file, header); Mesh *temp_val = convert_ply_to_mesh(*data, mesh, import_params); if (import_params.merge_verts && temp_val != mesh) { @@ -223,7 +248,5 @@ void importer_main(Main *bmain, DEG_id_tag_update_ex(bmain, &obj->id, flags); DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS); DEG_relations_tag_update(bmain); - - infile.close(); } } // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import.hh b/source/blender/io/ply/importer/ply_import.hh index 0141ebd98a4..c10482377e9 100644 --- a/source/blender/io/ply/importer/ply_import.hh +++ b/source/blender/io/ply/importer/ply_import.hh @@ -9,12 +9,10 @@ #include "IO_ply.h" #include "ply_data.hh" -namespace blender { -class fstream; -} - namespace blender::io::ply { +class PlyReadBuffer; + void splitstr(std::string str, Vector &words, const StringRef &deli); /* Main import function used from within Blender. */ @@ -27,6 +25,6 @@ void importer_main(Main *bmain, const PLYImportParams &import_params, wmOperator *op); -const char *read_header(fstream &file, PlyHeader &r_header); +const char *read_header(PlyReadBuffer &file, PlyHeader &r_header); } // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_buffer.cc b/source/blender/io/ply/importer/ply_import_buffer.cc new file mode 100644 index 00000000000..78924a1d1e2 --- /dev/null +++ b/source/blender/io/ply/importer/ply_import_buffer.cc @@ -0,0 +1,114 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include "ply_import_buffer.hh" + +#include "BLI_fileops.h" + +static inline bool is_newline(char ch) +{ + return ch == '\n' || ch == '\r'; +} + +namespace blender::io::ply { + +PlyReadBuffer::PlyReadBuffer(const char *file_path, size_t read_buffer_size) + : buffer_(read_buffer_size), read_buffer_size_(read_buffer_size) +{ + file_ = BLI_fopen(file_path, "rb"); +} + +PlyReadBuffer::~PlyReadBuffer() +{ + if (file_ != nullptr) { + fclose(file_); + } +} + +void PlyReadBuffer::after_header(bool is_binary) +{ + is_binary_ = is_binary; +} + +Span PlyReadBuffer::read_line() +{ + if (is_binary_) { + throw std::runtime_error("PLY read_line should not be used in binary mode"); + } + if (pos_ >= last_newline_) { + refill_buffer(); + } + BLI_assert(last_newline_ <= buffer_.size()); + int res_begin = pos_; + while (pos_ < last_newline_ && !is_newline(buffer_[pos_])) { + pos_++; + } + int res_end = pos_; + /* Move past newlines (possibly multiple for different line endings). */ + while (pos_ < buf_used_ && is_newline(buffer_[pos_])) { + pos_++; + } + return Span(buffer_.data() + res_begin, res_end - res_begin); +} + +bool PlyReadBuffer::read_bytes(void *dst, size_t size) +{ + while (size > 0) { + if (pos_ + size > buf_used_) { + if (!refill_buffer()) { + return false; + } + } + int to_copy = int(size); + if (to_copy > buf_used_) + to_copy = buf_used_; + memcpy(dst, buffer_.data() + pos_, to_copy); + pos_ += to_copy; + dst = (char *)dst + to_copy; + size -= to_copy; + } + return true; +} + +bool PlyReadBuffer::refill_buffer() +{ + BLI_assert(pos_ <= buf_used_); + BLI_assert(pos_ <= buffer_.size()); + BLI_assert(buf_used_ <= buffer_.size()); + + if (file_ == nullptr || at_eof_) { + return false; /* File is fully read. */ + } + + /* Move any leftover to start of buffer. */ + int keep = buf_used_ - pos_; + if (keep > 0) { + memmove(buffer_.data(), buffer_.data() + pos_, keep); + } + /* Read in data from the file. */ + size_t read = fread(buffer_.data() + keep, 1, read_buffer_size_ - keep, file_) + keep; + at_eof_ = read < read_buffer_size_; + pos_ = 0; + buf_used_ = int(read); + + /* Find last newline. */ + if (!is_binary_) { + int last_nl = buf_used_; + if (!at_eof_) { + while (last_nl > 0) { + --last_nl; + if (is_newline(buffer_[last_nl])) { + break; + } + } + if (!is_newline(buffer_[last_nl])) { + /* Whole line did not fit into our read buffer. */ + throw std::runtime_error("PLY text line did not fit into the read buffer"); + } + } + last_newline_ = last_nl; + } + + return true; +} + +} // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_buffer.hh b/source/blender/io/ply/importer/ply_import_buffer.hh new file mode 100644 index 00000000000..06f50712ace --- /dev/null +++ b/source/blender/io/ply/importer/ply_import_buffer.hh @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup ply + */ + +#pragma once + +#include +#include + +#include "BLI_array.hh" +#include "BLI_span.hh" + +namespace blender::io::ply { + +class PlyReadBuffer { + public: + PlyReadBuffer(const char *file_path, size_t read_buffer_size = 64 * 1024); + ~PlyReadBuffer(); + + void after_header(bool is_binary); + + Span read_line(); + bool read_bytes(void *dst, size_t size); + + private: + bool refill_buffer(); + + private: + FILE *file_ = nullptr; + Array buffer_; + int pos_ = 0; + int buf_used_ = 0; + int last_newline_ = 0; + size_t read_buffer_size_ = 0; + bool at_eof_ = false; + bool is_binary_ = false; +}; + +} // namespace blender::io::ply diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index 85f8248eac9..78b8eacd6da 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -5,7 +5,8 @@ */ #include "ply_import_data.hh" -#include "ply_functions.hh" +#include "ply_data.hh" +#include "ply_import_buffer.hh" #include "BLI_endian_switch.h" @@ -13,7 +14,6 @@ #include #include -#include static bool is_whitespace(char c) { @@ -145,10 +145,9 @@ static int2 get_uv_index(const PlyElement &element) return {get_index(element, "s"), get_index(element, "t")}; } -static void parse_row_ascii(fstream &file, Vector &r_values) +static void parse_row_ascii(PlyReadBuffer &file, Vector &r_values) { - std::string line; - safe_getline(file, line); + Span line = file.read_line(); /* Parse whole line as floats. */ const char *p = line.data(); @@ -161,19 +160,6 @@ static void parse_row_ascii(fstream &file, Vector &r_values) } } -static void check_file_errors(const fstream &file) -{ - if (file.bad()) { - throw std::ios_base::failure("Read/Write error on io operation"); - } - if (file.fail()) { - throw std::ios_base::failure("Logical error on io operation"); - } - if (file.eof()) { - throw std::ios_base::failure("Reached end of the file"); - } -} - template static T get_binary_value(PlyDataTypes type, const uint8_t *&r_ptr) { T val = 0; @@ -218,7 +204,7 @@ template static T get_binary_value(PlyDataTypes type, const uint8_t return val; } -static void parse_row_binary(fstream &file, +static void parse_row_binary(PlyReadBuffer &file, const PlyHeader &header, const PlyElement &element, Vector &r_scratch, @@ -230,8 +216,9 @@ static void parse_row_binary(fstream &file, } BLI_assert(r_scratch.size() == element.stride); BLI_assert(r_values.size() == element.properties.size()); - file.read((char *)r_scratch.data(), r_scratch.size()); - check_file_errors(file); + if (!file.read_bytes(r_scratch.data(), r_scratch.size())) { + throw std::runtime_error("Could not read row of binary property"); + } const uint8_t *ptr = r_scratch.data(); if (header.type == PlyFormatType::BINARY_LE) { @@ -256,7 +243,7 @@ static void parse_row_binary(fstream &file, } } -static void load_vertex_element(fstream &file, +static void load_vertex_element(PlyReadBuffer &file, const PlyHeader &header, const PlyElement &element, PlyData *data) @@ -344,13 +331,13 @@ static void load_vertex_element(fstream &file, } } -static uint32_t read_list_count(fstream &file, +static uint32_t read_list_count(PlyReadBuffer &file, const PlyProperty &prop, Vector &scratch, bool big_endian) { scratch.resize(8); - file.read((char *)scratch.data(), data_type_size[prop.count_type]); + file.read_bytes(scratch.data(), data_type_size[prop.count_type]); const uint8_t *ptr = scratch.data(); if (big_endian) endian_switch((uint8_t *)ptr, data_type_size[prop.count_type]); @@ -358,23 +345,23 @@ static uint32_t read_list_count(fstream &file, return count; } -static void skip_property(fstream &file, +static void skip_property(PlyReadBuffer &file, const PlyProperty &prop, Vector &scratch, bool big_endian) { if (prop.count_type == PlyDataTypes::NONE) { scratch.resize(8); - file.read((char *)scratch.data(), data_type_size[prop.type]); + file.read_bytes(scratch.data(), data_type_size[prop.type]); } else { uint32_t count = read_list_count(file, prop, scratch, big_endian); scratch.resize(count * data_type_size[prop.type]); - file.read((char *)scratch.data(), scratch.size()); + file.read_bytes(scratch.data(), scratch.size()); } } -static void load_face_element(fstream &file, +static void load_face_element(PlyReadBuffer &file, const PlyHeader &header, const PlyElement &element, PlyData *data) @@ -397,8 +384,7 @@ static void load_face_element(fstream &file, if (header.type == PlyFormatType::ASCII) { for (int i = 0; i < header.face_count; i++) { /* Read line */ - std::string line; - getline(file, line); + Span line = file.read_line(); const char *p = line.data(); const char *end = p + line.size(); @@ -455,7 +441,7 @@ static void load_face_element(fstream &file, Array face(count); scratch.resize(count * data_type_size[prop.type]); - file.read((char *)scratch.data(), scratch.size()); + file.read_bytes(scratch.data(), scratch.size()); ptr = scratch.data(); if (header.type == PlyFormatType::BINARY_BE) endian_switch_array((uint8_t *)ptr, data_type_size[prop.type], count); @@ -477,7 +463,7 @@ static void load_face_element(fstream &file, } } -static void load_tristrips_element(fstream &file, +static void load_tristrips_element(PlyReadBuffer &file, const PlyHeader &header, const PlyElement &element, PlyData *data) @@ -496,8 +482,7 @@ static void load_tristrips_element(fstream &file, Vector strip; if (header.type == PlyFormatType::ASCII) { - std::string line; - getline(file, line); + Span line = file.read_line(); const char *p = line.data(); const char *end = p + line.size(); @@ -523,7 +508,7 @@ static void load_tristrips_element(fstream &file, strip.resize(count); scratch.resize(count * data_type_size[prop.type]); - file.read((char *)scratch.data(), scratch.size()); + file.read_bytes(scratch.data(), scratch.size()); ptr = scratch.data(); if (header.type == PlyFormatType::BINARY_BE) endian_switch_array((uint8_t *)ptr, data_type_size[prop.type], count); @@ -562,7 +547,7 @@ static void load_tristrips_element(fstream &file, } } -static void load_edge_element(fstream &file, +static void load_edge_element(PlyReadBuffer &file, const PlyHeader &header, const PlyElement &element, PlyData *data) @@ -593,7 +578,7 @@ static void load_edge_element(fstream &file, } } -std::unique_ptr import_ply_data(fstream &file, PlyHeader &header) +std::unique_ptr import_ply_data(PlyReadBuffer &file, PlyHeader &header) { std::unique_ptr data = std::make_unique(); diff --git a/source/blender/io/ply/importer/ply_import_data.hh b/source/blender/io/ply/importer/ply_import_data.hh index cd023d48035..7531d648941 100644 --- a/source/blender/io/ply/importer/ply_import_data.hh +++ b/source/blender/io/ply/importer/ply_import_data.hh @@ -6,18 +6,18 @@ #pragma once -#include "BLI_fileops.hh" - #include "ply_data.hh" namespace blender::io::ply { +class PlyReadBuffer; + /** * Loads the information from a PLY file to a #PlyData data-structure. * \param file: The PLY file that was opened. * \param header: The information in the PLY header. * \return The #PlyData data-structure that can be used for conversion to a Mesh. */ -std::unique_ptr import_ply_data(fstream &file, PlyHeader &header); +std::unique_ptr import_ply_data(PlyReadBuffer &file, PlyHeader &header); } // namespace blender::io::ply diff --git a/source/blender/io/ply/intern/ply_functions.cc b/source/blender/io/ply/intern/ply_functions.cc deleted file mode 100644 index 5cc61547dbe..00000000000 --- a/source/blender/io/ply/intern/ply_functions.cc +++ /dev/null @@ -1,51 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -/** \file - * \ingroup ply - */ - -#include "ply_functions.hh" - -namespace blender::io::ply { - -line_ending safe_getline(fstream &file, std::string &line) -{ - line.clear(); - std::streambuf *sb = file.rdbuf(); - std::istream::sentry se(file, true); - - line_ending possible = UNSET; - char c; - while (sb->sgetc() != std::streambuf::traits_type::eof()) { - c = char(sb->sgetc()); - switch (c) { - case '\n': - if (possible == UNSET) { - possible = LF; - } - else if (possible == CR) { - possible = CR_LF; - } - break; - case '\r': - if (possible == UNSET) { - possible = CR; - } - else if (possible == LF) { - possible = LF_CR; - } - break; - default: - /* If a different character is encountered after the line ending is set, we know to return. - */ - if (possible != UNSET) { - return possible; - } - line += c; - break; - } - sb->sbumpc(); - } - return possible; -} -} // namespace blender::io::ply diff --git a/source/blender/io/ply/intern/ply_functions.hh b/source/blender/io/ply/intern/ply_functions.hh deleted file mode 100644 index e454167be87..00000000000 --- a/source/blender/io/ply/intern/ply_functions.hh +++ /dev/null @@ -1,26 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -/** \file - * \ingroup ply - */ - -#pragma once - -#include "BLI_fileops.hh" -#include - -namespace blender::io::ply { - -enum line_ending { CR_LF, LF, CR, LF_CR, UNSET }; - -/** - * Reads a line in the ply file in a line-ending safe manner. All different line endings are - * supported. This also supports a mix of different line endings in the same file. CR (\\r), LF - * (\\n), CR/LF (\\r\\n), LF/CR (\\n\\r). - * \param file: The file stream. - * \param line: The string you want to read to. - * \return The line ending enum if you're interested. - */ -line_ending safe_getline(fstream &file, std::string &line); - -} // namespace blender::io::ply diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index 1233817278f..c91041448dd 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -6,6 +6,7 @@ #include "BLI_hash_mm2a.h" #include "ply_import.hh" +#include "ply_import_buffer.hh" #include "ply_import_data.hh" namespace blender::io::ply { @@ -25,7 +26,8 @@ class ply_import_test : public testing::Test { { std::string ply_path = blender::tests::flags_test_asset_dir() + "/io_tests/ply/" + path; - fstream infile(ply_path, std::ios::in | std::ios::binary); + /* Use a small read buffer size for better coverage of buffer refilling behavior. */ + PlyReadBuffer infile(ply_path.c_str(), 128); PlyHeader header; const char *header_err = read_header(infile, header); if (header_err != nullptr) { -- 2.30.2 From 7fb7a5852bcb3d7ea345a01df2959c71a01b4b2b Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 16 Mar 2023 21:22:14 +0200 Subject: [PATCH 07/10] PLY: optimize importing by having flat index buffer instead of an array for each face --- .../io/ply/exporter/ply_export_data.cc | 5 +- .../io/ply/exporter/ply_export_header.cc | 6 +- .../ply/exporter/ply_export_load_plydata.cc | 10 +-- source/blender/io/ply/importer/ply_import.cc | 26 ++------ .../io/ply/importer/ply_import_data.cc | 64 +++++++++---------- .../io/ply/importer/ply_import_mesh.cc | 32 ++++------ source/blender/io/ply/intern/ply_data.hh | 13 +--- .../io/ply/tests/io_ply_exporter_test.cc | 5 +- .../io/ply/tests/io_ply_importer_test.cc | 23 +++---- 9 files changed, 72 insertions(+), 112 deletions(-) diff --git a/source/blender/io/ply/exporter/ply_export_data.cc b/source/blender/io/ply/exporter/ply_export_data.cc index 1c4379f24ec..f929a04432d 100644 --- a/source/blender/io/ply/exporter/ply_export_data.cc +++ b/source/blender/io/ply/exporter/ply_export_data.cc @@ -39,8 +39,9 @@ void write_vertices(FileBuffer &buffer, const PlyData &ply_data) void write_faces(FileBuffer &buffer, const PlyData &ply_data) { - for (const Array &face : ply_data.faces) { - buffer.write_face(char(face.size()), face); + const uint32_t *indices = ply_data.face_vertices.data(); + for (uint32_t face_size : ply_data.face_sizes) { + buffer.write_face(char(face_size), Span(indices, face_size)); } buffer.write_to_file(); } diff --git a/source/blender/io/ply/exporter/ply_export_header.cc b/source/blender/io/ply/exporter/ply_export_header.cc index 014aa9de897..8dac491e298 100644 --- a/source/blender/io/ply/exporter/ply_export_header.cc +++ b/source/blender/io/ply/exporter/ply_export_header.cc @@ -49,13 +49,13 @@ void write_header(FileBuffer &buffer, buffer.write_header_scalar_property("float", "t"); } - if (!ply_data.faces.is_empty()) { - buffer.write_header_element("face", int32_t(ply_data.faces.size())); + if (!ply_data.face_sizes.is_empty()) { + buffer.write_header_element("face", int(ply_data.face_sizes.size())); buffer.write_header_list_property("uchar", "uint", "vertex_indices"); } if (!ply_data.edges.is_empty()) { - buffer.write_header_element("edge", int32_t(ply_data.edges.size())); + buffer.write_header_element("edge", int(ply_data.edges.size())); buffer.write_header_scalar_property("int", "vertex1"); buffer.write_header_scalar_property("int", "vertex2"); } 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 6cf09474d61..37db9089d14 100644 --- a/source/blender/io/ply/exporter/ply_export_load_plydata.cc +++ b/source/blender/io/ply/exporter/ply_export_load_plydata.cc @@ -120,11 +120,12 @@ void load_plydata(PlyData &plyData, Depsgraph *depsgraph, const PLYExportParams &export_object_eval_, export_params.forward_axis, export_params.up_axis); /* Load faces into plyData. */ + plyData.face_vertices.reserve(mesh->totloop); + plyData.face_sizes.reserve(mesh->totpoly); int loop_offset = 0; const Span loops = mesh->loops(); for (const MPoly &poly : mesh->polys()) { const Span poly_loops = loops.slice(poly.loopstart, poly.totloop); - Array poly_verts(poly_loops.size()); for (int i = 0; i < poly_loops.size(); ++i) { float2 uv; @@ -136,11 +137,10 @@ void load_plydata(PlyData &plyData, Depsgraph *depsgraph, const PLYExportParams } UV_vertex_key key = UV_vertex_key(uv, poly_loops[i].v); int ply_vertex_index = vertex_map.lookup(key); - poly_verts[i] = (uint32_t(ply_vertex_index + vertex_offset)); + plyData.face_vertices.append(ply_vertex_index + vertex_offset); } - loop_offset += poly_loops.size(); - - plyData.faces.append(std::move(poly_verts)); + loop_offset += poly.totloop; + plyData.face_sizes.append(poly.totloop); } Array mesh_vertex_index_LUT(vertex_map.size()); diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index 33b86f05720..242ffa8da28 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -91,14 +91,14 @@ static PlyDataTypes type_from_string(Span word) const char *read_header(PlyReadBuffer &file, PlyHeader &r_header) { - Span word; + Span word, line; + line = file.read_line(); + if (StringRef(line.data(), line.size()) != "ply") { + return "Invalid PLY header."; + } while (true) { /* We break when end_header is encountered. */ - Span line = file.read_line(); - if (r_header.header_size == 0 && StringRef(line.data(), line.size()) != "ply") { - return "Invalid PLY header."; - } - r_header.header_size++; + line = file.read_line(); if (parse_keyword(line, "format")) { skip_space(line); @@ -122,20 +122,6 @@ const char *read_header(PlyReadBuffer &file, PlyHeader &r_header) word = parse_word(line); element.count = std::stoi(std::string(word.data(), word.size())); r_header.elements.append(element); - - if (element.name == "vertex") { - r_header.vertex_count = element.count; - } - else if (element.name == "face") { - r_header.face_count = element.count; - } - else if (element.name == "tristrips") { - /* Will get changed later after strip decoding. */ - r_header.face_count = element.count; - } - else if (element.name == "edge") { - r_header.edge_count = element.count; - } } else if (parse_keyword(line, "property")) { PlyProperty property; diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index 78b8eacd6da..2cc0a619107 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -110,7 +110,7 @@ void PlyElement::calc_stride() } static const float data_type_normalizer[] = { - 1.0f, 127.0f, 255.0f, 32767.0f, 65535.0f, INT_MAX, UINT_MAX, 1.0f, 1.0f}; + 1.0f, 127.0f, 255.0f, 32767.0f, 65535.0f, float(INT_MAX), float(UINT_MAX), 1.0f, 1.0f}; static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, "PLY data type normalization factor table mismatch"); @@ -265,6 +265,17 @@ static void load_vertex_element(PlyReadBuffer &file, throw std::runtime_error("Vertex positions are not present in the file"); } + data->vertices.reserve(element.count); + if (has_color) { + data->vertex_colors.reserve(element.count); + } + if (has_normal) { + data->vertex_normals.reserve(element.count); + } + if (has_uv) { + data->uv_coordinates.reserve(element.count); + } + float4 color_norm = {1, 1, 1, 1}; if (has_color) { color_norm.x = data_type_normalizer[element.properties[color_index.x].type]; @@ -281,7 +292,7 @@ static void load_vertex_element(PlyReadBuffer &file, scratch.resize(element.stride); } - for (int i = 0; i < header.vertex_count; i++) { + for (int i = 0; i < element.count; i++) { if (header.type == PlyFormatType::ASCII) { parse_row_ascii(file, value_vec); @@ -381,8 +392,11 @@ static void load_face_element(PlyReadBuffer &file, throw std::runtime_error("Face element vertex indices property must be a list"); } + data->face_vertices.reserve(element.count * 3); + data->face_sizes.reserve(element.count); + if (header.type == PlyFormatType::ASCII) { - for (int i = 0; i < header.face_count; i++) { + for (int i = 0; i < element.count; i++) { /* Read line */ Span line = file.read_line(); @@ -408,23 +422,18 @@ static void load_face_element(PlyReadBuffer &file, /* Parse vertex indices list. */ p = parse_int(p, end, 0, count); - Array vertex_indices(count); for (int j = 0; j < count; j++) { int index; p = parse_int(p, end, 0, index); - /* If the face has a vertex index that is outside the range. */ - if (index >= header.vertex_count) { - throw std::runtime_error("Vertex index out of bounds"); - } - vertex_indices[j] = index; + data->face_vertices.append(index); } - data->faces.append(vertex_indices); + data->face_sizes.append(count); } } else { Vector scratch(64); - for (int i = 0; i < header.face_count; i++) { + for (int i = 0; i < element.count; i++) { const uint8_t *ptr; /* Skip any properties before vertex indices. */ @@ -439,7 +448,6 @@ static void load_face_element(PlyReadBuffer &file, //@TODO: check invalid face size values? - Array face(count); scratch.resize(count * data_type_size[prop.type]); file.read_bytes(scratch.data(), scratch.size()); ptr = scratch.data(); @@ -447,12 +455,9 @@ static void load_face_element(PlyReadBuffer &file, endian_switch_array((uint8_t *)ptr, data_type_size[prop.type], count); for (int j = 0; j < count; ++j) { uint32_t index = get_binary_value(prop.type, ptr); - if (index >= header.vertex_count) { - throw std::runtime_error("Vertex index out of bounds"); - } - face[j] = index; + data->face_vertices.append(index); } - data->faces.append(face); + data->face_sizes.append(count); /* Skip any properties after vertex indices. */ for (int j = prop_index + 1; j < element.properties.size(); j++) { @@ -468,7 +473,7 @@ static void load_tristrips_element(PlyReadBuffer &file, const PlyElement &element, PlyData *data) { - if (header.face_count != 1) { + if (element.count != 1) { throw std::runtime_error("Tristrips element should contain one row"); } if (element.properties.size() != 1) { @@ -493,9 +498,6 @@ static void load_tristrips_element(PlyReadBuffer &file, for (int j = 0; j < count; j++) { int index; p = parse_int(p, end, 0, index); - if (index >= header.vertex_count) { - throw std::runtime_error("Tristrip vertex index out of bounds"); - } strip[j] = index; } } @@ -514,15 +516,11 @@ static void load_tristrips_element(PlyReadBuffer &file, endian_switch_array((uint8_t *)ptr, data_type_size[prop.type], count); for (int j = 0; j < count; ++j) { int index = get_binary_value(prop.type, ptr); - if (index >= header.vertex_count) { - throw std::runtime_error("Tristrip vertex index out of bounds"); - } strip[j] = index; } } /* Decode triangle strip (with possible -1 restart indices) into faces. */ - Array triangle(3); size_t start = 0; for (size_t i = 0; i < strip.size(); i++) { @@ -538,10 +536,10 @@ static void load_tristrips_element(PlyReadBuffer &file, } /* Add triangle if it's not degenerate. */ if (a != b && a != c && b != c) { - triangle[0] = a; - triangle[1] = b; - triangle[2] = c; - data->faces.append(triangle); + data->face_vertices.append(a); + data->face_vertices.append(b); + data->face_vertices.append(c); + data->face_sizes.append(3); } } } @@ -558,13 +556,15 @@ static void load_edge_element(PlyReadBuffer &file, throw std::runtime_error("Edge element does not contain vertex1 and vertex2 properties"); } + data->edges.reserve(element.count); + Vector value_vec(element.properties.size()); Vector scratch; if (header.type != PlyFormatType::ASCII) { scratch.resize(element.stride); } - for (int i = 0; i < header.edge_count; i++) { + for (int i = 0; i < element.count; i++) { if (header.type == PlyFormatType::ASCII) { parse_row_ascii(file, value_vec); } @@ -573,7 +573,6 @@ static void load_edge_element(PlyReadBuffer &file, } int index1 = value_vec[prop_vertex1]; int index2 = value_vec[prop_vertex2]; - //@TODO: bounds check data->edges.append(std::make_pair(index1, index2)); } } @@ -590,10 +589,7 @@ std::unique_ptr import_ply_data(PlyReadBuffer &file, PlyHeader &header) load_face_element(file, header, element, data.get()); } else if (element.name == "tristrips") { - int64_t prev_face_count = data->faces.size(); load_tristrips_element(file, header, element, data.get()); - header.face_count -= 1; - header.face_count += data->faces.size() - prev_face_count; } else if (element.name == "edge") { load_edge_element(file, header, element, data.get()); diff --git a/source/blender/io/ply/importer/ply_import_mesh.cc b/source/blender/io/ply/importer/ply_import_mesh.cc index 44c2ceb24ba..157172cd5b3 100644 --- a/source/blender/io/ply/importer/ply_import_mesh.cc +++ b/source/blender/io/ply/importer/ply_import_mesh.cc @@ -39,30 +39,24 @@ Mesh *convert_ply_to_mesh(PlyData &data, Mesh *mesh, const PLYImportParams ¶ } /* Add faces to the mesh. */ - if (!data.faces.is_empty()) { - /* Specify amount of total faces. */ - mesh->totpoly = int(data.faces.size()); - mesh->totloop = 0; - for (int i = 0; i < data.faces.size(); i++) { - /* Add number of loops from the vertex indices in the face. */ - mesh->totloop += data.faces[i].size(); - } + if (!data.face_sizes.is_empty()) { + /* Create poly and loop layers. */ + mesh->totpoly = int(data.face_sizes.size()); + mesh->totloop = int(data.face_vertices.size()); CustomData_add_layer(&mesh->pdata, CD_MPOLY, CD_SET_DEFAULT, mesh->totpoly); CustomData_add_layer(&mesh->ldata, CD_MLOOP, CD_SET_DEFAULT, mesh->totloop); MutableSpan polys = mesh->polys_for_write(); MutableSpan loops = mesh->loops_for_write(); - int offset = 0; - /* Iterate over amount of faces. */ + /* Fill in face data. */ + //@TODO: validation of the indices + uint32_t offset = 0; for (int i = 0; i < mesh->totpoly; i++) { - int size = int(data.faces[i].size()); - /* Set the index from where this face starts and specify the amount of edges it has. */ + uint32_t size = data.face_sizes[i]; polys[i].loopstart = offset; polys[i].totloop = size; - for (int j = 0; j < size; j++) { - /* Set the vertex index of the loop to the one in PlyData. */ - loops[offset + j].v = data.faces[i][j]; + loops[offset + j].v = data.face_vertices[offset + j]; } offset += size; } @@ -93,12 +87,8 @@ Mesh *convert_ply_to_mesh(PlyData &data, Mesh *mesh, const PLYImportParams ¶ if (!data.uv_coordinates.is_empty()) { bke::SpanAttributeWriter uv_map = attributes.lookup_or_add_for_write_only_span( "UVMap", ATTR_DOMAIN_CORNER); - int counter = 0; - for (int i = 0; i < data.faces.size(); i++) { - for (int j = 0; j < data.faces[i].size(); j++) { - uv_map.span[counter] = data.uv_coordinates[data.faces[i][j]]; - counter++; - } + for (size_t i = 0; i < data.face_vertices.size(); i++) { + uv_map.span[i] = data.uv_coordinates[data.face_vertices[i]]; } uv_map.finish(); } diff --git a/source/blender/io/ply/intern/ply_data.hh b/source/blender/io/ply/intern/ply_data.hh index 36d1519e353..8d95dd9c4ac 100644 --- a/source/blender/io/ply/intern/ply_data.hh +++ b/source/blender/io/ply/intern/ply_data.hh @@ -17,11 +17,10 @@ enum PlyDataTypes { NONE, CHAR, UCHAR, SHORT, USHORT, INT, UINT, FLOAT, DOUBLE, struct PlyData { Vector vertices; Vector vertex_normals; - /* Value between 0 and 1. */ - Vector vertex_colors; + Vector vertex_colors; /* Linear space, 0..1 range colors. */ Vector> edges; - Vector edge_colors; - Vector> faces; + Vector face_vertices; + Vector face_sizes; Vector uv_coordinates; }; @@ -43,13 +42,7 @@ struct PlyElement { }; struct PlyHeader { - int vertex_count = 0; - int edge_count = 0; - int face_count = 0; - int header_size = 0; - Vector elements; - PlyFormatType type; }; 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 60a7ead15bf..79e0e26513b 100644 --- a/source/blender/io/ply/tests/io_ply_exporter_test.cc +++ b/source/blender/io/ply/tests/io_ply_exporter_test.cc @@ -68,8 +68,9 @@ static std::unique_ptr load_cube(PLYExportParams ¶ms) {-1.122082, -1.122082, -1.122082}, }; - plyData->faces = { - {0, 2, 6, 4}, {3, 7, 6, 2}, {7, 5, 4, 6}, {5, 7, 3, 1}, {1, 3, 2, 0}, {5, 1, 0, 4}}; + plyData->face_sizes = {4, 4, 4, 4, 4, 4}; + plyData->face_vertices = {0, 2, 6, 4, 3, 7, 6, 2, 7, 5, 4, 6, + 5, 7, 3, 1, 1, 3, 2, 0, 5, 1, 0, 4}; if (params.export_normals) plyData->vertex_normals = { diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index c91041448dd..78f1e239f2d 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -45,25 +45,21 @@ class ply_import_test : public testing::Test { } /* Test expected amount of vertices, edges, and faces. */ - ASSERT_EQ(header.vertex_count, exp.totvert); ASSERT_EQ(data->vertices.size(), exp.totvert); - ASSERT_EQ(header.edge_count, exp.totedge); ASSERT_EQ(data->edges.size(), exp.totedge); - ASSERT_EQ(header.face_count, exp.totpoly); - ASSERT_EQ(data->faces.size(), exp.totpoly); + ASSERT_EQ(data->face_sizes.size(), exp.totpoly); + ASSERT_EQ(data->face_vertices.size(), exp.totindex); /* Test hash of face and edge index data. */ BLI_HashMurmur2A hash; BLI_hash_mm2a_init(&hash, 0); - int indexCount = 0; - for (const auto &f : data->faces) { - indexCount += f.size(); - BLI_hash_mm2a_add(&hash, (const unsigned char *)f.data(), f.size() * sizeof(f[0])); + uint32_t offset = 0; + for (uint32_t face_size : data->face_sizes) { + BLI_hash_mm2a_add(&hash, (const unsigned char *)&data->face_vertices[offset], face_size * 4); + offset += face_size; } - ASSERT_EQ(indexCount, exp.totindex); uint16_t face_hash = BLI_hash_mm2a_end(&hash); - - if (!data->faces.is_empty()) { + if (!data->face_vertices.is_empty()) { ASSERT_EQ(face_hash, exp.polyhash); } @@ -275,13 +271,10 @@ TEST_F(ply_import_test, PlyImportVertexCompOrder) } //@TODO: test with vertex element having list properties -//@TODO: test with face element having vertex_indices list as non-first property -//@TODO: test big endian binaries //@TODO: test with edges starting with non-vertex index properties //@TODO: test various malformed headers -//@TODO: line endings //@TODO: UVs with: s,t; u,v; texture_u,texture_v; texture_s,texture_t (from miniply) //@TODO: colors with: r,g,b in addition to red,green,blue (from miniply) -//@TODO: vertex_index in addition to vertex_indices (from miniply) +//@TODO: importing bunny2 with old importer results in smooth shading; flat shading with new one } // namespace blender::io::ply -- 2.30.2 From 711a0ea7f2697097876078d2d462c6ee5d3c355f Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Fri, 17 Mar 2023 09:56:03 +0200 Subject: [PATCH 08/10] PLY: do not use exceptions in inner loops, fix exporter face indexing Using exceptions to indicate errors from "hot" code paths does incur non-trivial overhead. Instead, report errors using oldskool method of returning error messages. --- .../io/ply/exporter/ply_export_data.cc | 1 + source/blender/io/ply/importer/ply_import.cc | 36 ++++--- .../io/ply/importer/ply_import_data.cc | 102 ++++++++++-------- source/blender/io/ply/intern/ply_data.hh | 1 + .../io/ply/tests/io_ply_exporter_test.cc | 20 ++-- .../io/ply/tests/io_ply_importer_test.cc | 8 +- 6 files changed, 97 insertions(+), 71 deletions(-) diff --git a/source/blender/io/ply/exporter/ply_export_data.cc b/source/blender/io/ply/exporter/ply_export_data.cc index f929a04432d..28204339eb3 100644 --- a/source/blender/io/ply/exporter/ply_export_data.cc +++ b/source/blender/io/ply/exporter/ply_export_data.cc @@ -42,6 +42,7 @@ void write_faces(FileBuffer &buffer, const PlyData &ply_data) const uint32_t *indices = ply_data.face_vertices.data(); for (uint32_t face_size : ply_data.face_sizes) { buffer.write_face(char(face_size), Span(indices, face_size)); + indices += face_size; } buffer.write_to_file(); } diff --git a/source/blender/io/ply/importer/ply_import.cc b/source/blender/io/ply/importer/ply_import.cc index 242ffa8da28..1ddb31837d7 100644 --- a/source/blender/io/ply/importer/ply_import.cc +++ b/source/blender/io/ply/importer/ply_import.cc @@ -184,9 +184,26 @@ void importer_main(Main *bmain, return; } + /* Parse actual file data. */ + std::unique_ptr data = import_ply_data(file, header); + if (data == nullptr) { + fprintf(stderr, "PLY Importer: failed importing %s, unknown error\n", ob_name); + BKE_report(op->reports, RPT_ERROR, "PLY Importer: failed importing, unknown error."); + return; + } + if (!data->error.empty()) { + fprintf(stderr, "PLY Importer: failed importing %s: %s\n", ob_name, data->error.c_str()); + BKE_report(op->reports, RPT_ERROR, "PLY Importer: failed importing, unknown error."); + return; + } + if (data->vertices.is_empty()) { + fprintf(stderr, "PLY Importer: file %s contains no vertices\n", ob_name); + BKE_report(op->reports, RPT_ERROR, "PLY Importer: failed importing, no vertices."); + return; + } + /* Create mesh and do all prep work. */ Mesh *mesh = BKE_mesh_add(bmain, ob_name); - BKE_view_layer_base_deselect_all(scene, view_layer); LayerCollection *lc = BKE_layer_collection_get_active(view_layer); Object *obj = BKE_object_add_only_object(bmain, OB_MESH, ob_name); @@ -196,19 +213,10 @@ void importer_main(Main *bmain, Base *base = BKE_view_layer_base_find(view_layer, obj); BKE_view_layer_base_select_and_set_active(view_layer, base); - /* Parse actual file data. */ - try { - std::unique_ptr data = import_ply_data(file, header); - - Mesh *temp_val = convert_ply_to_mesh(*data, mesh, import_params); - if (import_params.merge_verts && temp_val != mesh) { - BKE_mesh_nomain_to_mesh(temp_val, mesh, obj); - } - } - catch (std::exception &e) { - fprintf(stderr, "PLY Importer: failed to read file. %s.\n", e.what()); - BKE_report(op->reports, RPT_ERROR, "PLY Importer: failed to parse file."); - return; + /* Stuff ply data into the mesh. */ + Mesh *temp_val = convert_ply_to_mesh(*data, mesh, import_params); + if (import_params.merge_verts && temp_val != mesh) { + BKE_mesh_nomain_to_mesh(temp_val, mesh, obj); } /* Object matrix and finishing up. */ diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index 2cc0a619107..46f70189bad 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -145,7 +145,7 @@ static int2 get_uv_index(const PlyElement &element) return {get_index(element, "s"), get_index(element, "t")}; } -static void parse_row_ascii(PlyReadBuffer &file, Vector &r_values) +static const char *parse_row_ascii(PlyReadBuffer &file, Vector &r_values) { Span line = file.read_line(); @@ -158,6 +158,7 @@ static void parse_row_ascii(PlyReadBuffer &file, Vector &r_values) p = parse_float(p, end, 0.0f, val); r_values[value_idx++] = val; } + return nullptr; } template static T get_binary_value(PlyDataTypes type, const uint8_t *&r_ptr) @@ -199,25 +200,24 @@ template static T get_binary_value(PlyDataTypes type, const uint8_t r_ptr += 8; break; default: - throw std::runtime_error("Unknown property type"); + BLI_assert_msg(false, "Unknown property type"); } return val; } -static void parse_row_binary(PlyReadBuffer &file, - const PlyHeader &header, - const PlyElement &element, - Vector &r_scratch, - Vector &r_values) +static const char *parse_row_binary(PlyReadBuffer &file, + const PlyHeader &header, + const PlyElement &element, + Vector &r_scratch, + Vector &r_values) { if (element.stride == 0) { - throw std::runtime_error( - "Vertex/Edge element contains list properties, this is not supported"); + return "Vertex/Edge element contains list properties, this is not supported"; } BLI_assert(r_scratch.size() == element.stride); BLI_assert(r_values.size() == element.properties.size()); if (!file.read_bytes(r_scratch.data(), r_scratch.size())) { - throw std::runtime_error("Could not read row of binary property"); + return "Could not read row of binary property"; } const uint8_t *ptr = r_scratch.data(); @@ -239,14 +239,15 @@ static void parse_row_binary(PlyReadBuffer &file, } } else { - throw std::runtime_error("Unknown binary ply format for vertex element"); + return "Unknown binary ply format for vertex element"; } + return nullptr; } -static void load_vertex_element(PlyReadBuffer &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) +static const char *load_vertex_element(PlyReadBuffer &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) { /* Figure out vertex component indices. */ int3 vertex_index = get_vertex_index(element); @@ -262,7 +263,7 @@ static void load_vertex_element(PlyReadBuffer &file, bool has_alpha = alpha_index >= 0; if (!has_vertex) { - throw std::runtime_error("Vertex positions are not present in the file"); + return "Vertex positions are not present in the file"; } data->vertices.reserve(element.count); @@ -294,11 +295,15 @@ static void load_vertex_element(PlyReadBuffer &file, for (int i = 0; i < element.count; i++) { + const char *error = nullptr; if (header.type == PlyFormatType::ASCII) { - parse_row_ascii(file, value_vec); + error = parse_row_ascii(file, value_vec); } else { - parse_row_binary(file, header, element, scratch, value_vec); + error = parse_row_binary(file, header, element, scratch, value_vec); + } + if (error != nullptr) { + return error; } /* Vertex coord */ @@ -340,6 +345,7 @@ static void load_vertex_element(PlyReadBuffer &file, data->uv_coordinates.append(uvmap); } } + return nullptr; } static uint32_t read_list_count(PlyReadBuffer &file, @@ -372,10 +378,10 @@ static void skip_property(PlyReadBuffer &file, } } -static void load_face_element(PlyReadBuffer &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) +static const char *load_face_element(PlyReadBuffer &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) { int prop_index = get_index(element, "vertex_indices"); if (prop_index < 0) { @@ -385,11 +391,11 @@ static void load_face_element(PlyReadBuffer &file, prop_index = 0; } if (prop_index < 0) { - throw std::runtime_error("Face element does not contain vertex indices property"); + return "Face element does not contain vertex indices property"; } const PlyProperty &prop = element.properties[prop_index]; if (prop.count_type == PlyDataTypes::NONE) { - throw std::runtime_error("Face element vertex indices property must be a list"); + return "Face element vertex indices property must be a list"; } data->face_vertices.reserve(element.count * 3); @@ -466,22 +472,23 @@ static void load_face_element(PlyReadBuffer &file, } } } + return nullptr; } -static void load_tristrips_element(PlyReadBuffer &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) +static const char *load_tristrips_element(PlyReadBuffer &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) { if (element.count != 1) { - throw std::runtime_error("Tristrips element should contain one row"); + return "Tristrips element should contain one row"; } if (element.properties.size() != 1) { - throw std::runtime_error("Tristrips element should contain one property"); + return "Tristrips element should contain one property"; } const PlyProperty &prop = element.properties[0]; if (prop.count_type == PlyDataTypes::NONE) { - throw std::runtime_error("Tristrips element property must be a list"); + return "Tristrips element property must be a list"; } Vector strip; @@ -543,17 +550,18 @@ static void load_tristrips_element(PlyReadBuffer &file, } } } + return nullptr; } -static void load_edge_element(PlyReadBuffer &file, - const PlyHeader &header, - const PlyElement &element, - PlyData *data) +static const char *load_edge_element(PlyReadBuffer &file, + const PlyHeader &header, + const PlyElement &element, + PlyData *data) { int prop_vertex1 = get_index(element, "vertex1"); int prop_vertex2 = get_index(element, "vertex2"); if (prop_vertex1 < 0 || prop_vertex2 < 0) { - throw std::runtime_error("Edge element does not contain vertex1 and vertex2 properties"); + return "Edge element does not contain vertex1 and vertex2 properties"; } data->edges.reserve(element.count); @@ -565,16 +573,21 @@ static void load_edge_element(PlyReadBuffer &file, } for (int i = 0; i < element.count; i++) { + const char *error = nullptr; if (header.type == PlyFormatType::ASCII) { - parse_row_ascii(file, value_vec); + error = parse_row_ascii(file, value_vec); } else { - parse_row_binary(file, header, element, scratch, value_vec); + error = parse_row_binary(file, header, element, scratch, value_vec); + } + if (error != nullptr) { + return error; } int index1 = value_vec[prop_vertex1]; int index2 = value_vec[prop_vertex2]; data->edges.append(std::make_pair(index1, index2)); } + return nullptr; } std::unique_ptr import_ply_data(PlyReadBuffer &file, PlyHeader &header) @@ -582,17 +595,22 @@ std::unique_ptr import_ply_data(PlyReadBuffer &file, PlyHeader &header) std::unique_ptr data = std::make_unique(); for (const PlyElement &element : header.elements) { + const char *error = nullptr; if (element.name == "vertex") { - load_vertex_element(file, header, element, data.get()); + error = load_vertex_element(file, header, element, data.get()); } else if (element.name == "face") { - load_face_element(file, header, element, data.get()); + error = load_face_element(file, header, element, data.get()); } else if (element.name == "tristrips") { - load_tristrips_element(file, header, element, data.get()); + error = load_tristrips_element(file, header, element, data.get()); } else if (element.name == "edge") { - load_edge_element(file, header, element, data.get()); + error = load_edge_element(file, header, element, data.get()); + } + if (error != nullptr) { + data->error = error; + return data; } } diff --git a/source/blender/io/ply/intern/ply_data.hh b/source/blender/io/ply/intern/ply_data.hh index 8d95dd9c4ac..db14fdeefc0 100644 --- a/source/blender/io/ply/intern/ply_data.hh +++ b/source/blender/io/ply/intern/ply_data.hh @@ -22,6 +22,7 @@ struct PlyData { Vector face_vertices; Vector face_sizes; Vector uv_coordinates; + std::string error; }; enum PlyFormatType { ASCII, BINARY_LE, BINARY_BE }; 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 79e0e26513b..d8f163bdfc4 100644 --- a/source/blender/io/ply/tests/io_ply_exporter_test.cc +++ b/source/blender/io/ply/tests/io_ply_exporter_test.cc @@ -21,7 +21,7 @@ namespace blender::io::ply { -class PlyExportTest : public BlendfileLoadingBaseTest { +class ply_export_test : public BlendfileLoadingBaseTest { public: bool load_file_and_depsgraph(const std::string &filepath, const eEvaluationMode eval_mode = DAG_EVAL_VIEWPORT) @@ -127,7 +127,7 @@ static std::vector read_temp_file_in_vectorchar(const std::string &file_pa return res; } -TEST_F(PlyExportTest, WriteHeaderAscii) +TEST_F(ply_export_test, WriteHeaderAscii) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -165,7 +165,7 @@ TEST_F(PlyExportTest, WriteHeaderAscii) ASSERT_STREQ(result.c_str(), expected.c_str()); } -TEST_F(PlyExportTest, WriteHeaderBinary) +TEST_F(ply_export_test, WriteHeaderBinary) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -203,7 +203,7 @@ TEST_F(PlyExportTest, WriteHeaderBinary) ASSERT_STREQ(result.c_str(), expected.c_str()); } -TEST_F(PlyExportTest, WriteVerticesAscii) +TEST_F(ply_export_test, WriteVerticesAscii) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -235,7 +235,7 @@ TEST_F(PlyExportTest, WriteVerticesAscii) ASSERT_STREQ(result.c_str(), expected.c_str()); } -TEST_F(PlyExportTest, WriteVerticesBinary) +TEST_F(ply_export_test, WriteVerticesBinary) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -271,7 +271,7 @@ TEST_F(PlyExportTest, WriteVerticesBinary) } } -TEST_F(PlyExportTest, WriteFacesAscii) +TEST_F(ply_export_test, WriteFacesAscii) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -301,7 +301,7 @@ TEST_F(PlyExportTest, WriteFacesAscii) ASSERT_STREQ(result.c_str(), expected.c_str()); } -TEST_F(PlyExportTest, WriteFacesBinary) +TEST_F(ply_export_test, WriteFacesBinary) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -337,7 +337,7 @@ TEST_F(PlyExportTest, WriteFacesBinary) } } -TEST_F(PlyExportTest, WriteVertexNormalsAscii) +TEST_F(ply_export_test, WriteVertexNormalsAscii) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -369,7 +369,7 @@ TEST_F(PlyExportTest, WriteVertexNormalsAscii) ASSERT_STREQ(result.c_str(), expected.c_str()); } -TEST_F(PlyExportTest, WriteVertexNormalsBinary) +TEST_F(ply_export_test, WriteVertexNormalsBinary) { std::string filePath = get_temp_ply_filename(temp_file_path); PLYExportParams _params = {}; @@ -411,7 +411,7 @@ TEST_F(PlyExportTest, WriteVertexNormalsBinary) } } -class ply_exporter_ply_data_test : public PlyExportTest { +class ply_exporter_ply_data_test : public ply_export_test { public: PlyData load_ply_data_from_blendfile(const std::string &blendfile, PLYExportParams ¶ms) { diff --git a/source/blender/io/ply/tests/io_ply_importer_test.cc b/source/blender/io/ply/tests/io_ply_importer_test.cc index 78f1e239f2d..73c9580e419 100644 --- a/source/blender/io/ply/tests/io_ply_importer_test.cc +++ b/source/blender/io/ply/tests/io_ply_importer_test.cc @@ -34,11 +34,9 @@ class ply_import_test : public testing::Test { ADD_FAILURE(); return; } - std::unique_ptr data; - try { - data = import_ply_data(infile, header); - } - catch (std::exception &e) { + std::unique_ptr data = import_ply_data(infile, header); + if (!data->error.empty()) { + fprintf(stderr, "%s\n", data->error.c_str()); ASSERT_EQ(0, exp.totvert); ASSERT_EQ(0, exp.totpoly); return; -- 2.30.2 From fafca64d00fe3a8b808a5fe3270a0d25aa790d22 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Fri, 17 Mar 2023 11:40:07 +0200 Subject: [PATCH 09/10] PLY: cleanup, comments, index validation --- .../io/ply/importer/ply_import_buffer.hh | 10 ++++ .../io/ply/importer/ply_import_data.cc | 48 +++++++------------ .../io/ply/importer/ply_import_mesh.cc | 22 +++++++-- 3 files changed, 45 insertions(+), 35 deletions(-) diff --git a/source/blender/io/ply/importer/ply_import_buffer.hh b/source/blender/io/ply/importer/ply_import_buffer.hh index 06f50712ace..692ca806495 100644 --- a/source/blender/io/ply/importer/ply_import_buffer.hh +++ b/source/blender/io/ply/importer/ply_import_buffer.hh @@ -14,14 +14,24 @@ namespace blender::io::ply { +/** + * Reads underlying PLY file in large chunks, and provides interface for ascii/header + * parsing to read individual lines, and for binary parsing to read chunks of bytes. + */ class PlyReadBuffer { public: PlyReadBuffer(const char *file_path, size_t read_buffer_size = 64 * 1024); ~PlyReadBuffer(); + /** After header is parsed, indicate whether the rest of reading will be ascii or binary. */ void after_header(bool is_binary); + /** Gets the next line from the file as a Span. The line does not include any newline characters. + */ Span read_line(); + + /** Reads a number of bytes into provided destination pointer. Returns false if this amount of + * bytes can not be read. */ bool read_bytes(void *dst, size_t size); private: diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index 46f70189bad..63f7e6bb42f 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -97,6 +97,11 @@ namespace blender::io::ply { static const int data_type_size[] = {0, 1, 1, 2, 2, 4, 4, 4, 8}; static_assert(std::size(data_type_size) == PLY_TYPE_COUNT, "PLY data type size table mismatch"); +static const float data_type_normalizer[] = { + 1.0f, 127.0f, 255.0f, 32767.0f, 65535.0f, float(INT_MAX), float(UINT_MAX), 1.0f, 1.0f}; +static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, + "PLY data type normalization factor table mismatch"); + void PlyElement::calc_stride() { stride = 0; @@ -109,11 +114,6 @@ void PlyElement::calc_stride() } } -static const float data_type_normalizer[] = { - 1.0f, 127.0f, 255.0f, 32767.0f, 65535.0f, float(INT_MAX), float(UINT_MAX), 1.0f, 1.0f}; -static_assert(std::size(data_type_normalizer) == PLY_TYPE_COUNT, - "PLY data type normalization factor table mismatch"); - static int get_index(const PlyElement &element, StringRef property) { for (int i = 0, n = int(element.properties.size()); i != n; i++) { @@ -125,26 +125,6 @@ static int get_index(const PlyElement &element, StringRef property) return -1; } -static int3 get_vertex_index(const PlyElement &element) -{ - return {get_index(element, "x"), get_index(element, "y"), get_index(element, "z")}; -} - -static int3 get_color_index(const PlyElement &element) -{ - return {get_index(element, "red"), get_index(element, "green"), get_index(element, "blue")}; -} - -static int3 get_normal_index(const PlyElement &element) -{ - return {get_index(element, "nx"), get_index(element, "ny"), get_index(element, "nz")}; -} - -static int2 get_uv_index(const PlyElement &element) -{ - return {get_index(element, "s"), get_index(element, "t")}; -} - static const char *parse_row_ascii(PlyReadBuffer &file, Vector &r_values) { Span line = file.read_line(); @@ -250,10 +230,12 @@ static const char *load_vertex_element(PlyReadBuffer &file, PlyData *data) { /* Figure out vertex component indices. */ - int3 vertex_index = get_vertex_index(element); - int3 color_index = get_color_index(element); - int3 normal_index = get_normal_index(element); - int2 uv_index = get_uv_index(element); + int3 vertex_index = {get_index(element, "x"), get_index(element, "y"), get_index(element, "z")}; + int3 color_index = { + get_index(element, "red"), get_index(element, "green"), get_index(element, "blue")}; + int3 normal_index = { + get_index(element, "nx"), get_index(element, "ny"), get_index(element, "nz")}; + int2 uv_index = {get_index(element, "s"), get_index(element, "t")}; int alpha_index = get_index(element, "alpha"); bool has_vertex = vertex_index.x >= 0 && vertex_index.y >= 0 && vertex_index.z >= 0; @@ -427,6 +409,9 @@ static const char *load_face_element(PlyReadBuffer &file, /* Parse vertex indices list. */ p = parse_int(p, end, 0, count); + if (count < 1 || count > 255) { + return "Invalid face size, must be between 1 and 255"; + } for (int j = 0; j < count; j++) { int index; @@ -451,8 +436,9 @@ static const char *load_face_element(PlyReadBuffer &file, /* Read vertex indices list. */ uint32_t count = read_list_count( file, prop, scratch, header.type == PlyFormatType::BINARY_BE); - - //@TODO: check invalid face size values? + if (count < 1 || count > 255) { + return "Invalid face size, must be between 1 and 255"; + } scratch.resize(count * data_type_size[prop.type]); file.read_bytes(scratch.data(), scratch.size()); diff --git a/source/blender/io/ply/importer/ply_import_mesh.cc b/source/blender/io/ply/importer/ply_import_mesh.cc index 157172cd5b3..c66884b8ea0 100644 --- a/source/blender/io/ply/importer/ply_import_mesh.cc +++ b/source/blender/io/ply/importer/ply_import_mesh.cc @@ -33,8 +33,18 @@ Mesh *convert_ply_to_mesh(PlyData &data, Mesh *mesh, const PLYImportParams ¶ CustomData_add_layer(&mesh->edata, CD_MEDGE, CD_SET_DEFAULT, mesh->totedge); MutableSpan edges = mesh->edges_for_write(); for (int i = 0; i < mesh->totedge; i++) { - edges[i].v1 = data.edges[i].first; - edges[i].v2 = data.edges[i].second; + uint32_t v1 = data.edges[i].first; + uint32_t v2 = data.edges[i].second; + if (v1 >= mesh->totvert) { + fprintf(stderr, "Invalid PLY vertex index in edge %i/1: %i\n", i, v1); + v1 = 0; + } + if (v2 >= mesh->totvert) { + fprintf(stderr, "Invalid PLY vertex index in edge %i/2: %i\n", i, v2); + v2 = 0; + } + edges[i].v1 = v1; + edges[i].v2 = v2; } } @@ -49,14 +59,18 @@ Mesh *convert_ply_to_mesh(PlyData &data, Mesh *mesh, const PLYImportParams ¶ MutableSpan loops = mesh->loops_for_write(); /* Fill in face data. */ - //@TODO: validation of the indices uint32_t offset = 0; for (int i = 0; i < mesh->totpoly; i++) { uint32_t size = data.face_sizes[i]; polys[i].loopstart = offset; polys[i].totloop = size; for (int j = 0; j < size; j++) { - loops[offset + j].v = data.face_vertices[offset + j]; + uint32_t v = data.face_vertices[offset + j]; + if (v >= mesh->totvert) { + fprintf(stderr, "Invalid PLY vertex index in face %i loop %i: %i\n", i, j, v); + v = 0; + } + loops[offset + j].v = v; } offset += size; } -- 2.30.2 From f15f92fa6f01c92b73189434dc58d1f9ad281720 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Fri, 17 Mar 2023 12:08:54 +0200 Subject: [PATCH 10/10] PLY: fix linux build error/warnings --- source/blender/io/ply/importer/ply_import_buffer.cc | 2 ++ source/blender/io/ply/importer/ply_import_data.cc | 2 +- source/blender/io/ply/importer/ply_import_mesh.cc | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/source/blender/io/ply/importer/ply_import_buffer.cc b/source/blender/io/ply/importer/ply_import_buffer.cc index 78924a1d1e2..3b1d5de9eb7 100644 --- a/source/blender/io/ply/importer/ply_import_buffer.cc +++ b/source/blender/io/ply/importer/ply_import_buffer.cc @@ -4,6 +4,8 @@ #include "BLI_fileops.h" +#include + static inline bool is_newline(char ch) { return ch == '\n' || ch == '\r'; diff --git a/source/blender/io/ply/importer/ply_import_data.cc b/source/blender/io/ply/importer/ply_import_data.cc index 63f7e6bb42f..fc396d5aad2 100644 --- a/source/blender/io/ply/importer/ply_import_data.cc +++ b/source/blender/io/ply/importer/ply_import_data.cc @@ -28,7 +28,7 @@ static const char *drop_whitespace(const char *p, const char *end) return p; } -const char *drop_non_whitespace(const char *p, const char *end) +static const char *drop_non_whitespace(const char *p, const char *end) { while (p < end && !is_whitespace(*p)) { ++p; diff --git a/source/blender/io/ply/importer/ply_import_mesh.cc b/source/blender/io/ply/importer/ply_import_mesh.cc index c66884b8ea0..efefebb6930 100644 --- a/source/blender/io/ply/importer/ply_import_mesh.cc +++ b/source/blender/io/ply/importer/ply_import_mesh.cc @@ -36,11 +36,11 @@ Mesh *convert_ply_to_mesh(PlyData &data, Mesh *mesh, const PLYImportParams ¶ uint32_t v1 = data.edges[i].first; uint32_t v2 = data.edges[i].second; if (v1 >= mesh->totvert) { - fprintf(stderr, "Invalid PLY vertex index in edge %i/1: %i\n", i, v1); + fprintf(stderr, "Invalid PLY vertex index in edge %i/1: %u\n", i, v1); v1 = 0; } if (v2 >= mesh->totvert) { - fprintf(stderr, "Invalid PLY vertex index in edge %i/2: %i\n", i, v2); + fprintf(stderr, "Invalid PLY vertex index in edge %i/2: %u\n", i, v2); v2 = 0; } edges[i].v1 = v1; @@ -67,7 +67,7 @@ Mesh *convert_ply_to_mesh(PlyData &data, Mesh *mesh, const PLYImportParams ¶ for (int j = 0; j < size; j++) { uint32_t v = data.face_vertices[offset + j]; if (v >= mesh->totvert) { - fprintf(stderr, "Invalid PLY vertex index in face %i loop %i: %i\n", i, j, v); + fprintf(stderr, "Invalid PLY vertex index in face %i loop %i: %u\n", i, j, v); v = 0; } loops[offset + j].v = v; -- 2.30.2