From 22b2ccc474de7337db7997e3ab1eb9b15b48f7cd Mon Sep 17 00:00:00 2001 From: Boleslaw Ciesielski Date: Sun, 7 Feb 2021 15:41:21 -0800 Subject: [PATCH] Fixes issue #5979 - NULL pointer crash in ModelObject::split() ModelObject::split() expects a non-NULL new_objects vector where it adds pointers to the new models resulting from the split. But in the CLI case the caller does not care about this and passes NULL which causes a crash. To fix the crash we could pass a dummy vector but it turns out that we actually have a use for the results because we should assign a unique name to each new model the same way as the GUI does. These names show up as comments in the gcode so this change makes the gcode produced by the GUI and the CLI more similar and diffable. @lukasmatena has amended the original commit by @combolek (pull request #5991) in order to avoid code duplication --- src/PrusaSlicer.cpp | 3 ++- src/libslic3r/Model.cpp | 4 +++- src/slic3r/GUI/Plater.cpp | 4 ---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/PrusaSlicer.cpp b/src/PrusaSlicer.cpp index a68c5cd0f..ed2d70272 100644 --- a/src/PrusaSlicer.cpp +++ b/src/PrusaSlicer.cpp @@ -402,7 +402,8 @@ int CLI::run(int argc, char **argv) for (Model &model : m_models) { size_t num_objects = model.objects.size(); for (size_t i = 0; i < num_objects; ++ i) { - model.objects.front()->split(nullptr); + ModelObjectPtrs new_objects; + model.objects.front()->split(&new_objects); model.delete_object(size_t(0)); } } diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp index a07708669..807221964 100644 --- a/src/libslic3r/Model.cpp +++ b/src/libslic3r/Model.cpp @@ -1305,6 +1305,7 @@ void ModelObject::split(ModelObjectPtrs* new_objects) ModelVolume* volume = this->volumes.front(); TriangleMeshPtrs meshptrs = volume->mesh().split(); + size_t counter = 1; for (TriangleMesh *mesh : meshptrs) { // FIXME: crashes if not satisfied @@ -1314,7 +1315,8 @@ void ModelObject::split(ModelObjectPtrs* new_objects) // XXX: this seems to be the only real usage of m_model, maybe refactor this so that it's not needed? ModelObject* new_object = m_model->add_object(); - new_object->name = this->name; + new_object->name = this->name + (meshptrs.size() > 1 ? "_" + std::to_string(counter++) : ""); + // Don't copy the config's ID. new_object->config.assign_config(this->config); assert(new_object->config.id().valid()); diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 9912e7aa0..b6dda66fd 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -2892,10 +2892,6 @@ void Plater::priv::split_object() { Plater::TakeSnapshot snapshot(q, _L("Split to Objects")); - unsigned int counter = 1; - for (ModelObject* m : new_objects) - m->name = current_model_object->name + "_" + std::to_string(counter++); - remove(obj_idx); // load all model objects at once, otherwise the plate would be rearranged after each one