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/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index 9c6dae471a..120766c516 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -14,6 +14,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 @@ -68,7 +69,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)); +}