diff --git a/src/libslic3r/Format/DRC.cpp b/src/libslic3r/Format/DRC.cpp index d489f04dba..c07655f69b 100644 --- a/src/libslic3r/Format/DRC.cpp +++ b/src/libslic3r/Format/DRC.cpp @@ -48,24 +48,37 @@ bool load_drc(const char *path, TriangleMesh *meshptr) indexed_triangle_set its; const PointAttribute *const positions = dracoMesh.GetNamedAttribute(GeometryAttribute::POSITION); + if (positions == nullptr) { + BOOST_LOG_TRIVIAL(error) << "load_drc: the mesh has no POSITION attribute"; + return false; + } size_t num_vertices = positions->size(); its.vertices.reserve(num_vertices); for (AttributeValueIndex i(0); i < num_vertices; ++ i) { float pos[3]; - positions->ConvertValue(i, 3, pos); + if (!positions->ConvertValue(i, 3, pos)) { + BOOST_LOG_TRIVIAL(error) << "load_drc: invalid vertex position"; + return false; + } its.vertices.emplace_back(pos[0], pos[1], pos[2]); } + // The Draco decoder does not check face indices against the point count. + const uint32_t num_points = dracoMesh.num_points(); size_t num_faces = dracoMesh.num_faces(); its.indices.reserve(num_faces); for (FaceIndex i(0); i < num_faces; ++ i) { - Mesh::Face face = dracoMesh.face(i); - - its.indices.emplace_back( - positions->mapped_index(face[0]).value(), - positions->mapped_index(face[1]).value(), - positions->mapped_index(face[2]).value() - ); + const Mesh::Face &face = dracoMesh.face(i); + stl_triangle_vertex_indices facet; + for (int k = 0; k < 3; ++ k) { + const size_t vertex_idx = face[k].value() < num_points ? positions->mapped_index(face[k]).value() : num_vertices; + if (vertex_idx >= num_vertices) { + BOOST_LOG_TRIVIAL(error) << "load_drc: invalid vertex index"; + return false; + } + facet[k] = static_cast(vertex_idx); + } + its.indices.emplace_back(facet); } *meshptr = TriangleMesh(std::move(its)); diff --git a/src/libslic3r/Format/OBJ.cpp b/src/libslic3r/Format/OBJ.cpp index 10abe8e4de..de91efb64d 100644 --- a/src/libslic3r/Format/OBJ.cpp +++ b/src/libslic3r/Format/OBJ.cpp @@ -166,10 +166,15 @@ bool load_obj(const char *path, TriangleMesh *meshptr, ObjInfo& obj_info, std::s obj_info.uv_map_pngs[face_index] = png_name; } if (data.textureCoordinates.size() > 0) { - Vec2f uv0(data.textureCoordinates[uvs[0] * 2], data.textureCoordinates[uvs[0] * 2 + 1]); - Vec2f uv1(data.textureCoordinates[uvs[1] * 2], data.textureCoordinates[uvs[1] * 2 + 1]); - Vec2f uv2(data.textureCoordinates[uvs[2] * 2], data.textureCoordinates[uvs[2] * 2 + 1]); - std::array uv_array{uv0, uv1, uv2}; + // A face vertex may omit vt or reference a missing one. Fall back to (0, 0) rather than + // skipping the face, so obj_info.uvs stays aligned with the face indices. + const int uv_count = static_cast(data.textureCoordinates.size() / OBJ_TEXCOORD_LENGTH); + auto uv_at = [&data, uv_count](int idx) -> Vec2f { + if (idx < 0 || idx >= uv_count) + return Vec2f::Zero(); + return Vec2f(data.textureCoordinates[idx * OBJ_TEXCOORD_LENGTH], data.textureCoordinates[idx * OBJ_TEXCOORD_LENGTH + 1]); + }; + std::array uv_array{uv_at(uvs[0]), uv_at(uvs[1]), uv_at(uvs[2])}; obj_info.uvs.emplace_back(uv_array); } obj_info.face_colors.emplace_back(face_color); diff --git a/src/libslic3r/Format/objparser.cpp b/src/libslic3r/Format/objparser.cpp index 886fa423bd..c5f33a7d0a 100644 --- a/src/libslic3r/Format/objparser.cpp +++ b/src/libslic3r/Format/objparser.cpp @@ -51,19 +51,18 @@ static bool obj_parseline(const char *line, ObjData &data) line = endptr; EATWS(); } - /*double w = 0; + // The optional w is accepted but not stored: only u and v are used. if (*line != 0) { - w = strtod(line, &endptr); + strtod(line, &endptr); if (endptr == 0 || (*endptr != ' ' && *endptr != '\t' && *endptr != 0)) return false; line = endptr; EATWS(); - }*/ + } if (*line != 0) return false; data.textureCoordinates.push_back((float)u); data.textureCoordinates.push_back((float)v); - //data.textureCoordinates.push_back((float)w); break; } case 'n': @@ -245,7 +244,7 @@ static bool obj_parseline(const char *line, ObjData &data) else -- vertex.normalIdx; if (vertex.textureCoordIdx < 0) - vertex.textureCoordIdx += (int)data.textureCoordinates.size() / 3; + vertex.textureCoordIdx += (int)data.textureCoordinates.size() / OBJ_TEXCOORD_LENGTH; else -- vertex.textureCoordIdx; data.vertices.push_back(vertex); diff --git a/src/libslic3r/Format/objparser.hpp b/src/libslic3r/Format/objparser.hpp index 58afd015a8..e711bdf6f4 100644 --- a/src/libslic3r/Format/objparser.hpp +++ b/src/libslic3r/Format/objparser.hpp @@ -92,6 +92,7 @@ inline bool operator==(const ObjSmoothingGroup &v1, const ObjSmoothingGroup &v2) } #define OBJ_VERTEX_COLOR_ALPHA 6 #define OBJ_VERTEX_LENGTH 7 // x, y, z, color_x,color_y,color_z,color_w +#define OBJ_TEXCOORD_LENGTH 2 // u, v #define ONE_FACE_SIZE 4//ONE_FACE format: f 8/4/6 7/3/6 6/2/6 -1/-1/-1 struct ObjData { // Version of the data structure for load / store in the private binary format. @@ -100,7 +101,7 @@ struct ObjData { // x, y, z, color_x,color_y,color_z,color_w std::vector coordinates; bool has_vertex_color{false}; - // u, v, w + // u, v std::vector textureCoordinates; // x, y, z std::vector normals; diff --git a/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index 5ee438a0e4..2c7ac2e648 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -16,6 +16,7 @@ add_executable(${_TEST_NAME}_tests test_clipper_utils.cpp test_config.cpp test_config_variant_expansion.cpp + test_drc.cpp test_toolordering_nozzle_group.cpp test_preset_bundle_loading.cpp test_preset_setting_id.cpp @@ -34,6 +35,7 @@ add_executable(${_TEST_NAME}_tests test_mutable_priority_queue.cpp test_minimum_spanning_tree.cpp test_nozzle_volume_type.cpp + test_obj.cpp test_step.cpp test_stl.cpp test_triangle_selector.cpp @@ -69,7 +71,9 @@ if (TARGET OpenVDB::openvdb) target_sources(${_TEST_NAME}_tests PRIVATE test_hollowing.cpp) endif() -target_link_libraries(${_TEST_NAME}_tests test_common libslic3r Catch2::Catch2WithMain) +# libslic3r links Draco privately; test_drc.cpp encodes its own fixtures. +find_package(Draco REQUIRED) +target_link_libraries(${_TEST_NAME}_tests test_common libslic3r draco::draco Catch2::Catch2WithMain) target_include_directories(${_TEST_NAME}_tests PRIVATE ${CMAKE_SOURCE_DIR}/src) set_property(TARGET ${_TEST_NAME}_tests PROPERTY FOLDER "tests") diff --git a/tests/libslic3r/test_drc.cpp b/tests/libslic3r/test_drc.cpp new file mode 100644 index 0000000000..917fd5c714 --- /dev/null +++ b/tests/libslic3r/test_drc.cpp @@ -0,0 +1,71 @@ +#include + +#include "libslic3r/Format/DRC.hpp" +#include "libslic3r/TriangleMesh.hpp" + +#include + +#include +#include + +#include "test_utils.hpp" + +using namespace Slic3r; + +namespace { + +// Encodes one triangle whose vertices carry a single attribute of the given type. +// The decoder accepts both a mesh without POSITION and a face index past the point count. +void write_drc_triangle(const std::string &path, draco::GeometryAttribute::Type attribute_type, uint32_t second_point) +{ + draco::Mesh mesh; + mesh.set_num_points(3); + draco::GeometryAttribute attribute; + attribute.Init(attribute_type, nullptr, 3, draco::DT_FLOAT32, false, sizeof(float) * 3, 0); + const int attribute_id = mesh.AddAttribute(attribute, true, 3); + const float points[3][3] = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}}; + for (int i = 0; i < 3; ++i) + mesh.attribute(attribute_id)->SetAttributeValue(draco::AttributeValueIndex(i), points[i]); + draco::Mesh::Face face; + face[0] = draco::PointIndex(0); + face[1] = draco::PointIndex(second_point); + face[2] = draco::PointIndex(2); + mesh.AddFace(face); + + draco::Encoder encoder; + encoder.SetEncodingMethod(draco::MESH_SEQUENTIAL_ENCODING); + draco::EncoderBuffer buffer; + REQUIRE(encoder.EncodeMeshToBuffer(mesh, &buffer).ok()); + boost::nowide::ofstream out(path, std::ios::binary); + out.write(buffer.data(), static_cast(buffer.size())); +} + +} // namespace + +TEST_CASE("A Draco triangle with positions loads", "[DRC]") +{ + ScopedTemporaryFile drc(".drc"); + write_drc_triangle(drc.string(), draco::GeometryAttribute::POSITION, 1); + + TriangleMesh mesh; + REQUIRE(load_drc(drc.string().c_str(), &mesh)); + CHECK(mesh.facets_count() == 1); +} + +TEST_CASE("A Draco mesh without a POSITION attribute fails to load", "[DRC][Regression]") +{ + ScopedTemporaryFile drc(".drc"); + write_drc_triangle(drc.string(), draco::GeometryAttribute::GENERIC, 1); + + TriangleMesh mesh; + CHECK_FALSE(load_drc(drc.string().c_str(), &mesh)); +} + +TEST_CASE("A Draco face referencing a missing point fails to load", "[DRC][Regression]") +{ + ScopedTemporaryFile drc(".drc"); + write_drc_triangle(drc.string(), draco::GeometryAttribute::POSITION, 200); + + TriangleMesh mesh; + CHECK_FALSE(load_drc(drc.string().c_str(), &mesh)); +} diff --git a/tests/libslic3r/test_obj.cpp b/tests/libslic3r/test_obj.cpp new file mode 100644 index 0000000000..d198a69c8c --- /dev/null +++ b/tests/libslic3r/test_obj.cpp @@ -0,0 +1,122 @@ +#include + +#include "libslic3r/TriangleMesh.hpp" +#include "libslic3r/Format/OBJ.hpp" + +#include + +#include "test_utils.hpp" + +using namespace Slic3r; +using Catch::Matchers::WithinAbs; + +namespace { + +struct LoadedObj +{ + bool ok{false}; + TriangleMesh mesh; + ObjInfo info; +}; + +// A tetrahedron with a material and two texture coordinates, (0.25, 0.5) and (0.75, 1). +// Only the first face and the vt lines are varied; the other three faces reference vt 1. +LoadedObj load_textured_tetrahedron(const std::string &first_face, const std::string &vts = "vt 0.25 0.5\nvt 0.75 1\n") +{ + ScopedTemporaryFile obj(".obj"); + ScopedTemporaryFile mtl(".mtl"); + { + boost::nowide::ofstream out(mtl.string()); + out << "newmtl a\nKd 1 0 0\n"; + } + { + boost::nowide::ofstream out(obj.string()); + out << "mtllib " << mtl.path().filename().string() << "\n" + << "v 0 0 0\nv 10 0 0\nv 0 10 0\nv 0 0 10\n" + << vts + << "usemtl a\n" + << first_face << "\n" + << "f 1/1 2/1 4/1\nf 1/1 4/1 3/1\nf 2/1 3/1 4/1\n"; + } + LoadedObj loaded; + std::string message; + loaded.ok = load_obj(obj.string().c_str(), &loaded.mesh, loaded.info, message); + return loaded; +} + +} // namespace + +TEST_CASE("An out-of-range texture index falls back to a zero UV and keeps the geometry", "[OBJ][Regression]") +{ + const LoadedObj loaded = load_textured_tetrahedron("f 1/1000000000 3/1 2/1"); + + REQUIRE(loaded.ok); + CHECK(loaded.mesh.facets_count() == 4); + REQUIRE(loaded.info.uvs.size() == 4); + const std::array &uv = loaded.info.uvs.front(); + CHECK_THAT(uv[0].x(), WithinAbs(0., 1e-6)); + CHECK_THAT(uv[0].y(), WithinAbs(0., 1e-6)); + CHECK_THAT(uv[1].x(), WithinAbs(0.25, 1e-6)); + CHECK_THAT(uv[1].y(), WithinAbs(0.5, 1e-6)); +} + +TEST_CASE("A face without texture indices loads among faces that have them", "[OBJ][Regression]") +{ + const LoadedObj loaded = load_textured_tetrahedron("f 1 3 2"); + + REQUIRE(loaded.ok); + CHECK(loaded.mesh.facets_count() == 4); + // One UV entry per face, so later faces keep their own coordinates. + REQUIRE(loaded.info.uvs.size() == 4); + for (const Vec2f &uv : loaded.info.uvs.front()) { + CHECK_THAT(uv.x(), WithinAbs(0., 1e-6)); + CHECK_THAT(uv.y(), WithinAbs(0., 1e-6)); + } + CHECK_THAT(loaded.info.uvs[1][0].x(), WithinAbs(0.25, 1e-6)); + CHECK_THAT(loaded.info.uvs[1][0].y(), WithinAbs(0.5, 1e-6)); +} + +TEST_CASE("A negative texture index counts back from the last texture coordinate", "[OBJ]") +{ + // -1 is the most recent vt (0.75, 1), -2 the one before it (0.25, 0.5). + const LoadedObj loaded = load_textured_tetrahedron("f 1/-2 3/-1 2/-1"); + + REQUIRE(loaded.ok); + CHECK(loaded.mesh.facets_count() == 4); + REQUIRE(loaded.info.uvs.size() == 4); + const std::array &uv = loaded.info.uvs.front(); + CHECK_THAT(uv[0].x(), WithinAbs(0.25, 1e-6)); + CHECK_THAT(uv[0].y(), WithinAbs(0.5, 1e-6)); + CHECK_THAT(uv[1].x(), WithinAbs(0.75, 1e-6)); + CHECK_THAT(uv[1].y(), WithinAbs(1., 1e-6)); +} + +TEST_CASE("Texture coordinates with a w component are kept", "[OBJ][Regression]") +{ + const LoadedObj loaded = load_textured_tetrahedron("f 1/1 3/2 2/2", "vt 0.25 0.5 0\nvt 0.75 1 0\n"); + + REQUIRE(loaded.ok); + CHECK(loaded.mesh.facets_count() == 4); + REQUIRE(loaded.info.uvs.size() == 4); + const std::array &uv = loaded.info.uvs.front(); + CHECK_THAT(uv[0].x(), WithinAbs(0.25, 1e-6)); + CHECK_THAT(uv[0].y(), WithinAbs(0.5, 1e-6)); + CHECK_THAT(uv[1].x(), WithinAbs(0.75, 1e-6)); + CHECK_THAT(uv[1].y(), WithinAbs(1., 1e-6)); +} + +TEST_CASE("A texture coordinate with w does not shift the indices of the ones after it", "[OBJ][Regression]") +{ + // The w on the first vt used to drop that line, so vt 2 resolved to the third coordinate. + const LoadedObj loaded = load_textured_tetrahedron("f 1/2 3/3 2/-1", "vt 0.1 0.2 0\nvt 0.25 0.5\nvt 0.75 1\n"); + + REQUIRE(loaded.ok); + REQUIRE(loaded.info.uvs.size() == 4); + const std::array &uv = loaded.info.uvs.front(); + CHECK_THAT(uv[0].x(), WithinAbs(0.25, 1e-6)); + CHECK_THAT(uv[0].y(), WithinAbs(0.5, 1e-6)); + CHECK_THAT(uv[1].x(), WithinAbs(0.75, 1e-6)); + CHECK_THAT(uv[1].y(), WithinAbs(1., 1e-6)); + CHECK_THAT(uv[2].x(), WithinAbs(0.75, 1e-6)); + CHECK_THAT(uv[2].y(), WithinAbs(1., 1e-6)); +}